Skip to content

fix(server): resume after self_exit fails with 'resumeAgentId not found' - #254

Merged
aterrylu merged 1 commit into
mainfrom
terry/resume-after-self-exit
Jun 26, 2026
Merged

fix(server): resume after self_exit fails with 'resumeAgentId not found'#254
aterrylu merged 1 commit into
mainfrom
terry/resume-after-self-exit

Conversation

@aterrylu

Copy link
Copy Markdown
Owner

Problem

Resuming an agent that called self_exit fails with resumeAgentId "<id>" not found.

Root cause: two "terminate" paths that look equivalent are asymmetric:

Path Route Effect Resumable?
kill_agent POST /api/agents/:id/killkillAttachmentmarkExited record kept as status:"exited"
self_exit DELETE /api/agents/:iddeleteAgentRawrmSync + cache.delete record hard-deleted

The resume path (spawnAgent with resumeAgentId) looks the agent up via getAgent(id) — a pure cache/disk read. After self_exit the record was rmSync'd off disk, so getAgent returns undefined and runtime.ts throws resumeAgentId "<id>" not found.

The reporting agent's theory ("autonomOS purges the session record on self_exit") was correct — the precise mechanism is the DELETE route's rmSync, not markExited.

sequenceDiagram
    participant A as Agent
    participant CS as channel-server
    participant API as server API
    participant Store as agents store

    rect rgb(255,235,235)
    note over A,Store: BEFORE — self_exit hard-deletes, resume fails
    A->>CS: self_exit
    CS->>API: DELETE /api/agents/:id
    API->>Store: deleteAgentRaw → rmSync + cache.delete
    Note over Store: record GONE
    A-->>API: create_agent({resumeSessionId})
    API->>Store: getAgent(id) → undefined
    API-->>A: ❌ resumeAgentId "id" not found
    end

    rect rgb(235,255,235)
    note over A,Store: AFTER — self_exit soft-exits, resume works
    A->>CS: self_exit
    CS->>API: POST /:id/kill {reason:"self_exited"}
    API->>Store: killAttachment → markExited("self_exited")
    Note over Store: record KEPT (status: exited)
    A-->>API: create_agent({resumeSessionId})
    API->>Store: getAgent(id) → record ✓
    API-->>A: ✅ resumed (status: running)
    end
Loading

Not a recent regression

