diff --git a/cmd/entire/cli/agent/geminicli/lifecycle.go b/cmd/entire/cli/agent/geminicli/lifecycle.go index f9117e6603..c0631ef2a0 100644 --- a/cmd/entire/cli/agent/geminicli/lifecycle.go +++ b/cmd/entire/cli/agent/geminicli/lifecycle.go @@ -18,14 +18,21 @@ var ( _ agent.HookResponseWriter = (*GeminiCLIAgent)(nil) ) -// WriteHookResponse outputs a JSON hook response to stdout. -// Gemini CLI reads this JSON and displays the systemMessage to the user. +// WriteHookResponse outputs a hook response message as plain text to stdout. +// +// Why plain text and not JSON? Gemini CLI (as of v0.40.0) double-displays +// systemMessage when it arrives in JSON form: once via emitHookSystemMessage +// (rendered with the [hookName] source tag) and again via the SessionStart +// path's direct historyManager.addItem (rendered without a tag). With plain +// text, gemini's convertPlainTextToHookOutput synthesizes a systemMessage +// internally, the JSON-only emitHookSystemMessage event doesn't fire, and +// the user sees the banner exactly once. func (g *GeminiCLIAgent) WriteHookResponse(message string) error { - resp := struct { - SystemMessage string `json:"systemMessage,omitempty"` - }{SystemMessage: message} - if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { - return fmt.Errorf("failed to encode hook response: %w", err) + if message == "" { + return nil + } + if _, err := fmt.Fprintln(os.Stdout, message); err != nil { + return fmt.Errorf("failed to write hook response: %w", err) } return nil } diff --git a/cmd/entire/cli/agent/geminicli/lifecycle_test.go b/cmd/entire/cli/agent/geminicli/lifecycle_test.go index dfbac91396..098cb25266 100644 --- a/cmd/entire/cli/agent/geminicli/lifecycle_test.go +++ b/cmd/entire/cli/agent/geminicli/lifecycle_test.go @@ -2,6 +2,8 @@ package geminicli import ( "context" + "io" + "os" "strings" "testing" @@ -471,3 +473,50 @@ func TestReadAndParse_AgentHookInput(t *testing.T) { t.Errorf("expected hook_event_name 'before-agent', got %q", result.HookEventName) } } + +// captureStdout swaps os.Stdout for a pipe, runs fn, and returns what was +// written. Sequential (no t.Parallel) because os.Stdout is process-global. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + require.NoError(t, err) + + original := os.Stdout + os.Stdout = w + + done := make(chan []byte, 1) + go func() { + data, _ := io.ReadAll(r) //nolint:errcheck // best-effort drain + done <- data + }() + + fn() + require.NoError(t, w.Close()) + os.Stdout = original + got := <-done + require.NoError(t, r.Close()) + return string(got) +} + +// TestWriteHookResponse_PlainText_NoJSON verifies the response is emitted as +// plain text (not JSON). Gemini CLI v0.40.0 double-displays JSON systemMessage +// (once with the [hookName] tag, once without) — plain text takes only the +// non-tagged path so the user sees the banner once. +func TestWriteHookResponse_PlainText_NoJSON(t *testing.T) { + ag := &GeminiCLIAgent{} + out := captureStdout(t, func() { + require.NoError(t, ag.WriteHookResponse("hello banner")) + }) + + require.Equal(t, "hello banner\n", out, "expected exact plain-text output (no JSON envelope)") + require.False(t, strings.HasPrefix(strings.TrimSpace(out), "{"), + "output must not start with '{' — gemini's JSON parser would route it through the duplicate-display path") +} + +func TestWriteHookResponse_EmptyMessage_WritesNothing(t *testing.T) { + ag := &GeminiCLIAgent{} + out := captureStdout(t, func() { + require.NoError(t, ag.WriteHookResponse("")) + }) + require.Empty(t, out, "empty message should produce no output") +} diff --git a/cmd/entire/cli/agent/registry.go b/cmd/entire/cli/agent/registry.go index 2651af2ed9..af0aa4e94a 100644 --- a/cmd/entire/cli/agent/registry.go +++ b/cmd/entire/cli/agent/registry.go @@ -3,7 +3,10 @@ package agent import ( "context" "fmt" + "path/filepath" + "runtime" "slices" + "strings" "sync" "github.com/entireio/cli/cmd/entire/cli/agent/types" @@ -98,6 +101,68 @@ func Detect(ctx context.Context) (Agent, error) { return detected[0], nil } +// AgentForTranscriptPath returns the registered agent whose session directory +// for repoPath contains the given transcript path. Used to disambiguate which +// agent owns a session when multiple agents' hooks fire for the same session +// ID — a Cursor transcript path uniquely identifies a Cursor session even +// when Claude Code's hook is the one firing. +// +// Returns (nil, false) if transcriptPath is empty, no agent claims it, or any +// registry lookup fails. Match is by directory prefix (with a separator) so +// "/x/.claude/projects/abc.jsonl" doesn't accidentally match an agent rooted +// at "/x/.claude/projects/ab". +// +//nolint:revive // AgentForTranscriptPath: stutter is intentional for package callers (agent.AgentForTranscriptPath reads naturally) +func AgentForTranscriptPath(transcriptPath, repoPath string) (Agent, bool) { + if transcriptPath == "" { + return nil, false + } + abs, err := filepath.Abs(transcriptPath) + if err != nil { + abs = transcriptPath + } + for _, name := range List() { + ag, err := Get(name) + if err != nil { + continue + } + dir, err := ag.GetSessionDir(repoPath) + if err != nil || dir == "" { + continue + } + dirAbs, err := filepath.Abs(dir) + if err != nil { + dirAbs = dir + } + if pathHasDirPrefix(abs, dirAbs) { + return ag, true + } + } + return nil, false +} + +// pathHasDirPrefix reports whether path is contained within dir (or equals it). +// Adds a trailing separator before prefix-matching so /a/bc doesn't match /a/b. +// +// On Windows, comparison is case-insensitive: NTFS/ReFS treat paths as +// case-insensitive, and filepath.Abs preserves whatever casing the input had, +// so a transcript path like `C:\Users\Bob\.cursor\...` and a session dir like +// `c:\users\bob\.cursor\...` refer to the same location but would not match +// under a byte-wise comparison. +func pathHasDirPrefix(path, dir string) bool { + if runtime.GOOS == "windows" { + path = strings.ToLower(path) + dir = strings.ToLower(dir) + } + if path == dir { + return true + } + if !strings.HasSuffix(dir, string(filepath.Separator)) { + dir += string(filepath.Separator) + } + return strings.HasPrefix(path, dir) +} + // Agent name constants (registry keys) const ( AgentNameClaudeCode types.AgentName = "claude-code" diff --git a/cmd/entire/cli/agent/registry_test.go b/cmd/entire/cli/agent/registry_test.go index 52515196b0..ceb33eca6d 100644 --- a/cmd/entire/cli/agent/registry_test.go +++ b/cmd/entire/cli/agent/registry_test.go @@ -2,6 +2,7 @@ package agent import ( "context" + "runtime" "strings" "testing" @@ -134,6 +135,134 @@ func (d *detectableAgent) DetectPresence(_ context.Context) (bool, error) { return true, nil } +// sessionDirAgent is a mock with a configurable session dir, for path-prefix tests. +type sessionDirAgent struct { + mockAgent + + name types.AgentName + agentType types.AgentType + sessionDir string +} + +func (s *sessionDirAgent) Name() types.AgentName { return s.name } +func (s *sessionDirAgent) Type() types.AgentType { return s.agentType } +func (s *sessionDirAgent) GetSessionDir(_ string) (string, error) { return s.sessionDir, nil } + +func TestAgentForTranscriptPath(t *testing.T) { + originalRegistry := make(map[types.AgentName]Factory) + registryMu.Lock() + for k, v := range registry { + originalRegistry[k] = v + } + registry = make(map[types.AgentName]Factory) + registryMu.Unlock() + t.Cleanup(func() { + registryMu.Lock() + registry = originalRegistry + registryMu.Unlock() + }) + + cursor := &sessionDirAgent{ + name: types.AgentName("cursor"), + agentType: types.AgentType("Cursor"), + sessionDir: "/home/u/.cursor/projects/repo/agent-transcripts", + } + claude := &sessionDirAgent{ + name: types.AgentName("claude-code"), + agentType: types.AgentType("Claude Code"), + sessionDir: "/home/u/.claude/projects/repo", + } + Register(cursor.name, func() Agent { return cursor }) + Register(claude.name, func() Agent { return claude }) + + cases := []struct { + name string + transcript string + wantAgent types.AgentType + wantOK bool + }{ + { + name: "cursor IDE nested layout", + transcript: "/home/u/.cursor/projects/repo/agent-transcripts/abc/abc.jsonl", + wantAgent: cursor.Type(), + wantOK: true, + }, + { + name: "cursor CLI flat layout", + transcript: "/home/u/.cursor/projects/repo/agent-transcripts/abc.jsonl", + wantAgent: cursor.Type(), + wantOK: true, + }, + { + name: "claude code transcript", + transcript: "/home/u/.claude/projects/repo/abc.jsonl", + wantAgent: claude.Type(), + wantOK: true, + }, + { + name: "empty transcript path returns false", + transcript: "", + wantOK: false, + }, + { + name: "unrelated path returns false", + transcript: "/home/u/somewhere/else/transcript.jsonl", + wantOK: false, + }, + { + name: "directory-prefix collision is rejected", + // Without a separator-aware prefix check, this would erroneously + // match an agent rooted at /home/u/.cursor/projects/rep. + transcript: "/home/u/.cursor/projects/repository/agent-transcripts/x.jsonl", + wantOK: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ag, ok := AgentForTranscriptPath(tc.transcript, "/repo") + if ok != tc.wantOK { + t.Fatalf("ok = %v, want %v", ok, tc.wantOK) + } + if !tc.wantOK { + return + } + if ag.Type() != tc.wantAgent { + t.Errorf("agent = %q, want %q", ag.Type(), tc.wantAgent) + } + }) + } +} + +// TestPathHasDirPrefix_CaseSensitivity verifies the platform-dependent +// case-handling of pathHasDirPrefix. On Windows, NTFS/ReFS are case- +// insensitive and filepath.Abs preserves whatever casing the input had, so +// the transcript-path override must match across casing differences. On Unix +// the comparison stays case-sensitive (different cases are different files). +func TestPathHasDirPrefix_CaseSensitivity(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + // Mixed-case paths that refer to the same NTFS location must match. + if !pathHasDirPrefix(`C:\Users\Bob\.cursor\projects\repo\agent-transcripts\abc.jsonl`, + `c:\users\bob\.cursor\projects\repo\agent-transcripts`) { + t.Errorf("expected case-insensitive match on Windows for mixed-case prefix") + } + // Equality with different casing should also match. + if !pathHasDirPrefix(`C:\Users\Bob\.cursor\projects\repo`, + `c:\users\bob\.cursor\projects\repo`) { + t.Errorf("expected case-insensitive equality match on Windows") + } + return + } + + // Unix: case-sensitive — different casing means different files. + if pathHasDirPrefix("/Home/u/.cursor/projects/repo/x.jsonl", + "/home/u/.cursor/projects/repo") { + t.Errorf("expected case-sensitive comparison on %s", runtime.GOOS) + } +} + func TestAgentNameConstants(t *testing.T) { if AgentNameClaudeCode != "claude-code" { t.Errorf("expected AgentNameClaudeCode %q, got %q", "claude-code", AgentNameClaudeCode) diff --git a/cmd/entire/cli/integration_test/cursor_forwarding_test.go b/cmd/entire/cli/integration_test/cursor_forwarding_test.go new file mode 100644 index 0000000000..de2390bfc9 --- /dev/null +++ b/cmd/entire/cli/integration_test/cursor_forwarding_test.go @@ -0,0 +1,101 @@ +//go:build integration + +package integration + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestDispatcher_ForwardedStopFromNonOwnerIsSkipped verifies the dispatcher +// skip end-to-end through the real `entire hooks claude-code stop` binary +// invocation. Cursor IDE forwards Stop to both .cursor/hooks.json and +// .claude/settings.json — when the SessionState records Cursor as the owner, +// the claude-code-side hook must no-op so we don't double-write checkpoints +// and metadata. +func TestDispatcher_ForwardedStopFromNonOwnerIsSkipped(t *testing.T) { + t.Parallel() + env := NewRepoWithCommit(t) + + sessionID := "test-cursor-forward-stop" + statePath := filepath.Join(env.RepoDir, ".git", "entire-sessions", sessionID+".json") + require.NoError(t, os.MkdirAll(filepath.Dir(statePath), 0o755)) + + // Pre-record state with AgentType=Cursor: the firing claude-code hook + // should be recognized as a forwarded duplicate. + initialState := map[string]any{ + "session_id": sessionID, + "agent_type": "Cursor", + "base_commit": env.GetHeadHash(), + "started_at": time.Now().Format(time.RFC3339Nano), + "step_count": 0, + } + initialBytes, err := json.Marshal(initialState) + require.NoError(t, err) + require.NoError(t, os.WriteFile(statePath, initialBytes, 0o600)) + + // Modify a file so the stop handler would normally have a checkpoint to write. + env.WriteFile("changed.txt", "modification") + + session := env.NewSession() + session.ID = sessionID + session.CreateTranscript("change a file", []FileChange{ + {Path: "changed.txt", Content: "modification"}, + }) + + require.NoError(t, env.SimulateStop(sessionID, session.TranscriptPath)) + + // If the dispatcher had let the event through, SaveStep would have run and + // step_count would have advanced. The skip leaves the state file untouched. + afterBytes, err := os.ReadFile(statePath) + require.NoError(t, err) + + var after map[string]any + require.NoError(t, json.Unmarshal(afterBytes, &after)) + + require.Equal(t, "Cursor", after["agent_type"], "AgentType must remain Cursor") + require.EqualValues(t, 0, after["step_count"], "StepCount must not advance when the stop hook was skipped") +} + +// TestDispatcher_ForwardedSessionEndFromNonOwnerIsSkipped is the SessionEnd +// variant. Cursor IDE also forwards sessionEnd to .claude/settings.json. When +// the owning agent is Cursor, the claude-code SessionEnd must not transition +// the session to ENDED, eager-condense, or alter step counts. +func TestDispatcher_ForwardedSessionEndFromNonOwnerIsSkipped(t *testing.T) { + t.Parallel() + env := NewRepoWithCommit(t) + + sessionID := "test-cursor-forward-sessionend" + statePath := filepath.Join(env.RepoDir, ".git", "entire-sessions", sessionID+".json") + require.NoError(t, os.MkdirAll(filepath.Dir(statePath), 0o755)) + + initialState := map[string]any{ + "session_id": sessionID, + "agent_type": "Cursor", + "base_commit": env.GetHeadHash(), + "started_at": time.Now().Format(time.RFC3339Nano), + "step_count": 0, + } + initialBytes, err := json.Marshal(initialState) + require.NoError(t, err) + require.NoError(t, os.WriteFile(statePath, initialBytes, 0o600)) + + require.NoError(t, env.SimulateSessionEnd(sessionID)) + + afterBytes, err := os.ReadFile(statePath) + require.NoError(t, err) + + var after map[string]any + require.NoError(t, json.Unmarshal(afterBytes, &after)) + + // markSessionEnded would have set ended_at and transitioned phase to "ended". + require.Nil(t, after["ended_at"], "ended_at must remain nil when SessionEnd was skipped") + if phase, ok := after["phase"]; ok { + require.NotEqual(t, "ended", phase, "phase must not transition to ENDED when SessionEnd was skipped") + } +} diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 6d3fee047e..ec42d0d6c9 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -28,6 +28,17 @@ import ( "github.com/entireio/cli/perf" ) +// eventBypassesAgentOwnershipCheck reports whether an event must run +// regardless of the recorded session-owning agent: +// - SessionStart fires before SessionState exists; the hint file dedup +// in handleLifecycleSessionStart already prevents a duplicate banner. +// - TurnStart needs to reach InitializeSession so transcript-path +// resolution can repair a wrongly-set AgentType. Skipping here would +// lock in a bad state. +func eventBypassesAgentOwnershipCheck(t agent.EventType) bool { + return t == agent.SessionStart || t == agent.TurnStart +} + // DispatchLifecycleEvent routes a normalized lifecycle event to the appropriate handler. // Returns nil if the event was handled successfully. func DispatchLifecycleEvent(ctx context.Context, ag agent.Agent, event *agent.Event) error { @@ -38,6 +49,23 @@ func DispatchLifecycleEvent(ctx context.Context, ag agent.Agent, event *agent.Ev return errors.New("event cannot be nil") } + // Filter forwarded hooks: when Cursor IDE forwards events to both + // .cursor/hooks.json and .claude/settings.json, only the agent that owns + // the session should process them — otherwise checkpoints, metadata + // writes, and step counts double. + if event.SessionID != "" && !eventBypassesAgentOwnershipCheck(event.Type) { + if state, _ := strategy.LoadSessionState(ctx, event.SessionID); state != nil && state.AgentType != "" && state.AgentType != ag.Type() { //nolint:errcheck // a load failure means we can't filter; let the event reach its handler, which surfaces its own load error + logging.Info(logging.WithAgent(logging.WithComponent(ctx, "lifecycle"), ag.Name()), + "skipping forwarded hook for non-owning agent", + slog.String("event", event.Type.String()), + slog.String("session_id", event.SessionID), + slog.String("owning_agent", string(state.AgentType)), + slog.String("firing_agent", string(ag.Type())), + ) + return nil + } + } + switch event.Type { case agent.SessionStart: return handleLifecycleSessionStart(ctx, ag, event) @@ -78,6 +106,16 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age return fmt.Errorf("invalid %s event: %w", event.Type, err) } + // Claim the session for this agent. First-writer-wins: subsequent agents + // firing SessionStart for the same session ID are no-ops. Used by + // InitializeSession (TurnStart) and the dispatcher skip in + // DispatchLifecycleEvent for cross-agent disambiguation when Cursor IDE + // forwards hooks to both .cursor/hooks.json and .claude/settings.json. + if _, hintErr := strategy.StoreAgentTypeHint(ctx, event.SessionID, ag.Type()); hintErr != nil { + logging.Warn(logCtx, "failed to store agent hint on session start", + slog.String("error", hintErr.Error())) + } + // Build informational message — warn early if repo has no commits yet, // since checkpoints require at least one commit to work. message := sessionStartMessage(ag.Name(), false) @@ -100,15 +138,30 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age // Output informational message if the agent supports hook responses. // Claude Code reads JSON from stdout; agents that don't implement // HookResponseWriter silently skip (avoids raw JSON in their terminal). + // + // Banner display is gated by ClaimSessionStartBanner — separate from the + // agent-ownership claim above. If the ownership winner can't write banners + // (Cursor), we'd suppress the banner entirely on a Cursor+Claude race; + // the banner marker is only claimed inside this branch so a non-writer + // winner can't consume the user's only banner. _, hookResponseSpan := perf.Start(ctx, "write_hook_response") if event.ResponseMessage != "" { message = event.ResponseMessage } if writer, ok := agent.AsHookResponseWriter(ag); ok { - if err := writer.WriteHookResponse(message); err != nil { - hookResponseSpan.RecordError(err) - hookResponseSpan.End() - return fmt.Errorf("failed to write hook response: %w", err) + bannerFirst, bErr := strategy.ClaimSessionStartBanner(ctx, event.SessionID) + if bErr != nil { + // Better to duplicate the banner than to suppress the only one. + logging.Warn(logCtx, "failed to claim session start banner marker", + slog.String("error", bErr.Error())) + bannerFirst = true + } + if bannerFirst { + if err := writer.WriteHookResponse(message); err != nil { + hookResponseSpan.RecordError(err) + hookResponseSpan.End() + return fmt.Errorf("failed to write hook response: %w", err) + } } } hookResponseSpan.End() diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index 0875f9d951..5886a12331 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -118,6 +118,214 @@ func TestDispatchLifecycleEvent_NilEvent(t *testing.T) { } } +// TestDispatchLifecycleEvent_SkipsForwardedHookFromNonOwningAgent verifies the +// dispatcher-level dedup: when SessionState records a different owning agent, +// non-SessionStart / non-TurnStart events from forwarded hooks no-op. This +// covers the Cursor IDE → .claude/settings.json forwarding scenario for Stop, +// SubagentStart/End, Compaction, SessionEnd, and ModelUpdate events. +func TestDispatchLifecycleEvent_SkipsForwardedHookFromNonOwningAgent(t *testing.T) { + setupStopTestRepo(t) + + sessionID := "test-skip-nonowning" + require.NoError(t, strategy.SaveSessionState(context.Background(), &strategy.SessionState{ + SessionID: sessionID, + AgentType: agent.AgentTypeCursor, + BaseCommit: "abc123", + StartedAt: time.Now(), + })) + + // Claude Code fires SessionEnd for Cursor's session (Cursor IDE forwarded hook). + claudeAgent := newMockAgent() + claudeAgent.agentType = agent.AgentTypeClaudeCode + + require.NoError(t, DispatchLifecycleEvent(context.Background(), claudeAgent, &agent.Event{ + Type: agent.SessionEnd, + SessionID: sessionID, + Timestamp: time.Now(), + })) + + // If the dispatcher had let the event through, markSessionEnded would have + // transitioned to ENDED and set EndedAt. + state, err := strategy.LoadSessionState(context.Background(), sessionID) + require.NoError(t, err) + require.NotNil(t, state) + require.Nil(t, state.EndedAt, "non-owning agent's SessionEnd must not transition the session") +} + +// TestDispatchLifecycleEvent_AllowsTurnStartFromMismatchedAgent verifies that +// TurnStart bypasses the dispatcher-level skip so InitializeSession runs (and +// can repair a wrongly-set AgentType via transcript-path resolution). +func TestDispatchLifecycleEvent_AllowsTurnStartFromMismatchedAgent(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + repo, err := strategy.OpenRepository(ctx) + require.NoError(t, err) + head, err := repo.Head() + require.NoError(t, err) + + sessionID := "test-turnstart-mismatch" + require.NoError(t, strategy.SaveSessionState(ctx, &strategy.SessionState{ + SessionID: sessionID, + AgentType: agent.AgentTypeClaudeCode, + BaseCommit: head.Hash().String(), + StartedAt: time.Now(), + })) + + cursorAgent := newMockAgent() + cursorAgent.agentType = agent.AgentTypeCursor + + require.NoError(t, DispatchLifecycleEvent(ctx, cursorAgent, &agent.Event{ + Type: agent.TurnStart, + SessionID: sessionID, + Timestamp: time.Now(), + })) + + // InitializeSession generates a fresh TurnID on every dispatch. If the + // dispatcher had skipped, TurnID would still be empty. + state, err := strategy.LoadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + require.NotEmpty(t, state.TurnID, "TurnStart must dispatch (and generate a TurnID) even when the firing agent disagrees with the recorded owner") +} + +// TestDispatchLifecycleEvent_SkipsAllNonBypassEventsFromNonOwner verifies the +// skip applies uniformly to every non-bypass event type. If the dispatcher +// had let any of these through, downstream handlers would either error +// (transcript file not found, etc.) or mutate state — both are detectable. +func TestDispatchLifecycleEvent_SkipsAllNonBypassEventsFromNonOwner(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-skip-all-events" + require.NoError(t, strategy.SaveSessionState(ctx, &strategy.SessionState{ + SessionID: sessionID, + AgentType: agent.AgentTypeCursor, + BaseCommit: "abc123", + StartedAt: time.Now(), + ModelName: "initial-model", + })) + + nonOwner := newMockAgent() + nonOwner.agentType = agent.AgentTypeClaudeCode + + skipEligible := []agent.EventType{ + agent.TurnEnd, + agent.Compaction, + agent.SubagentStart, + agent.SubagentEnd, + agent.ModelUpdate, + agent.SessionEnd, + } + + for _, et := range skipEligible { + t.Run(et.String(), func(t *testing.T) { + err := DispatchLifecycleEvent(ctx, nonOwner, &agent.Event{ + Type: et, + SessionID: sessionID, + SessionRef: "/nonexistent/transcript.jsonl", // would fail in handler + Model: "would-overwrite-on-modelupdate", + Timestamp: time.Now(), + }) + require.NoError(t, err, "skip must return nil; downstream handler would have errored on missing transcript") + }) + } + + // Side-effect assertions: the handlers most likely to mutate state never ran. + state, err := strategy.LoadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + require.Nil(t, state.EndedAt, "SessionEnd skipped: EndedAt should remain nil") + require.Equal(t, "initial-model", state.ModelName, "ModelUpdate skipped: ModelName should not have been overwritten") +} + +// TestDispatchLifecycleEvent_DoesNotSkipWhenOwnerMatches verifies that when +// the firing agent IS the recorded owner, the event runs normally. +func TestDispatchLifecycleEvent_DoesNotSkipWhenOwnerMatches(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-owner-match" + require.NoError(t, strategy.SaveSessionState(ctx, &strategy.SessionState{ + SessionID: sessionID, + AgentType: agent.AgentTypeCursor, + BaseCommit: "abc123", + StartedAt: time.Now(), + })) + + owner := newMockAgent() + owner.agentType = agent.AgentTypeCursor + + require.NoError(t, DispatchLifecycleEvent(ctx, owner, &agent.Event{ + Type: agent.SessionEnd, + SessionID: sessionID, + Timestamp: time.Now(), + })) + + // Owner's SessionEnd must run markSessionEnded → EndedAt is set. + state, err := strategy.LoadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + require.NotNil(t, state.EndedAt, "SessionEnd from the owning agent must transition the session") +} + +// TestDispatchLifecycleEvent_DoesNotSkipWhenAgentTypeUnset verifies the early +// bootstrap window: SessionStart fired but TurnStart hasn't yet, so +// state.AgentType is empty. The skip must NOT engage in this state. +func TestDispatchLifecycleEvent_DoesNotSkipWhenAgentTypeUnset(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-agenttype-unset" + require.NoError(t, strategy.SaveSessionState(ctx, &strategy.SessionState{ + SessionID: sessionID, + AgentType: "", // unset + BaseCommit: "abc123", + StartedAt: time.Now(), + })) + + ag := newMockAgent() + ag.agentType = agent.AgentTypeClaudeCode + + require.NoError(t, DispatchLifecycleEvent(ctx, ag, &agent.Event{ + Type: agent.SessionEnd, + SessionID: sessionID, + Timestamp: time.Now(), + })) + + // Without a recorded owner, the dispatcher cannot tell who is forwarded; + // the event must reach the handler. + state, err := strategy.LoadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + require.NotNil(t, state.EndedAt, "with no recorded owner, SessionEnd must run regardless of firing agent") +} + +func TestEventBypassesAgentOwnershipCheck(t *testing.T) { + t.Parallel() + + bypassed := []agent.EventType{agent.SessionStart, agent.TurnStart} + for _, et := range bypassed { + if !eventBypassesAgentOwnershipCheck(et) { + t.Errorf("%s must bypass the ownership check", et) + } + } + + notBypassed := []agent.EventType{ + agent.TurnEnd, + agent.Compaction, + agent.SubagentStart, + agent.SubagentEnd, + agent.ModelUpdate, + agent.SessionEnd, + } + for _, et := range notBypassed { + if eventBypassesAgentOwnershipCheck(et) { + t.Errorf("%s must be subject to the ownership check", et) + } + } +} + func TestDispatchLifecycleEvent_UnknownEventType(t *testing.T) { t.Parallel() @@ -179,6 +387,139 @@ func newMockHookResponseAgent() *mockHookResponseAgent { } } +// TestHandleLifecycleSessionStart_StoresAgentTypeHint verifies the +// SessionStart hook claims the session for its agent so a wrapper agent's +// later TurnStart hook (e.g., Cursor IDE forwarding to Claude Code's hook +// system) cannot re-label the session. +func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { + setupStopTestRepo(t) + + ag := newMockHookResponseAgent() + ag.agentType = agent.AgentTypeCursor + event := &agent.Event{ + Type: agent.SessionStart, + SessionID: "test-agent-hint", + Timestamp: time.Now(), + } + require.NoError(t, handleLifecycleSessionStart(context.Background(), ag, event)) + + got := strategy.LoadAgentTypeHint(context.Background(), "test-agent-hint") + require.Equal(t, agent.AgentTypeCursor, got) +} + +// TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins verifies that +// when multiple agents fire SessionStart for the same session ID, only the +// first agent's claim is recorded AND only the first emits the banner. This +// matches both the Cursor cross-agent and the Gemini repeat-source +// (startup → resume) cases — the user must see the banner only once. +func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-agent-hint-race" + + first := newMockHookResponseAgent() + first.agentType = agent.AgentTypeCursor + require.NoError(t, handleLifecycleSessionStart(ctx, first, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.NotEmpty(t, first.lastMessage, "first SessionStart must emit the banner") + + second := newMockHookResponseAgent() + second.agentType = agent.AgentTypeClaudeCode + require.NoError(t, handleLifecycleSessionStart(ctx, second, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.Empty(t, second.lastMessage, "subsequent SessionStarts for the same session must not emit the banner again") + + got := strategy.LoadAgentTypeHint(ctx, sessionID) + require.Equal(t, agent.AgentTypeCursor, got, "first SessionStart caller must own the session") +} + +// TestHandleLifecycleSessionStart_NonWriterClaimDoesNotSuppressBanner covers +// the Cursor + Claude Code forwarding race: Cursor IDE forwards SessionStart +// to both .cursor/hooks.json (Cursor agent — no HookResponseWriter) and +// .claude/settings.json (Claude Code — has HookResponseWriter). When Cursor +// wins the ownership claim, Claude Code must still emit the banner; otherwise +// the user sees nothing ~50% of the time (the original Bugbot finding). +func TestHandleLifecycleSessionStart_NonWriterClaimDoesNotSuppressBanner(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-non-writer-claim" + + // Non-writer agent (Cursor) wins the ownership race. + nonWriter := newMockAgent() + nonWriter.agentType = agent.AgentTypeCursor + require.NoError(t, handleLifecycleSessionStart(ctx, nonWriter, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + + // Writer-capable agent (Claude Code) fires SessionStart for the same session. + writer := newMockHookResponseAgent() + writer.agentType = agent.AgentTypeClaudeCode + require.NoError(t, handleLifecycleSessionStart(ctx, writer, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.NotEmpty(t, writer.lastMessage, + "banner-capable agent must emit the banner even after a non-writer claimed ownership") + + // Ownership still belongs to whoever called StoreAgentTypeHint first. + require.Equal(t, agent.AgentTypeCursor, strategy.LoadAgentTypeHint(ctx, sessionID), + "first SessionStart caller still owns the session") +} + +// TestHandleLifecycleSessionStart_BannerClaimedOnce verifies that once a +// banner-capable agent has shown the banner, a subsequent banner-capable +// agent firing SessionStart for the same session ID does not duplicate it. +func TestHandleLifecycleSessionStart_BannerClaimedOnce(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-banner-claimed-once" + + first := newMockHookResponseAgent() + first.agentType = agent.AgentTypeClaudeCode + require.NoError(t, handleLifecycleSessionStart(ctx, first, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.NotEmpty(t, first.lastMessage) + + second := newMockHookResponseAgent() + second.agentType = agent.AgentTypeGemini + require.NoError(t, handleLifecycleSessionStart(ctx, second, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.Empty(t, second.lastMessage, + "banner must not be re-emitted once a writer agent has shown it") +} + +// TestHandleLifecycleSessionStart_GeminiRepeatSourceDoesNotDuplicate covers +// the specific case the user reported: Gemini fires SessionStart twice for +// the same session (e.g., source=startup followed by source=resume) and we +// were emitting the banner both times. +func TestHandleLifecycleSessionStart_GeminiRepeatSourceDoesNotDuplicate(t *testing.T) { + setupStopTestRepo(t) + + ctx := context.Background() + sessionID := "test-gemini-repeat" + + ag := newMockHookResponseAgent() + ag.agentType = agent.AgentTypeGemini + + require.NoError(t, handleLifecycleSessionStart(ctx, ag, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + first := ag.lastMessage + require.NotEmpty(t, first) + + ag.lastMessage = "" + require.NoError(t, handleLifecycleSessionStart(ctx, ag, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + require.Empty(t, ag.lastMessage, "second SessionStart from the same agent must not re-emit the banner") +} + func TestHandleLifecycleSessionStart_EmptyRepoWarning(t *testing.T) { // Cannot use t.Parallel() because we use t.Chdir() tmpDir := t.TempDir() diff --git a/cmd/entire/cli/strategy/agent_resolution_test.go b/cmd/entire/cli/strategy/agent_resolution_test.go new file mode 100644 index 0000000000..7086377b0f --- /dev/null +++ b/cmd/entire/cli/strategy/agent_resolution_test.go @@ -0,0 +1,238 @@ +package strategy + +import ( + "context" + "path/filepath" + "sync" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/agent" + + // Register agents so AgentForTranscriptPath can resolve them. + _ "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" + _ "github.com/entireio/cli/cmd/entire/cli/agent/cursor" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// withCursorSessionDir points the Cursor agent at a temp directory and returns +// a sample transcript path inside it. +func withCursorSessionDir(t *testing.T) string { + t.Helper() + sessionDir := filepath.Join(t.TempDir(), "agent-transcripts") + t.Setenv("ENTIRE_TEST_CURSOR_PROJECT_DIR", sessionDir) + return filepath.Join(sessionDir, "abc-123.jsonl") +} + +// withClaudeSessionDir does the same for Claude Code. +func withClaudeSessionDir(t *testing.T) string { + t.Helper() + sessionDir := filepath.Join(t.TempDir(), "claude-projects") + t.Setenv("ENTIRE_TEST_CLAUDE_PROJECT_DIR", sessionDir) + return filepath.Join(sessionDir, "abc-123.jsonl") +} + +func TestResolveSessionAgentType_TranscriptPathBeatsHook(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + got := resolveSessionAgentType(context.Background(), "test-session-1", agent.AgentTypeClaudeCode, cursorTranscript) + assert.Equal(t, agent.AgentTypeCursor, got, + "transcript path inside Cursor's session dir must override the firing hook's agent type") +} + +func TestResolveSessionAgentType_HintBeatsHookWhenNoTranscript(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + ctx := context.Background() + _, err := StoreAgentTypeHint(ctx, "test-session-2", agent.AgentTypeCursor) + require.NoError(t, err) + + got := resolveSessionAgentType(ctx, "test-session-2", agent.AgentTypeClaudeCode, "") + assert.Equal(t, agent.AgentTypeCursor, got, + "SessionStart hint must override the firing hook's agent type when no transcript path is given") +} + +func TestResolveSessionAgentType_FallsBackToHook(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + got := resolveSessionAgentType(context.Background(), "test-session-3", agent.AgentTypeClaudeCode, "/somewhere/unrelated.jsonl") + assert.Equal(t, agent.AgentTypeClaudeCode, got, + "with no hint and an unrelated transcript path, the hook's agent type wins") +} + +func TestResolveSessionAgentType_TranscriptOverridesEvenWithHint(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + ctx := context.Background() + // Wrong-first-writer scenario: Claude Code's SessionStart fired first by accident. + _, err := StoreAgentTypeHint(ctx, "test-session-4", agent.AgentTypeClaudeCode) + require.NoError(t, err) + + got := resolveSessionAgentType(ctx, "test-session-4", agent.AgentTypeClaudeCode, cursorTranscript) + assert.Equal(t, agent.AgentTypeCursor, got, + "transcript path is the strongest signal — even an existing (wrong) hint must lose to it") +} + +func TestCorrectSessionAgentType_TranscriptDisagrees(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + corrected, changed := correctSessionAgentType(context.Background(), agent.AgentTypeClaudeCode, cursorTranscript) + assert.True(t, changed) + assert.Equal(t, agent.AgentTypeCursor, corrected) +} + +func TestCorrectSessionAgentType_TranscriptAgrees_NoChange(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + corrected, changed := correctSessionAgentType(context.Background(), agent.AgentTypeCursor, cursorTranscript) + assert.False(t, changed) + assert.Equal(t, agent.AgentTypeCursor, corrected) +} + +func TestCorrectSessionAgentType_NoTranscript_NoChange(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + corrected, changed := correctSessionAgentType(context.Background(), agent.AgentTypeClaudeCode, "") + assert.False(t, changed) + assert.Equal(t, agent.AgentTypeClaudeCode, corrected) +} + +// TestInitializeSession_HintWinsRaceAtTurnStart simulates the bug: +// Cursor fires SessionStart first (wins the hint). Claude Code's TurnStart fires +// first (without transcript path). The session must still be recorded as Cursor. +func TestInitializeSession_HintWinsRaceAtTurnStart(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "test-session-hint-race" + + // SessionStart phase: Cursor fires first, then Claude Code (no-op). + created, err := StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor) + require.NoError(t, err) + require.True(t, created) + created, err = StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode) + require.NoError(t, err) + require.False(t, created) + + // TurnStart phase: Claude Code fires first with no transcript path (the bug condition). + s := &ManualCommitStrategy{} + err = s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "first prompt", "") + require.NoError(t, err) + + state, err := s.loadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + assert.Equal(t, agent.AgentTypeCursor, state.AgentType, + "the SessionStart hint should claim the session for Cursor even when Claude Code's TurnStart fires first") +} + +// TestInitializeSession_TranscriptPathRepairsExistingState simulates a session +// already recorded with the wrong AgentType. Cursor's TurnStart fires with a +// Cursor transcript path on a subsequent turn — state.AgentType must be repaired. +func TestInitializeSession_TranscriptPathRepairsExistingState(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + ctx := context.Background() + sessionID := "test-session-repair" + + // Bootstrap an already-wrong session. + s := &ManualCommitStrategy{} + require.NoError(t, s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "first prompt", "")) + state, err := s.loadSessionState(ctx, sessionID) + require.NoError(t, err) + require.Equal(t, agent.AgentTypeClaudeCode, state.AgentType) + + // Subsequent turn arrives via Cursor's hook with a Cursor transcript path. + err = s.InitializeSession(ctx, sessionID, agent.AgentTypeCursor, cursorTranscript, "second prompt", "") + require.NoError(t, err) + + state, err = s.loadSessionState(ctx, sessionID) + require.NoError(t, err) + assert.Equal(t, agent.AgentTypeCursor, state.AgentType, + "a turn whose transcript path lives in Cursor's session dir must repair the wrong agent type") + assert.Equal(t, cursorTranscript, state.TranscriptPath) +} + +// TestInitializeSession_ConcurrentRaceRepairsOnNextTurn simulates the bug +// from the field: two processes (Cursor IDE forwarding the prompt to both +// .cursor/hooks.json and .claude/settings.json) call InitializeSession +// concurrently for the same session ID. We do not serialize them — the +// race may leave AgentType wrong for a single turn (both goroutines see +// no existing state and run the "initialize new session" path; last +// writer wins). The contract is eventual consistency: the next turn that +// arrives with the cursor transcript path repairs AgentType via +// correctSessionAgentType. +func TestInitializeSession_ConcurrentRaceRepairsOnNextTurn(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + cursorTranscript := withCursorSessionDir(t) + + ctx := context.Background() + sessionID := "test-session-concurrent" + + // Reproduce the SessionStart hint race where Claude Code happened to win. + _, err := StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode) + require.NoError(t, err) + + // Two parallel hook processes: claude-code (no transcript) and cursor + // (with transcript). Errors are non-fatal — without serialization the + // goroutines may step on each other's writes; convergence is asserted + // via the next-turn call below. + s := &ManualCommitStrategy{} + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + _ = s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "prompt", "") //nolint:errcheck // see comment above + }() + go func() { + defer wg.Done() + _ = s.InitializeSession(ctx, sessionID, agent.AgentTypeCursor, cursorTranscript, "prompt", "") //nolint:errcheck // see comment above + }() + wg.Wait() + + // Simulate the next turn: cursor's hook fires again with its transcript + // path. correctSessionAgentType repairs any wrong AgentType from the race. + require.NoError(t, s.InitializeSession(ctx, sessionID, agent.AgentTypeCursor, cursorTranscript, "next prompt", "")) + + state, lErr := s.loadSessionState(ctx, sessionID) + require.NoError(t, lErr) + require.NotNil(t, state) + assert.Equal(t, agent.AgentTypeCursor, state.AgentType, + "the transcript-bearing turn must repair AgentType to Cursor regardless of how the prior race resolved") +} + +// TestInitializeSession_ClaudeCodeTranscriptKeepsClaudeCode verifies the negative case: +// a Claude Code transcript path doesn't get rewritten just because some other agent's +// hook also fired earlier. +func TestInitializeSession_ClaudeCodeTranscriptKeepsClaudeCode(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + claudeTranscript := withClaudeSessionDir(t) + + ctx := context.Background() + sessionID := "test-session-claude" + + s := &ManualCommitStrategy{} + err := s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, claudeTranscript, "first prompt", "") + require.NoError(t, err) + + state, err := s.loadSessionState(ctx, sessionID) + require.NoError(t, err) + assert.Equal(t, agent.AgentTypeClaudeCode, state.AgentType) +} diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index 06905aba17..32da8ad64f 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -2143,6 +2143,58 @@ func addCheckpointTrailerWithComment(message string, checkpointID id.CheckpointI return userContent + "\n\n" + trailer + "\n" + comment + "\n\n" + gitComments } +// resolveSessionAgentType picks the most reliable identifier for which agent +// owns a session, given the agent whose hook is firing right now (callerAgentType), +// an optional transcript path, and the SessionStart hint stored under the +// session ID. +// +// Priority (strongest signal wins): +// 1. Transcript path — when transcriptPath is inside a registered agent's +// GetSessionDir(repoRoot), the file's location is direct evidence. +// 2. SessionStart hint — first agent to fire SessionStart for this session. +// Useful when transcriptPath is empty (e.g., Claude Code's first TurnStart +// in a forwarded-hook scenario). +// 3. callerAgentType — the agent whose hook called us. Used when no stronger +// signal exists. +func resolveSessionAgentType(ctx context.Context, sessionID string, callerAgentType types.AgentType, transcriptPath string) types.AgentType { + if repoRoot, err := paths.WorktreeRoot(ctx); err == nil && transcriptPath != "" { + if owner, ok := agent.AgentForTranscriptPath(transcriptPath, repoRoot); ok { + return owner.Type() + } + } + if hint := LoadAgentTypeHint(ctx, sessionID); hint != "" && hint != agent.AgentTypeUnknown { + return hint + } + return callerAgentType +} + +// correctSessionAgentType returns the agent type that owns transcriptPath when +// it disagrees with currentType. Returns (currentType, false) if there's no +// disagreement or no transcript signal — callers should treat that as a no-op. +// +// Used to repair sessions whose AgentType was set incorrectly on an earlier +// turn (e.g., a wrapper agent's hook fired first before the real agent's +// transcript path was available). Only the transcript path is considered; +// the SessionStart hint is intentionally ignored here because it can also be +// the wrong-first-writer that we're trying to correct. +func correctSessionAgentType(ctx context.Context, currentType types.AgentType, transcriptPath string) (types.AgentType, bool) { + if transcriptPath == "" { + return currentType, false + } + repoRoot, err := paths.WorktreeRoot(ctx) + if err != nil { + return currentType, false + } + owner, ok := agent.AgentForTranscriptPath(transcriptPath, repoRoot) + if !ok { + return currentType, false + } + if owner.Type() == currentType { + return currentType, false + } + return owner.Type(), true +} + // InitializeSession creates session state for a new session or updates an existing one. // This implements the optional SessionInitializer interface. // Called during UserPromptSubmit to allow git hooks to detect active sessions. @@ -2158,11 +2210,21 @@ func addCheckpointTrailerWithComment(message string, checkpointID id.CheckpointI // userPrompt is the user's prompt text (stored truncated as LastPrompt for display). // model is the LLM model identifier (e.g., "claude-sonnet-4-20250514"); empty if unknown. func (s *ManualCommitStrategy) InitializeSession(ctx context.Context, sessionID string, agentType types.AgentType, transcriptPath string, userPrompt string, model string) error { + // Concurrent calls for the same session_id are accepted; convergence on + // AgentType is via correctSessionAgentType on the next turn. repo, err := OpenRepository(ctx) if err != nil { return fmt.Errorf("failed to open git repository: %w", err) } + // Resolve which agent actually owns this session. The hook firing isn't + // authoritative when multiple agents' hooks fire for the same session ID + // (e.g., Cursor IDE forwarding to both .cursor/hooks.json and + // .claude/settings.json). Stronger signals: transcript path (file lives in + // a specific agent's session dir) and the SessionStart hint (first agent + // to claim the session ID). + resolvedAgentType := resolveSessionAgentType(ctx, sessionID, agentType, transcriptPath) + // Check if session already exists state, err := s.loadSessionState(ctx, sessionID) if err != nil { @@ -2184,9 +2246,20 @@ func (s *ManualCommitStrategy) InitializeSession(ctx context.Context, sessionID } state.TurnID = turnID.String() - // Set AgentType from hook context if not yet set - if state.AgentType == "" && agentType != "" { - state.AgentType = agentType + // Update AgentType when: + // - it isn't yet set, or + // - the resolved type came from a stronger signal (transcript path) + // than what's currently stored. correctSessionAgentType handles the + // "transcript path proves we're a different agent" case. + if state.AgentType == "" && resolvedAgentType != "" { + state.AgentType = resolvedAgentType + } else if corrected, changed := correctSessionAgentType(ctx, state.AgentType, transcriptPath); changed { + logging.Info(logging.WithComponent(ctx, "hooks"), "corrected session agent type from transcript path", + slog.String("session_id", sessionID), + slog.String("from", string(state.AgentType)), + slog.String("to", string(corrected)), + slog.String("transcript_path", transcriptPath)) + state.AgentType = corrected } // Update ModelName if provided (model can change between turns) @@ -2256,7 +2329,7 @@ func (s *ManualCommitStrategy) InitializeSession(ctx context.Context, sessionID // Continue below to properly initialize it // Initialize new session - state, err = s.initializeSession(ctx, repo, sessionID, agentType, transcriptPath, userPrompt, model) + state, err = s.initializeSession(ctx, repo, sessionID, resolvedAgentType, transcriptPath, userPrompt, model) if err != nil { return fmt.Errorf("failed to initialize session: %w", err) } diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index a541604828..b160dc3875 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -2,12 +2,15 @@ package strategy import ( "context" + "errors" "fmt" "log/slog" "os" "path/filepath" "strings" + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/types" "github.com/entireio/cli/cmd/entire/cli/jsonutil" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" @@ -267,6 +270,125 @@ func LoadModelHint(ctx context.Context, sessionID string) string { return strings.TrimSpace(string(data)) } +// StoreAgentTypeHint records the agent type that owns a session before +// SessionState exists. Used by the lifecycle dispatcher when SessionStart fires +// (state isn't created until TurnStart, so we need a place to remember which +// agent claimed the session first). +// +// Semantics: first writer wins. When multiple agents fire hooks for the same +// session ID — e.g., Cursor IDE running cursor-agent while also forwarding to +// Claude Code's hook system — only the agent that fires SessionStart first +// gets recorded. Subsequent calls return nil without overwriting. +// +// At TurnStart, InitializeSession reads this hint to override agentType when +// the hook firing isn't the same agent that owns the session. After the state +// file is written, the hint is unused but remains until ClearSessionState +// removes it alongside the state file. +// +// Returns (created=true) when this call wrote the hint, (created=false) when +// the hint already existed (no-op) or agentType was empty/Unknown. +// +// Banner display is gated separately via ClaimSessionStartBanner — winning +// the ownership claim does NOT mean this agent should also print the banner, +// because the winner may not implement HookResponseWriter (e.g., Cursor). +func StoreAgentTypeHint(ctx context.Context, sessionID string, agentType types.AgentType) (created bool, err error) { + if vErr := validation.ValidateSessionID(sessionID); vErr != nil { + return false, fmt.Errorf("invalid session ID: %w", vErr) + } + if agentType == "" || agentType == agent.AgentTypeUnknown { + return false, nil + } + + stateDir, sErr := getSessionStateDir(ctx) + if sErr != nil { + return false, fmt.Errorf("failed to get session state directory: %w", sErr) + } + if mErr := os.MkdirAll(stateDir, 0o750); mErr != nil { + return false, fmt.Errorf("failed to create session state directory: %w", mErr) + } + + hintFile := filepath.Join(stateDir, sessionID+".agent") + f, oErr := os.OpenFile(hintFile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // hintFile path is built from validated sessionID + if oErr != nil { + if errors.Is(oErr, os.ErrExist) { + // First-writer-wins: another caller already claimed this session. + return false, nil + } + return false, fmt.Errorf("failed to create agent hint file: %w", oErr) + } + defer f.Close() + if _, wErr := f.WriteString(string(agentType)); wErr != nil { + return false, fmt.Errorf("failed to write agent hint file: %w", wErr) + } + return true, nil +} + +// ClaimSessionStartBanner records that the SessionStart banner has been emitted +// for a session. First-writer-wins semantics, separate from StoreAgentTypeHint +// so a non-banner-capable agent winning the ownership race (e.g. Cursor, which +// doesn't implement HookResponseWriter) doesn't suppress the banner from a +// banner-capable agent that fires SessionStart for the same session. +// +// Callers MUST only invoke this from within the HookResponseWriter branch — the +// claim represents "a banner was actually shown", not just "an agent considered +// showing one". Otherwise a non-writer claimant would re-introduce the bug. +// +// Returns (claimed=true) when this call won the race and the caller should +// emit the banner; (claimed=false) when an earlier call already claimed it. +func ClaimSessionStartBanner(ctx context.Context, sessionID string) (claimed bool, err error) { + if vErr := validation.ValidateSessionID(sessionID); vErr != nil { + return false, fmt.Errorf("invalid session ID: %w", vErr) + } + + stateDir, sErr := getSessionStateDir(ctx) + if sErr != nil { + return false, fmt.Errorf("failed to get session state directory: %w", sErr) + } + if mErr := os.MkdirAll(stateDir, 0o750); mErr != nil { + return false, fmt.Errorf("failed to create session state directory: %w", mErr) + } + + markerFile := filepath.Join(stateDir, sessionID+".banner") + f, oErr := os.OpenFile(markerFile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // markerFile path is built from validated sessionID + if oErr != nil { + if errors.Is(oErr, os.ErrExist) { + return false, nil + } + return false, fmt.Errorf("failed to create banner marker file: %w", oErr) + } + _ = f.Close() + return true, nil +} + +// LoadAgentTypeHint reads the agent type hint written by SessionStart. +// Returns empty string if the hint file doesn't exist, can't be read, or the +// value isn't a registered agent type. +func LoadAgentTypeHint(ctx context.Context, sessionID string) types.AgentType { + if err := validation.ValidateSessionID(sessionID); err != nil { + return "" + } + + stateDir, err := getSessionStateDir(ctx) + if err != nil { + logging.Warn(logging.WithComponent(ctx, "session"), "failed to resolve state dir for agent hint", + slog.String("session_id", sessionID), + slog.Any("error", err)) + return "" + } + + hintPath := filepath.Join(stateDir, sessionID+".agent") + data, err := os.ReadFile(hintPath) //nolint:gosec // sessionID is validated above + if err != nil { + if !os.IsNotExist(err) { + logging.Warn(logging.WithComponent(ctx, "session"), "failed to read agent hint file", + slog.String("path", hintPath), + slog.Any("error", err)) + } + return "" + } + return types.AgentType(strings.TrimSpace(string(data))) +} + // ClearSessionState removes the session state file for the given session ID. func ClearSessionState(ctx context.Context, sessionID string) error { // Validate session ID to prevent path traversal diff --git a/cmd/entire/cli/strategy/session_state_test.go b/cmd/entire/cli/strategy/session_state_test.go index 27a1e37c7f..ac054ee757 100644 --- a/cmd/entire/cli/strategy/session_state_test.go +++ b/cmd/entire/cli/strategy/session_state_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/types" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/session" "github.com/go-git/go-git/v6" @@ -581,6 +583,163 @@ func TestLoadModelHint_TrimsWhitespace(t *testing.T) { } } +// --- Agent type hint file tests --- + +func TestStoreAgentTypeHint_RoundTrip(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "2026-01-01-agent-roundtrip" + + created, err := StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor) + require.NoError(t, err) + require.True(t, created, "first call must report it created the hint") + + got := LoadAgentTypeHint(ctx, sessionID) + require.Equal(t, agent.AgentTypeCursor, got) +} + +func TestStoreAgentTypeHint_FirstWriterWins(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "2026-01-01-agent-firstwriter" + + // Cursor claims the session first. + created, err := StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor) + require.NoError(t, err) + require.True(t, created) + + // Claude Code's hook fires next (concurrent forwarded-hook scenario). + // Should be a no-op — does not overwrite the existing hint. + created, err = StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode) + require.NoError(t, err) + require.False(t, created, "second call must report it did not create the hint") + + got := LoadAgentTypeHint(ctx, sessionID) + require.Equal(t, agent.AgentTypeCursor, got, "first writer's hint must persist") +} + +func TestStoreAgentTypeHint_EmptyOrUnknown_NoOp(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + + for _, tc := range []struct { + sid string + at types.AgentType + }{ + {"2026-01-01-empty", ""}, + {"2026-01-01-unknown", agent.AgentTypeUnknown}, + } { + created, hErr := StoreAgentTypeHint(ctx, tc.sid, tc.at) + require.NoError(t, hErr) + require.False(t, created, "empty/Unknown must report created=false") + } + + stateDir, sdErr := getSessionStateDir(ctx) + require.NoError(t, sdErr) + + for _, sid := range []string{"2026-01-01-empty", "2026-01-01-unknown"} { + hintPath := filepath.Join(stateDir, sid+".agent") + _, statErr := os.Stat(hintPath) + require.True(t, os.IsNotExist(statErr), "no hint file should be created for empty/Unknown agent type") + } +} + +func TestLoadAgentTypeHint_NoFile_ReturnsEmpty(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + got := LoadAgentTypeHint(context.Background(), "2026-01-01-nonexistent") + require.Empty(t, string(got)) +} + +func TestStoreAgentTypeHint_InvalidSessionID_ReturnsError(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + _, err = StoreAgentTypeHint(context.Background(), "../../../etc/passwd", agent.AgentTypeCursor) + require.Error(t, err) +} + +func TestClaimSessionStartBanner_FirstWriterWins(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "2026-01-01-banner-claim" + + claimed, err := ClaimSessionStartBanner(ctx, sessionID) + require.NoError(t, err) + require.True(t, claimed, "first call must win the banner claim") + + claimed, err = ClaimSessionStartBanner(ctx, sessionID) + require.NoError(t, err) + require.False(t, claimed, "subsequent calls must report the banner already claimed") +} + +func TestClaimSessionStartBanner_InvalidSessionID_ReturnsError(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + _, err = ClaimSessionStartBanner(context.Background(), "../../../etc/passwd") + require.Error(t, err) +} + +func TestClearSessionState_RemovesBannerMarker(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "2026-01-01-clear-banner" + + _, err = ClaimSessionStartBanner(ctx, sessionID) + require.NoError(t, err) + require.NoError(t, ClearSessionState(ctx, sessionID)) + + // After clear, the marker is gone — the next claim wins again. + claimed, err := ClaimSessionStartBanner(ctx, sessionID) + require.NoError(t, err) + require.True(t, claimed, "ClearSessionState should remove the banner marker") +} + +func TestClearSessionState_RemovesAgentHint(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + require.NoError(t, err) + t.Chdir(dir) + + ctx := context.Background() + sessionID := "2026-01-01-clear-agent-hint" + + _, err = StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor) + require.NoError(t, err) + require.NoError(t, ClearSessionState(ctx, sessionID)) + + got := LoadAgentTypeHint(ctx, sessionID) + require.Empty(t, string(got)) +} + func TestClearSessionState_RemovesHintFile(t *testing.T) { dir := t.TempDir() _, err := git.PlainInit(dir, false)