Conversation
_resume_at_cwd returns None when it can't launch, and both call sites —
resume_transcript (the resume-any picker) and import_session (the target
half of a cross-host migration) — threw that away. The command is ACKed
whether the agent ran it or declined it, so the hub could not tell a
refusal from a slow spawn: a refused import left the move sitting in
`importing` until MIGRATE_TIMEOUT_MS and then failed with no reason, and
a refused resume left the Sessions page spinning out SPAWN_FOLLOW_MS.
Agent: every refusal in _resume_at_cwd, import_session and export_session
now goes through _refuse_start, which logs as before AND stages
{cmdId, migrationId, error} onto the beat's new `spawnFailures` list —
same held-across-a-failed-POST lifecycle as ticketStatusResults. The
reason is operator-facing, because it is what the UI shows. Also covered
inline, being the same silent hang in the same feature: the export half's
refusals (no transcript, pack failure, over-cap bundle, failed upload —
_migration_upload now answers whether the POST landed) and _resume_at_cwd's
launch exception, whose record is `status:"error"` and so never satisfies
the hub's wait for a RUNNING session either.
Hub: ingestSpawnFailures caches them per cmdId as `spawnRefusals` — served
with the record rather than stripped like the other caches, since the
client following that spawn is who needs it — and stamps `m.refusal` on a
matching in-flight migration, which advanceMigrations applies AFTER its
handoff check so a success always wins the tie. Only a host actually in
the move may fail it (every agent shares one token). Absent field = "that
agent can't tell", i.e. the old timeout wait.
Web: the Sessions page ends its spawn-follow wait on a refusal and toasts
the reason instead of spinning out the window; a refused MOVE stays silent
there because the migration follow already words it.
Android has no spawn-follow to surface this in — PARITY.md line added.
…erator QA (FAIL) found three defects in the first cut and two pre-existing ones it interacts with. Fixing what it proved: D1 (HIGH, caused here). spawnRefusals is the first per-host cache that is SERVED with the record, so agentRecordSize counts it — and the ceiling check runs BEFORE the ingest. The reason interpolates exception text and was unbounded, and making spawnFailures a known heartbeat key removed the 64 KiB unknown-field bound that used to cover it. QA drove two 4 MiB beats: the record went past AGENT_RECORD_MAX, every later beat 413'd (including the ones that would have swept the entry), the 8 MiB payload persisted to state.json and survived a hub restart, and the host was permanently offline to the hub. That is XERK-235's failure class. Both ends now cap the reason (agent SPAWN_FAILURE_REASON_MAX, hub SPAWN_FAILURE_ERROR_MAX), matching the 500 _set_error already truncated errorMsg to, and the hub bounds the cmdId key too. D2 (HIGH, pre-existing, but it nullified the fix). sessions.html has no `migrations` SSE listener, and the fleet payload is polled once at load while SSE is healthy — so cache.migrations never advanced in the browser. The follow never saw importCmdId, and no reason for a failed move ever reached the operator, which is precisely what the ticket set out to fix. Added the listener; the ticket's benefit is now actually delivered. D3 (MEDIUM, caused here). The migration stamp was host-checked but the per-cmdId cache was not, and the page scans the whole fleet — so one host could end another host's spawn wait with arbitrary text (QA proved it in a browser). A cmdId is now accepted only from the host the hub actually queued it to (prev.commands, which still holds it: the agent stages a refusal in the same handle_commands call that acks it). D4/D6 (LOW, caused here / pre-existing pattern). A `__proto__` cmdId hit the prototype setter instead of storing an entry, silently dropping the refusal; a non-array spawnFailures threw mid-handler after the record was replaced, costing the host its commands. Both guarded. Also: the client now looks up the reported session BEFORE reading a refusal, mirroring the hub's "a success wins the tie" — a refusal stays served for ten minutes, so the old order let a stale one abandon a spawn that had since landed. D5 (LOW, pre-existing) stands as documented, not fixed: _resume_at_cwd's launch-exception path leaves an `error` record holding a ttyd port. That is _set_error working as intended, so the PR claim is corrected rather than the behaviour. Tests: the four escaped mutants QA reported are now caught (verified by re-applying each), plus the launch-exception refusal, the truncation, the own-command check, the __proto__/non-string/non-array shapes, the record ceiling, an age-out asserted well under the count cap (the old one passed for the wrong reason), the SSE-driven move follow, and the tie-break.
…a hub restart costs QA's second pass (PARTIAL, nothing blocking) proved every fix and raised three LOW items. Two are comments, not code: F3 — the cmdId length cap and the __proto__/constructor/prototype key refusal cannot fail a test, because ownCmdIds already blocks anything they would catch (queueCommand mints ids from crypto.randomBytes). They stay as belt-and-braces behind that check, now saying so, rather than carrying a test that can never fail. F1 — a hub restart inside scheduleSave's 30s debounce forgets prev.commands and so drops a legitimate refusal, degrading that wait to the pre-XERK-265 timeout. That is correct: the hub can no longer tell whose command it was. Noted at the guard, with the reason not to 'fix' it by trusting the beat's own ackedCommands — that is the agent's word, which is the thing being checked. F2 (an SSE event landing during an in-flight poll is clobbered by refresh() rebinding cache) is older and wider than this change and is filed as XERK-274.
Main moved while QA ran: XERK-256 (prune on a worker thread), XERK-264
(one toast surface) and XERK-266 (the blob-relay scoping I filed) all
landed. The PR was CONFLICTING, which is why GitHub ran no checks on it.
Two things the merge needed beyond the textual conflict:
XERK-256 added _claim_worktree, which declines a resume while a prune is
removing that worktree — THE refusal this ticket cites as its reason to
exist ("ordinary timing between two features neither of which the
operator is thinking about"). Git merged it cleanly because it is a
different line, but it still only log()'d, so the change would have
shipped without covering its own motivating case. Both sites now report:
_resume_at_cwd's, and resume()'s (which needed cmd_id threading through
handle_commands).
resume()'s refusal has a worse failure mode than the one this ticket
describes, and the client half is NOT fixed here: a killed resume keeps
its id, so the dashboard deep-links ?session=<id>, whose wait resolves
only a running session and never times out — forever and silent, versus
90s and a toast. The reason is now on the wire either way; surfacing it
needs a decision about how the page correlates a by-id resume, filed as
XERK-276.
XERK-264 made TurmaNav.toast the page's ONE failure surface, so the
tests' toast assertions move from the page-local element to the harness's
`toasts` array. The conflicted line just needed both `sse` and `toasts`.
Suites: 1301 python, 1100 node.
…reviving Main moved again mid-review — XERK-263 (the migration bundle spools to disk instead of the hub's heap), XERK-262, XERK-278 — leaving the PR CONFLICTING, so GitHub ran no checks on it. Two textual conflicts, both taking main's shape and keeping mine beside it: the migration record gains blobPath/blobSize/uploading next to XERK-265's `refusal`, and PARITY.md keeps my XERK-265 gap line beside main's narrowed XERK-246 one. The conflicts git DIDN'T flag mattered more: - The refusal branch in advanceMigrations still did `m.blob = null`, which is now a no-op that leaks the spool FILE. It calls dropMigrationBlob(m), exactly as the timeout branch beside it does. - A move can now settle WHILE its bundle is being spooled, and the upload route flipped it back to `importing` unconditionally when the write finished — queueing an importSession for a move already reported failed, and killing the source once the target came up. XERK-265 is what makes this reachable: a refusal fails a move within one beat instead of after MIGRATE_TIMEOUT_MS, and the agent stages exactly this refusal when its own upload POST times out — i.e. precisely while the hub is still spooling. The route's entry check cannot see it (the phase was `exporting` when the body started), so the completion path re-checks and drops the now-orphaned file, answering the same uniform 404 as every other refusal there (XERK-266). Test notes, both of the "passed for the wrong reason" kind: - The bystander test parked a migration in `exporting` forever, and MIGRATE_INFLIGHT_MAX is 4, so it silently refused a LATER test's /migrate. It settles its own record now. - The first resurrection test only reached the entry guard, so removing the new check left it green. The replacement races a real body: half the bundle, then the refusal beat, then the rest — and it fails without the check. Verified by re-applying the mutant. Suites: 1301 python, 1128 node.
A newly-published HIGH is failing the agent image scan on every PR that rebuilds the image, mine included. It is not first-party: Trivy reports exactly one carrier, /usr/local/bin/terraform (1.15.8), which compiles in golang.org/x/net v0.55.0; the fix is x/net 0.56.0. There is nothing to bump to. Checked against HashiCorp's release index today: 1.15.8 (2026-07-08) is still the newest STABLE terraform, and everything after it is a 1.16.0 rc/alpha, which does not belong in a shipped image. That is the same position the existing terraform entries in this file are in. So it takes the documented route: a reviewed entry with the reason and an expiry (2026-10-25, matching the most recent batch), which forces the revisit rather than muting it. The blast radius is a DoS of one terraform invocation a session runs by hand — nothing in this image parses DNS responses from a resolver the operator did not already choose. Separate from XERK-265 deliberately: a suppression on a security gate should be reviewable on its own, not buried in a feature diff.
… can pass The agent image scan on this PR fails on a HIGH in the bundled terraform binary that has nothing to do with this change and blocks every PR in the repo. The triage for it is its own reviewable PR (#441) — merge that one first; these identical lines then merge as a no-op.
Contributor
Author
|
Note on the stacked commit: the agent image scan here was failing on The triage for it is #441, kept separate so a suppression on a security gate is reviewable on its own. This branch now merges that commit in so its own scan can pass. Merge #441 first — the identical lines then merge as a no-op here. The download flake that ate four scan jobs while sorting this out is filed as XERK-283. |
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.
The bug
_resume_at_cwd()returns the new session record, orNonewhen it could not launch — and both call sites threw that away:resume_transcript()(the resume-any picker) andimport_session()(the target half of a cross-host migration, XERK-101).A command is ACKed whether the agent ran it or declined it, so a refusal that only reaches the container log is indistinguishable from a slow spawn.
advanceMigrationswaits for the target to report a session whosespawnCmdIdis the recordedimportCmdId; a refused import never reports one, so the move sat inimportingfor the wholeMIGRATE_TIMEOUT_MSand then failed with no reason attached. The Sessions page spun out its 90s follow window the same way.It fails in the safe direction — the source is only killed once the target reports up — which is why the ticket is Low. XERK-256 is what makes it worth fixing now: it adds a refusal (
_claim_worktree, when a prune is removing the target worktree) that is ordinary timing between two features rather than operator error, and reads as "migration randomly hangs for the timeout".This takes the ticket's second, preferred option: a wire path.
The change
Agent. Every refusal in
_resume_at_cwd,resume_transcript,import_sessionandexport_sessiongoes through a new_refuse_start, which logs as before and stages{cmdId, migrationId, error}onto the heartbeat's newspawnFailures— same held-across-a-failed-POST lifecycle asticketStatusResults. The reason is operator-facing, because it is what the UI and the migration record show.Hub.
ingestSpawnFailurescaches them per cmdId asspawnRefusals— served with the record rather than stripped like the other caches, since the client following that spawn is who needs it — and stampsm.refusal, whichadvanceMigrationsapplies after its handoff check so a success always wins the tie.Web. The Sessions page ends its spawn-follow wait on a refusal and toasts the reason. A refused move stays silent there, because the migration follow already words it as "Move failed: …".
An absent field means "that agent can't tell" — i.e. the old timeout wait — on both halves, per the heartbeat wire contract.
Fixed inline, being the same silent hang in the same feature
exportingfor the full timeout, identically._migration_uploadnow answers whether the POST landed._resume_at_cwd's launch exception. Its record isstatus:"error", which never satisfies the hub's wait for a running session either.QA
Two passes by the
qaagent, against a real hub, the real agent and a real browser. First pass: FAIL — findings below. Second pass: PARTIAL, nothing blocking, with every fix independently re-proven, including D1 against its original attack verbatim.spawnRefusalsis the first served cache thatagentRecordSizecounts, and the ceiling check runs before the ingest. The reason interpolated exception text and was unbounded; addingspawnFailurestoHEARTBEAT_KNOWN_KEYSalso removed the 64 KiB unknown-field bound. Two 4 MiB beats pushed the record pastAGENT_RECORD_MAX, after which every beat 413'd — including the ones that would have swept the entry — the 8 MiB payload persisted tostate.jsonand survived a hub restart, and the host was permanently offline to the hub. XERK-235's failure class._set_error), plus the cmdId key. Re-verified: reason stored at 500 chars, payload 2,083 bytes (was 8,391,357), next five beats all 200 (were all 413). Also held against 60 consecutive at-cap beats, 20,000 entries in one beat, and 5 MiB cmdId/migrationId.sessions.htmlhas nomigrationsSSE listener and polls the fleet once at load while SSE is healthy, socache.migrationsnever advanced in the browser: the follow never sawimportCmdId, and no reason for a failed move ever reached the operator.Move failed: cwd '…' is not resumable on this host.__proto__cmdId hit the prototype setter instead of storing an entry; a non-arrayspawnFailuresthrew mid-handler after the record was replaced, costing the host its commands.42/{}/"boom"/trueall 200 (were 400).errorrecord holding its ttyd port._set_errorworking as intended. Because the client now looks up the session first, that record wins the tie, so the operator lands on a card showingerrorMsgwhile the hub still fails the move. One toast, one visible card, no silent hole.scheduleSave's debounce forgetsprev.commands, dropping a legitimate refusal.ackedCommands— that is the agent's word, which is the thing being checked.refresh()rebindscache, so an SSE event landing during an in-flight poll is clobbered — affects every listener on the page, not just the new one.ownCmdIdsalready blocks anything they would catch.Also corrected: one of my own tests passed for the wrong reason (the age-out entry was also the oldest, so the count trim retired it). Split into a separate 3-entry test, far under the cap.
Mutation: the four mutants QA found escaping the first cut are now caught — I re-applied each, and so did QA independently. Of ten mutants against the new guards, eight are caught; the two that escape are F3's, by design.
Unverified, and why: a real
claude/tmux/ttyd launch and two realhub-agent.pyprocesses. Every refusal path returns before_launch_tmux, so they were driven unstubbed against the realSessionManager; what remains unexercised is the successful import of a migrated session onto a second live agent, from both sides. Android/glasses/veiller were not built — no code changed there, and Android's decode is lenient.Separate findings, routed
<host>segment and never use it, so any agent-authed caller can upload or download any host's transcript bundle. The attachment relay 60 lines below does scope by host. Out of scope here; this PR adds that same check on the path it introduces.Parity
Android has no spawn-follow to surface a refusal in (its Resume is fire-and-forget), so
android/PARITY.mdrecords the gap. The SSE listener closes no Android gap — it polls rather than streams.Tests
python3 -m unittest discover -s agent/tests -p "test_*.py"— 1288 pass.node --test turma/tests/*.test.js agent/tests/*.test.js .github/scripts/tests/*.test.js— 1078 pass. Instruction files under the gate (CLAUDE.md27,930;.claude/rules/turma-sessions.md19,874).