Skip to content

fix: skip restart request for named sessions on self-handoff#927

Merged
julianknutsen merged 2 commits intogastownhall:mainfrom
quad341:fix/744-handoff-named-session-no-restart
Apr 19, 2026
Merged

fix: skip restart request for named sessions on self-handoff#927
julianknutsen merged 2 commits intogastownhall:mainfrom
quad341:fix/744-handoff-named-session-no-restart

Conversation

@quad341
Copy link
Copy Markdown
Collaborator

@quad341 quad341 commented Apr 19, 2026

Summary

  • Fixes gc handoff: self-handoff crashes human-attended (named) sessions on PreCompact #744: the default PreCompact hook runs gc handoff "context cycle", which called setRestartRequested unconditionally. Named (human-attended) sessions like the mayor are started by the user, and the controller cannot respawn them — so every context compaction killed Claude Code to the user's shell.
  • Before requesting a restart, doHandoff now looks up the open session bead for the current session and skips both the tmux and bead restart flags when the bead is a configured named session. Mail is still sent — context preservation was the whole point.

Why

The fix the issue describes — "check whether the current session is a named session, if so skip the restart and return after sending the mail" — matches what I did. isNamedSessionBead (the existing helper from cmd/gc/named_sessions.go) returns true when Metadata["configured_named_session"] == "true". A new local helper sessionIsNamed(store, sessionName) scans open session beads by the existing gc:session label and returns true only for a positive match, so pool sessions (which either have no bead at all or a non-named one) stay on the old restart path.

Test plan

  • New regression test: TestDoHandoff_Regression744_NamedSessionSkipsRestart in cmd/gc/cmd_handoff_test.go seeds a named-session bead and asserts mail is created, dops.restartRequested is NOT set, bead restart_requested is NOT set, and stdout does not promise a restart. Fails on main, passes after the fix.
  • Sibling tests pass: go test ./cmd/gc/ -run 'TestHandoff|TestDoHandoff' — the pre-existing TestHandoffSuccess / TestHandoffWithMessage / TestHandoffRemote* all continue to pass.
  • go vet ./cmd/gc/ clean.
  • go build ./cmd/gc/ clean.

🤖 Generated with Claude Code

@quad341 quad341 requested a review from julianknutsen as a code owner April 19, 2026 18:08
@github-actions github-actions Bot added the status/needs-triage Inbox — we haven't looked at it yet label Apr 19, 2026
gc handoff called dops.setRestartRequested (and setBeadRestartRequested)
unconditionally. Named (human-attended) sessions like the mayor are
started by the user — the controller cannot respawn them — so every
PreCompact "gc handoff context cycle" tick killed Claude Code to the
user's shell and lost any in-flight context-recovery work the mail
would have set up.

Before requesting a restart, look up the open session bead for the
current sessionName. If it is a configured named session, return after
sending the mail. Do not touch the restart flags. Headless pool sessions
continue to behave exactly as before (the store lookup either misses or
finds a non-named bead, and the restart path runs).

TestDoHandoff_Regression744_NamedSessionSkipsRestart seeds a named
session bead and asserts: mail is still created, dops.restartRequested
is NOT set, bead restart_requested is NOT set, and stdout does not
promise a restart.

Fixes gastownhall#744.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@julianknutsen julianknutsen force-pushed the fix/744-handoff-named-session-no-restart branch from 2c57560 to cd0aacd Compare April 19, 2026 21:13
@julianknutsen
Copy link
Copy Markdown
Collaborator

Adopted Review Summary

I adopted this PR and pushed a maintainer fixup on top of the contributor commit while preserving contributor authorship.

What changed during adoption

  • Rebasing brought the branch onto current main.
  • gc handoff now returns instead of blocking when a named on-demand session skips restart.
  • The restartability check now distinguishes controller-managed always named sessions from on-demand named sessions.
  • gc handoff --target skips killing on-demand named targets that the controller cannot restart.
  • gc runtime request-restart now uses the same on-demand named-session guard.
  • Skip paths clear stale restart metadata and fail closed if stale cleanup cannot be completed.
  • CLI help/reference docs were updated for the new semantics.

Review result

Triple-model review validated the core #744 fix: handoff mail is still created, but on-demand named sessions no longer request controller restart or emit a misleading drain event. A follow-up review finding about swallowed stale-cleanup errors was fixed before push.

Remaining broader policy questions are not blockers for this scoped bug fix: whether gc session reset should also refuse or warn for on-demand named sessions, and whether the reconciler should enforce named-session restartability as a defense-in-depth invariant.

Local verification

  • go test ./cmd/gc -run 'TestHandoff|TestDoHandoff|TestRuntimeRequestRestart|TestRequestRestartAcceptsNoArgs|TestDoRuntimeRequestRestart|TestReconcileSessionBeads_.*Restart|TestReconcileAndWake_Restart' -count=1
  • go vet ./cmd/gc
  • go test ./test/docsync -count=1

@julianknutsen julianknutsen force-pushed the fix/744-handoff-named-session-no-restart branch 2 times, most recently from f021bc8 to de82d6f Compare April 19, 2026 21:40
@julianknutsen julianknutsen force-pushed the fix/744-handoff-named-session-no-restart branch from de82d6f to 84c0e19 Compare April 19, 2026 21:54
@julianknutsen julianknutsen merged commit 514bd28 into gastownhall:main Apr 19, 2026
60 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-triage Inbox — we haven't looked at it yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gc handoff: self-handoff crashes human-attended (named) sessions on PreCompact

2 participants