Make daemon/client build skew visible and resolvable with hunk daemon status/restart - #1099
Merged
Conversation
Contributor
|
Too many files changed for review (59 files, 50 file limit). Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Snapshot the fullest registration and snapshot payload for every review launch mode into a fixture whose name embeds the daemon revision. A payload change against the current fixture now fails with instructions to bump the revision and regenerate, and every corpus entry must round-trip through the daemon parsers. Exhaustive key lists tie the corpus to the session types so a new field cannot be left out.
The pre-commit formatter reflows the fixture JSON, so compare parsed values rather than text.
The daemon closes a refused registration or snapshot with a fixed reason and nothing else, which turned the last daemon/client skew into a bisect. Thread a key path through the wire parsers so the innermost rejection is recorded, and log the parser name and path (never the payload) to the daemon's stderr under HUNK_DEBUG=1.
…hello A post-hello 1008 close for an incompatible registration or snapshot was a silent reconnect. Treat it as a warning-bearing close with its own message and keep the reconnect loop running so a daemon restart recovers the window.
Expose two admin actions, status and stop, behind the ordinary signed caller handshake. The scope authenticates against its own frozen contract version instead of the app revision, using a second authenticator instance with the same daemon identity and credentials, so a Hunk build on a different revision can still ask a daemon what it is and ask it to retire. Its caller sessions are unknown to the main authenticator and cannot reach the session API. Status reports the daemon revision, app version, pid, uptime, and the attached sessions with the revision each producer presented in its hello. Stop closes attached producers with 'Session daemon restarting.' after the acknowledgement is sent, then runs the existing graceful shutdown. Hunk wires the scope into its daemon; the Bun and Node runtime fixtures cover a caller from another revision.
After a refused hello the broker client now asks the daemon's admin scope what build it is and publishes a sticky, direction-aware notice: a window newer than the daemon is told to run `hunk daemon restart` and keeps polling so it attaches once the daemon is replaced; a window older than the daemon is told to relaunch and backs off to a slow poll. A post-hello registration rejection gets its own notice. The status bar keeps the notice until the link reaches connected; only genuine faults still go to the console. The daemon can impersonate another revision through an internal, test-only environment override so PTY coverage can drive both directions across real processes.
Replace the generic 'incompatible or missing required support' failure with a DaemonBuildMismatchError that names both builds, counts the attached windows from the daemon's admin scope, and recommends 'restart-daemon' or 'use-newer-hunk' by direction. A daemon that predates the admin scope falls back to its launch metadata. Under --json the error is written in-band on stdout with exit code 1 so agents can decide before restarting. The review skill now tells agents to report both builds and ask before running `hunk daemon restart`.
'status' reads the daemon's admin scope and prints its build, uptime, pid, and attached windows, marking windows that could not reconnect to a daemon started from this CLI; a daemon from before the admin scope is described from its launch metadata, and no daemon exits 0. 'restart' prints the same summary, asks for confirmation (or takes --yes; non-TTY without it exits 1), takes the daemon launch lock so no attached window can respawn the old binary, stops the daemon through the admin scope, waits for the port to clear, spawns the replacement from this CLI's own binary, waits for health, and reports the new daemon. A pre-admin daemon is stopped by SIGTERM to the pid in its launch metadata only after a second explicit confirmation. The cross-process integration test attaches a window from the old build and a refused window from this build, restarts, and checks that the daemon pid and revision changed, the refused window attached, and the old window was told to relaunch. The PTY skew test now replaces the daemon through the command the notice names.
Recognize the renamed built-revision declaration in the install-VM daemon upgrade fixture, and give the restart integration test's CLI-backed polls headroom for a saturated full-suite run.
Bun 1.4's --no-orphans kills every descendant of a spawned process the moment it exits, so a replacement daemon started by a short-lived spawned 'hunk daemon restart' never survived the suite. Run the restart in-process through the same command implementation the CLI calls, with the replacement spawned from Hunk's real entrypoint. The window fixture now also resolves its daemon launch command to Hunk's entrypoint rather than to itself, so a slow health probe cannot make it spawn copies of itself.
Complete the status-line broker stub and cover a sticky daemon notice alongside a timed notice at wide and overflowing widths. Keep the warning after the ordinary notice expires and clear it only on reconnect.
The window's own notice promises that closing older Hunk windows lets it reconnect automatically, which depends on the incumbent going quiescent: the daemon deliberately does not count a refused producer's connection attempts as activity. Probing the admin scope for the incumbent's build undid that, because the control handler counted every read as activity and the window re-probed on each reconnect, so the daemon it was waiting out could never idle. Stop treating admin status as activity, and probe one incumbent once (keyed on its launch fingerprint) instead of on every reconnect, which also stops the refined notice from being overwritten by the generic one and stops churning caller sessions on the daemon.
The wait notice told the user to close older Hunk windows and promised an automatic reconnect, without mentioning the restart command that does it immediately or the fact that the automatic route waits for the incumbent's idle timeout. Name both routes. Retry the build probe against the same incumbent when the probe itself did not land, so a transient failure cannot strand a window on the generic notice for that daemon's whole life — including when the daemon is the newer build and closing older windows cannot help. A daemon that answered, even to refuse the admin scope, is still asked only once. Bind the install-VM upgrade scenario's expected warning to the exported message instead of a copied fragment.
Keep sticky notices within an 80-column status line and share build relations, restart costs, and relaunch guidance across the TUI and CLI. Remove internal revision rendering and misleading per-window markers without changing JSON or wire contracts.
Adapt the daemon-notice width coverage to this branch's status bar, which shows one notice by priority rather than composing several items, and keep the three widths so an overlong notice still fails loudly. Describe the final behavior in a single changeset instead of adding a second note for wording inside an unreleased feature.
Member
Author
|
Rebased onto
Local checks on the rebased tree: |
elucid
force-pushed
the
daemon-version-skew
branch
from
September 11, 2026 18:38
be77eef to
1dd412c
Compare
The daemon notice test was narrowed while this branch still targeted the old single-notice status bar, which could only show one notice by priority. StatusLine composes several items, so restore the version that proves the sticky daemon notice coexists with a timed notice, outlives its expiry, and survives an 80-column row rather than merely outranking it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After a Hunk upgrade, a daemon from the previous build keeps running for as long as any old window holds it open. New windows that connect to it are refused at the hello (revision mismatch) or — worse, as in the recent
hunk showincident — accepted at the hello and then have their registration rejected one parser deeper because #1064 changed a payload shape without bumpingHUNK_SESSION_DAEMON_VERSION. In both cases nothing told the user what happened, agents got a generic "incompatible or missing required support" message, and the only way to replace the daemon waskill.This PR makes version skew visible on every surface and gives users one supported way to resolve it. It does not replace the daemon automatically: old and new windows are peers, and the daemon is replaced only when the user runs
hunk daemon restartand confirms.What changes
Guard against the root cause
wire.snapshot.test.tsbuilds the fullest registration + snapshot payload for every launch mode (all three review-descriptor kinds incl.displayRevisionand comparison commits, catalog, capability digest, experimental features, live comments withparentId, review notes, …) through the realcreateSessionRegistration/updateSessionRegistration, round-trips it through the daemon parsers, and compares it tofixtures/session-wire.v<N>.json. Changing the payload without bumpingHUNK_SESSION_DAEMON_VERSIONfails with instructions;bun run generate:session-wireregenerates. Compile-time exhaustive key lists tie the corpus to the session types so a new field cannot be left out.AGENTS.mddocuments the rule.Diagnosability
rejected registration from session <id>: parseExtensionReviewDescriptor returned null at info.reviewunderHUNK_DEBUG=1(parser name and path only, never payload contents).1008 "Incompatible session registration."/"…snapshot."close is now a warning-bearing close instead of a silent reconnect; the reconnect loop keeps running so a daemon restart recovers the window.Revision-tolerant admin scope (
@hunk/session-broker)statusandstop, behind the ordinary signed caller handshake and on-disk credentials, authenticated by a secondSessionBrokerAuthenticatorwhose fixed contract usesSESSION_BROKER_ADMIN_SCOPE_VERSION(=1) in place of the app revision, on its own paths (/session-admin/*). Its caller sessions are unknown to the main authenticator and cannot reach/session-api; the ordinary hello still requires an exact match. Response schemas are frozen per scope version.statusreports daemon revision, app version, pid, uptime, and attached sessions with the revision each producer presented in its hello.stopsends the signed ack, then closes producers with"Session daemon restarting."and runs the existing graceful shutdown.admin-scopemode): a caller from another revision reachesstatus/stop, is refused forlist, and an admin caller session cannot reach the API.Direction-aware window notices (TUI)
Not connected to the session daemon (daemon build 0.21.1, this window 0.22.0). Runhunk daemon restart.— keeps polling and attaches on its own once the daemon is replaced. When both builds report the same package version, the revision is added (0.22.0 (revision 14)), which is exactly the incident's shape.This window is on an older Hunk build than the session daemon. Relaunch it to reconnect (notes in this window will be lost).— backs off to a 30 s poll.The session daemon rejected this window's registration; runhunk daemon restart.App, distinct from the 4 s timed session notice) and clears when the link reachesconnected. It flowsSessionBrokerClient.subscribeConnectionNotice→useHunkSessionBridge→App; UI code never touches the connection. Only genuine faults still reach the console.Agent-facing structured error
hunk session *command now fails withDaemonBuildMismatchError(kind: "daemon-build-mismatch") carryingdaemon,cli,attachedSessions(count + list when the admin scope answered),launchmetadata for a pre-admin daemon, andrecommendedAction: "restart-daemon" | "use-newer-hunk". Under--jsonit is returned in-band on stdout with exit 1.hunk daemon restart --yes, and to re-runhunk session listafterwards rather than relaunching windows.hunk daemon status/hunk daemon restartstatus [--json]: daemon build, uptime, pid, attached windows (marked(older build)when they could not reconnect to a daemon started from this CLI); launch-metadata fallback for a pre-admin daemon; "No session daemon is running." with exit 0.restart [--yes] [--json]: prints the summary, asksRestarting will disconnect N attached windows. …Continue? [y/N](non-TTY without--yesexits 1), takes the daemon launch lock before stopping anything so no attached window can respawn the old binary, stops via the admin scope, waits for the port to clear, spawns the replacement from this CLI's own binary, waits for health, releases the lock, reports the new daemon. A pre-admin daemon (the one being upgraded from) is stopped by SIGTERM to the pid in its launch metadata only after a second explicit confirmation; nothing ever signals a pid otherwise.Notes
HUNK_SESSION_DAEMON_VERSIONstays 15: nothing on the producer wire changed. The admin scope is versioned independently.HUNK_INTERNAL_SESSION_DAEMON_VERSIONoverride so cross-process skew tests can run without building a second binary; the install-VM upgrade fixture regex was updated to recognize the renamed built-revision declaration.bun test --no-orphanskills every descendant of a spawned process the moment it exits, so the restart integration tests run the restart in-process through the same command implementation the CLI calls (the CLI contract itself is covered by spawned help/non-TTY runs).Verification
bun run typecheck,bun run lint,bun run deps:checkclean.bun run test: only pre-existing failures on this macOS machine (install-VM harness path tests, one jj test); everything else green including the newtest/session/daemon-restart.test.tsend-to-end (old daemon + attached old window + refused new window → restart → new window attaches, old window told to relaunch, pid/revision changed).bun run test:integration: newtest/pty/daemon-version-skew-integration.test.tspasses for both directions and for the notice clearing after restart; the remaining PTY failures (lifecycle signals, extension trust prompt) reproduce onmain.bun run test:session-broker-nodeandbun test ./test/review-conformancegreen.hunk daemon status,hunk session list(text and--json),hunk daemon restart(non-TTY refusal),hunk daemon restart --yes, thenhunk daemon statusshowing the matched replacement — all as described above.