Skip to content

fix(server): resume external Claude Code sessions — adopt-into-managed (ADR-056) - #283

Merged
aterrylu merged 5 commits into
mainfrom
terry/external-cc-resume
Jul 18, 2026
Merged

fix(server): resume external Claude Code sessions — adopt-into-managed (ADR-056)#283
aterrylu merged 5 commits into
mainfrom
terry/external-cc-resume

Conversation

@aterrylu

Copy link
Copy Markdown
Owner

Problem

A real user hit this during onboarding: Claude Code sessions started from a terminal (claude, outside autonomOS) show up in the dashboard Projects panel, but clicking resume returns failed to resume session. Terry: "I don't think we had it work in a very long time."

Discovery was never broken. GET /api/projects lists external sessions correctly. The break was entirely resume-side — and it was one throw:

flowchart TD
    A["Click resume in Projects panel<br/>Sidebar.tsx"] --> B["store.ts sends<br/>resumeSessionId = CC uuid ✓"]
    B --> C["store.ts REWRITES<br/>resumeSessionId → resumeAgentId ✗<br/>(assumes agent.id == CC uuid)"]
    C --> D["POST /api/agents → spawnAgent"]
    D --> E["getAgent(resumeAgentId) → undefined<br/>throw 'not found' → HTTP 404"]
    E --> F["UI: 'failed to resume session'"]
    style C fill:#e8b4b4,color:#000
    style E fill:#e8b4b4,color:#000
Loading

Root cause, precisely dated: commit 9ac5d5b"unify Agent + Session (#165)", 2026-05-04. It deleted routes/sessions.ts (whose POST /api/sessions passed resumeSessionId straight to claude --resume <id>, no record required) for the agent-record-gated spawnAgent, collapsing two distinct id-spaces — a raw CC session id vs. an internal agent-record id — into one resumeAgentId resolved only by getAgent(). External sessions have no record, so every entry point (UI, POST /api/agents, /attach, MCP create_agent) 404'd. Never captured as an ADR — silent drift.

The provider's --resume <uuid> argv and the lazy-JSONL hasResumableSession probe survived intact. This PR reconnects the plumbing; it doesn't rebuild the engine.

Solution

spawnAgent gains a distinctly-named resumeSessionId (raw CC session id), separate from resumeAgentId:

flowchart TD
    R["resumeSessionId (raw CC id)"] --> L{"resolve in agent store<br/>by providerSessionId, or agent id"}
    L -->|hit| RA["REATTACH managed record<br/>markRunning + --resume"]
    L -->|miss| G{"assertAdoptable<br/>provider can probe? id is a UUID?"}
    G -->|no| X["reject 422 / 400<br/>no record created"]
    G -->|yes| P{"hasResumableSession<br/>transcript on disk?"}
    P -->|no| X2["reject 422 'nothing to resume'<br/>no orphan record"]
    P -->|yes| AD["ADOPT → new persistent<br/>managed record + --resume"]
    style RA fill:#b4d7e8,color:#000
    style AD fill:#b4e8c0,color:#000
    style X fill:#e8b4b4,color:#000
    style X2 fill:#e8b4b4,color:#000
Loading

resumeAgentId and /attach are untouched, so the ADR-049 dev-restart path carries no regression risk. Two names for two id-spaces is also the exact lesson of the bug being fixed.

Also fixes the dual-id footgun (reported by Terry during implementation): resuming a managed agent needed the autonomOS id, but the CC session id got tried first, 404'd, then retried. New spawns (fresh/fork/adopt) now use one id for both (id == providerSessionId). Pre-existing split-id agents resolve via a providerSessionId fallback on /attach — no id migration, since an id is referenced by managerId, layout panes, and persisted sessions.

Fail-closed by design

A silently-empty session is worse than an error, so adoption refuses rather than guesses. Each of these was found by /polish review agents and is covered by tests or QA:

