From 3f5087d02f494b915e565ad13551359ad8f60314 Mon Sep 17 00:00:00 2001 From: Alan Green Date: Tue, 9 Jun 2026 07:03:25 +1000 Subject: [PATCH] feat(session): non-destructive duplicate Claude session-ID resolution (prototype) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two live Claude sessions in the same project dir end up sharing a ClaudeSessionID, UpdateClaudeSessionsWithDedup blanks the newer one's ID and zeroes ClaudeDetectedAt. CanFork() requires a non-empty, recently-detected ID, so the newer session becomes un-forkable — the TUI never renders `f` — and because its tmux env still asserts the shared ID it re-adopts it on the next poll and flip-flops forever. This is the "can't fork a same-cwd session" symptom. Add RepairDuplicateClaudeSessions, which resolves the collision non-destructively: the older session (by CreatedAt) keeps the ID; the newer collider is re-keyed to its OWN fresh session ID, forked from the shared conversation (claude --session-id --resume --fork-session) via the existing fork machinery. It stays forkable, and once restarted the two sessions have genuinely distinct CLAUDE_SESSION_IDs so restart_sweep no longer cross-kills them (issue #1246 family). Falls back to the legacy blank when the owner cannot be forked. Prototype scope: adds the function plus unit + characterization tests; not yet wired into the dedup call sites (the caller must Restart() the returned instances for the fork to materialize). Opening as a draft for design review of that wiring (auto-restart vs. user-prompted "split this session?"). Refs #1147, #1246 Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/session/fork_dedup_rekey_test.go | 161 ++++++++++++++++++++++ internal/session/instance.go | 74 ++++++++++ 2 files changed, 235 insertions(+) create mode 100644 internal/session/fork_dedup_rekey_test.go diff --git a/internal/session/fork_dedup_rekey_test.go b/internal/session/fork_dedup_rekey_test.go new file mode 100644 index 000000000..d005c6b9f --- /dev/null +++ b/internal/session/fork_dedup_rekey_test.go @@ -0,0 +1,161 @@ +package session + +import ( + "strings" + "testing" + "time" +) + +// TestLegacyDedup_ReproducesUnforkableSymptom characterizes the CURRENT +// (shipped) behavior that this prototype targets: UpdateClaudeSessionsWithDedup +// blanks the newer collider's ID and zeroes ClaudeDetectedAt, leaving it +// un-forkable. This is the exact mechanism behind "f does nothing on a +// same-cwd session". Pinning it here makes the contrast with +// RepairDuplicateClaudeSessions explicit and guards against silent +// behavior drift. +func TestLegacyDedup_ReproducesUnforkableSymptom(t *testing.T) { + const shared = "8bb987b1-7d26-4c85-a606-c6a8a80f8645" + + older := NewInstance("make beads", "/home/src/tbd") + older.Tool = "claude" + older.Status = StatusRunning + older.ClaudeSessionID = shared + older.ClaudeDetectedAt = time.Now() + older.CreatedAt = time.Now().Add(-2 * time.Hour) + + newer := NewInstance("r/55 show knowledge", "/home/src/tbd") + newer.Tool = "claude" + newer.Status = StatusRunning + newer.ClaudeSessionID = shared + newer.ClaudeDetectedAt = time.Now() + newer.CreatedAt = time.Now().Add(-1 * time.Hour) + + // Both are forkable before dedup runs. + if !newer.CanFork() { + t.Fatalf("precondition: newer should be forkable before dedup") + } + + UpdateClaudeSessionsWithDedup([]*Instance{older, newer}) + + // Symptom: newer is blanked + zeroed → CanFork() false → no `f`. + if newer.ClaudeSessionID != "" { + t.Errorf("expected legacy dedup to blank newer's ID, got %q", newer.ClaudeSessionID) + } + if !newer.ClaudeDetectedAt.IsZero() { + t.Errorf("expected legacy dedup to zero ClaudeDetectedAt") + } + if newer.CanFork() { + t.Errorf("symptom not reproduced: newer is still forkable after legacy dedup") + } + // Older keeps the ID and stays forkable. + if older.ClaudeSessionID != shared || !older.CanFork() { + t.Errorf("older should retain ID and remain forkable") + } +} + +// TestRepairDuplicateClaudeSessions_RekeysNewerCollider is the prototype +// regression for the "can't fork a same-cwd session" symptom. +// +// Repro: two Claude sessions in the same project dir end up sharing one +// ClaudeSessionID (both resumed the same conversation). The legacy +// UpdateClaudeSessionsWithDedup blanks the newer one's ID and zeroes +// ClaudeDetectedAt, so CanFork() returns false and the TUI never renders +// the `f` key — and because the tmux env still asserts the shared ID, it +// flip-flops forever. +// +// RepairDuplicateClaudeSessions instead re-keys the newer collider to its +// OWN fresh forked session ID, keeping it forkable and making the two +// sessions genuinely distinct. +func TestRepairDuplicateClaudeSessions_RekeysNewerCollider(t *testing.T) { + const shared = "8bb987b1-7d26-4c85-a606-c6a8a80f8645" + + older := NewInstance("make beads", "/home/src/tbd") + older.Tool = "claude" + older.Status = StatusRunning + older.ClaudeSessionID = shared + older.ClaudeDetectedAt = time.Now() + older.CreatedAt = time.Now().Add(-2 * time.Hour) + + newer := NewInstance("r/55 show knowledge", "/home/src/tbd") + newer.Tool = "claude" + newer.Status = StatusRunning + newer.ClaudeSessionID = shared + newer.ClaudeDetectedAt = time.Now() + newer.CreatedAt = time.Now().Add(-1 * time.Hour) // newer than `older` + + // Precondition: both currently collide and the symptom is present + // (newer is forkable only because the IDs haven't been resolved yet). + if !older.CanFork() { + t.Fatalf("precondition: older should be forkable") + } + + rekeyed := RepairDuplicateClaudeSessions([]*Instance{older, newer}) + + // Owner (older) keeps the shared ID. + if older.ClaudeSessionID != shared { + t.Errorf("older lost its ID: got %q want %q", older.ClaudeSessionID, shared) + } + + // Newer is re-keyed to a distinct, non-empty ID (NOT blanked). + if newer.ClaudeSessionID == "" { + t.Fatalf("newer was blanked — legacy destructive behavior, symptom NOT fixed") + } + if newer.ClaudeSessionID == shared { + t.Errorf("newer still shares the ID: %q", newer.ClaudeSessionID) + } + + // ClaudeDetectedAt stays fresh so CanFork() is true (the actual fix). + if newer.ClaudeDetectedAt.IsZero() { + t.Errorf("newer.ClaudeDetectedAt was zeroed — CanFork() would fail") + } + if !newer.CanFork() { + t.Errorf("newer is not forkable after repair — symptom not fixed") + } + + // Newer is set up to materialize as an independent fork on next start. + if !newer.IsForkAwaitingStart { + t.Errorf("newer.IsForkAwaitingStart not set") + } + if !strings.Contains(newer.Command, "--fork-session") || + !strings.Contains(newer.Command, "--resume "+shared) { + t.Errorf("newer.Command is not a fork-from-shared command: %q", newer.Command) + } + + // The re-keyed instance is reported so the caller can Restart() it. + if len(rekeyed) != 1 || rekeyed[0] != newer { + t.Errorf("expected exactly the newer instance returned for restart, got %v", rekeyed) + } +} + +// TestRepairDuplicateClaudeSessions_FallsBackToBlankWhenOwnerNotForkable +// covers the degenerate case: if the owner can't be forked (no fresh +// detection / no conversation to inherit), there's nothing to fork from, +// so the newer collider is blanked as before. +func TestRepairDuplicateClaudeSessions_FallsBackToBlankWhenOwnerNotForkable(t *testing.T) { + const shared = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + + older := NewInstance("owner", "/home/src/tbd") + older.Tool = "claude" + older.ClaudeSessionID = shared + older.ClaudeDetectedAt = time.Now().Add(-10 * time.Minute) // stale → CanFork() false + older.CreatedAt = time.Now().Add(-2 * time.Hour) + + newer := NewInstance("dup", "/home/src/tbd") + newer.Tool = "claude" + newer.ClaudeSessionID = shared + newer.ClaudeDetectedAt = time.Now() + newer.CreatedAt = time.Now().Add(-1 * time.Hour) + + if older.CanFork() { + t.Fatalf("precondition: older should NOT be forkable (stale)") + } + + rekeyed := RepairDuplicateClaudeSessions([]*Instance{older, newer}) + + if newer.ClaudeSessionID != "" { + t.Errorf("expected legacy blank when owner not forkable, got %q", newer.ClaudeSessionID) + } + if len(rekeyed) != 0 { + t.Errorf("expected no re-keyed instances, got %d", len(rekeyed)) + } +} diff --git a/internal/session/instance.go b/internal/session/instance.go index 8dbbe876a..c8577e792 100644 --- a/internal/session/instance.go +++ b/internal/session/instance.go @@ -7859,6 +7859,80 @@ func UpdateClaudeSessionsWithDedup(instances []*Instance) { // Sessions will get their IDs from UpdateClaudeSession() during normal status updates } +// RepairDuplicateClaudeSessions resolves Claude session-ID collisions +// non-destructively. When two live instances share a ClaudeSessionID +// (e.g. two sessions in the same project dir that both resumed the same +// conversation), the older one (by CreatedAt) keeps the ID. +// +// The legacy UpdateClaudeSessionsWithDedup blanks the newer one's ID and +// zeroes ClaudeDetectedAt. That permanently breaks forking — CanFork() +// requires a non-empty, recently-detected ID — and because the session's +// tmux env still asserts the shared ID, UpdateClaudeSession re-adopts it +// on the next poll, so it flip-flops forever. This is why `f` does +// nothing on the newer of two same-cwd sessions. +// +// Instead, this re-keys the newer collider to its OWN fresh session ID, +// forked from the shared conversation (claude --session-id +// --resume --fork-session) — the same machinery a manual fork +// uses. The instance stays forkable, and once restarted the two sessions +// have genuinely distinct CLAUDE_SESSION_IDs, so restart_sweep no longer +// cross-kills them (issue #1246 family). The returned slice holds the +// re-keyed instances; the caller must Restart() them for the fork command +// to materialize the new conversation on disk and write the fresh ID into +// the tmux env. +// +// Falls back to the legacy blank-and-zero when the owner cannot be forked +// (no fresh detection / nothing to inherit) — there is no conversation to +// fork from, so the newer session must re-acquire an ID on its own. +func RepairDuplicateClaudeSessions(instances []*Instance) []*Instance { + // Work on a copy so callers don't observe order mutation as a side effect. + ordered := make([]*Instance, len(instances)) + copy(ordered, instances) + + // Sort by CreatedAt (older first keeps its ID; newer ones get re-keyed). + sort.SliceStable(ordered, func(i, j int) bool { + return ordered[i].CreatedAt.Before(ordered[j].CreatedAt) + }) + + var rekeyed []*Instance + idOwner := make(map[string]*Instance) + for _, inst := range ordered { + if !IsClaudeCompatible(inst.Tool) || inst.ClaudeSessionID == "" { + continue + } + owner, exists := idOwner[inst.ClaudeSessionID] + if !exists { + idOwner[inst.ClaudeSessionID] = inst + continue + } + + // Collision: `inst` is newer than `owner` (slice is CreatedAt-sorted). + if owner.CanFork() { + // buildClaudeForkCommandForTarget assigns inst.ClaudeSessionID a + // fresh UUID and builds the fork command resuming the shared ID. + cmd, err := owner.buildClaudeForkCommandForTarget(inst, inst.GetClaudeOptions()) + if err == nil { + inst.Command = cmd + inst.IsForkAwaitingStart = true + inst.ClaudeDetectedAt = time.Now() + idOwner[inst.ClaudeSessionID] = inst // claim the fresh, distinct ID + rekeyed = append(rekeyed, inst) + continue + } + sessionLog.Warn("dedup_rekey_failed_fallback_blank", + slog.String("instance_id", inst.ID), + slog.String("shared_id", owner.ClaudeSessionID), + slog.String("error", err.Error())) + } + + // Legacy fallback: nothing forkable to inherit — blank it so the + // session re-acquires its own ID from tmux env on the next poll. + inst.ClaudeSessionID = "" + inst.ClaudeDetectedAt = time.Time{} + } + return rekeyed +} + // wrapIgnoreSuspend wraps cmd in a bash -c invocation that disables CTRL+Z // suspension before running the command. This is the sole bash -c layer // in the command chain — ensureSandboxContainer returns a plain docker exec