self_exit has hard-DELETEd since it was introduced (#124); #165 retargeted it /api/sessions/api/agents (per-file hard delete) at the same time the resume feature began requiring a persisted exited record — a latent design conflict since the #165 unification. Verified not from #249 (UI-only, zero server files) or #237 (Codex providerThreadId path only).

Solution

Route self_exit through the same soft-exit path as kill/crash, keeping the record resumable with an honest exit reason:

  • agents/runtime.tskillAttachment(id, reason: ExitReason = "user_killed") parametrized; threads reason into markExited + the agent.exited WS delta.
  • routes/agents.tsPOST /:id/kill accepts an optional {reason} body, validated via isExitReason (defaults to user_killed; logs unrecognized values rather than silently coercing). Guards a null JSON body.
  • channel-server/index.tsself_exit POSTs /:id/kill {reason:"self_exited"} instead of DELETE. Also surfaces HTTP-level kill failures to stderr (serverFetch resolves non-2xx as isError, so the prior .catch couldn't see them). Bundle (dist.mjs) regenerated.
  • core/types/agent.tsExitReason now derives from a single EXIT_REASONS tuple + isExitReason typeguard, so adding a reason can't desync a hand-maintained allowlist.

Testing

Unit (resume-after-self-exit.test.ts, 5 tests): self-exited record stays findable across in-memory + on-disk reload; old deleteAgentRaw path documented as the breakage; isExitReason accepts the union and rejects 8 malformed inputs.

Real-spawn e2e (live dev server, isolated config, real claude PTY):

  1. spawn → running
  2. POST /:id/kill {reason:"self_exited"}GET /:id returns 200 exited/self_exited (was 404)
  3. POST /api/agents {resumeAgentId}201, re-attached (was "not found")
  4. DELETE /:id → 200 then 404 (hard-delete path unchanged)
  5. /kill no-body → user_killed; bogus reason → user_killed + server logs unrecognized reason

Full make check green: 558 server + 318 dashboard tests.

Risks / trade-offs

  • Behavior change: a self-exiting manager's reports are no longer reparented (DELETE did this; /kill doesn't). Intentional — now symmetric with kill_agent, and keeping the exited manager in the chart is the point of the fix.
  • Disk growth: self_exit was the only auto-cleanup path. Self-exited agents now accumulate like killed/crashed ones already do (there is no exited-record pruning anywhere today). This makes growth consistent rather than introducing a new unbounded class — but a bounded-retention prune is a sensible follow-up (filed separately, out of scope here).

🤖 Generated with Claude Code

`self_exit` issued a hard `DELETE /api/agents/:id`, which rmSync'd the agent
record off disk and dropped it from the in-memory cache. The resume path
(`spawnAgent` with `resumeAgentId`) looks the agent up via `getAgent(id)`,
which then returned undefined → `throw resumeAgentId "<id>" not found`.

This was asymmetric with `kill_agent`, which soft-exits via `POST /:id/kill`
→ `killAttachment` → `markExited` and KEEPS the record as `status:"exited"`
(resumable). The contradiction has existed since the #165 Agent+Session
unification — self_exit has hard-DELETEd since #124, while resume began
requiring a persisted exited record. Not a regression from #249 (UI-only) or
#237 (Codex providerThreadId only).

Fix: route `self_exit` through the same soft-exit path as kill/crash so the
record survives and stays resumable, with an honest exit reason:
- `killAttachment(id, reason: ExitReason = "user_killed")` — parametrize reason
- `POST /:id/kill` accepts an optional `{reason}` body, validated against the
  ExitReason union (defaults to user_killed; logs unrecognized values instead
  of silently coercing them)
- `self_exit` POSTs `/:id/kill {reason:"self_exited"}` instead of DELETE
- `ExitReason` now derives from a single `EXIT_REASONS` tuple + `isExitReason`
  typeguard (drift-proof: a new reason can't desync a hand-maintained allowlist)

Verified end-to-end against a live dev server: spawn → self_exit → record kept
as exited/self_exited (HTTP 200, was 404) → resume succeeds (HTTP 201, was
"not found"); DELETE still hard-deletes (404 after). Regression test added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158xdYsvWMjXfEXP4n9fVCd
@aterrylu
aterrylu enabled auto-merge (squash) June 26, 2026 08:33

@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 — clean fix for the self_exit / resume asymmetry, and the implementation is tight.

Reviewed the incremental diff. Verified:

  • Diagnosis is correct. runtime.ts:243-245 does getAgent(params.resumeAgentId) and throws resumeAgentId "<id>" not found on undefined — exactly the symptom. Routing self_exit through POST /:id/killkillAttachmentmarkExited("self_exited") keeps the record on disk so the resume lookup resolves.
  • killAttachment signature change is backward-compatible. The new reason: ExitReason = "user_killed" parameter is defaulted; the lone other caller (mcp.ts:249, single-arg) is unaffected.
  • /:id/kill body parsing is safe. c.req.json().catch(() => null) handles the no-body case (dashboard killSession at store.ts:1192 and kill_agent MCP at channel-server/index.ts:435 both send none) → silent user_killed default. Unknown reasons get logged, not silently coerced — good call.
  • isExitReason + EXIT_REASONS single-source-of-truth removes the desync risk that motivated the type refactor.
  • serverFetch error surfacing in channel-server/index.ts:520-524 is correct. Confirmed serverFetch resolves (not rejects) on non-2xx with { isError: true } (see channel-server/index.ts:235-241), so the prior .catch-only path really was blind to HTTP-level failures.
  • markExited is idempotent ("first reason wins", store.ts:341), so the inevitable onExit handler firing after pty.kill() won't overwrite "self_exited" with "crashed" on disk. (There's a pre-existing stale-WS-delta race in runtime.ts:605-623 where the onExit handler emits an agent.exited delta with its locally-computed reason regardless of what markExited actually persisted — but that's not introduced by this PR and applies equally to operator kill.)
  • Bundle (dist.mjs) regenerated in sync with channel-server/index.ts.
  • Behavior-change trade-offs are honest. Self-exiting manager's reports no longer reparenting is intentional and symmetric with operator kill; unbounded exited-record growth from self_exit now matches existing kill/crash paths (already unpruned) — a separate retention prune is a reasonable follow-up, not a blocker.

Tests cover the store-level invariant the fix hinges on. Ship it.

@aterrylu
aterrylu merged commit 5b57cfe into main Jun 26, 2026
9 of 10 checks passed
@aterrylu
aterrylu deleted the terry/resume-after-self-exit branch June 26, 2026 08:36
aterrylu added a commit that referenced this pull request Jun 26, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYB9dBo9Ap9ViBYKERneu7
aterrylu added a commit that referenced this pull request Jun 26, 2026
…e) (#257)

* feat(settings): per-provider permission modes (replaces autonomousMode)

Replace the coarse `autonomousMode: boolean` with a provider-agnostic
`permissionMode` enum (default | auto | plan | bypass) that each provider
maps to its native permission surface:

- Claude Code: --permission-mode default|acceptEdits|plan, with the legacy
  --dangerously-skip-permissions for bypass
- Gemini CLI:  --approval-mode default|auto_edit|plan|yolo
- Codex:       approval_policy=on-request|on-failure|never (sandbox stays
  danger-full-access — autonomOS is the trust boundary). No plan mode, so
  plan is disabled for Codex in the UI and clamps to default with a warning.

Settings holds a global default; Create Agent overrides per spawn. A shared
PermissionModeSelect renders a "?" current-selection explainer sourced from
core's PERMISSION_MODE_INFO (single source of truth). Templates carry their
own default mode.

Migration is accept-and-discard at every persisted layer (agent records,
sessions.json, user templates, dashboard localStorage): autonomousMode:true
-> bypass, false -> default; the old field is scrubbed with a warning. An
unspecified mode resolves to bypass to preserve prior behavior.

See ADR-045.

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

* docs(decisions): record PR number in ADR-045

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

* test: migrate #254 resume test fixture to permissionMode

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

* test: prove upgrade backward-compat for legacy autonomousMode records

Add store-migration.test.ts covering the main upgrade path: an old server's
on-disk per-agent records (autonomousMode boolean, no permissionMode) load
under the new store without crashing on the now-required field, derive the
correct mode (true->bypass, false->default, missing->default), coerce a
malformed value, and scrub the legacy field on disk on next write.

Also harden templates-migration.test.ts to do all fs/env setup inside before()
rather than at module top-level (avoids import-time side effects that can crash
the Linux e2e collector).

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

* test: pin prompt-delivery integration spawn to permissionMode default

The raw POST /api/agents path's pre-ADR-045 default resolved to supervised
((body.autonomousMode ?? tmpl?.autonomousMode) === true is false when absent).
Unifying to DEFAULT_PERMISSION_MODE=bypass flipped it to bypass, which emits
--dangerously-skip-permissions — refused by Claude Code when the CI runner is
root, so the agent died code=1 and the prompt never delivered. Prompt delivery
is permission-mode-agnostic, so pin this raw-REST test to default mode.

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

* test(integration): set IS_SANDBOX so root CI can spawn bypass-mode agents

The spawn default is now 'bypass' (--dangerously-skip-permissions), which
Claude Code refuses as root — the CI runner. The real-spawn integration suites
(agent-spawn-prompt, usage-queue, embedded-mode) spawn via the default and so
died code=1 as root. IS_SANDBOX=1 is Claude Code's documented escape hatch for
ephemeral CI sandboxes; bootEmbedded spreads it into every spawned agent, so the
suites now exercise the real production-default spawn instead of dying. Reverts
the earlier per-test permissionMode pin in favor of this systemic fix.

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

* fix(claude): 'default' mode emits no flag; pin integration spawns to default

Two coupled fixes for the bypass-default-in-CI breakage:

1. claude-code 'default' mode now emits NO permission flag instead of
   --permission-mode default. It IS claude's built-in behavior, so the flag was
   redundant — and it perturbed the interactive TUI startup. This matches the
   pre-ADR-045 flag-less supervised spawn exactly.

2. The real-spawn integration suites (agent-spawn-prompt, usage-queue,
   usage-queue-sim, embedded-mode) POST with no mode, which now defaults to
   bypass (--dangerously-skip-permissions) — which the real claude binary exits
   1 on in CI. Pin them to permissionMode 'default' (supervised): they test
   spawn mechanics, not permission semantics. Reverts the ineffective IS_SANDBOX
   attempt. Product default stays bypass (Terry's call, pending).

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

---------

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