Guard Behavior
Provider can't probe disk (Codex, Gemini) reject 422 — ADR-056's scope is now enforced, not just documented
Session id isn't a UUID reject 400 — the id becomes a record filename; blocks path traversal
No transcript on disk reject 422, no orphan record
Probe throws fail closed on adopt (fail-open is only correct for reattach)
Adopt crashes on boot safety net not armed — it would overwrite providerSessionId and erase the only pointer to the conversation
Reattach with a wrong cwd uses the record's workingDirectory, not the caller's guess
Empty-string resume id reject 400 instead of silently spawning fresh

Dashboard: stop rewriting resumeSessionId → resumeAgentId (client-side root cause), surface the server's error reason instead of a generic status, and match sessions on both id-spaces so a running split-id agent switches panes instead of erroring.

Testing

Real end-to-end — a genuine terminal-started claude session (outside autonomOS, env stripped), conversed to force the lazy JSONL write, then resumed through autonomOS:

Check Result
External session discovered by /api/projects isAutonomosAgent: null
POST /api/agents {resumeSessionId} → adopt 201, id == providerSessionId == CC id
--resume <external id> in argv, no --session-id ✅ real resume, not fresh
Same transcript continued, not forked ✅ 1 file, grew 56959→57315 bytes
Prior context actually loaded ✅ recalled PURPLE-OTTER-42
MCP create_agent(resumeSessionId) 201, recalled SILVER-BADGER-99
Re-resume → reattach, no duplicate ✅ same id, count stays 1
Codex adopt ✅ rejected 422
../../../../tmp/pwn as id ✅ rejected 400, no traversal write
Empty-string id ✅ rejected 400
No-transcript id ✅ rejected 422, agent count unchanged
Reattach with wrong cwd ✅ warned, used the record's, resume succeeded

The context-recall checks are the ones that matter — a session can spawn successfully with --resume and still come up empty, which is exactly the failure mode this bug class produces.

Automated: 11 new unit tests (store resolver, id == providerSessionId invariant, and the assertAdoptable guard — including path-traversal and unsupported-provider rejection). Full gate green: biome check (no --write), make check, and AUTONOMOS_INTEGRATION=1 make check653 server + 254 dashboard tests, 0 failures.

