Skip to content

Commit bcced02

Browse files
committed
fix(session): claim source on deferred cross-common-dir adopt to prevent double-adopt
The deferred cross-common-dir auto-adopt (DeferSourceRetire) registers the adopted session in the target and stamps PendingSourceRetire, but left the source session untouched at prepare time. Two targets that concurrently discover the same unique live source could both pass the exactly-one-candidate check, acquire the shared source lock in turn, and each register their own copy of the same SessionID — a double-adopt, because neither wrote anything the other would observe. The in-band tombstone the deferral removed had provided that cross-process mutual exclusion. Restore it without re-tombstoning at prepare time: under the same source-common- dir lock, stamp a non-destructive AdoptClaim (target common dir + timestamp) on the source. It does not end the session, so the source agent keeps checkpointing (preserving the deferral's abort-safety). A second concurrent adopt re-reads the source under the lock, observes a fresh claim by a different target, and refuses (sourceClaimedError -> auto-adopt skips). The candidate discovery filter drops a claimed source too. post-commit finalize's retire supersedes the claim; a claim older than adoptRecentWindow is ignored, so an abandoned claim from an aborted commit self-heals. A re-claim by the same target is idempotent. Entire-Checkpoint: 01KYX128X8JKV7FH75QR1FV8NW
1 parent e15c05d commit bcced02

5 files changed

Lines changed: 399 additions & 10 deletions

File tree

cmd/entire/cli/session/state.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,20 @@ type State struct {
143143
// (finalizePendingSourceRetires). nil in the normal steady state.
144144
PendingSourceRetire *PendingSourceRetire `json:"pending_source_retire,omitempty"`
145145

146+
// AdoptClaim is a non-destructive, in-flight cross-common-dir adoption
147+
// marker written on the SOURCE session at prepare-commit-msg time when the
148+
// destructive retire is deferred to post-commit (DeferSourceRetire). It is
149+
// what restores the cross-process mutual exclusion the in-band tombstone used
150+
// to provide: a second target that concurrently discovers the same unique
151+
// candidate observes a FRESH claim by a different common dir and skips, so the
152+
// session is never double-adopted. Unlike a tombstone it does NOT end the
153+
// session — the source agent keeps checkpointing, preserving the abort-safety
154+
// of the deferral. Superseded by finalizePendingSourceRetires' retire. A claim
155+
// older than the adopt-recency window is ignored, so an abandoned claim (an
156+
// aborted commit that never reached post-commit) self-heals and frees the
157+
// source for a later legitimate adopt. nil in the normal steady state.
158+
AdoptClaim *AdoptClaim `json:"adopt_claim,omitempty"`
159+
146160
// Branch is the git branch HEAD pointed at the last time this session took a
147161
// turn. Captured on each turn start so it tracks branches created or renamed
148162
// after the session began. Empty when HEAD was detached or for sessions
@@ -391,6 +405,22 @@ type PendingSourceRetire struct {
391405
SourceWorktreePath string `json:"source_worktree_path,omitempty"`
392406
}
393407

408+
// AdoptClaim stamps a source session as being adopted, in-flight, by a specific
409+
// target worktree while its destructive retire is deferred to post-commit. It is
410+
// the lightweight lock marker that a concurrent adopter must observe to avoid
411+
// double-adopting the same unique session across processes. See State.AdoptClaim.
412+
type AdoptClaim struct {
413+
// ByCommonDir is the git common dir of the TARGET worktree that claimed this
414+
// source. A claim by a *different* common dir blocks a new adoption; a
415+
// re-claim by the same common dir is idempotent (re-adopt is fine).
416+
ByCommonDir string `json:"by_common_dir"`
417+
// ByWorktreePath is the target worktree root (best-effort, for logging).
418+
ByWorktreePath string `json:"by_worktree_path,omitempty"`
419+
// At is when the claim was written. A claim older than the adopt-recency
420+
// window is treated as abandoned (aborted commit) and no longer blocks.
421+
At time.Time `json:"at"`
422+
}
423+
394424
// PromptAttribution captures line-level attribution data at the start of each prompt.
395425
// By recording what changed since the last checkpoint BEFORE the agent works,
396426
// we can accurately separate user edits from agent contributions.

cmd/entire/cli/session_adopt.go

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,14 @@ func adoptFromExternalSessionStore(
171171
if !isAdoptableSourceSession(sourceState) {
172172
return fmt.Errorf("session %s is ended or fully condensed and cannot be adopted", sessionID)
173173
}
174+
// Cross-process mutual exclusion for the deferred (auto-adopt) path. The
175+
// shared sourceCommonDir lock serializes two concurrent adopts of the same
176+
// unique candidate; this makes the loser OBSERVE the winner's claim and
177+
// refuse, so the session is adopted into exactly one repo. A stale claim
178+
// (aborted commit) or a re-claim by this same target does not block.
179+
if opts.DeferSourceRetire && adoptClaimBlocks(sourceState.AdoptClaim, targetCommonDir) {
180+
return &sourceClaimedError{sessionID: sessionID, claimedBy: adoptClaimLabel(sourceState.AdoptClaim)}
181+
}
174182
if !sessionBelongsToSourceWorktree(sourceState, sourceWorktree, sourceWorktreeID) {
175183
return fmt.Errorf("session %s belongs to %s, not %s",
176184
sessionID, adoptSessionWorktreeLabel(sourceState), sourceWorktree)
@@ -209,7 +217,26 @@ func adoptFromExternalSessionStore(
209217
return fmt.Errorf("save adopted session state: %w", err)
210218
}
211219
if opts.DeferSourceRetire {
212-
// Non-destructive prepare phase: target registered, source untouched.
220+
// Non-destructive prepare phase: stamp a claim on the SOURCE (under the
221+
// shared source lock) so a concurrent adopt by a different target sees
222+
// it and skips, then leave the source otherwise ACTIVE so it keeps
223+
// checkpointing until post-commit finalizes the retire. The claim is
224+
// recency-bounded, so an aborted commit (no post-commit) self-heals once
225+
// it ages out.
226+
sourceState.AdoptClaim = &session.AdoptClaim{
227+
ByCommonDir: targetCommonDir,
228+
ByWorktreePath: next.WorktreePath,
229+
At: time.Now(),
230+
}
231+
if err := sourceStore.Save(ctx, sourceState); err != nil {
232+
// Roll back the target registration so a failed claim never leaves a
233+
// claimless double-adopt window. Use a non-cancelable context for the
234+
// same reason the retire path does (the caller's ctx may be timed out).
235+
if rollbackErr := rollbackExternalAdoptTarget(context.WithoutCancel(ctx), targetStore, next.SessionID, existing); rollbackErr != nil {
236+
return &adoptRollbackFailedError{retireErr: err, rollbackErr: rollbackErr}
237+
}
238+
return fmt.Errorf("claim source session state: %w", err)
239+
}
213240
adopted = next
214241
filesTouched = touched
215242
return nil
@@ -276,6 +303,8 @@ func retireAdoptedSourceSession(source, target *session.State) session.State {
276303
retired.FilesTouched = nil
277304
retired.TurnID = ""
278305
retired.TurnCheckpointIDs = nil
306+
// The tombstone supersedes any in-flight adoption claim on the source.
307+
retired.AdoptClaim = nil
279308
retired.AdoptedIntoWorktreePath = target.WorktreePath
280309
retired.AdoptedIntoWorktreeID = target.WorktreeID
281310
return retired
@@ -515,6 +544,51 @@ func isAdoptableSourceSession(state *session.State) bool {
515544
!state.FullyCondensed
516545
}
517546

547+
// adoptClaimBlocks reports whether a source session's in-flight adoption claim
548+
// should block a NEW deferred adoption by targetCommonDir. A claim blocks only
549+
// when it is (a) present, (b) FRESH — within the adopt-recency window, so a stale
550+
// claim left by an aborted commit self-heals rather than pinning the source
551+
// forever — and (c) held by a DIFFERENT common dir, since a re-adopt by the same
552+
// target is idempotent. Reuses adoptRecentWindow (= session.LiveSessionMaxAge);
553+
// no separate TTL is introduced.
554+
func adoptClaimBlocks(claim *session.AdoptClaim, targetCommonDir string) bool {
555+
if claim == nil || claim.At.IsZero() {
556+
return false
557+
}
558+
if time.Since(claim.At) > adoptRecentWindow {
559+
return false
560+
}
561+
return !sameAdoptStore(claim.ByCommonDir, targetCommonDir)
562+
}
563+
564+
func adoptClaimLabel(claim *session.AdoptClaim) string {
565+
if claim == nil {
566+
return unknownPlaceholder
567+
}
568+
if claim.ByWorktreePath != "" {
569+
return claim.ByWorktreePath
570+
}
571+
if claim.ByCommonDir != "" {
572+
return claim.ByCommonDir
573+
}
574+
return unknownPlaceholder
575+
}
576+
577+
// sourceClaimedError is returned by adoptFromExternalSessionStore when the source
578+
// session already carries a FRESH cross-common-dir adoption claim by a DIFFERENT
579+
// target — a concurrent commit is mid-adopt of the same unique session. It is a
580+
// benign "lost the race" outcome that preserves adopt-exactly-once, not
581+
// corruption, so auto-adopt logs it at Debug and moves on rather than warning.
582+
type sourceClaimedError struct {
583+
sessionID string
584+
claimedBy string
585+
}
586+
587+
func (e *sourceClaimedError) Error() string {
588+
return fmt.Sprintf("session %s is already claimed for cross-common-dir adoption by %s",
589+
e.sessionID, e.claimedBy)
590+
}
591+
518592
func sessionLastSeen(state *session.State) time.Time {
519593
if state.LastInteractionTime != nil {
520594
return *state.LastInteractionTime
@@ -570,6 +644,9 @@ func buildAdoptedSessionState(ctx context.Context, source *session.State) (*sess
570644
adopted.WorktreeID = worktreeID
571645
adopted.AdoptedIntoWorktreePath = ""
572646
adopted.AdoptedIntoWorktreeID = ""
647+
// AdoptClaim is a source-side marker; the adopted (target) copy never carries
648+
// one. The caller re-stamps it on the source under the shared lock.
649+
adopted.AdoptClaim = nil
573650
adopted.Branch = branch
574651
adopted.LastInteractionTime = &now
575652
adopted.Phase = session.PhaseActive
@@ -631,6 +708,10 @@ func cloneAdoptSourceState(source *session.State) session.State {
631708
pending := clonePromptAttribution(*source.PendingPromptAttribution)
632709
adopted.PendingPromptAttribution = &pending
633710
}
711+
if source.AdoptClaim != nil {
712+
claim := *source.AdoptClaim
713+
adopted.AdoptClaim = &claim
714+
}
634715
return adopted
635716
}
636717

cmd/entire/cli/session_auto_adopt.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,23 @@ func tryAutoAdoptCrossCommonDirSession(ctx context.Context) {
188188
adoptCancel()
189189
if adoptErr != nil {
190190
var rollbackFailed *adoptRollbackFailedError
191-
if errors.As(adoptErr, &rollbackFailed) {
191+
var claimed *sourceClaimedError
192+
switch {
193+
case errors.As(adoptErr, &rollbackFailed):
192194
// Retire failed AND rollback failed: the session is now registered in
193195
// both the source and target repos. This is corruption, not a miss.
194196
logging.Error(logCtx, "auto-adopt: adopt left session registered in both repos",
195197
slog.String("session_id", source.SessionID),
196198
slog.String("error", adoptErr.Error()),
197199
)
198-
} else {
200+
case errors.As(adoptErr, &claimed):
201+
// Lost the concurrent-adopt race: another target claimed this source
202+
// first, under the shared source lock. Adopt-exactly-once is preserved,
203+
// so this is an expected skip, not a failure.
204+
logging.Debug(logCtx, "auto-adopt: source already claimed by a concurrent adopt",
205+
slog.String("session_id", source.SessionID),
206+
)
207+
default:
199208
logging.Warn(logCtx, "auto-adopt: adopt failed",
200209
slog.String("session_id", source.SessionID),
201210
slog.String("error", adoptErr.Error()),
@@ -396,7 +405,7 @@ func collectRegistryAutoAdoptCandidates(
396405
// alone therefore cannot steal across unrelated repos.
397406
resolved++
398407
resolveCtx, resolveCancel := context.WithTimeout(ctx, autoAdoptGitResolveTimeout)
399-
cand, ok := candidateFromSource(resolveCtx, entry.WorktreePath, entry.SessionID, staged, owner, hasOwner)
408+
cand, ok := candidateFromSource(resolveCtx, entry.WorktreePath, entry.SessionID, targetCommonDir, staged, owner, hasOwner)
400409
resolveCancel()
401410
if ok {
402411
out = append(out, cand)
@@ -456,7 +465,7 @@ func collectSiblingAutoAdoptCandidates(
456465
if !isRecentAdoptCandidate(state) {
457466
continue
458467
}
459-
cand, ok := candidateFromLoaded(store, worktree, commonDir, state, staged, owner, hasOwner)
468+
cand, ok := candidateFromLoaded(store, worktree, commonDir, targetCommonDir, state, staged, owner, hasOwner)
460469
if ok {
461470
out = append(out, cand)
462471
}
@@ -468,6 +477,7 @@ func collectSiblingAutoAdoptCandidates(
468477
func candidateFromSource(
469478
ctx context.Context,
470479
sourceWorktree, sessionID string,
480+
targetCommonDir string,
471481
staged []string,
472482
owner proclive.Identity,
473483
hasOwner bool,
@@ -480,12 +490,13 @@ func candidateFromSource(
480490
if err != nil || state == nil {
481491
return autoAdoptCandidate{}, false
482492
}
483-
return candidateFromLoaded(store, worktree, commonDir, state, staged, owner, hasOwner)
493+
return candidateFromLoaded(store, worktree, commonDir, targetCommonDir, state, staged, owner, hasOwner)
484494
}
485495

486496
func candidateFromLoaded(
487497
store *session.StateStore,
488498
worktree, commonDir string,
499+
targetCommonDir string,
489500
state *session.State,
490501
staged []string,
491502
owner proclive.Identity,
@@ -494,6 +505,14 @@ func candidateFromLoaded(
494505
if !isRecentAdoptCandidate(state) {
495506
return autoAdoptCandidate{}, false
496507
}
508+
// A source already carrying a FRESH adoption claim by a DIFFERENT target is
509+
// mid-adopt by another commit; drop it so this commit does not double-adopt.
510+
// Our own claim (same common dir) and a stale claim (aborted commit) do not
511+
// exclude. This is the best-effort discovery-time skip; the authoritative
512+
// serialization is the under-lock claim check in adoptFromExternalSessionStore.
513+
if adoptClaimBlocks(state.AdoptClaim, targetCommonDir) {
514+
return autoAdoptCandidate{}, false
515+
}
497516
// Auto-adopt is ACTIVE-only (matches the live-registry prefilter; see
498517
// docs/architecture/sessions-and-checkpoints.md, "Automatic cross-common-dir
499518
// adoption").

0 commit comments

Comments
 (0)