Skip to content

Commit e983e08

Browse files
Sophclaude
andcommitted
Drop per-session file lock; rely on transcript-path repair
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) <noreply@anthropic.com> Entire-Checkpoint: 14da5eb72d51
1 parent afec89d commit e983e08

8 files changed

Lines changed: 26 additions & 280 deletions

File tree

.golangci.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ linters:
108108
- grpc.DialOption
109109
- github.com/entireio/cli/cmd/entire/cli/agent\..+
110110
- github.com/entireio/cli/cmd/entire/cli/checkpoint.CommittedReader
111-
- github.com/entireio/cli/cmd/entire/cli/filelock\..+
112111
- github.com/entireio/cli/cmd/entire/cli/strategy.Strategy
113112
- github.com/entireio/cli/cmd/entire/cli/summarize.Generator
114113
- github.com/go-git/go-git/v6/plumbing/storer.ReferenceIter

cmd/entire/cli/filelock/filelock.go

Lines changed: 0 additions & 41 deletions
This file was deleted.

cmd/entire/cli/filelock/filelock_test.go

Lines changed: 0 additions & 87 deletions
This file was deleted.

cmd/entire/cli/filelock/filelock_unix.go

Lines changed: 0 additions & 36 deletions
This file was deleted.

cmd/entire/cli/filelock/filelock_windows.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

cmd/entire/cli/strategy/agent_resolution_test.go

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -168,17 +168,16 @@ func TestInitializeSession_TranscriptPathRepairsExistingState(t *testing.T) {
168168
assert.Equal(t, cursorTranscript, state.TranscriptPath)
169169
}
170170

171-
// TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner simulates
172-
// the bug from the field: two processes (Cursor IDE forwarding the prompt to
173-
// both .cursor/hooks.json and .claude/settings.json) call InitializeSession
174-
// concurrently for the same session ID. Without the per-session lock, the
175-
// last writer wins. With the lock the runner-up sees the existing state and
176-
// applies correctSessionAgentType.
177-
//
178-
// The transcript-bearing call is the authoritative one. Regardless of which
179-
// goroutine wins the lock first, the final AgentType must come from the
180-
// transcript path.
181-
func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T) {
171+
// TestInitializeSession_ConcurrentRaceRepairsOnNextTurn simulates the bug
172+
// from the field: two processes (Cursor IDE forwarding the prompt to both
173+
// .cursor/hooks.json and .claude/settings.json) call InitializeSession
174+
// concurrently for the same session ID. We do not serialize them — the
175+
// race may leave AgentType wrong for a single turn (both goroutines see
176+
// no existing state and run the "initialize new session" path; last
177+
// writer wins). The contract is eventual consistency: the next turn that
178+
// arrives with the cursor transcript path repairs AgentType via
179+
// correctSessionAgentType.
180+
func TestInitializeSession_ConcurrentRaceRepairsOnNextTurn(t *testing.T) {
182181
dir := setupGitRepo(t)
183182
t.Chdir(dir)
184183
cursorTranscript := withCursorSessionDir(t)
@@ -191,16 +190,14 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T
191190
require.NoError(t, err)
192191

193192
// Two parallel hook processes: claude-code (no transcript) and cursor
194-
// (with transcript). Whichever grabs the lock first creates the state;
195-
// the other follows and either matches or repairs via correctSessionAgentType.
193+
// (with transcript). Errors are non-fatal — without serialization the
194+
// goroutines may step on each other's writes; convergence is asserted
195+
// via the next-turn call below.
196196
s := &ManualCommitStrategy{}
197197
var wg sync.WaitGroup
198198
wg.Add(2)
199199
go func() {
200200
defer wg.Done()
201-
// Errors here are non-fatal — the goroutines race; either may turn
202-
// out to be a no-op or hit a benign concurrent-update edge case.
203-
// What we assert is the final state below.
204201
_ = s.InitializeSession(ctx, sessionID, agent.AgentTypeClaudeCode, "", "prompt", "") //nolint:errcheck // see comment above
205202
}()
206203
go func() {
@@ -209,11 +206,15 @@ func TestInitializeSession_ConcurrentCallsConvergeToTranscriptOwner(t *testing.T
209206
}()
210207
wg.Wait()
211208

209+
// Simulate the next turn: cursor's hook fires again with its transcript
210+
// path. correctSessionAgentType repairs any wrong AgentType from the race.
211+
require.NoError(t, s.InitializeSession(ctx, sessionID, agent.AgentTypeCursor, cursorTranscript, "next prompt", ""))
212+
212213
state, lErr := s.loadSessionState(ctx, sessionID)
213214
require.NoError(t, lErr)
214215
require.NotNil(t, state)
215216
assert.Equal(t, agent.AgentTypeCursor, state.AgentType,
216-
"the transcript-bearing call must determine the final AgentType regardless of goroutine ordering")
217+
"the transcript-bearing turn must repair AgentType to Cursor regardless of how the prior race resolved")
217218
}
218219

219220
// TestInitializeSession_ClaudeCodeTranscriptKeepsClaudeCode verifies the negative case:

cmd/entire/cli/strategy/manual_commit_hooks.go

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,29 +2210,14 @@ func correctSessionAgentType(ctx context.Context, currentType types.AgentType, t
22102210
// userPrompt is the user's prompt text (stored truncated as LastPrompt for display).
22112211
// model is the LLM model identifier (e.g., "claude-sonnet-4-20250514"); empty if unknown.
22122212
func (s *ManualCommitStrategy) InitializeSession(ctx context.Context, sessionID string, agentType types.AgentType, transcriptPath string, userPrompt string, model string) error {
2213-
// Serialize concurrent InitializeSession calls for the same session ID.
2214-
// Cursor IDE forwards a single user prompt to both .cursor/hooks.json and
2215-
// .claude/settings.json, so two `entire` hook processes can race here. The
2216-
// lock ensures the runner-up sees the state file the first process wrote
2217-
// and goes through the cheap update path (correctSessionAgentType handles
2218-
// any AgentType mismatch via the transcript-path signal).
2219-
lock, lockErr := AcquireSessionLock(ctx, sessionID)
2220-
if lockErr != nil {
2221-
// Logged but not fatal — fall through and accept the existing race.
2222-
// The most we lose is the correctness improvement; pre-fix behavior.
2223-
logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to acquire session lock; proceeding without serialization",
2224-
slog.String("session_id", sessionID),
2225-
slog.String("error", lockErr.Error()))
2226-
} else {
2227-
defer func() {
2228-
if rErr := lock.Release(); rErr != nil {
2229-
logging.Warn(logging.WithComponent(ctx, "hooks"), "failed to release session lock",
2230-
slog.String("session_id", sessionID),
2231-
slog.String("error", rErr.Error()))
2232-
}
2233-
}()
2234-
}
2235-
2213+
// Concurrent InitializeSession calls for the same session ID are possible
2214+
// — Cursor IDE forwards one prompt to both .cursor/hooks.json and
2215+
// .claude/settings.json, so two `entire` processes race here. We do not
2216+
// serialize them: the SessionStart hint plus correctSessionAgentType
2217+
// converge on the right AgentType across turns. The cost of the race is
2218+
// at most one redundant full setup (idempotent shadow-branch creation)
2219+
// and a possibly-wrong AgentType for one turn, which the next turn's
2220+
// transcript-path repair fixes.
22362221
repo, err := OpenRepository(ctx)
22372222
if err != nil {
22382223
return fmt.Errorf("failed to open git repository: %w", err)

cmd/entire/cli/strategy/session_state.go

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"github.com/entireio/cli/cmd/entire/cli/agent"
1313
"github.com/entireio/cli/cmd/entire/cli/agent/types"
14-
"github.com/entireio/cli/cmd/entire/cli/filelock"
1514
"github.com/entireio/cli/cmd/entire/cli/jsonutil"
1615
"github.com/entireio/cli/cmd/entire/cli/logging"
1716
"github.com/entireio/cli/cmd/entire/cli/paths"
@@ -271,38 +270,6 @@ func LoadModelHint(ctx context.Context, sessionID string) string {
271270
return strings.TrimSpace(string(data))
272271
}
273272

274-
// AcquireSessionLock takes an exclusive per-session-id lock. Used to
275-
// serialize concurrent processes that initialize the same session — when
276-
// Cursor IDE forwards a single user prompt to both its own hook system and
277-
// Claude Code's, two `entire` hook processes can otherwise each
278-
// create-from-scratch in parallel and race for the last write. With the lock
279-
// the second arrival sees the existing state and goes through the cheap
280-
// update path (correctSessionAgentType repairs AgentType if the transcript
281-
// path indicates a different agent).
282-
//
283-
// Lock files live under a dedicated `entire-locks/` directory in the git
284-
// common dir — separate from `entire-sessions/` so callers that enumerate
285-
// session state files (status, doctor, tests) don't have to filter lock
286-
// artifacts.
287-
func AcquireSessionLock(ctx context.Context, sessionID string) (*filelock.Lock, error) {
288-
if err := validation.ValidateSessionID(sessionID); err != nil {
289-
return nil, fmt.Errorf("invalid session ID: %w", err)
290-
}
291-
commonDir, err := GetGitCommonDir(ctx)
292-
if err != nil {
293-
return nil, fmt.Errorf("failed to get git common dir: %w", err)
294-
}
295-
lockDir := filepath.Join(commonDir, "entire-locks")
296-
if err := os.MkdirAll(lockDir, 0o750); err != nil {
297-
return nil, fmt.Errorf("failed to create lock directory: %w", err)
298-
}
299-
lock, err := filelock.Acquire(filepath.Join(lockDir, sessionID+".lock"))
300-
if err != nil {
301-
return nil, fmt.Errorf("failed to acquire session lock: %w", err)
302-
}
303-
return lock, nil
304-
}
305-
306273
// StoreAgentTypeHint records the agent type that owns a session before
307274
// SessionState exists. Used by the lifecycle dispatcher when SessionStart fires
308275
// (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 {
405372
_ = os.Remove(f)
406373
}
407374

408-
// Remove the lock file (lives in a separate directory; see AcquireSessionLock).
409-
if commonDir, cdErr := GetGitCommonDir(ctx); cdErr == nil {
410-
_ = os.Remove(filepath.Join(commonDir, "entire-locks", sessionID+".lock"))
411-
}
412-
413375
return nil
414376
}

0 commit comments

Comments
 (0)