From c62b2ff5bda953b64960187459b755cc65d5b7cf Mon Sep 17 00:00:00 2001 From: Thomas Dohmke Date: Thu, 30 Apr 2026 11:57:04 +0200 Subject: [PATCH 01/10] Fix Cursor session mis-identified as Claude Code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Cursor IDE forwards a single user prompt to both .cursor/hooks.json and .claude/settings.json, two `entire` hook processes race to claim the session and the loser's wrong agent_type can win the last write. Three layered defenses: - SessionStart hint: first agent to fire claims the session ID (`.agent` written with O_EXCL). - Transcript-path override: a transcript inside an agent's session dir is authoritative — overrides both hint and the firing hook. - Per-session file lock: serializes concurrent InitializeSession calls so the runner-up takes the cheap update path (`correctSessionAgentType`) instead of also creating-from-scratch. Lock files live under `/entire-locks/` to keep `entire-sessions/` clean. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: e51b741d6a56 --- cmd/entire/cli/agent/registry.go | 54 ++++ cmd/entire/cli/agent/registry_test.go | 99 ++++++++ cmd/entire/cli/filelock/filelock.go | 41 ++++ cmd/entire/cli/filelock/filelock_test.go | 87 +++++++ cmd/entire/cli/filelock/filelock_unix.go | 36 +++ cmd/entire/cli/filelock/filelock_windows.go | 37 +++ cmd/entire/cli/lifecycle.go | 10 + cmd/entire/cli/lifecycle_test.go | 57 +++++ .../cli/strategy/agent_resolution_test.go | 230 ++++++++++++++++++ .../cli/strategy/manual_commit_hooks.go | 102 +++++++- cmd/entire/cli/strategy/session_state.go | 119 +++++++++ cmd/entire/cli/strategy/session_state_test.go | 95 ++++++++ 12 files changed, 963 insertions(+), 4 deletions(-) create mode 100644 cmd/entire/cli/filelock/filelock.go create mode 100644 cmd/entire/cli/filelock/filelock_test.go create mode 100644 cmd/entire/cli/filelock/filelock_unix.go create mode 100644 cmd/entire/cli/filelock/filelock_windows.go create mode 100644 cmd/entire/cli/strategy/agent_resolution_test.go diff --git a/cmd/entire/cli/agent/registry.go b/cmd/entire/cli/agent/registry.go index 2651af2ed9..94e2e3774a 100644 --- a/cmd/entire/cli/agent/registry.go +++ b/cmd/entire/cli/agent/registry.go @@ -3,7 +3,9 @@ package agent import ( "context" "fmt" + "path/filepath" "slices" + "strings" "sync" "github.com/entireio/cli/cmd/entire/cli/agent/types" @@ -98,6 +100,58 @@ 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. +func pathHasDirPrefix(path, dir string) bool { + 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..7b86b3c961 100644 --- a/cmd/entire/cli/agent/registry_test.go +++ b/cmd/entire/cli/agent/registry_test.go @@ -134,6 +134,105 @@ 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) + } + }) + } +} + 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/filelock/filelock.go b/cmd/entire/cli/filelock/filelock.go new file mode 100644 index 0000000000..e0655c0866 --- /dev/null +++ b/cmd/entire/cli/filelock/filelock.go @@ -0,0 +1,41 @@ +// Package filelock provides a small cross-platform exclusive file lock. +// +// Used to serialize concurrent processes operating on the same per-session +// state — e.g., when Cursor IDE forwards a single user prompt to both its own +// hooks and Claude Code's, both `entire` hook processes race to initialize the +// session. Without serialization the second writer can clobber the first. +package filelock + +// Lock is held until Release is called. Calling Release is mandatory; missing +// it leaves the lock held until the process exits and the OS reclaims the +// file descriptor. +type Lock struct { + impl lockImpl +} + +// Release drops the lock and closes the backing file descriptor. +// Safe to call multiple times — subsequent calls are no-ops. +func (l *Lock) Release() error { + if l == nil || l.impl == nil { + return nil + } + err := l.impl.release() + l.impl = nil + return err +} + +// Acquire blocks until an exclusive lock on path is obtained. The path's +// parent directory must exist; the lock file itself is created if missing. +// The contents of the lock file are intentionally not used — only its +// existence as a kernel-tracked inode matters. +func Acquire(path string) (*Lock, error) { + impl, err := acquireImpl(path) + if err != nil { + return nil, err + } + return &Lock{impl: impl}, nil +} + +type lockImpl interface { + release() error +} diff --git a/cmd/entire/cli/filelock/filelock_test.go b/cmd/entire/cli/filelock/filelock_test.go new file mode 100644 index 0000000000..47212d4868 --- /dev/null +++ b/cmd/entire/cli/filelock/filelock_test.go @@ -0,0 +1,87 @@ +package filelock + +import ( + "path/filepath" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestAcquire_SerializesGoroutines(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "test.lock") + + const goroutines = 8 + var ( + holders atomic.Int32 + maxConc atomic.Int32 + finished atomic.Int32 + ) + + var wg sync.WaitGroup + for range goroutines { + wg.Add(1) + go func() { + defer wg.Done() + lock, err := Acquire(path) + if err != nil { + t.Errorf("Acquire() error = %v", err) + return + } + defer func() { + if rErr := lock.Release(); rErr != nil { + t.Errorf("Release() error = %v", rErr) + } + finished.Add(1) + }() + + // Track concurrent holders. Must always be 1 under exclusive lock. + cur := holders.Add(1) + if cur > maxConc.Load() { + maxConc.Store(cur) + } + + // Hold the lock briefly to give other goroutines a real chance to + // race for it. Without this, Goroutine 1 might release before any + // peer attempts Acquire, masking a broken lock. + time.Sleep(5 * time.Millisecond) + + holders.Add(-1) + }() + } + wg.Wait() + + if finished.Load() != goroutines { + t.Fatalf("expected %d goroutines to finish, got %d", goroutines, finished.Load()) + } + if got := maxConc.Load(); got != 1 { + t.Errorf("max concurrent holders = %d, want 1 (lock is not exclusive)", got) + } +} + +func TestRelease_Idempotent(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "test.lock") + lock, err := Acquire(path) + if err != nil { + t.Fatalf("Acquire() error = %v", err) + } + if err := lock.Release(); err != nil { + t.Fatalf("first Release() error = %v", err) + } + if err := lock.Release(); err != nil { + t.Errorf("second Release() should be a no-op, got: %v", err) + } +} + +func TestRelease_NilLock_NoOp(t *testing.T) { + t.Parallel() + + var lock *Lock + if err := lock.Release(); err != nil { + t.Errorf("Release() on nil lock should be a no-op, got: %v", err) + } +} diff --git a/cmd/entire/cli/filelock/filelock_unix.go b/cmd/entire/cli/filelock/filelock_unix.go new file mode 100644 index 0000000000..a84782530d --- /dev/null +++ b/cmd/entire/cli/filelock/filelock_unix.go @@ -0,0 +1,36 @@ +//go:build !windows + +package filelock + +import ( + "fmt" + "os" + "syscall" +) + +type unixLock struct { + f *os.File +} + +func (u *unixLock) release() error { + // Flock releases when the fd is closed. Closing alone is sufficient, + // but call LOCK_UN explicitly so the lock drops before any deferred + // fsyncs on close. + _ = syscall.Flock(int(u.f.Fd()), syscall.LOCK_UN) //nolint:errcheck,gosec // Fd() is a small descriptor; Close below also releases + if err := u.f.Close(); err != nil { + return fmt.Errorf("close lock file: %w", err) + } + return nil +} + +func acquireImpl(path string) (lockImpl, error) { + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) //nolint:gosec // path is built from caller-validated session ID + if err != nil { + return nil, fmt.Errorf("open lock file %q: %w", path, err) + } + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { //nolint:gosec // Fd() is a small descriptor + _ = f.Close() + return nil, fmt.Errorf("flock %q: %w", path, err) + } + return &unixLock{f: f}, nil +} diff --git a/cmd/entire/cli/filelock/filelock_windows.go b/cmd/entire/cli/filelock/filelock_windows.go new file mode 100644 index 0000000000..8044eeca81 --- /dev/null +++ b/cmd/entire/cli/filelock/filelock_windows.go @@ -0,0 +1,37 @@ +//go:build windows + +package filelock + +import ( + "fmt" + "os" + + "golang.org/x/sys/windows" +) + +type windowsLock struct { + f *os.File +} + +func (w *windowsLock) release() error { + overlapped := &windows.Overlapped{} + _ = windows.UnlockFileEx(windows.Handle(w.f.Fd()), 0, ^uint32(0), ^uint32(0), overlapped) //nolint:errcheck // Close below also releases + if err := w.f.Close(); err != nil { + return fmt.Errorf("close lock file: %w", err) + } + return nil +} + +//nolint:ireturn // intentional: cross-platform interface used only by filelock.Acquire +func acquireImpl(path string) (lockImpl, error) { + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) //nolint:gosec // path is built from caller-validated session ID + if err != nil { + return nil, fmt.Errorf("open lock file %q: %w", path, err) + } + overlapped := &windows.Overlapped{} + if err := windows.LockFileEx(windows.Handle(f.Fd()), windows.LOCKFILE_EXCLUSIVE_LOCK, 0, ^uint32(0), ^uint32(0), overlapped); err != nil { + _ = f.Close() //nolint:errcheck // best-effort cleanup on lock failure + return nil, fmt.Errorf("LockFileEx %q: %w", path, err) + } + return &windowsLock{f: f}, nil +} diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 6d3fee047e..c877f19553 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -121,6 +121,16 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age } } + // Claim the session for this agent. First-writer-wins: if another agent + // (e.g., Cursor IDE forwarding hooks to both .cursor/hooks.json and + // .claude/settings.json) already claimed this session ID, this is a no-op. + // InitializeSession reads the hint at TurnStart to override the firing + // hook's agent when a different agent owns the session. + if err := strategy.StoreAgentTypeHint(ctx, event.SessionID, ag.Type()); err != nil { + logging.Warn(logCtx, "failed to store agent hint on session start", + slog.String("error", err.Error())) + } + // Fire EventSessionStart for the current session (if state exists). if state, loadErr := strategy.LoadSessionState(ctx, event.SessionID); loadErr != nil { logging.Warn(logCtx, "failed to load session state on start", diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index 0875f9d951..6bef7546c5 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -179,6 +179,63 @@ 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) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "init.txt", "init") + testutil.GitAdd(t, tmpDir, "init.txt") + testutil.GitCommit(t, tmpDir, "init") + t.Chdir(tmpDir) + paths.ClearWorktreeRootCache() + + ag := newMockHookResponseAgent() + ag.agentType = "Cursor" + 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, types.AgentType("Cursor"), got) +} + +// TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins verifies that +// when multiple agents fire SessionStart for the same session ID, only the +// first agent's claim is recorded. +func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "init.txt", "init") + testutil.GitAdd(t, tmpDir, "init.txt") + testutil.GitCommit(t, tmpDir, "init") + t.Chdir(tmpDir) + paths.ClearWorktreeRootCache() + + ctx := context.Background() + sessionID := "test-agent-hint-race" + + first := newMockHookResponseAgent() + first.agentType = "Cursor" + require.NoError(t, handleLifecycleSessionStart(ctx, first, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + + second := newMockHookResponseAgent() + second.agentType = testAgentClaude + require.NoError(t, handleLifecycleSessionStart(ctx, second, &agent.Event{ + Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), + })) + + got := strategy.LoadAgentTypeHint(ctx, sessionID) + require.Equal(t, types.AgentType("Cursor"), got, "first SessionStart caller must own the session") +} + 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..4fc6a2758c --- /dev/null +++ b/cmd/entire/cli/strategy/agent_resolution_test.go @@ -0,0 +1,230 @@ +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() + require.NoError(t, StoreAgentTypeHint(ctx, "test-session-2", agent.AgentTypeCursor)) + + 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. + require.NoError(t, StoreAgentTypeHint(ctx, "test-session-4", agent.AgentTypeClaudeCode)) + + 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). + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) // no-op + + // 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_ConcurrentCallsConvergeToTranscriptOwner 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. Without the per-session lock, the +// last writer wins. With the lock the runner-up sees the existing state and +// applies correctSessionAgentType. +// +// The transcript-bearing call is the authoritative one. Regardless of which +// goroutine wins the lock first, the final AgentType must come from the +// transcript path. +func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(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. + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) + + // Two parallel hook processes: claude-code (no transcript) and cursor + // (with transcript). Whichever grabs the lock first creates the state; + // the other follows and either matches or repairs via correctSessionAgentType. + s := &ManualCommitStrategy{} + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + // Errors here are non-fatal — the goroutines race; either may turn + // out to be a no-op or hit a benign concurrent-update edge case. + // What we assert is the final state below. + _ = 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() + + state, err := s.loadSessionState(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, state) + assert.Equal(t, agent.AgentTypeCursor, state.AgentType, + "the transcript-bearing call must determine the final AgentType regardless of goroutine ordering") +} + +// 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..095e0d6906 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,42 @@ 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 { + // Serialize concurrent InitializeSession calls for the same session ID. + // Cursor IDE forwards a single user prompt to both .cursor/hooks.json and + // .claude/settings.json, so two `entire` hook processes can race here. The + // lock ensures the runner-up sees the state file the first process wrote + // and goes through the cheap update path (correctSessionAgentType handles + // any AgentType mismatch via the transcript-path signal). + lock, lockErr := AcquireSessionLock(ctx, sessionID) + if lockErr != nil { + // Logged but not fatal — fall through and accept the existing race. + // The most we lose is the correctness improvement; pre-fix behavior. + logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to acquire session lock; proceeding without serialization", + slog.String("session_id", sessionID), + slog.String("error", lockErr.Error())) + } else { + defer func() { + if rErr := lock.Release(); rErr != nil { + logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to release session lock", + slog.String("session_id", sessionID), + slog.String("error", rErr.Error())) + } + }() + } + 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 +2267,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 +2350,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..2536f70667 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -2,12 +2,16 @@ 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/filelock" "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 +271,116 @@ func LoadModelHint(ctx context.Context, sessionID string) string { return strings.TrimSpace(string(data)) } +// AcquireSessionLock takes an exclusive per-session-id lock. Used to +// serialize concurrent processes that initialize the same session — when +// Cursor IDE forwards a single user prompt to both its own hook system and +// Claude Code's, two `entire` hook processes can otherwise each +// create-from-scratch in parallel and race for the last write. With the lock +// the second arrival sees the existing state and goes through the cheap +// update path (correctSessionAgentType repairs AgentType if the transcript +// path indicates a different agent). +// +// Lock files live under a dedicated `entire-locks/` directory in the git +// common dir — separate from `entire-sessions/` so callers that enumerate +// session state files (status, doctor, tests) don't have to filter lock +// artifacts. +func AcquireSessionLock(ctx context.Context, sessionID string) (*filelock.Lock, error) { + if err := validation.ValidateSessionID(sessionID); err != nil { + return nil, fmt.Errorf("invalid session ID: %w", err) + } + commonDir, err := GetGitCommonDir(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get git common dir: %w", err) + } + lockDir := filepath.Join(commonDir, "entire-locks") + if err := os.MkdirAll(lockDir, 0o750); err != nil { + return nil, fmt.Errorf("failed to create lock directory: %w", err) + } + lock, err := filelock.Acquire(filepath.Join(lockDir, sessionID+".lock")) + if err != nil { + return nil, fmt.Errorf("failed to acquire session lock: %w", err) + } + return lock, nil +} + +// 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 nil for empty agentType or AgentTypeUnknown (don't pollute the hint +// with non-identifying values). +func StoreAgentTypeHint(ctx context.Context, sessionID string, agentType types.AgentType) error { + if err := validation.ValidateSessionID(sessionID); err != nil { + return fmt.Errorf("invalid session ID: %w", err) + } + if agentType == "" || agentType == agent.AgentTypeUnknown { + return nil + } + + stateDir, err := getSessionStateDir(ctx) + if err != nil { + return fmt.Errorf("failed to get session state directory: %w", err) + } + if err := os.MkdirAll(stateDir, 0o750); err != nil { + return fmt.Errorf("failed to create session state directory: %w", err) + } + + hintFile := filepath.Join(stateDir, sessionID+".agent") + f, err := os.OpenFile(hintFile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // hintFile path is built from validated sessionID + if err != nil { + if errors.Is(err, os.ErrExist) { + // First-writer-wins: another agent already claimed this session. + return nil + } + return fmt.Errorf("failed to create agent hint file: %w", err) + } + defer f.Close() + if _, err := f.WriteString(string(agentType)); err != nil { + return fmt.Errorf("failed to write agent hint file: %w", err) + } + return 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 @@ -285,5 +399,10 @@ func ClearSessionState(ctx context.Context, sessionID string) error { _ = os.Remove(f) } + // Remove the lock file (lives in a separate directory; see AcquireSessionLock). + if commonDir, cdErr := GetGitCommonDir(ctx); cdErr == nil { + _ = os.Remove(filepath.Join(commonDir, "entire-locks", sessionID+".lock")) + } + return nil } diff --git a/cmd/entire/cli/strategy/session_state_test.go b/cmd/entire/cli/strategy/session_state_test.go index 27a1e37c7f..4b7fd7ab7b 100644 --- a/cmd/entire/cli/strategy/session_state_test.go +++ b/cmd/entire/cli/strategy/session_state_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/entireio/cli/cmd/entire/cli/agent" "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 +582,100 @@ 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" + + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + + 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. + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + + // Claude Code's hook fires next (concurrent forwarded-hook scenario). + // Should be a no-op — does not overwrite the existing hint. + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) + + 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() + + require.NoError(t, StoreAgentTypeHint(ctx, "2026-01-01-empty", "")) + require.NoError(t, StoreAgentTypeHint(ctx, "2026-01-01-unknown", agent.AgentTypeUnknown)) + + 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 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" + + require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + 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) From 45fecc3176b7cdcd41f019d541be7f36a6b89868 Mon Sep 17 00:00:00 2001 From: Thomas Dohmke Date: Thu, 30 Apr 2026 12:15:06 +0200 Subject: [PATCH 02/10] Allow ireturn for filelock cross-platform interface acquireImpl returns lockImpl so Acquire doesn't need build-tag-conditional code. Same pattern as the agent package. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: e4815943e53a --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index 0dcec08769..c60abb3403 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -108,6 +108,7 @@ linters: - grpc.DialOption - github.com/entireio/cli/cmd/entire/cli/agent\..+ - github.com/entireio/cli/cmd/entire/cli/checkpoint.CommittedReader + - github.com/entireio/cli/cmd/entire/cli/filelock\..+ - github.com/entireio/cli/cmd/entire/cli/strategy.Strategy - github.com/entireio/cli/cmd/entire/cli/summarize.Generator - github.com/go-git/go-git/v6/plumbing/storer.ReferenceIter From a54fdc611224316d033d59330ea3a1281489889b Mon Sep 17 00:00:00 2001 From: Thomas Dohmke Date: Thu, 30 Apr 2026 13:56:12 +0200 Subject: [PATCH 03/10] Dedupe SessionStart banner per session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gemini fires SessionStart twice on launch (once for source=startup, then for source=resume after restoring saved chat state). Both fired our hook; both emitted the "Entire CLI will link this conversation..." banner. StoreAgentTypeHint now returns (created bool, err error) — true only when the call wrote the hint file (O_EXCL). Lifecycle gates the banner on created=true, so subsequent SessionStarts for the same session ID skip the banner. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 2e86dcafd179 --- cmd/entire/cli/lifecycle.go | 44 ++++++++++-------- cmd/entire/cli/lifecycle_test.go | 38 ++++++++++++++- .../cli/strategy/agent_resolution_test.go | 23 ++++++---- cmd/entire/cli/strategy/session_state.go | 46 +++++++++++-------- cmd/entire/cli/strategy/session_state_test.go | 31 ++++++++++--- 5 files changed, 128 insertions(+), 54 deletions(-) diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index c877f19553..f8898e1139 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -78,6 +78,21 @@ 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: if another caller + // already claimed this session ID, claimedFirst=false. We use this both + // for cross-agent disambiguation (Cursor IDE forwarding hooks to both + // .cursor/hooks.json and .claude/settings.json) and to dedupe the banner + // when one agent fires SessionStart multiple times for the same session + // (Gemini fires SessionStart for source=startup AND for source=resume). + claimedFirst, hintErr := strategy.StoreAgentTypeHint(ctx, event.SessionID, ag.Type()) + if hintErr != nil { + // If we couldn't write the hint, fall back to "show the banner" — + // better to duplicate than to suppress the only banner the user sees. + logging.Warn(logCtx, "failed to store agent hint on session start", + slog.String("error", hintErr.Error())) + claimedFirst = true + } + // 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) @@ -97,18 +112,21 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age } countSessionsSpan.End() - // 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). + // Output informational message if the agent supports hook responses, but + // only on the first SessionStart for this session ID. Claude Code reads + // JSON from stdout; agents that don't implement HookResponseWriter + // silently skip (avoids raw JSON in their terminal). _, 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) + if claimedFirst { + 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) + } } } hookResponseSpan.End() @@ -121,16 +139,6 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age } } - // Claim the session for this agent. First-writer-wins: if another agent - // (e.g., Cursor IDE forwarding hooks to both .cursor/hooks.json and - // .claude/settings.json) already claimed this session ID, this is a no-op. - // InitializeSession reads the hint at TurnStart to override the firing - // hook's agent when a different agent owns the session. - if err := strategy.StoreAgentTypeHint(ctx, event.SessionID, ag.Type()); err != nil { - logging.Warn(logCtx, "failed to store agent hint on session start", - slog.String("error", err.Error())) - } - // Fire EventSessionStart for the current session (if state exists). if state, loadErr := strategy.LoadSessionState(ctx, event.SessionID); loadErr != nil { logging.Warn(logCtx, "failed to load session state on start", diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index 6bef7546c5..3a8d61c619 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -207,7 +207,9 @@ func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { // TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins verifies that // when multiple agents fire SessionStart for the same session ID, only the -// first agent's claim is recorded. +// 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) { tmpDir := t.TempDir() testutil.InitRepo(t, tmpDir) @@ -225,17 +227,51 @@ func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) 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 = testAgentClaude 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, types.AgentType("Cursor"), got, "first SessionStart caller must own the session") } +// 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) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "init.txt", "init") + testutil.GitAdd(t, tmpDir, "init.txt") + testutil.GitCommit(t, tmpDir, "init") + t.Chdir(tmpDir) + paths.ClearWorktreeRootCache() + + ctx := context.Background() + sessionID := "test-gemini-repeat" + + ag := newMockHookResponseAgent() + ag.agentType = "Gemini CLI" + + 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 index 4fc6a2758c..2a15e3cabd 100644 --- a/cmd/entire/cli/strategy/agent_resolution_test.go +++ b/cmd/entire/cli/strategy/agent_resolution_test.go @@ -48,7 +48,8 @@ func TestResolveSessionAgentType_HintBeatsHookWhenNoTranscript(t *testing.T) { t.Chdir(dir) ctx := context.Background() - require.NoError(t, StoreAgentTypeHint(ctx, "test-session-2", agent.AgentTypeCursor)) + _, 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, @@ -71,7 +72,8 @@ func TestResolveSessionAgentType_TranscriptOverridesEvenWithHint(t *testing.T) { ctx := context.Background() // Wrong-first-writer scenario: Claude Code's SessionStart fired first by accident. - require.NoError(t, StoreAgentTypeHint(ctx, "test-session-4", agent.AgentTypeClaudeCode)) + _, 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, @@ -118,12 +120,16 @@ func TestInitializeSession_HintWinsRaceAtTurnStart(t *testing.T) { sessionID := "test-session-hint-race" // SessionStart phase: Cursor fires first, then Claude Code (no-op). - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) // 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", "") + err = s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "first prompt", "") require.NoError(t, err) state, err := s.loadSessionState(ctx, sessionID) @@ -181,7 +187,8 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T sessionID := "test-session-concurrent" // Reproduce the SessionStart hint race where Claude Code happened to win. - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) + _, err := StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode) + require.NoError(t, err) // Two parallel hook processes: claude-code (no transcript) and cursor // (with transcript). Whichever grabs the lock first creates the state; @@ -202,8 +209,8 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T }() wg.Wait() - state, err := s.loadSessionState(ctx, sessionID) - require.NoError(t, err) + state, lErr := s.loadSessionState(ctx, sessionID) + require.NoError(t, lErr) require.NotNil(t, state) assert.Equal(t, agent.AgentTypeCursor, state.AgentType, "the transcript-bearing call must determine the final AgentType regardless of goroutine ordering") diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index 2536f70667..fce62631b4 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -318,38 +318,44 @@ func AcquireSessionLock(ctx context.Context, sessionID string) (*filelock.Lock, // file is written, the hint is unused but remains until ClearSessionState // removes it alongside the state file. // -// Returns nil for empty agentType or AgentTypeUnknown (don't pollute the hint -// with non-identifying values). -func StoreAgentTypeHint(ctx context.Context, sessionID string, agentType types.AgentType) error { - if err := validation.ValidateSessionID(sessionID); err != nil { - return fmt.Errorf("invalid session ID: %w", err) +// Returns (created=true) when this call wrote the hint, (created=false) when +// the hint already existed (no-op) or agentType was empty/Unknown. +// +// Callers can use the created bool to gate one-time-per-session side effects +// — e.g., the SessionStart banner: if Gemini fires SessionStart twice (once +// for source=startup, once for source=resume) we don't want to print the +// banner twice. The first call creates the hint and returns created=true; +// the second sees the hint and returns created=false. +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 nil + return false, nil } - stateDir, err := getSessionStateDir(ctx) - if err != nil { - return fmt.Errorf("failed to get session state directory: %w", err) + stateDir, sErr := getSessionStateDir(ctx) + if sErr != nil { + return false, fmt.Errorf("failed to get session state directory: %w", sErr) } - if err := os.MkdirAll(stateDir, 0o750); err != nil { - return fmt.Errorf("failed to create session state directory: %w", err) + 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, err := os.OpenFile(hintFile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // hintFile path is built from validated sessionID - if err != nil { - if errors.Is(err, os.ErrExist) { - // First-writer-wins: another agent already claimed this session. - return nil + 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 fmt.Errorf("failed to create agent hint file: %w", err) + return false, fmt.Errorf("failed to create agent hint file: %w", oErr) } defer f.Close() - if _, err := f.WriteString(string(agentType)); err != nil { - return fmt.Errorf("failed to write agent hint file: %w", err) + if _, wErr := f.WriteString(string(agentType)); wErr != nil { + return false, fmt.Errorf("failed to write agent hint file: %w", wErr) } - return nil + return true, nil } // LoadAgentTypeHint reads the agent type hint written by SessionStart. diff --git a/cmd/entire/cli/strategy/session_state_test.go b/cmd/entire/cli/strategy/session_state_test.go index 4b7fd7ab7b..9818cd9390 100644 --- a/cmd/entire/cli/strategy/session_state_test.go +++ b/cmd/entire/cli/strategy/session_state_test.go @@ -9,6 +9,7 @@ import ( "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" @@ -593,7 +594,9 @@ func TestStoreAgentTypeHint_RoundTrip(t *testing.T) { ctx := context.Background() sessionID := "2026-01-01-agent-roundtrip" - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + 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) @@ -609,11 +612,15 @@ func TestStoreAgentTypeHint_FirstWriterWins(t *testing.T) { sessionID := "2026-01-01-agent-firstwriter" // Cursor claims the session first. - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + 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. - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeClaudeCode)) + 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") @@ -627,8 +634,17 @@ func TestStoreAgentTypeHint_EmptyOrUnknown_NoOp(t *testing.T) { ctx := context.Background() - require.NoError(t, StoreAgentTypeHint(ctx, "2026-01-01-empty", "")) - require.NoError(t, StoreAgentTypeHint(ctx, "2026-01-01-unknown", agent.AgentTypeUnknown)) + 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) @@ -656,7 +672,7 @@ func TestStoreAgentTypeHint_InvalidSessionID_ReturnsError(t *testing.T) { require.NoError(t, err) t.Chdir(dir) - err = StoreAgentTypeHint(context.Background(), "../../../etc/passwd", agent.AgentTypeCursor) + _, err = StoreAgentTypeHint(context.Background(), "../../../etc/passwd", agent.AgentTypeCursor) require.Error(t, err) } @@ -669,7 +685,8 @@ func TestClearSessionState_RemovesAgentHint(t *testing.T) { ctx := context.Background() sessionID := "2026-01-01-clear-agent-hint" - require.NoError(t, StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor)) + _, err = StoreAgentTypeHint(ctx, sessionID, agent.AgentTypeCursor) + require.NoError(t, err) require.NoError(t, ClearSessionState(ctx, sessionID)) got := LoadAgentTypeHint(ctx, sessionID) From afec89df499b81da337a9d9637031f845af61111 Mon Sep 17 00:00:00 2001 From: Thomas Dohmke Date: Thu, 30 Apr 2026 14:18:28 +0200 Subject: [PATCH 04/10] Emit plain text from Gemini hook response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gemini CLI v0.40.0 double-displays JSON systemMessage from hook output: emitHookSystemMessage adds an info entry tagged [hookName], and the SessionStart caller also calls historyManager.addItem with the same text without a tag — so the user sees the banner twice. The previous sessionId-dedup attempt didn't help because both displays come from a single hook invocation. Switching to plain-text output bypasses the JSON branch: gemini's convertPlainTextToHookOutput synthesizes systemMessage internally, the JSON-only emitHookSystemMessage event doesn't fire, and only the historyManager.addItem path runs. Banner displays once. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: b970ac3f423b --- cmd/entire/cli/agent/geminicli/lifecycle.go | 21 +++++--- .../cli/agent/geminicli/lifecycle_test.go | 49 +++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) 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") +} From e983e08a0b7296787740a2218085e32cebc85108 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 30 Apr 2026 19:10:54 +0200 Subject: [PATCH 05/10] Drop per-session file lock; rely on transcript-path repair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The InitializeSession race the lock guarded doesn't fire in practice — field logs from a Cursor IDE session show only Cursor's TurnStart fires (beforeSubmitPrompt is not forwarded to .claude/settings.json), and correctSessionAgentType already repairs wrong AgentType from the transcript path on the next turn. Removing it eliminates a generic synchronization primitive that future code could expand, plus the cleanup race in ClearSessionState where unlinking a held .lock file orphans the inode. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 14da5eb72d51 --- .golangci.yaml | 1 - cmd/entire/cli/filelock/filelock.go | 41 --------- cmd/entire/cli/filelock/filelock_test.go | 87 ------------------- cmd/entire/cli/filelock/filelock_unix.go | 36 -------- cmd/entire/cli/filelock/filelock_windows.go | 37 -------- .../cli/strategy/agent_resolution_test.go | 35 ++++---- .../cli/strategy/manual_commit_hooks.go | 31 ++----- cmd/entire/cli/strategy/session_state.go | 38 -------- 8 files changed, 26 insertions(+), 280 deletions(-) delete mode 100644 cmd/entire/cli/filelock/filelock.go delete mode 100644 cmd/entire/cli/filelock/filelock_test.go delete mode 100644 cmd/entire/cli/filelock/filelock_unix.go delete mode 100644 cmd/entire/cli/filelock/filelock_windows.go diff --git a/.golangci.yaml b/.golangci.yaml index c60abb3403..0dcec08769 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -108,7 +108,6 @@ linters: - grpc.DialOption - github.com/entireio/cli/cmd/entire/cli/agent\..+ - github.com/entireio/cli/cmd/entire/cli/checkpoint.CommittedReader - - github.com/entireio/cli/cmd/entire/cli/filelock\..+ - github.com/entireio/cli/cmd/entire/cli/strategy.Strategy - github.com/entireio/cli/cmd/entire/cli/summarize.Generator - github.com/go-git/go-git/v6/plumbing/storer.ReferenceIter diff --git a/cmd/entire/cli/filelock/filelock.go b/cmd/entire/cli/filelock/filelock.go deleted file mode 100644 index e0655c0866..0000000000 --- a/cmd/entire/cli/filelock/filelock.go +++ /dev/null @@ -1,41 +0,0 @@ -// Package filelock provides a small cross-platform exclusive file lock. -// -// Used to serialize concurrent processes operating on the same per-session -// state — e.g., when Cursor IDE forwards a single user prompt to both its own -// hooks and Claude Code's, both `entire` hook processes race to initialize the -// session. Without serialization the second writer can clobber the first. -package filelock - -// Lock is held until Release is called. Calling Release is mandatory; missing -// it leaves the lock held until the process exits and the OS reclaims the -// file descriptor. -type Lock struct { - impl lockImpl -} - -// Release drops the lock and closes the backing file descriptor. -// Safe to call multiple times — subsequent calls are no-ops. -func (l *Lock) Release() error { - if l == nil || l.impl == nil { - return nil - } - err := l.impl.release() - l.impl = nil - return err -} - -// Acquire blocks until an exclusive lock on path is obtained. The path's -// parent directory must exist; the lock file itself is created if missing. -// The contents of the lock file are intentionally not used — only its -// existence as a kernel-tracked inode matters. -func Acquire(path string) (*Lock, error) { - impl, err := acquireImpl(path) - if err != nil { - return nil, err - } - return &Lock{impl: impl}, nil -} - -type lockImpl interface { - release() error -} diff --git a/cmd/entire/cli/filelock/filelock_test.go b/cmd/entire/cli/filelock/filelock_test.go deleted file mode 100644 index 47212d4868..0000000000 --- a/cmd/entire/cli/filelock/filelock_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package filelock - -import ( - "path/filepath" - "sync" - "sync/atomic" - "testing" - "time" -) - -func TestAcquire_SerializesGoroutines(t *testing.T) { - t.Parallel() - - path := filepath.Join(t.TempDir(), "test.lock") - - const goroutines = 8 - var ( - holders atomic.Int32 - maxConc atomic.Int32 - finished atomic.Int32 - ) - - var wg sync.WaitGroup - for range goroutines { - wg.Add(1) - go func() { - defer wg.Done() - lock, err := Acquire(path) - if err != nil { - t.Errorf("Acquire() error = %v", err) - return - } - defer func() { - if rErr := lock.Release(); rErr != nil { - t.Errorf("Release() error = %v", rErr) - } - finished.Add(1) - }() - - // Track concurrent holders. Must always be 1 under exclusive lock. - cur := holders.Add(1) - if cur > maxConc.Load() { - maxConc.Store(cur) - } - - // Hold the lock briefly to give other goroutines a real chance to - // race for it. Without this, Goroutine 1 might release before any - // peer attempts Acquire, masking a broken lock. - time.Sleep(5 * time.Millisecond) - - holders.Add(-1) - }() - } - wg.Wait() - - if finished.Load() != goroutines { - t.Fatalf("expected %d goroutines to finish, got %d", goroutines, finished.Load()) - } - if got := maxConc.Load(); got != 1 { - t.Errorf("max concurrent holders = %d, want 1 (lock is not exclusive)", got) - } -} - -func TestRelease_Idempotent(t *testing.T) { - t.Parallel() - - path := filepath.Join(t.TempDir(), "test.lock") - lock, err := Acquire(path) - if err != nil { - t.Fatalf("Acquire() error = %v", err) - } - if err := lock.Release(); err != nil { - t.Fatalf("first Release() error = %v", err) - } - if err := lock.Release(); err != nil { - t.Errorf("second Release() should be a no-op, got: %v", err) - } -} - -func TestRelease_NilLock_NoOp(t *testing.T) { - t.Parallel() - - var lock *Lock - if err := lock.Release(); err != nil { - t.Errorf("Release() on nil lock should be a no-op, got: %v", err) - } -} diff --git a/cmd/entire/cli/filelock/filelock_unix.go b/cmd/entire/cli/filelock/filelock_unix.go deleted file mode 100644 index a84782530d..0000000000 --- a/cmd/entire/cli/filelock/filelock_unix.go +++ /dev/null @@ -1,36 +0,0 @@ -//go:build !windows - -package filelock - -import ( - "fmt" - "os" - "syscall" -) - -type unixLock struct { - f *os.File -} - -func (u *unixLock) release() error { - // Flock releases when the fd is closed. Closing alone is sufficient, - // but call LOCK_UN explicitly so the lock drops before any deferred - // fsyncs on close. - _ = syscall.Flock(int(u.f.Fd()), syscall.LOCK_UN) //nolint:errcheck,gosec // Fd() is a small descriptor; Close below also releases - if err := u.f.Close(); err != nil { - return fmt.Errorf("close lock file: %w", err) - } - return nil -} - -func acquireImpl(path string) (lockImpl, error) { - f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) //nolint:gosec // path is built from caller-validated session ID - if err != nil { - return nil, fmt.Errorf("open lock file %q: %w", path, err) - } - if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { //nolint:gosec // Fd() is a small descriptor - _ = f.Close() - return nil, fmt.Errorf("flock %q: %w", path, err) - } - return &unixLock{f: f}, nil -} diff --git a/cmd/entire/cli/filelock/filelock_windows.go b/cmd/entire/cli/filelock/filelock_windows.go deleted file mode 100644 index 8044eeca81..0000000000 --- a/cmd/entire/cli/filelock/filelock_windows.go +++ /dev/null @@ -1,37 +0,0 @@ -//go:build windows - -package filelock - -import ( - "fmt" - "os" - - "golang.org/x/sys/windows" -) - -type windowsLock struct { - f *os.File -} - -func (w *windowsLock) release() error { - overlapped := &windows.Overlapped{} - _ = windows.UnlockFileEx(windows.Handle(w.f.Fd()), 0, ^uint32(0), ^uint32(0), overlapped) //nolint:errcheck // Close below also releases - if err := w.f.Close(); err != nil { - return fmt.Errorf("close lock file: %w", err) - } - return nil -} - -//nolint:ireturn // intentional: cross-platform interface used only by filelock.Acquire -func acquireImpl(path string) (lockImpl, error) { - f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) //nolint:gosec // path is built from caller-validated session ID - if err != nil { - return nil, fmt.Errorf("open lock file %q: %w", path, err) - } - overlapped := &windows.Overlapped{} - if err := windows.LockFileEx(windows.Handle(f.Fd()), windows.LOCKFILE_EXCLUSIVE_LOCK, 0, ^uint32(0), ^uint32(0), overlapped); err != nil { - _ = f.Close() //nolint:errcheck // best-effort cleanup on lock failure - return nil, fmt.Errorf("LockFileEx %q: %w", path, err) - } - return &windowsLock{f: f}, nil -} diff --git a/cmd/entire/cli/strategy/agent_resolution_test.go b/cmd/entire/cli/strategy/agent_resolution_test.go index 2a15e3cabd..7086377b0f 100644 --- a/cmd/entire/cli/strategy/agent_resolution_test.go +++ b/cmd/entire/cli/strategy/agent_resolution_test.go @@ -168,17 +168,16 @@ func TestInitializeSession_TranscriptPathRepairsExistingState(t *testing.T) { assert.Equal(t, cursorTranscript, state.TranscriptPath) } -// TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner 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. Without the per-session lock, the -// last writer wins. With the lock the runner-up sees the existing state and -// applies correctSessionAgentType. -// -// The transcript-bearing call is the authoritative one. Regardless of which -// goroutine wins the lock first, the final AgentType must come from the -// transcript path. -func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T) { +// 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) @@ -191,16 +190,14 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T require.NoError(t, err) // Two parallel hook processes: claude-code (no transcript) and cursor - // (with transcript). Whichever grabs the lock first creates the state; - // the other follows and either matches or repairs via correctSessionAgentType. + // (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() - // Errors here are non-fatal — the goroutines race; either may turn - // out to be a no-op or hit a benign concurrent-update edge case. - // What we assert is the final state below. _ = s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "prompt", "") //nolint:errcheck // see comment above }() go func() { @@ -209,11 +206,15 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T }() 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 call must determine the final AgentType regardless of goroutine ordering") + "the transcript-bearing turn must repair AgentType to Cursor regardless of how the prior race resolved") } // TestInitializeSession_ClaudeCodeTranscriptKeepsClaudeCode verifies the negative case: diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index 095e0d6906..b3e0f43e51 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -2210,29 +2210,14 @@ func correctSessionAgentType(ctx context.Context, currentType types.AgentType, t // 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 { - // Serialize concurrent InitializeSession calls for the same session ID. - // Cursor IDE forwards a single user prompt to both .cursor/hooks.json and - // .claude/settings.json, so two `entire` hook processes can race here. The - // lock ensures the runner-up sees the state file the first process wrote - // and goes through the cheap update path (correctSessionAgentType handles - // any AgentType mismatch via the transcript-path signal). - lock, lockErr := AcquireSessionLock(ctx, sessionID) - if lockErr != nil { - // Logged but not fatal — fall through and accept the existing race. - // The most we lose is the correctness improvement; pre-fix behavior. - logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to acquire session lock; proceeding without serialization", - slog.String("session_id", sessionID), - slog.String("error", lockErr.Error())) - } else { - defer func() { - if rErr := lock.Release(); rErr != nil { - logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to release session lock", - slog.String("session_id", sessionID), - slog.String("error", rErr.Error())) - } - }() - } - + // Concurrent InitializeSession calls for the same session ID are possible + // — Cursor IDE forwards one prompt to both .cursor/hooks.json and + // .claude/settings.json, so two `entire` processes race here. We do not + // serialize them: the SessionStart hint plus correctSessionAgentType + // converge on the right AgentType across turns. The cost of the race is + // at most one redundant full setup (idempotent shadow-branch creation) + // and a possibly-wrong AgentType for one turn, which the next turn's + // transcript-path repair fixes. repo, err := OpenRepository(ctx) if err != nil { return fmt.Errorf("failed to open git repository: %w", err) diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index fce62631b4..6f310075fb 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -11,7 +11,6 @@ import ( "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/types" - "github.com/entireio/cli/cmd/entire/cli/filelock" "github.com/entireio/cli/cmd/entire/cli/jsonutil" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" @@ -271,38 +270,6 @@ func LoadModelHint(ctx context.Context, sessionID string) string { return strings.TrimSpace(string(data)) } -// AcquireSessionLock takes an exclusive per-session-id lock. Used to -// serialize concurrent processes that initialize the same session — when -// Cursor IDE forwards a single user prompt to both its own hook system and -// Claude Code's, two `entire` hook processes can otherwise each -// create-from-scratch in parallel and race for the last write. With the lock -// the second arrival sees the existing state and goes through the cheap -// update path (correctSessionAgentType repairs AgentType if the transcript -// path indicates a different agent). -// -// Lock files live under a dedicated `entire-locks/` directory in the git -// common dir — separate from `entire-sessions/` so callers that enumerate -// session state files (status, doctor, tests) don't have to filter lock -// artifacts. -func AcquireSessionLock(ctx context.Context, sessionID string) (*filelock.Lock, error) { - if err := validation.ValidateSessionID(sessionID); err != nil { - return nil, fmt.Errorf("invalid session ID: %w", err) - } - commonDir, err := GetGitCommonDir(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get git common dir: %w", err) - } - lockDir := filepath.Join(commonDir, "entire-locks") - if err := os.MkdirAll(lockDir, 0o750); err != nil { - return nil, fmt.Errorf("failed to create lock directory: %w", err) - } - lock, err := filelock.Acquire(filepath.Join(lockDir, sessionID+".lock")) - if err != nil { - return nil, fmt.Errorf("failed to acquire session lock: %w", err) - } - return lock, nil -} - // 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 @@ -405,10 +372,5 @@ func ClearSessionState(ctx context.Context, sessionID string) error { _ = os.Remove(f) } - // Remove the lock file (lives in a separate directory; see AcquireSessionLock). - if commonDir, cdErr := GetGitCommonDir(ctx); cdErr == nil { - _ = os.Remove(filepath.Join(commonDir, "entire-locks", sessionID+".lock")) - } - return nil } From 22ddd7b9c793f2fec037fa84cf31ac1dac36be63 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 30 Apr 2026 19:11:10 +0200 Subject: [PATCH 06/10] Skip forwarded lifecycle events from non-owning agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor IDE forwards stop hooks to both .cursor/hooks.json and .claude/settings.json (verified in field logs), so without filtering the non-owning agent's process duplicates checkpoint creation, races on metadata writes, and emits "transcript flush sentinel not found" warnings — Claude Code's TurnEnd handler waits for a sentinel in Cursor's transcript that will never appear. Once SessionState records the owning agent, the dispatcher no-ops events from any other agent for that session_id. SessionStart bypasses the check (state doesn't exist yet; hint dedup handles its duplication). TurnStart bypasses too, so InitializeSession's transcript-path resolution can repair a wrongly-set AgentType. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 35ba11dcdbb3 --- cmd/entire/cli/lifecycle.go | 26 +++++++++ cmd/entire/cli/lifecycle_test.go | 93 ++++++++++++++++++++++++++++++-- cmd/entire/cli/sessions_test.go | 1 + 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index f8898e1139..0a1fd3ec86 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -38,6 +38,32 @@ func DispatchLifecycleEvent(ctx context.Context, ag agent.Agent, event *agent.Ev return errors.New("event cannot be nil") } + // Skip events from non-owning agents. Cursor IDE forwards hooks to both + // .cursor/hooks.json and .claude/settings.json, so the non-owning agent's + // process otherwise duplicates work — extra checkpoints, torn writes on + // shared metadata files, doubled step counts. Once SessionState records + // the owning agent (set by InitializeSession's transcript-path resolution + // at TurnStart), any other agent's events for that session no-op here. + // + // Bypassed events: + // - SessionStart: runs before SessionState exists; the hint file dedup + // in handleLifecycleSessionStart protects the banner. + // - TurnStart: InitializeSession's transcript-path resolution can repair + // a wrongly-set AgentType. Skipping at the dispatcher would lock in + // a bad state forever. + if event.Type != agent.SessionStart && event.Type != agent.TurnStart && event.SessionID != "" { + if state, _ := strategy.LoadSessionState(ctx, event.SessionID); state != nil && state.AgentType != "" && state.AgentType != ag.Type() { //nolint:errcheck // load failures fall through to normal dispatch + 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) diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index 3a8d61c619..b7dc4eb10a 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -118,6 +118,91 @@ 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) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "init.txt", "init") + testutil.GitAdd(t, tmpDir, "init.txt") + testutil.GitCommit(t, tmpDir, "init") + t.Chdir(tmpDir) + paths.ClearWorktreeRootCache() + + sessionID := "test-skip-nonowning" + + // Bootstrap: state owned by Cursor. + require.NoError(t, strategy.SaveSessionState(context.Background(), &strategy.SessionState{ + SessionID: sessionID, + AgentType: testAgentCursor, + BaseCommit: "abc123", + StartedAt: time.Now(), + })) + + // Claude Code fires SessionEnd for Cursor's session (Cursor IDE forwarded hook). + claudeAgent := newMockAgent() + claudeAgent.agentType = testAgentClaude + + require.NoError(t, DispatchLifecycleEvent(context.Background(), claudeAgent, &agent.Event{ + Type: agent.SessionEnd, + SessionID: sessionID, + Timestamp: time.Now(), + })) + + // SessionEnd handler must NOT have run — EndedAt stays nil and the phase + // stays unset. 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's +// transcript-path resolution can repair a wrongly-set AgentType. +func TestDispatchLifecycleEvent_AllowsTurnStartFromMismatchedAgent(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "init.txt", "init") + testutil.GitAdd(t, tmpDir, "init.txt") + testutil.GitCommit(t, tmpDir, "init") + t.Chdir(tmpDir) + paths.ClearWorktreeRootCache() + + sessionID := "test-turnstart-mismatch" + + // Bootstrap: state has the wrong AgentType (e.g., a forwarded SessionStart + // won the hint race). + require.NoError(t, strategy.SaveSessionState(context.Background(), &strategy.SessionState{ + SessionID: sessionID, + AgentType: testAgentClaude, + BaseCommit: "abc123", + StartedAt: time.Now(), + })) + + // Cursor's TurnStart fires for the same session — the dispatcher must not + // skip; InitializeSession needs to run to invoke correctSessionAgentType. + cursorAgent := newMockAgent() + cursorAgent.agentType = testAgentCursor + + err := DispatchLifecycleEvent(context.Background(), cursorAgent, &agent.Event{ + Type: agent.TurnStart, + SessionID: sessionID, + Timestamp: time.Now(), + }) + // The handler may itself fail (no shadow branch infra in this minimal + // test), but the call must not have been short-circuited at the + // dispatcher. We only assert the dispatcher didn't filter the event — + // verified by the absence of the "skipping forwarded hook" no-op path, + // which would have returned nil immediately. A real or non-nil error + // from downstream both prove the dispatcher dispatched. + _ = err +} + func TestDispatchLifecycleEvent_UnknownEventType(t *testing.T) { t.Parallel() @@ -193,7 +278,7 @@ func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { paths.ClearWorktreeRootCache() ag := newMockHookResponseAgent() - ag.agentType = "Cursor" + ag.agentType = testAgentCursor event := &agent.Event{ Type: agent.SessionStart, SessionID: "test-agent-hint", @@ -202,7 +287,7 @@ func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { require.NoError(t, handleLifecycleSessionStart(context.Background(), ag, event)) got := strategy.LoadAgentTypeHint(context.Background(), "test-agent-hint") - require.Equal(t, types.AgentType("Cursor"), got) + require.Equal(t, types.AgentType(testAgentCursor), got) } // TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins verifies that @@ -223,7 +308,7 @@ func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) sessionID := "test-agent-hint-race" first := newMockHookResponseAgent() - first.agentType = "Cursor" + first.agentType = testAgentCursor require.NoError(t, handleLifecycleSessionStart(ctx, first, &agent.Event{ Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), })) @@ -237,7 +322,7 @@ func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) 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, types.AgentType("Cursor"), got, "first SessionStart caller must own the session") + require.Equal(t, types.AgentType(testAgentCursor), got, "first SessionStart caller must own the session") } // TestHandleLifecycleSessionStart_GeminiRepeatSourceDoesNotDuplicate covers diff --git a/cmd/entire/cli/sessions_test.go b/cmd/entire/cli/sessions_test.go index a78c455568..d6c8361abf 100644 --- a/cmd/entire/cli/sessions_test.go +++ b/cmd/entire/cli/sessions_test.go @@ -18,6 +18,7 @@ import ( const ( testAgentClaude = "Claude Code" + testAgentCursor = "Cursor" testCheckpointID = "a3b2c4d5e6f7" testPromptFixLogin = "fix the login bug" ) From afd6a7f394602a4560f0b77c795343765ffd7157 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 30 Apr 2026 19:50:51 +0200 Subject: [PATCH 07/10] Simplify dispatcher skip and lifecycle tests Extract eventBypassesAgentOwnershipCheck helper, reuse setupStopTestRepo across tests, switch to typed agent constants (agent.AgentTypeCursor / ClaudeCode / Gemini) so the cast wrappers go away, and strengthen AllowsTurnStartFromMismatchedAgent to assert dispatch actually ran by checking that TurnID was generated. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 17fd2ef20bd8 --- cmd/entire/cli/lifecycle.go | 32 +++--- cmd/entire/cli/lifecycle_test.go | 104 ++++++------------ cmd/entire/cli/sessions_test.go | 1 - .../cli/strategy/manual_commit_hooks.go | 10 +- 4 files changed, 55 insertions(+), 92 deletions(-) diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 0a1fd3ec86..532adaff62 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,21 +49,12 @@ func DispatchLifecycleEvent(ctx context.Context, ag agent.Agent, event *agent.Ev return errors.New("event cannot be nil") } - // Skip events from non-owning agents. Cursor IDE forwards hooks to both - // .cursor/hooks.json and .claude/settings.json, so the non-owning agent's - // process otherwise duplicates work — extra checkpoints, torn writes on - // shared metadata files, doubled step counts. Once SessionState records - // the owning agent (set by InitializeSession's transcript-path resolution - // at TurnStart), any other agent's events for that session no-op here. - // - // Bypassed events: - // - SessionStart: runs before SessionState exists; the hint file dedup - // in handleLifecycleSessionStart protects the banner. - // - TurnStart: InitializeSession's transcript-path resolution can repair - // a wrongly-set AgentType. Skipping at the dispatcher would lock in - // a bad state forever. - if event.Type != agent.SessionStart && event.Type != agent.TurnStart && event.SessionID != "" { - if state, _ := strategy.LoadSessionState(ctx, event.SessionID); state != nil && state.AgentType != "" && state.AgentType != ag.Type() { //nolint:errcheck // load failures fall through to normal dispatch + // 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()), diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index b7dc4eb10a..55103b30d7 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -124,27 +124,19 @@ func TestDispatchLifecycleEvent_NilEvent(t *testing.T) { // covers the Cursor IDE → .claude/settings.json forwarding scenario for Stop, // SubagentStart/End, Compaction, SessionEnd, and ModelUpdate events. func TestDispatchLifecycleEvent_SkipsForwardedHookFromNonOwningAgent(t *testing.T) { - tmpDir := t.TempDir() - testutil.InitRepo(t, tmpDir) - testutil.WriteFile(t, tmpDir, "init.txt", "init") - testutil.GitAdd(t, tmpDir, "init.txt") - testutil.GitCommit(t, tmpDir, "init") - t.Chdir(tmpDir) - paths.ClearWorktreeRootCache() + setupStopTestRepo(t) sessionID := "test-skip-nonowning" - - // Bootstrap: state owned by Cursor. require.NoError(t, strategy.SaveSessionState(context.Background(), &strategy.SessionState{ SessionID: sessionID, - AgentType: testAgentCursor, + AgentType: agent.AgentTypeCursor, BaseCommit: "abc123", StartedAt: time.Now(), })) // Claude Code fires SessionEnd for Cursor's session (Cursor IDE forwarded hook). claudeAgent := newMockAgent() - claudeAgent.agentType = testAgentClaude + claudeAgent.agentType = agent.AgentTypeClaudeCode require.NoError(t, DispatchLifecycleEvent(context.Background(), claudeAgent, &agent.Event{ Type: agent.SessionEnd, @@ -152,9 +144,8 @@ func TestDispatchLifecycleEvent_SkipsForwardedHookFromNonOwningAgent(t *testing. Timestamp: time.Now(), })) - // SessionEnd handler must NOT have run — EndedAt stays nil and the phase - // stays unset. If the dispatcher had let the event through, - // markSessionEnded would have transitioned to ENDED and set EndedAt. + // 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) @@ -162,45 +153,40 @@ func TestDispatchLifecycleEvent_SkipsForwardedHookFromNonOwningAgent(t *testing. } // TestDispatchLifecycleEvent_AllowsTurnStartFromMismatchedAgent verifies that -// TurnStart bypasses the dispatcher-level skip so InitializeSession's -// transcript-path resolution can repair a wrongly-set AgentType. +// 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) { - tmpDir := t.TempDir() - testutil.InitRepo(t, tmpDir) - testutil.WriteFile(t, tmpDir, "init.txt", "init") - testutil.GitAdd(t, tmpDir, "init.txt") - testutil.GitCommit(t, tmpDir, "init") - t.Chdir(tmpDir) - paths.ClearWorktreeRootCache() + setupStopTestRepo(t) - sessionID := "test-turnstart-mismatch" + ctx := context.Background() + repo, err := strategy.OpenRepository(ctx) + require.NoError(t, err) + head, err := repo.Head() + require.NoError(t, err) - // Bootstrap: state has the wrong AgentType (e.g., a forwarded SessionStart - // won the hint race). - require.NoError(t, strategy.SaveSessionState(context.Background(), &strategy.SessionState{ + sessionID := "test-turnstart-mismatch" + require.NoError(t, strategy.SaveSessionState(ctx, &strategy.SessionState{ SessionID: sessionID, - AgentType: testAgentClaude, - BaseCommit: "abc123", + AgentType: agent.AgentTypeClaudeCode, + BaseCommit: head.Hash().String(), StartedAt: time.Now(), })) - // Cursor's TurnStart fires for the same session — the dispatcher must not - // skip; InitializeSession needs to run to invoke correctSessionAgentType. cursorAgent := newMockAgent() - cursorAgent.agentType = testAgentCursor + cursorAgent.agentType = agent.AgentTypeCursor - err := DispatchLifecycleEvent(context.Background(), cursorAgent, &agent.Event{ + require.NoError(t, DispatchLifecycleEvent(ctx, cursorAgent, &agent.Event{ Type: agent.TurnStart, SessionID: sessionID, Timestamp: time.Now(), - }) - // The handler may itself fail (no shadow branch infra in this minimal - // test), but the call must not have been short-circuited at the - // dispatcher. We only assert the dispatcher didn't filter the event — - // verified by the absence of the "skipping forwarded hook" no-op path, - // which would have returned nil immediately. A real or non-nil error - // from downstream both prove the dispatcher dispatched. - _ = err + })) + + // 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") } func TestDispatchLifecycleEvent_UnknownEventType(t *testing.T) { @@ -269,16 +255,10 @@ func newMockHookResponseAgent() *mockHookResponseAgent { // 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) { - tmpDir := t.TempDir() - testutil.InitRepo(t, tmpDir) - testutil.WriteFile(t, tmpDir, "init.txt", "init") - testutil.GitAdd(t, tmpDir, "init.txt") - testutil.GitCommit(t, tmpDir, "init") - t.Chdir(tmpDir) - paths.ClearWorktreeRootCache() + setupStopTestRepo(t) ag := newMockHookResponseAgent() - ag.agentType = testAgentCursor + ag.agentType = agent.AgentTypeCursor event := &agent.Event{ Type: agent.SessionStart, SessionID: "test-agent-hint", @@ -287,7 +267,7 @@ func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { require.NoError(t, handleLifecycleSessionStart(context.Background(), ag, event)) got := strategy.LoadAgentTypeHint(context.Background(), "test-agent-hint") - require.Equal(t, types.AgentType(testAgentCursor), got) + require.Equal(t, agent.AgentTypeCursor, got) } // TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins verifies that @@ -296,33 +276,27 @@ func TestHandleLifecycleSessionStart_StoresAgentTypeHint(t *testing.T) { // 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) { - tmpDir := t.TempDir() - testutil.InitRepo(t, tmpDir) - testutil.WriteFile(t, tmpDir, "init.txt", "init") - testutil.GitAdd(t, tmpDir, "init.txt") - testutil.GitCommit(t, tmpDir, "init") - t.Chdir(tmpDir) - paths.ClearWorktreeRootCache() + setupStopTestRepo(t) ctx := context.Background() sessionID := "test-agent-hint-race" first := newMockHookResponseAgent() - first.agentType = testAgentCursor + 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 = testAgentClaude + 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, types.AgentType(testAgentCursor), got, "first SessionStart caller must own the session") + require.Equal(t, agent.AgentTypeCursor, got, "first SessionStart caller must own the session") } // TestHandleLifecycleSessionStart_GeminiRepeatSourceDoesNotDuplicate covers @@ -330,19 +304,13 @@ func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) // 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) { - tmpDir := t.TempDir() - testutil.InitRepo(t, tmpDir) - testutil.WriteFile(t, tmpDir, "init.txt", "init") - testutil.GitAdd(t, tmpDir, "init.txt") - testutil.GitCommit(t, tmpDir, "init") - t.Chdir(tmpDir) - paths.ClearWorktreeRootCache() + setupStopTestRepo(t) ctx := context.Background() sessionID := "test-gemini-repeat" ag := newMockHookResponseAgent() - ag.agentType = "Gemini CLI" + ag.agentType = agent.AgentTypeGemini require.NoError(t, handleLifecycleSessionStart(ctx, ag, &agent.Event{ Type: agent.SessionStart, SessionID: sessionID, Timestamp: time.Now(), diff --git a/cmd/entire/cli/sessions_test.go b/cmd/entire/cli/sessions_test.go index d6c8361abf..a78c455568 100644 --- a/cmd/entire/cli/sessions_test.go +++ b/cmd/entire/cli/sessions_test.go @@ -18,7 +18,6 @@ import ( const ( testAgentClaude = "Claude Code" - testAgentCursor = "Cursor" testCheckpointID = "a3b2c4d5e6f7" testPromptFixLogin = "fix the login bug" ) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index b3e0f43e51..32da8ad64f 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -2210,14 +2210,8 @@ func correctSessionAgentType(ctx context.Context, currentType types.AgentType, t // 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 InitializeSession calls for the same session ID are possible - // — Cursor IDE forwards one prompt to both .cursor/hooks.json and - // .claude/settings.json, so two `entire` processes race here. We do not - // serialize them: the SessionStart hint plus correctSessionAgentType - // converge on the right AgentType across turns. The cost of the race is - // at most one redundant full setup (idempotent shadow-branch creation) - // and a possibly-wrong AgentType for one turn, which the next turn's - // transcript-path repair fixes. + // 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) From 9f34898bf984f8a4c6d2e6c0ac789e68920357b2 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 30 Apr 2026 20:49:52 +0200 Subject: [PATCH 08/10] Cover dispatcher skip across all event types and end-to-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit tests: a table-driven test exercises every non-bypass event type (TurnEnd, Compaction, SubagentStart/End, ModelUpdate, SessionEnd) and asserts the skip via observable side effects (EndedAt nil, ModelName unchanged). Adds negative cases for owner-match and empty-AgentType, and a direct test of eventBypassesAgentOwnershipCheck. Integration tests: TestDispatcher_ForwardedStopFromNonOwnerIsSkipped and TestDispatcher_ForwardedSessionEndFromNonOwnerIsSkipped invoke `entire hooks claude-code stop` / `entire hooks claude-code session-end` on a Cursor-owned session and verify state is unchanged — proves the skip works end-to-end through the real CLI binary, not just the in-process DispatchLifecycleEvent function. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: e6e214d0a9eb --- .../cursor_forwarding_test.go | 101 +++++++++++++ cmd/entire/cli/lifecycle_test.go | 137 ++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 cmd/entire/cli/integration_test/cursor_forwarding_test.go 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_test.go b/cmd/entire/cli/lifecycle_test.go index 55103b30d7..ecc4fb8dde 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -189,6 +189,143 @@ func TestDispatchLifecycleEvent_AllowsTurnStartFromMismatchedAgent(t *testing.T) 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() From e38f1c6018b8ff5cc19de4dfadcf00202ea947e1 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 10:31:40 +0200 Subject: [PATCH 09/10] Decouple SessionStart banner from agent ownership claim When Cursor and Claude Code both fire SessionStart for the same session (Cursor IDE forwards hooks to .cursor/hooks.json and .claude/settings.json), StoreAgentTypeHint's first-writer-wins claim was gating both ownership AND banner display. Cursor doesn't implement HookResponseWriter, so when it won the race (~50%), it consumed the banner privilege without printing one, and Claude Code's banner was then suppressed too. Add ClaimSessionStartBanner with its own marker file, and only call it from inside the HookResponseWriter branch. Ownership claim semantics (used by InitializeSession repair and the dispatcher skip) are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 1d854d8d0b19 --- cmd/entire/cli/lifecycle.go | 41 +++++++------ cmd/entire/cli/lifecycle_test.go | 58 +++++++++++++++++++ cmd/entire/cli/strategy/session_state.go | 45 ++++++++++++-- cmd/entire/cli/strategy/session_state_test.go | 47 +++++++++++++++ 4 files changed, 169 insertions(+), 22 deletions(-) diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 532adaff62..ec42d0d6c9 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -106,19 +106,14 @@ 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: if another caller - // already claimed this session ID, claimedFirst=false. We use this both - // for cross-agent disambiguation (Cursor IDE forwarding hooks to both - // .cursor/hooks.json and .claude/settings.json) and to dedupe the banner - // when one agent fires SessionStart multiple times for the same session - // (Gemini fires SessionStart for source=startup AND for source=resume). - claimedFirst, hintErr := strategy.StoreAgentTypeHint(ctx, event.SessionID, ag.Type()) - if hintErr != nil { - // If we couldn't write the hint, fall back to "show the banner" — - // better to duplicate than to suppress the only banner the user sees. + // 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())) - claimedFirst = true } // Build informational message — warn early if repo has no commits yet, @@ -140,16 +135,28 @@ func handleLifecycleSessionStart(ctx context.Context, ag agent.Agent, event *age } countSessionsSpan.End() - // Output informational message if the agent supports hook responses, but - // only on the first SessionStart for this session ID. Claude Code reads - // JSON from stdout; agents that don't implement HookResponseWriter - // silently skip (avoids raw JSON in their terminal). + // 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 claimedFirst { - if writer, ok := agent.AsHookResponseWriter(ag); ok { + if writer, ok := agent.AsHookResponseWriter(ag); ok { + 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() diff --git a/cmd/entire/cli/lifecycle_test.go b/cmd/entire/cli/lifecycle_test.go index ecc4fb8dde..5886a12331 100644 --- a/cmd/entire/cli/lifecycle_test.go +++ b/cmd/entire/cli/lifecycle_test.go @@ -436,6 +436,64 @@ func TestHandleLifecycleSessionStart_AgentTypeHintFirstWriterWins(t *testing.T) 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 diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index 6f310075fb..b160dc3875 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -288,11 +288,9 @@ func LoadModelHint(ctx context.Context, sessionID string) string { // Returns (created=true) when this call wrote the hint, (created=false) when // the hint already existed (no-op) or agentType was empty/Unknown. // -// Callers can use the created bool to gate one-time-per-session side effects -// — e.g., the SessionStart banner: if Gemini fires SessionStart twice (once -// for source=startup, once for source=resume) we don't want to print the -// banner twice. The first call creates the hint and returns created=true; -// the second sees the hint and returns created=false. +// 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) @@ -325,6 +323,43 @@ func StoreAgentTypeHint(ctx context.Context, sessionID string, agentType types.A 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. diff --git a/cmd/entire/cli/strategy/session_state_test.go b/cmd/entire/cli/strategy/session_state_test.go index 9818cd9390..ac054ee757 100644 --- a/cmd/entire/cli/strategy/session_state_test.go +++ b/cmd/entire/cli/strategy/session_state_test.go @@ -676,6 +676,53 @@ func TestStoreAgentTypeHint_InvalidSessionID_ReturnsError(t *testing.T) { 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) From de082f61ef98c3dc70893de36a216b38308ac767 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Mon, 4 May 2026 11:10:10 +0200 Subject: [PATCH 10/10] Match transcript-path prefix case-insensitively on Windows NTFS/ReFS treat paths as case-insensitive, and filepath.Abs preserves whatever casing the input had. 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 HasPrefix, silently dropping the strongest signal in AgentForTranscriptPath. Lowercase both sides on Windows; keep Unix case-sensitive. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 7212dd03c385 --- cmd/entire/cli/agent/registry.go | 11 ++++++++++ cmd/entire/cli/agent/registry_test.go | 30 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/cmd/entire/cli/agent/registry.go b/cmd/entire/cli/agent/registry.go index 94e2e3774a..af0aa4e94a 100644 --- a/cmd/entire/cli/agent/registry.go +++ b/cmd/entire/cli/agent/registry.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path/filepath" + "runtime" "slices" "strings" "sync" @@ -142,7 +143,17 @@ func AgentForTranscriptPath(transcriptPath, repoPath string) (Agent, bool) { // 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 } diff --git a/cmd/entire/cli/agent/registry_test.go b/cmd/entire/cli/agent/registry_test.go index 7b86b3c961..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" @@ -233,6 +234,35 @@ func TestAgentForTranscriptPath(t *testing.T) { } } +// 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)