Risks

  • Adopt → managed (Terry's call): an adopted session becomes a first-class managed agent that survives restart. Intended, but it does mean resuming an external session now creates a persistent record.
  • id-unification applies to new spawns only. Existing split-id agents keep their ids and rely on the providerSessionId fallback. Migrating them was rejected as too high-blast-radius.
  • Error classification is substring-based in the route handler. Ordering is now explicit and commented, but typed Error subclasses + a central onError would be the durable shape — deliberately out of scope.
  • Codex/Gemini external resume is not shipped. The Projects panel discovers sessions via the CC SDK listSessions(), which doesn't enumerate Codex threads; that needs its own discovery source. The guard makes this an honest 422 rather than a silent empty session.

Alternatives considered

  • One universal resumeSessionId replacing resumeAgentId//attach — rejected: risks the tested managed-restart path and re-conflates the id-spaces that caused this bug.
  • Migrate existing split-id records to id == providerSessionId — rejected: ids are referenced by managerId, layout panes, persisted sessions.
  • Adopt as an ephemeral attach — rejected per Terry: the value is that a discovered session becomes managed.
  • Silently start fresh when an adopt target has no transcript — rejected: the user asked for that conversation.

Notes

  • ADR-055 is intentionally skipped — reserved for AppSecurityAudit's planned two-listener split (referenced in ADR-054). This PR takes ADR-056.
  • No conflict with PR fix(security): require auth on /mcp — closes an unauthenticated RCE (ADR-054) #281 (server auth surface) — providers/claude-code.ts is unmodified; its argv builder and probe are reused as-is.
  • docs/FEATURES.md F-004 updated: resume-into-managed is no longer N/A for discovered sessions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi

…d (ADR-056)

Clicking resume on a terminal-started `claude` session in the Projects panel
failed with `failed to resume session`. Discovery was never broken — the panel
lists external sessions fine — but every resume path funneled through
`resumeAgentId`, resolved only by internal agent id. External sessions have no
autonomOS record, so the lookup missed and spawnAgent threw `not found` (404).

Regressed in 9ac5d5b (#165 "unify Agent + Session"), which collapsed two
distinct id-spaces — a raw CC session id vs an internal agent-record id — into
one param. The provider's `--resume <uuid>` machinery survived intact; only the
plumbing to reach it with an arbitrary id was removed.

spawnAgent gains a distinctly-named `resumeSessionId` (raw CC id) that resolves
against the store by providerSessionId (or agent id, for migrated records) →
reattach, else ADOPTS the external session into a new persistent managed record
and --resume's it. `resumeAgentId` + /attach are untouched, so the ADR-049
restart path carries no regression risk.

Also unifies `id == providerSessionId` for fresh/fork/adopt, removing the
chronic footgun where resuming a managed agent needed the autonomOS id but the
CC session id was tried first, 404'd, then retried. Pre-existing split-id agents
resolve via a providerSessionId fallback on /attach — no id migration (an id is
referenced by managerId, layout panes, and persisted sessions).

Adoption is fail-closed throughout, since a silently-empty session is worse than
an error:
- providers that can't prove a session exists on disk (Codex, Gemini) are
  rejected — ADR-056's scope is now enforced, not just documented
- session ids are validated as UUIDs before becoming a record filename
- a missing transcript returns 422 with no orphan record
- a thrown probe fails closed on adopt (fail-open is only right for reattach)
- a failed adopt no longer arms the fresh-session safety net, which would have
  overwritten providerSessionId and erased the pointer to the conversation
- reattach uses the record's own workingDirectory, not the caller's guess

Dashboard: stop rewriting resumeSessionId → resumeAgentId (the root cause on the
client side), surface the server's error reason instead of a generic status, and
match sessions on both id-spaces so a running split-id agent switches panes
instead of erroring.

Verified end-to-end against a real terminal-started session: discovered, adopted,
`--resume` emitted, same transcript continued, and the resumed agent recalled a
phrase planted before the adopt. Same via MCP create_agent. All guards verified
to reject with zero orphan records.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi
Comment thread packages/server/src/routes/agents.ts Outdated

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — restores an entry point that had been silently dead since #165, and the fail-closed adopt guards (UUID shape check, provider-capability gate, no-transcript reject, fail-closed probe, safety-net disarm) are the right calls given the alternative is a phantom empty conversation over the user's real one. The dual id-space naming (resumeSessionId vs resumeAgentId) is the direct antidote to the bug being fixed; test coverage on the store resolver + assertAdoptable is proportionate to their blast radius; dashboard client-side change removes exactly the rewrite that caused the 404. Real-terminal QA (context-recall, path-traversal, wrong-cwd, empty-string, no-transcript) covers the meaningful failure modes.

One minor follow-up (posted inline, non-blocking): the empty-string sanity check for resume/fork ids lives in the REST handler, but the HTTP MCP create_agent in mcp.ts calls spawnAgent directly and bypasses it — same silent-fresh-spawn class the check exists to prevent. Promoting the guard into spawnAgent itself (or a shared boundary) would cover both entry points and any future callers; can land separately.

@aterrylu
aterrylu marked this pull request as ready for review July 18, 2026 07:38
aterrylu and others added 4 commits July 18, 2026 01:41
…ups)

Follow-ups from /pr-review-toolkit:review-pr on #283.

Real bug: the dashboard's dual-id `matchesId` matched when BOTH the argument
and a session's providerSessionId were undefined — SessionInfo's id fields are
optional, so a malformed entry would match an arbitrary session and switch the
user to the wrong pane. Guarded on a truthy id.

Testability, not just behavior — two critical guards were correct but unpinned,
one refactor away from silently regressing:

- The adopt veto on the onExit safety net lived as a `resolution !== "adopt"`
  conjunction at the callsite, outside any test. Moved INSIDE
  resumeSafetyNetArmed as an `isAdopt` param so it's covered by the same tests
  as the rest of the arming logic. If it regresses, an adopted session that
  crashes on boot has its providerSessionId reset to a fresh UUID and is
  respawned — erasing the only pointer to the user's conversation, silently,
  after the API already returned 201.
- The runtime→HTTP status mapping matched phrases authored in runtime.ts from
  routes/agents.ts, a cross-file coupling nothing pinned. Extracted as
  `spawnErrorStatus(message)` and tested from both ends: each phrase maps to its
  status, AND assertAdoptable's actual thrown messages classify as 422/400.
  Rewording a runtime error had silently degraded an actionable 4xx to a 500.

Both ordering hazards the comments only asserted are now tests: a cwd containing
the literal text "not found" must still map 422 (not 404), and an
unsupported-provider message must resolve 422.

Also corrected a test-suite honesty problem: the "id == providerSessionId
invariant" block seeded unified-id records by hand and asserted lookups against
its own fixture — tautological, and a regression in the code that MINTS the ids
would have passed. Renamed to "unified-id lookup contract" and documented that
the minting is covered by the end-to-end QA, not here.

Verified: full browser click-through against an isolated server — clicked the
real resume row in the Projects panel, the session adopted into a managed agent,
a terminal opened, and typing into it returned the passphrase planted before the
adopt. biome + make check + AUTONOMOS_INTEGRATION=1 make check: 660 server + 254
dashboard tests, 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi
…s it

Review catch (nox-0x on #283): the present-but-empty resume/fork id check lived
in the REST route, but the HTTP MCP handler (mcp.ts) calls spawnAgent DIRECTLY
rather than through /api/agents — so an empty-string resumeSessionId from that
entry point bypassed the guard entirely. Zod's z.string().optional() accepts "",
and every dispatch in spawnAgent is truthiness-based, so an MCP caller who
intended to resume but lost the id would get a fresh empty agent reported as
success. Exactly the silent failure the guard exists to prevent.

Verified before fixing: mcp.ts calls spawnAgent directly; channel-server/index.ts
POSTs to /api/agents (so it was already covered); the guard was route-only.

Moved into spawnAgent — the shared boundary every caller inherits, including
future direct callers. The message carries the "invalid session id" prefix so
spawnErrorStatus still classifies it 400. Route-level duplicate removed, with a
comment recording why the check is not there.

Tests pin both halves: message→400 classification, and PLACEMENT — the new cases
call the real spawnAgent and assert it rejects an empty/whitespace-only id for
all three fields, plus a negative that absent fields are untouched (a guard that
fired on absence would break every plain spawn). These need no PTY or `claude`
binary because the guard runs before cwd validation and binary resolution.

biome + make check + AUTONOMOS_INTEGRATION=1 make check: 668 server + 254
dashboard tests, 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi
Round 3 of review on #283 (three specialist agents). The headline finding is a
hole in my own earlier fix.

CRITICAL — the adopt veto on the resume safety net was spawn-scoped. It gated on
the spawn-local `resolution`, so it protected only the spawn that DID the
adopting. Afterwards an adopted record is indistinguishable from a fresh one
(both satisfy id == providerSessionId), so every LATER resume took the reattach
path with the net fully armed — and the first failure there regenerated
providerSessionId, leaving `--resume` pointing at an id with no transcript. The
user gets a healthy, named, EMPTY agent. Same failure I "fixed", one retry later.

Provenance is now PERSISTED as Agent.adoptedExternal and read from the record, so
the veto holds for the life of the agent. (Two reviewers disagreed on the
consequence; the record's `id` does survive, so a retry reattaches this record
rather than adopting a duplicate — the comments claiming otherwise, in three
places, are corrected.)

HIGH — reattachCwd reassigned `cwd` AFTER the only statSync, so a record whose
directory had since been deleted reached the PTY spawn unvalidated. Routine here:
wt-sync removes merged worktrees out from under agents that ran in them. Either
node-pty throws (a 500 for a 400-class problem) or the child dies instantly and
arms the safety net for a reason unrelated to resumability. Now re-validated with
a message naming the actual cause.

Also fixed:
- Adopting passed the session SUMMARY as the agent name, which became its
  agent:// address AND was sent as `--name` on the resume — rewriting customTitle
  on the user's own external session. Newly-live behavior (this path 404'd
  before). The dashboard no longer forwards it; the server mints `<dir> · <id>`.
- Non-string resume/fork ids (`resumeSessionId: 3421`, `null`) were coerced to
  undefined by the route and answered with a fresh empty agent. The boundary
  guard now rejects present-but-not-a-non-empty-string, and the route forwards
  the raw value so the guard can see it.
- The adopt-failure notification is the ONLY signal (the net is disarmed), but
  was gated on `crashed` + <5s — narrower than the failure. A CC path that errors
  and exits 0 was silent. Now fires on any short-lived adopt exit, worded by exit
  code.
- `Invalid working directory` and `Cannot use both ...` fell through to 500;
  both are client errors → 400.
- /attach had a hand-rolled status mapping that contradicted spawnErrorStatus's
  own doc comment ("the two entry points agree"): a live namesake returned 500
  there and 409 from POST /. Now shares the classifier.
- Sidebar had two more id-space crossings identical to the matchesId bug:
  liveSessionIds (split-id agents never showed the live dot) and sessionMetaMap
  (they lost summary/project/branch enrichment).
- Comment corrections: the resumeSessionId agent-id fallback is CONTRACT (what
  create_agent advertises), not migration residue; the id==providerSessionId
  invariant is mint-time only (the force-fresh net re-splits it); the pre-flight
  fresh-fallback applies to reattach only; spawnErrorStatus's coupling is
  partially pinned, and its ordering is not injection-proof in both directions.
- ADR-056 gained a Decision §5 recording all the fail-closed guards — three of
  them MODIFY ADR-049 behavior and so belong in the durable record, not just the
  changeset. FEATURES.md row no longer contradicts its neighbor.

Re-verified e2e after the changes: non-string → 400; adopt → 201 with
adoptedExternal true, persisted on disk, and surviving a kill+reattach; name is
`<dir> · <id>` not the summary; --resume emitted with zero fresh-fallbacks; and
the resumed session recalled AMBER-LYNX-31, planted before the adopt.

biome + make check + AUTONOMOS_INTEGRATION=1 make check: 669 server + 254
dashboard tests, 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi
Type-design review follow-ups on #283. All three are bounded; the larger
recommendations from that review are deferred (see below).

SpawnError — the throw sites this PR ADDED now declare their own HTTP status
instead of the route inferring intent by substring-matching prose authored in
another file. That coupling was one this PR created, and the repo already had
the pattern to avoid it (CachePoisonedError + the router's onError). Messages
are byte-identical, so spawnErrorStatus's chain classifies them the same for any
caller that sees only text; it stays as the fallback for the pre-existing
untyped throws. A test asserts the two mechanisms AGREE, so a typed status and
the chain can't silently diverge.

getAgentByProviderSessionId returned readCache()'s first hit — i.e. filesystem
read order, arbitrary and unstable across restarts. Nothing enforces
providerSessionId uniqueness, and the ADR-049 safety net regenerates the field,
so the id space isn't collision-free by construction. Now mirrors its sibling
resolveAgentByName: prefer a running candidate, else most recently updated. The
asymmetry between two adjacent lookup functions — one treating multiplicity as
designed, the other as impossible — was the smell.

buildNewAgent's positional boolean (`buildNewAgent(psid, true)`) is now an
options object; the callsite reads `{ adoptedExternal: true }`.

Deferred deliberately, recorded so they aren't lost: extracting a pure
`resolveSpawnTarget` returning a SpawnTarget discriminated union (two reviewers
asked for this; it restructures the hottest path in the change right when the
current version is freshly tested); replacing `adoptedExternal?: boolean` with a
richer `origin` union unified with FEATURES.md's unimplemented `source` concept
(persisted-schema change during a bug fix is a bad trade, and the boolean is
forward-compatible); narrow AgentId/ProviderSessionId branding on store accessor
signatures only; and converting the remaining legacy throws.

Verified through the real HTTP layer: bad UUID → 400, Codex adopt → 422, no
transcript → 422, empty id → 400, zero agent records created by any rejection.
biome + make check + AUTONOMOS_INTEGRATION=1 make check: 670 server + 254
dashboard tests, 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnRSVXE4eMY4ZehhCBB3Qi
@aterrylu
aterrylu merged commit 4f1e5c6 into main Jul 18, 2026
5 checks passed
@aterrylu
aterrylu deleted the terry/external-cc-resume branch July 18, 2026 16:54
aterrylu added a commit that referenced this pull request Jul 24, 2026
…d queue warning

Two policies established by this PR, recorded per CLAUDE.md's append-only
decision-record convention:

1. The prompt-delivery receipt applies only to providers that emit hook events
   (`hooks.eventCount > 0`), never a provider-name check — so a provider that
   doesn't exist yet is classified by what it declares rather than by an
   allowlist someone has to remember to update.

2. Queued Codex inbound warns the operator once per stall episode at 5 minutes,
   replacing a ~45-minute silence. The 15-minute tolerance for long turns is
   unchanged; waiting was always correct, not saying so was the bug.

Records the consequence explicitly rather than leaving it to be rediscovered:
Codex spawn-with-prompt now has NO delivery detector, and a lost prompt is
indistinguishable from a finished agent because the daemon reports the thread
idle either way. A Codex-native detector via thread/status is a scoped
follow-up, deliberately not this ADR.

The ADR number appears in exactly two places (the entry and one code comment)
and in no log strings, so a renumber stays a two-minute change — a past
collision touched ~18 references across 10+ files.

Number assigned by TeamLead@autonomOS (055 reserved for #284, 056 merged
with #283). Rides with this PR per the #281/#283 precedent so the code and its
rationale stay co-located in git log.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP
aterrylu added a commit that referenced this pull request Jul 24, 2026
…d queue warning

Two policies established by this PR, recorded per CLAUDE.md's append-only
decision-record convention:

1. The prompt-delivery receipt applies only to providers that emit hook events
   (`hooks.eventCount > 0`), never a provider-name check — so a provider that
   doesn't exist yet is classified by what it declares rather than by an
   allowlist someone has to remember to update.

2. Queued Codex inbound warns the operator once per stall episode at 5 minutes,
   replacing a ~45-minute silence. The 15-minute tolerance for long turns is
   unchanged; waiting was always correct, not saying so was the bug.

Records the consequence explicitly rather than leaving it to be rediscovered:
Codex spawn-with-prompt now has NO delivery detector, and a lost prompt is
indistinguishable from a finished agent because the daemon reports the thread
idle either way. A Codex-native detector via thread/status is a scoped
follow-up, deliberately not this ADR.

The ADR number appears in exactly two places (the entry and one code comment)
and in no log strings, so a renumber stays a two-minute change — a past
collision touched ~18 references across 10+ files.

Number assigned by TeamLead@autonomOS (055 reserved for #284, 056 merged
with #283). Rides with this PR per the #281/#283 precedent so the code and its
rationale stay co-located in git log.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP
aterrylu added a commit that referenced this pull request Jul 26, 2026
…-delivery false alarm (#287)

* fix(codex): stop losing inbound messages in silence + kill the prompt-delivery false alarm

A Codex agent appeared to drop an inbound message: send() returned success,
the gateway logged the connection, and no `[codex-inbound] injected` line ever
appeared. Investigation found the message was never dropped — it was correctly
QUEUED behind an unfinished turn, in complete silence. From outside, a working
queue and a lost message produced byte-identical logs.

Root causes fixed:

1. Provider-blind prompt-delivery receipt. `trackPromptDelivery` reads the hook
   relay, but was gated only on `if (params.prompt)`. Codex emits zero hook
   events, so its SessionStart never arrived and EVERY prompted Codex agent
   logged "may have failed to boot" and pushed a dashboard SystemWarning — on
   agents that had already run their prompt correctly. That false alarm actively
   misdirected the original diagnosis. Now gated on the CAPABILITY
   (`hooks.eventCount > 0`) so it stays correct if Codex ever ships hooks.
   Nothing is lost: the re-delivery fallback needs a SessionStart to become
   reachable, so for Codex it never was. Gemini (11 events) is unaffected.

2. The inbound queue said nothing. Delivery is idle-gated by design (a
   `turn/start` mid-turn corrupts the thread), but the wait emitted no output:
   no enqueue log, a 15-minute silent poll, and an operator notification only
   after 3 consecutive failures — ~45 minutes. Now logs enqueue, logs every
   expired attempt, distinguishes "thread still active" from "status
   unreadable" (naming the cause), and notifies the operator once per stall
   episode after 5 minutes. The 15-minute tolerance for long turns is kept —
   waiting is correct, not SAYING you're waiting was the bug.

3. Silent drops on the delivery paths:
   - `dispose()` cleared the queue with no log — the module's only true message
     drop, reachable on kill, delete, PTY exit and resume-failure respawn.
   - The broadcast fan-out skipped endpoint-less Codex agents bare, and
     broadcast has no per-recipient ack.
   - Unicast fell through to the channel-server WS for a non-running Codex
     agent, which "succeeds" into a socket whose reader ignores inbound.
   - `broadcastToAllAgents` had no `.catch()`; one throw took out every
     recipient after the sender was already ack'd.

Also fixed: `statusLoop`'s escalation could never fire, because `queryIdle`
swallows read failures so the catch never ran and `statusFailures` reset every
cycle — a daemon that accepts the socket but never answers `thread/read` would
freeze the dashboard silently. And RPC timeouts were never cleared on success,
leaking a 30s timer per call (test suite exit: 31s -> 1.8s).

NOT fixed, deliberately: the original report's other symptom — a Codex thread
that goes active at spawn and never finishes — did not reproduce locally with
an identical spawn and is Codex-side, past our submission path. This change
makes that failure VISIBLE rather than claiming to fix it.

Tests: 9 delivery-observability + capability-gate cases against a fake
app-server daemon (swaps the global WebSocket, so production framing, id
matching and timeout logic all stay under test). Verified live against a real
`codex` agent on an isolated dev server.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* docs: add changeset for the Codex inbound observability fix

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* docs: record ADR-057 — capability-gated prompt receipt + 5-min inbound queue warning

Two policies established by this PR, recorded per CLAUDE.md's append-only
decision-record convention:

1. The prompt-delivery receipt applies only to providers that emit hook events
   (`hooks.eventCount > 0`), never a provider-name check — so a provider that
   doesn't exist yet is classified by what it declares rather than by an
   allowlist someone has to remember to update.

2. Queued Codex inbound warns the operator once per stall episode at 5 minutes,
   replacing a ~45-minute silence. The 15-minute tolerance for long turns is
   unchanged; waiting was always correct, not saying so was the bug.

Records the consequence explicitly rather than leaving it to be rediscovered:
Codex spawn-with-prompt now has NO delivery detector, and a lost prompt is
indistinguishable from a finished agent because the daemon reports the thread
idle either way. A Codex-native detector via thread/status is a scoped
follow-up, deliberately not this ADR.

The ADR number appears in exactly two places (the entry and one code comment)
and in no log strings, so a renumber stays a two-minute change — a past
collision touched ~18 references across 10+ files.

Number assigned by TeamLead@autonomOS (055 reserved for #284, 056 merged
with #283). Rides with this PR per the #281/#283 precedent so the code and its
rationale stay co-located in git log.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* test(gateway): cover the non-running-Codex delivery guard

The PR's one behavior change had zero test coverage — gateway-router.test.ts
never exercised the Codex branches at all. Adds both directions against a real
isolated agent store:

- an EXITED Codex agent whose channel-server socket is still open now gets a
  visible error and NO socket write (previously: the write "succeeded", the
  sender was told null/success, and the recipient discarded the bytes)
- a running Claude Code agent still delivers over that same socket — the guard
  must stay Codex-specific, since Claude Code genuinely reads inbound there

Found by asking whether the change was actually verified rather than assuming
the suite covered it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* test(codex): cover the two escalation paths that were unreachable or one-shot

Both are behavior, not logging — a notification that can never fire looks
exactly like a healthy system, which is the failure mode this whole PR is about.

- statusLoop: a daemon that accepts the WebSocket but never answers thread/read
  now warns. Previously queryIdle swallowed the failure, so the catch never ran,
  statusFailures reset every cycle, and the warning was unreachable for the
  likeliest daemon failure there is.
- noteFailure: re-notifies on a doubling backoff instead of exactly once per
  controller lifetime.

Moves ensureThread's hardcoded 1s poll into `timings` as threadPollMs — it was
the last wait in the module a test couldn't shrink, which is what made the
second case unreachable in under 6 seconds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* docs(adr-057): state the idle-gate premise as an untested assumption

ADR-057's Context asserted that injecting a turn/start mid-turn "interleaves
and corrupts the thread" and called the resulting idle gate right. Subsequent
testing disproved it: 8 injections across 5 thread states (blocking MCP call,
apply_patch mid-write, wait_agent block, reasoning, backgrounded shell, plus a
control) were all accepted with the original work completing intact.

The claim is reworded, not removed. Deleting it would erase the "we believed X,
then tested it" trail; leaving it would enter a known-false statement into an
append-only record, where a future reader could believe it and never find the
reversal — the exact misleading-signal failure this PR exists to fix. Stating
it as an assumption untested AT THE TIME is simply accurate about that moment.

Consistency is what decides it: the forthcoming reversal ADR is required to
label its one surviving safeguard "untested conservatism, not a measured
requirement." We don't get to hold an hours-old ADR of our own to a lower bar
than the one we're writing today.

Docs only — no behavior change. Nothing has entered the record yet; #287 is
unmerged, so this is not a rewrite of history.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

* fix(codex): address review — remove dead `warned` field, give statusLoop the same backoff

Two findings from nox-0x's review of #287:

1. `QueuedInbound.warned` was dead state — set to false in enqueue() and never
   read again, a leftover from moving warn-once tracking to the controller-
   scoped `longWaitWarned`. Its doc comment described per-message semantics the
   code deliberately rejects. Removed the field and corrected the comment.

2. `statusLoop`'s escalation was still `=== FAILURES_BEFORE_WARN` — the exact
   one-shot-per-lifetime flaw this PR fixes in `noteFailure`, and its own comment
   claimed to "mirror the delivery-path warning" after that mirror had broken.
   Now escalates on the same doubling backoff (nextStatusWarnAt, re-armed on a
   successful reconcile), so a daemon still unreachable an hour later keeps
   surfacing instead of going quiet after the first warning. New test pins it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YkDUfhTJQDjyXetrZEpP

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants