Skip to content

feat(session): non-destructive duplicate Claude session-ID resolution (prototype)#1338

Draft
alanvgreen wants to merge 1 commit into
asheshgoplani:mainfrom
alanvgreen:prototype/fork-dedup-rekey
Draft

feat(session): non-destructive duplicate Claude session-ID resolution (prototype)#1338
alanvgreen wants to merge 1 commit into
asheshgoplani:mainfrom
alanvgreen:prototype/fork-dedup-rekey

Conversation

@alanvgreen

Copy link
Copy Markdown

Summary

Prototype fix for the "can't fork a session in a multi-session project dir" symptom: pressing f (or agent-deck session fork) does nothing on the newer of two Claude sessions that share a working directory.

Root cause

New sessions and forks already get distinct --session-ids. But when two live Claude sessions in the same project dir end up sharing a ClaudeSessionID at runtime, UpdateClaudeSessionsWithDedup (internal/session/instance.go) resolves the collision destructively:

inst.ClaudeSessionID = ""          // blank the newer session
inst.ClaudeDetectedAt = time.Time{} // zero the freshness stamp

CanFork() requires a non-empty, recently-detected ID, so the newer session becomes un-forkable — the TUI never renders the f key. 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 change

Adds 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 <fresh> --resume <shared> --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 Default multi-instance mode (allow_multiple=true) lets concurrent instances tear down each other's live sessions #1246 family);
  • falls back to the legacy blank when the owner can't be forked (nothing to inherit).

⚠️ Prototype scope — opening as draft for design review

The function + tests are here, but it is not yet wired into the dedup call sites (internal/ui/home.go). Wiring requires the caller to Restart() the returned instances so the fork materializes. The open design question is how:

  • auto-restart the colliding session, vs.
  • surface a user-prompted "this session shares an ID — split it?" action.

Feedback on that wiring approach is what this draft is for.

Tests

  • TestLegacyDedup_ReproducesUnforkableSymptom — characterizes the current destructive behavior (blank + zero → CanFork() false).
  • TestRepairDuplicateClaudeSessions_RekeysNewerCollider — newer collider gets a fresh, distinct ID, stays forkable, fork command set.
  • TestRepairDuplicateClaudeSessions_FallsBackToBlankWhenOwnerNotForkable — degenerate case.

All pass (5/5 in a clean HOME); gofmt/go vet clean; full internal/session package shows zero new failures vs. baseline.

Refs #1147, #1246

🤖 Generated with Claude Code

… (prototype)

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 <fresh> --resume <shared> --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) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a690fbc-ae5e-4687-abdb-524675d4ebe1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@smorin

smorin commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@alanvgreen can you expland this to support the know f agents, which includes codex, pi and opencode?

@asheshgoplani

Copy link
Copy Markdown
Owner

Checking in on this prototype — a lot has shipped since it was opened (v1.9.50→56, incl. the #1352 session-id collision guards and #1384 credential-chain fix, which overlap this PR's problem space). Is the non-destructive duplicate-session-ID resolution still something you want to pursue? If yes: a rebase onto current main + un-draft and it'll get a prompt review. If the recent fixes cover your original pain, no hard feelings closing it — just let us know either way so the board stays accurate.

@asheshgoplani

Copy link
Copy Markdown
Owner

Rebase-or-close check — here's what's changed in this problem space, and where this PR still stands.

Not superseded. The two big collision-family fixes that shipped since this draft opened address adjacent symptoms, not this PR's core mechanism:

Neither touches the destructive dedup this PR targets. Confirmed on current main (v1.9.60): UpdateClaudeSessionsWithDedup still resolves a same-dir collision by blanking the newer session (inst.ClaudeSessionID = "" / inst.ClaudeDetectedAt = time.Time{}, internal/session/instance.go), and RepairDuplicateClaudeSessions is not present. So the original symptom — the newer of two Claude sessions sharing a working dir becomes un-forkable (the f key never renders) and flip-flops forever — is still reproducible. This PR's non-destructive re-key approach remains a real, unaddressed fix.

Caveat: the branch is ~91 commits behind main, and the function is still not wired into the dedup call sites (internal/ui/home.go) — by design, since the open question was the wiring (auto-restart the collider vs. a user-prompted "split this session?" action).

Next step — your call: if you still want this, a rebase onto current main + un-draft will get it a prompt review, and the maintainer can rule on the wiring approach at the same time (a user-prompted split is probably the safer default given it materializes a fork/restart). If the recent fixes have removed your original pain in practice, closing is fine — just let us know either way so the board reflects a clear state. (Also noting @smorin's ask above about generalizing to codex/pi/opencode — worth folding into the rebase scope if you proceed.)

@asheshgoplani

Copy link
Copy Markdown
Owner

This is a really nicely-scoped prototype. The characterization test that pins the current blank-and-zero behavior before introducing the non-destructive path is exactly the right instinct, and reusing buildClaudeForkCommandForTarget rather than hand-rolling a fork command keeps this aligned with how manual forks already work. The core approach is sound — re-keying the newer collider to its own --fork-session UUID is the right fix, and because it forks against the shared conversation it never mutates the owner's on-disk transcript, so there's no data loss.

A few things to consider on the way to completion:

  1. Wiring / consent. RepairDuplicateClaudeSessions isn't called yet, which is the open design question you flagged. A user-prompted "this session shares an ID — split it?" action feels safer than a silent auto-restart, since the repair force-restarts a live session (interrupting whatever the agent is doing).
  2. The CanFork() 5-minute freshness gate. If a collision is noticed more than 5 minutes after the owner's last detection, owner.CanFork() is false and the code silently falls back to the destructive blank — i.e. the symptom reappears intermittently. A timing-boundary test (and perhaps refreshing the owner's detection on this path) would close that gap.
  3. Idempotence. If the repair runs on a poll loop, guard against reprocessing a session that's already mid-repair (IsForkAwaitingStart set) before it restarts.
  4. Coverage / generalization. Worth covering 3+ colliders sharing one ID (each newer one gets its own key), and noting that this is Claude-only today — codex/pi/opencode have their own fork paths if you want to generalize later.

A rebase onto current main would help too. The underlying bug is confirmed still present, so this is solving a real problem — nice work. Leaving as draft per its status; flagging the above for whenever you pick it back up.

Copy link
Copy Markdown
Owner

🤖 autopilot: No CI check runs found for this PR. Cannot merge without a passing CI run. This PR also appears to address session-ID deduplication that may be superseded by #1526 — please check for overlap before reviving. Please push a commit or re-run CI to trigger checks if still relevant.


Generated by Claude Code

Copy link
Copy Markdown
Owner

🤖 autopilot 2026-06-29: DRAFT — PR is marked as draft and no CI checks have run. Additionally FLAGGED as data-sensitive (touches internal/session/instance.go — duplicate session-ID resolution). No action this run.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants