fix(opencode): address review feedback

Fix the OpenCode session handle and teardown behavior after PR review.

- return the resumed session ID instead of leaking workspace paths via Session.ID
- honor StopSession(ctx) with a bounded graceful shutdown path
- correct the managed-environment warning text in export usage recovery
- remove the dead errors import placeholder and add regression coverage for stop deadlines

Author: serghei-dev

internal/agent/opencode/command_test.go

@@ -2,7 +2,6 @@ package opencode
 
 import (
 	"encoding/json"
-	"errors"
 	"strings"
 	"testing"
 
@@ -482,6 +481,3 @@ func TestSSHRemoteCommand(t *testing.T) {
 		}
 	})
 }
-
-// Compile-time interface check.
-var _ = errors.New // keep errors imported if needed in future

internal/agent/opencode/opencode.go

@@ -100,7 +100,7 @@ func (a *OpenCodeAdapter) StartSession(_ context.Context, params domain.StartSes
 	}
 
 	return domain.Session{
-		ID:       params.WorkspacePath,
+		ID:       state.sessionID,
 		AgentPID: "",
 		Internal: state,
 	}, nil
@@ -449,7 +449,7 @@ func (a *OpenCodeAdapter) RunTurn(ctx context.Context, session domain.Session, p
 }
 
 // StopSession marks the session closed and terminates any active subprocess.
-func (a *OpenCodeAdapter) StopSession(_ context.Context, session domain.Session) error {
+func (a *OpenCodeAdapter) StopSession(ctx context.Context, session domain.Session) error {
 	state, ok := session.Internal.(*sessionState)
 	if !ok {
 		return &domain.AgentError{
@@ -461,19 +461,10 @@ func (a *OpenCodeAdapter) StopSession(_ context.Context, session domain.Session)
 	state.mu.Lock()
 	state.closed = true
 	active := state.active
+	state.active = nil
 	state.mu.Unlock()
 
-	if active == nil {
-		return nil
-	}
-
-	closeStop(active)
-	killTurnProcess(active)
-	<-active.readerDone
-	_ = waitForProcess(active)
-	clearActive(state, active)
-
-	return nil
+	return stopActiveTurn(ctx, active)
 }
 
 // EventStream returns nil because OpenCode events are delivered via the
@@ -695,6 +686,33 @@ func closeStop(runtime *turnRuntime) {
 	})
 }
 
+func stopActiveTurn(ctx context.Context, runtime *turnRuntime) error {
+	if runtime == nil {
+		return nil
+	}
+
+	closeStop(runtime)
+	if runtime.proc == nil {
+		return nil
+	}
+
+	_ = procutil.SignalGraceful(runtime.proc.Pid) //nolint:errcheck // best-effort signal; process may already be dead
+
+	graceTimer := time.NewTimer(5 * time.Second)
+	defer stopTimer(graceTimer)
+
+	select {
+	case <-runtime.waitCh:
+		return nil
+	case <-graceTimer.C:
+		killTurnProcess(runtime)
+		return nil
+	case <-ctx.Done():
+		killTurnProcess(runtime)
+		return ctx.Err()
+	}
+}
+
 func killTurnProcess(runtime *turnRuntime) {
 	if runtime == nil || runtime.proc == nil {
 		return

internal/agent/opencode/opencode_test.go

@@ -164,6 +164,9 @@ func TestStartSession_ResumeSession(t *testing.T) {
 	if state.sessionID != resumeID {
 		t.Errorf("sessionID = %q, want %q", state.sessionID, resumeID)
 	}
+	if session.ID != resumeID {
+		t.Errorf("session.ID = %q, want %q", session.ID, resumeID)
+	}
 }
 
 func TestRunTurn_WrongInternalType(t *testing.T) {
@@ -314,6 +317,73 @@ func TestStopSession_NoActiveTurn(t *testing.T) {
 	}
 }
 
+func TestStopSession_ContextDeadline(t *testing.T) {
+	t.Parallel()
+
+	testCtx, testCancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer testCancel()
+
+	tmpDir := t.TempDir()
+	script := writeOpenCodeScript(t, tmpDir, `case "$1" in
+  export) echo '{"messages":[]}'; exit 0;;
+esac
+trap '' TERM
+printf '{"type":"step_start","timestamp":1000,"sessionID":"ses_abc123","part":{"id":"p1","messageID":"m1","sessionID":"ses_abc123","snapshot":"","type":"step-start"}}\n'
+while :; do sleep 1; done`)
+
+	a, _ := NewOpenCodeAdapter(map[string]any{})
+	session, err := a.StartSession(testCtx, domain.StartSessionParams{
+		WorkspacePath: tmpDir,
+		AgentConfig:   domain.AgentConfig{Command: script},
+	})
+	if err != nil {
+		t.Fatalf("StartSession() error = %v", err)
+	}
+
+	gotEvent := make(chan struct{}, 1)
+	resultCh := make(chan domain.TurnResult, 1)
+	errCh := make(chan error, 1)
+	go func() {
+		result, runErr := a.RunTurn(context.Background(), session, domain.RunTurnParams{
+			Prompt: "work",
+			OnEvent: func(_ domain.AgentEvent) {
+				select {
+				case gotEvent <- struct{}{}:
+				default:
+				}
+			},
+		})
+		resultCh <- result
+		errCh <- runErr
+	}()
+
+	select {
+	case <-gotEvent:
+	case <-testCtx.Done():
+		t.Fatal("timed out waiting for first event")
+	}
+
+	stopCtx, stopCancel := context.WithTimeout(testCtx, 50*time.Millisecond)
+	defer stopCancel()
+
+	err = a.StopSession(stopCtx, session)
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("StopSession() error = %v, want %v", err, context.DeadlineExceeded)
+	}
+
+	select {
+	case result := <-resultCh:
+		if result.ExitReason != domain.EventTurnCancelled {
+			t.Errorf("ExitReason = %q, want %q", result.ExitReason, domain.EventTurnCancelled)
+		}
+		if runErr := <-errCh; runErr != nil {
+			t.Errorf("RunTurn() error = %v, want nil", runErr)
+		}
+	case <-testCtx.Done():
+		t.Fatal("RunTurn did not return after StopSession timeout")
+	}
+}
+
 func TestStopSession_WrongInternalType(t *testing.T) {
 	t.Parallel()
 

internal/agent/opencode/parse.go

@@ -178,7 +178,7 @@ func queryExportUsage(ctx context.Context, state *sessionState) exportUsage {
 
 	managedEnv, err := buildManagedEnv(state.passthrough)
 	if err != nil {
-		state.logger().Warn("failed to build opencode export command", slog.Any("error", err))
+		state.logger().Warn("failed to build opencode managed environment", slog.Any("error", err))
 		return exportUsage{}
 	}