diff --git a/cmd/entire/cli/agent/opencode/entire_plugin.ts b/cmd/entire/cli/agent/opencode/entire_plugin.ts index e311645ac..5ccc2d9d9 100644 --- a/cmd/entire/cli/agent/opencode/entire_plugin.ts +++ b/cmd/entire/cli/agent/opencode/entire_plugin.ts @@ -96,7 +96,7 @@ export const EntirePlugin: Plugin = async ({ $, directory }) => { // session.idle is deprecated and not reliably emitted in run mode. const props = (event as any).properties if (props?.status?.type !== "idle") break - const sessionID = props?.sessionID + const sessionID = props?.sessionID ?? currentSessionID if (!sessionID) break // Use sync variant: `opencode run` exits on the same idle event, // so an async hook would be killed before completing. diff --git a/cmd/entire/cli/agent/opencode/lifecycle.go b/cmd/entire/cli/agent/opencode/lifecycle.go index 9e0337065..57904ead2 100644 --- a/cmd/entire/cli/agent/opencode/lifecycle.go +++ b/cmd/entire/cli/agent/opencode/lifecycle.go @@ -12,6 +12,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/validation" ) // Hook name constants — these become CLI subcommands under `entire hooks opencode`. @@ -53,13 +54,10 @@ func (a *OpenCodeAgent) ParseHookEvent(ctx context.Context, hookName string, std if err != nil { return nil, err } - // Get the temp file path for this session (may not exist yet, but needed for pre-prompt state). - repoRoot, err := paths.WorktreeRoot(ctx) + transcriptPath, err := sessionTranscriptPath(ctx, raw.SessionID) if err != nil { - repoRoot = "." + return nil, err } - tmpDir := filepath.Join(repoRoot, paths.EntireTmpDir) - transcriptPath := filepath.Join(tmpDir, raw.SessionID+".json") return &agent.Event{ Type: agent.TurnStart, SessionID: raw.SessionID, @@ -73,10 +71,10 @@ func (a *OpenCodeAgent) ParseHookEvent(ctx context.Context, hookName string, std if err != nil { return nil, err } - // Call `opencode export` to get the transcript and write to temp file - transcriptPath, exportErr := a.fetchAndCacheExport(ctx, raw.SessionID) - if exportErr != nil { - return nil, fmt.Errorf("failed to export session: %w", exportErr) + // Export is deferred to PrepareTranscript; we just compute the path here. + transcriptPath, err := sessionTranscriptPath(ctx, raw.SessionID) + if err != nil { + return nil, err } return &agent.Event{ Type: agent.TurnEnd, @@ -141,6 +139,18 @@ func (a *OpenCodeAgent) PrepareTranscript(ctx context.Context, sessionRef string return err } +// sessionTranscriptPath validates the session ID and returns the expected transcript path. +func sessionTranscriptPath(ctx context.Context, sessionID string) (string, error) { + if err := validation.ValidateSessionID(sessionID); err != nil { + return "", fmt.Errorf("invalid session ID for transcript path: %w", err) + } + repoRoot, err := paths.WorktreeRoot(ctx) + if err != nil { + repoRoot = "." + } + return filepath.Join(repoRoot, paths.EntireTmpDir, sessionID+".json"), nil +} + // fetchAndCacheExport calls `opencode export ` and writes the result // to a temporary file. Returns the path to the temp file. // @@ -149,6 +159,10 @@ func (a *OpenCodeAgent) PrepareTranscript(ctx context.Context, sessionRef string // pre-write the transcript file to .entire/tmp/.json before // triggering the hook. See integration_test/hooks.go:SimulateOpenCodeTurnEnd. func (a *OpenCodeAgent) fetchAndCacheExport(ctx context.Context, sessionID string) (string, error) { + if err := validation.ValidateSessionID(sessionID); err != nil { + return "", fmt.Errorf("invalid session ID for export: %w", err) + } + // Get worktree root for the temp directory repoRoot, err := paths.WorktreeRoot(ctx) if err != nil { diff --git a/cmd/entire/cli/agent/opencode/lifecycle_test.go b/cmd/entire/cli/agent/opencode/lifecycle_test.go index 665eccea9..5d3a6a88a 100644 --- a/cmd/entire/cli/agent/opencode/lifecycle_test.go +++ b/cmd/entire/cli/agent/opencode/lifecycle_test.go @@ -67,11 +67,30 @@ func TestParseHookEvent_TurnStart(t *testing.T) { } } -// TestParseHookEvent_TurnEnd is skipped because it requires `opencode export` to be available. -// The TurnEnd handler calls `opencode export` to fetch the transcript, which won't work in unit tests. -// Integration tests cover the full TurnEnd flow. -func TestParseHookEvent_TurnEnd_RequiresOpenCode(t *testing.T) { - t.Skip("TurnEnd requires opencode CLI - tested in integration tests") +func TestParseHookEvent_TurnEnd(t *testing.T) { + t.Parallel() + + ag := &OpenCodeAgent{} + input := `{"session_id": "sess-2"}` + + event, err := ag.ParseHookEvent(context.Background(), HookNameTurnEnd, strings.NewReader(input)) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if event == nil { + t.Fatal("expected event, got nil") + } + if event.Type != agent.TurnEnd { + t.Errorf("expected TurnEnd, got %v", event.Type) + } + if event.SessionID != "sess-2" { + t.Errorf("expected session_id 'sess-2', got %q", event.SessionID) + } + // SessionRef is computed from session_id, should end with .json (same pattern as TurnStart) + if !strings.HasSuffix(event.SessionRef, "sess-2.json") { + t.Errorf("expected session ref to end with 'sess-2.json', got %q", event.SessionRef) + } } func TestParseHookEvent_Compaction(t *testing.T) { @@ -210,12 +229,9 @@ func TestHookNames(t *testing.T) { func TestPrepareTranscript_AlwaysRefreshesTranscript(t *testing.T) { t.Parallel() - // PrepareTranscript should always call fetchAndCacheExport to get fresh data, - // even when the file exists. This ensures resumed sessions get updated transcripts. - // In production, fetchAndCacheExport calls `opencode export`. - // In mock mode (ENTIRE_TEST_OPENCODE_MOCK_EXPORT=1), it reads from .entire/tmp/. - // Without mock mode and without opencode CLI, it will fail - which is expected. - + // PrepareTranscript should always attempt to refresh via fetchAndCacheExport, + // even when the file already exists. Without opencode CLI or mock mode, + // this means it must return an error (proving it tried to refresh). tmpDir := t.TempDir() transcriptPath := filepath.Join(tmpDir, "sess-123.json") @@ -227,15 +243,13 @@ func TestPrepareTranscript_AlwaysRefreshesTranscript(t *testing.T) { ag := &OpenCodeAgent{} err := ag.PrepareTranscript(context.Background(), transcriptPath) - // Without ENTIRE_TEST_OPENCODE_MOCK_EXPORT and without opencode CLI installed, - // PrepareTranscript will fail because fetchAndCacheExport can't run `opencode export`. - // This is expected behavior - the point is that it TRIES to refresh, not that it no-ops. + // Without opencode CLI, fetchAndCacheExport must fail — proving it attempted a refresh + // rather than short-circuiting because the file exists. if err == nil { - // If no error, either opencode CLI is installed or mock mode is enabled - t.Log("PrepareTranscript succeeded (opencode CLI available or mock mode enabled)") - } else { - // Expected: fails because we're not in mock mode and opencode CLI isn't installed - t.Logf("PrepareTranscript attempted refresh and failed (expected without opencode CLI): %v", err) + t.Fatal("expected error (refresh attempt should fail without opencode CLI), got nil") + } + if !strings.Contains(err.Error(), "opencode export failed") { + t.Errorf("expected 'opencode export failed' error, got: %v", err) } } @@ -257,11 +271,11 @@ func TestPrepareTranscript_ErrorOnInvalidPath(t *testing.T) { func TestPrepareTranscript_ErrorOnBrokenSymlink(t *testing.T) { t.Parallel() - // Create a broken symlink to test non-IsNotExist error handling + // Broken symlinks cause os.Stat to return a non-IsNotExist error. + // PrepareTranscript should surface this as a stat error. tmpDir := t.TempDir() transcriptPath := filepath.Join(tmpDir, "broken-link.json") - // Create symlink pointing to non-existent target if err := os.Symlink("/nonexistent/target", transcriptPath); err != nil { t.Skipf("cannot create symlink (permission denied?): %v", err) } @@ -269,20 +283,8 @@ func TestPrepareTranscript_ErrorOnBrokenSymlink(t *testing.T) { ag := &OpenCodeAgent{} err := ag.PrepareTranscript(context.Background(), transcriptPath) - // Broken symlinks cause os.Stat to return a specific error (not IsNotExist). - // The function should return a wrapped error explaining the issue. - // Note: On some systems, symlink to nonexistent target returns IsNotExist, - // so we accept either behavior here. - switch { - case err != nil && strings.Contains(err.Error(), "failed to stat OpenCode transcript path"): - // Good: proper error handling for broken symlink - t.Logf("Got expected stat error for broken symlink: %v", err) - case err != nil: - // Also acceptable: fetchAndCacheExport fails for other reasons - t.Logf("Got error (acceptable): %v", err) - default: - // Unexpected: should have gotten an error - t.Log("No error returned - symlink may have been treated as non-existent") + if err == nil { + t.Fatal("expected error for broken symlink, got nil") } } @@ -300,3 +302,35 @@ func TestPrepareTranscript_ErrorOnEmptySessionID(t *testing.T) { t.Errorf("expected 'empty session ID' error, got: %v", err) } } + +func TestParseHookEvent_TurnStart_InvalidSessionID(t *testing.T) { + t.Parallel() + + ag := &OpenCodeAgent{} + input := `{"session_id": "../escape", "prompt": "hello"}` + + _, err := ag.ParseHookEvent(context.Background(), HookNameTurnStart, strings.NewReader(input)) + + if err == nil { + t.Fatal("expected error for path-traversal session ID") + } + if !strings.Contains(err.Error(), "contains path separators") { + t.Errorf("expected 'contains path separators' error, got: %v", err) + } +} + +func TestParseHookEvent_TurnEnd_InvalidSessionID(t *testing.T) { + t.Parallel() + + ag := &OpenCodeAgent{} + input := `{"session_id": "../escape"}` + + _, err := ag.ParseHookEvent(context.Background(), HookNameTurnEnd, strings.NewReader(input)) + + if err == nil { + t.Fatal("expected error for path-traversal session ID") + } + if !strings.Contains(err.Error(), "contains path separators") { + t.Errorf("expected 'contains path separators' error, got: %v", err) + } +} diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 87015ee40..de567eb9f 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -171,6 +171,19 @@ func handleLifecycleTurnEnd(ctx context.Context, ag agent.Agent, event *agent.Ev if transcriptRef == "" { return errors.New("transcript file not specified") } + + // If agent implements TranscriptPreparer, materialize the transcript file. + // This must run BEFORE fileExists: agents like OpenCode lazily fetch transcripts + // via `opencode export`, so the file doesn't exist until PrepareTranscript creates it. + // Claude Code's PrepareTranscript just flushes (always succeeds). Agents without + // TranscriptPreparer (Gemini, Droid) are unaffected. + if preparer, ok := ag.(agent.TranscriptPreparer); ok { + if err := preparer.PrepareTranscript(ctx, transcriptRef); err != nil { + logging.Warn(logCtx, "failed to prepare transcript", + slog.String("error", err.Error())) + } + } + if !fileExists(transcriptRef) { return fmt.Errorf("transcript file not found: %s", transcriptRef) } @@ -191,14 +204,6 @@ func handleLifecycleTurnEnd(ctx context.Context, ag agent.Agent, event *agent.Ev return fmt.Errorf("failed to create session directory: %w", err) } - // If agent implements TranscriptPreparer, wait for transcript to be ready - if preparer, ok := ag.(agent.TranscriptPreparer); ok { - if err := preparer.PrepareTranscript(ctx, transcriptRef); err != nil { - logging.Warn(logCtx, "failed to prepare transcript", - slog.String("error", err.Error())) - } - } - // Copy transcript to session directory transcriptData, err := ag.ReadTranscript(transcriptRef) if err != nil { diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index d3f0000c2..d6aa6ff93 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -214,6 +214,65 @@ func TestHandleLifecycleTurnEnd_NonexistentTranscript(t *testing.T) { } } +// mockPreparerAgent is a mock that implements TranscriptPreparer. +// It creates the transcript file when PrepareTranscript is called, +// simulating OpenCode's lazy-fetch behavior. +type mockPreparerAgent struct { + mockLifecycleAgent + + prepareTranscriptCalled bool +} + +var _ agent.TranscriptPreparer = (*mockPreparerAgent)(nil) + +func (m *mockPreparerAgent) PrepareTranscript(_ context.Context, sessionRef string) error { + m.prepareTranscriptCalled = true + // Create the file (simulating opencode export writing to disk) + if err := os.MkdirAll(filepath.Dir(sessionRef), 0o750); err != nil { + return err + } + return os.WriteFile(sessionRef, m.transcriptData, 0o600) +} + +func TestHandleLifecycleTurnEnd_PreparerCreatesFile(t *testing.T) { + // Cannot use t.Parallel() because we use t.Chdir() + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + setupGitRepoWithCommit(t, tmpDir) + paths.ClearWorktreeRootCache() + + // Transcript file does NOT exist yet — PrepareTranscript should create it + transcriptPath := filepath.Join(tmpDir, ".entire", "tmp", "sess-lazy.json") + + ag := &mockPreparerAgent{ + mockLifecycleAgent: mockLifecycleAgent{ + name: "mock-preparer", + agentType: "Mock Preparer Agent", + transcriptData: []byte(`{"type":"user","message":"test"}`), + }, + } + event := &agent.Event{ + Type: agent.TurnEnd, + SessionID: "sess-lazy", + SessionRef: transcriptPath, + Timestamp: time.Now(), + } + + err := handleLifecycleTurnEnd(context.Background(), ag, event) + + // PrepareTranscript should have been called + if !ag.prepareTranscriptCalled { + t.Error("expected PrepareTranscript to be called") + } + + // The handler may fail later (no strategy state, etc), but it should NOT + // fail with "transcript file not found" — that was the bug. + if err != nil && strings.Contains(err.Error(), "transcript file not found") { + t.Errorf("handler failed with 'transcript file not found' — PrepareTranscript was not called before fileExists check: %v", err) + } +} + func TestHandleLifecycleTurnEnd_EmptyRepository(t *testing.T) { // Cannot use t.Parallel() because we use t.Chdir() tmpDir := t.TempDir() diff --git a/cmd/entire/cli/validation/validators.go b/cmd/entire/cli/validation/validators.go index 3cc1baad2..ab286442c 100644 --- a/cmd/entire/cli/validation/validators.go +++ b/cmd/entire/cli/validation/validators.go @@ -13,10 +13,11 @@ import ( // Used to validate IDs that will be used in file paths. var pathSafeRegex = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) -// ValidateSessionID validates that a session ID doesn't contain path separators. +// ValidateSessionID validates that a session ID doesn't contain path separators +// or other unsafe characters for use in file paths. // This prevents path traversal attacks when session IDs are used in file paths. func ValidateSessionID(id string) error { - if id == "" { + if strings.TrimSpace(id) == "" { return errors.New("session ID cannot be empty") } if strings.ContainsAny(id, "/\\") { diff --git a/cmd/entire/cli/validation/validators_test.go b/cmd/entire/cli/validation/validators_test.go index 8ab5a86a4..7cf029671 100644 --- a/cmd/entire/cli/validation/validators_test.go +++ b/cmd/entire/cli/validation/validators_test.go @@ -28,13 +28,19 @@ func TestValidateSessionID(t *testing.T) { sessionID: "session-2026.01.25_test@123", wantErr: false, }, - // Empty string (security-critical) + // Empty/whitespace-only (security-critical) { name: "empty session ID", sessionID: "", wantErr: true, errMsg: "session ID cannot be empty", }, + { + name: "whitespace-only session ID", + sessionID: " ", + wantErr: true, + errMsg: "session ID cannot be empty", + }, // Path separators (security-critical - path traversal prevention) { name: "session ID with forward slash",