fix: supersede active sessions when resuming in the same pane - #57
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 173e40290a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (or (nil? existing) | ||
| (and (contains? terminal-statuses | ||
| (:agent-status existing)) | ||
| (not (:superseded existing))))) |
There was a problem hiding this comment.
Restrict reactivation supersede to true resume events
This condition now supersedes other active sessions whenever an existing session is terminal and receives any :running update, but handle-hook! emits :running for many non-resume events (including default/unknown events), and the repo docs note asynchronous hook delivery (InstructionsLoaded), so a delayed event for an old completed session can incorrectly close the current active session in the same pane and revive stale state. Before this commit, that delayed update would not evict peers because supersede only ran for brand-new session IDs.
Useful? React with 👍 / 👎.
Summary
maybe-supersedeonly fired for brand-new session IDs. When Claude Code resumes a past session (--resume), hooks reuse the same session-id that already exists in the store, so supersede was skipped. The previously-active session in the same pane remained un-superseded, causing duplicate sessions in ceeker.maybe-supersedeto also fire when an existing session reactivates from a terminal state (:completed,:closed,:error), while excluding already-superseded sessions to prevent stale delayed hooks from evicting the current active session.terminal-statusesdefinition beforemaybe-supersedeto fix forward-reference lint error.Test plan
test-resume-supersedes-active-session-in-same-pane— core resume-dup scenariotest-resume-with-different-cwd-supersedes— different CWD same panetest-resume-does-not-affect-different-pane— parallel pane safetytest-resume-closed-session-supersedes— closed (non-superseded) session resumetest-ongoing-running-update-no-supersede— normal hook updates unaffectedtest-delayed-hook-on-superseded-session-no-eviction— stale hook safety (Codex review P1 fix)Residual risk
terminal-statusesto detect resume. If a new terminal status is added without updating the set, resume-supersede would not fire for that status.close-dup-pane-sessions!remains as a safety net for edge cases where supersede timing is insufficient.:runningreactivation viamerge-session-data.Generated with Claude Code