From c6dfaa2cacb14ebb10711668eeae4914589c38c6 Mon Sep 17 00:00:00 2001 From: Randalix Date: Thu, 15 Oct 2026 09:53:17 +0200 Subject: [PATCH 1/4] feat(agent): per-session agent inbox (discussion #558) A coordinator can leave text for a worker without typing into its pane. - Store (src/web/agent-inbox.ts): read until ack by id, seen only for the receiver, 16 KiB/message, 512 KiB + 200 messages per inbox, full = 409 CONFLICT (never drops the oldest), 12 h TTL from each message's createdAt. - Routes (routes/inbox-routes.ts): POST/GET /api/sessions/:id/inbox (?wait= parks while nothing is unseen), POST .../inbox/ack (receiver only, 403 otherwise), GET /api/agent-inbox/summary (owner-scoped counts, not in toState()). Caller = resolveParentSessionId(). - Inbox waiters are the wait registry's third kind: same caps, ended by cancelAll/cancelEverything. - Opt-in snapshot (agentInboxPersistEnabled, synced, default off): agent-inbox.json via dataPath(), 0600 tmp+rename, off deletes it, boot prune only after a successful restore. Dropped on delete, kept on detach. - CLI: codeman agent post | inbox [--wait] | ack, INBOX column in ls. - waitCapacityResponse/abortOnClientHangUp moved to route-helpers.ts unchanged. - Docs: api-reference, README, CLAUDE.md, architecture-invariants, skill + plugin mirror. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 4 +- README.md | 4 + docs/api-reference.md | 85 ++++- docs/architecture-invariants.md | 14 + plugins/codeman/skills/codeman/SKILL.md | 11 +- plugins/codeman/skills/codeman/preamble.sh | 6 +- .../skills/codeman/reference/endpoints.md | 16 + .../codeman/skills/codeman/reference/verbs.md | 43 +++ skills/codeman/SKILL.md | 11 +- skills/codeman/preamble.sh | 6 +- skills/codeman/reference/endpoints.md | 16 + skills/codeman/reference/verbs.md | 43 +++ src/cli-agent.ts | 247 +++++++++++++- src/web/agent-inbox-persistence.ts | 138 ++++++++ src/web/agent-inbox.ts | 302 ++++++++++++++++++ src/web/public/index.html | 7 + src/web/public/settings-ui.js | 3 + src/web/route-helpers.ts | 50 ++- src/web/routes/inbox-routes.ts | 181 +++++++++++ src/web/routes/index.ts | 1 + src/web/routes/session-routes.ts | 49 +-- src/web/routes/system-routes.ts | 5 + src/web/schemas.ts | 32 ++ src/web/sensitive-path.ts | 2 + src/web/server.ts | 28 ++ src/web/session-wait-registry.ts | 170 +++++++++- test/agent-inbox-persistence.test.ts | 104 ++++++ test/agent-inbox-server-wiring.test.ts | 80 +++++ test/agent-inbox.test.ts | 263 +++++++++++++++ test/cli-agent.test.ts | 146 +++++++++ test/routes/inbox-routes.test.ts | 255 +++++++++++++++ test/sensitive-path.test.ts | 1 + test/session-wait-registry.test.ts | 63 ++++ test/tab-layout-restore-gate.test.ts | 5 + 34 files changed, 2308 insertions(+), 83 deletions(-) create mode 100644 src/web/agent-inbox-persistence.ts create mode 100644 src/web/agent-inbox.ts create mode 100644 src/web/routes/inbox-routes.ts create mode 100644 test/agent-inbox-persistence.test.ts create mode 100644 test/agent-inbox-server-wiring.test.ts create mode 100644 test/agent-inbox.test.ts create mode 100644 test/routes/inbox-routes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 02704d0f..b1b55b37 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -203,6 +203,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Read My Mind intent profiles** (`readMyMindEnabled`, SYNCED, default OFF; `docs/readmymind-plan.md`): per-CASE profiles (goals + recent prompts) keyed by owner + realpath(workingDir), captured from the transcript (`transcript:user_prompt`), never the input paths; the listener must stay inside `startTranscriptWatcher()`'s `if (!watcher)` block. Store `src/intent-store.ts` → `intents.json`, ⚠️ written 0600 tmp+rename and never fed to `/api/search` (prompts carry secrets). Routes in `readmymind-routes.ts` (ownership via `findSessionOrFail` WITH `req`); predictor = pure `readmymind-context.ts` + IO in `readmymind-collectors.ts` + `readmymind-predictor.ts`, claude-only, one in flight per session (409). ⚠️ Suggestions render via value/`textContent` ONLY and nothing auto-sends, ever. Frontend `readmymind-ui.js`. → [architecture-invariants#read-my-mind-intent-profiles](docs/architecture-invariants.md#read-my-mind-intent-profiles) +**Agent inbox** (discussion #558; persistence `agentInboxPersistEnabled`, SYNCED, default OFF): per-session mailbox in `web/agent-inbox.ts`, routes `routes/inbox-routes.ts`, CLI `codeman agent post|inbox|ack`. Never writes to a pane, never in `toState()` (counts via `GET /api/agent-inbox/summary`). ⚠️ Read until ack by id; seen/ack only for the receiver resolved via `resolveParentSessionId()` (ack from anyone else = 403); `?wait=` parks while nothing is UNSEEN; full = 409, never drop the oldest; dropped on delete (`killMux`) only, boot prune only after a successful restore. → [architecture-invariants#agent-inbox](docs/architecture-invariants.md#agent-inbox) + **Voice dictation via Claude** (`claudeVoiceEnabled`, SYNCED, default OFF): the mic transcribes through this machine's Claude Code login (the CLI `/voice` backend) instead of Deepgram; the browser captures, `src/web/voice-stream.ts` relays to Anthropic. ⚠️ The OAuth token never reaches the page. ⚠️ Credentials are READ-ONLY (`src/claude-credentials.ts`): never refresh them (it rotates the refresh token and can sign the user out of their CLI). ⚠️ Capture must be linear16/16 kHz/mono via an AudioWorklet; `voice-pcm-worklet.js` borrows voice-input.js's `?v=` token, so edit the two together. ⚠️ Claude transcript frames are cumulative: replace, never append. → [architecture-invariants#voice-dictation-via-claude](docs/architecture-invariants.md#voice-dictation-via-claude) **Agent Teams**: `TeamWatcher` polls `~/.claude/teams/`, matches to sessions via `leadSessionId`. Teammates are in-process threads appearing as subagents. Enable: `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1`. See `docs/agent-teams/`. @@ -307,7 +309,7 @@ One module per domain in `src/web/routes/` (plus a barrel; `ls src/web/routes/` ## State Files -All in `~/.codeman/`: `state.json` (sessions, settings, respawn, orchestrator, cron jobs/runs, owner tab layouts, case default groups), `mux-sessions.json` (tmux recovery), `settings.json` (user prefs), `push-keys.json` + `push-subscriptions.json`, `session-lifecycle.jsonl` (audit log), `update-status.json` (self-updater progress, polled across the service restart), `docker-env-applied.json` (Compose deployment only: sha256 of the Dockerfile + compose file the running container was built from, written by `Start-Codeman.sh`, read by the self-updater's environment gate), `docker-build-source.json` (Compose deployment only: the checkout's HEAD commit and `package-lock.json` hash the `codeman-node-modules`/`codeman-dist` volumes currently reflect, written by both `Start-Codeman.sh` and a successful in-place self-update, compared to detect and refresh a volume left stale by an externally-triggered rebuild), `linked-cases.json`, `webviews.json` (saved web-tab dashboard URLs), `remote-hosts.json` + `remote-cases.json`, `docker-hosts.json` + `docker-cases.json` + `docker-exports/`, `subagent-window-states.json` + `subagent-parents.json` (subagent window layout, GET/PUT `/api/subagent-window-states`/`-parents`), `hook-secret` (per-instance), `users.json` (multi-user, mode 0600) + `admin-audit.jsonl`, `intents.json` (Read My Mind intent profiles, mode 0600), `webhook.json` (webhook notifications: enabled/service/scope plus the ntfy/Slack/Discord URL, a bearer secret, so mode 0600, kept out of `settings.json` and never returned by `/api/webhook`, which is admin-only in multi-user mode), `config-backup-id` (the random id config backups record as each manifest's `origin`, so instances sharing a backup folder keep to their own snapshots; mode 0600, never tracked or restored, and deleting it unlists this instance's earlier snapshots), `backups/config/` (the default config-backup folder, moved by `configBackupDir`: `cfg-YYYYMMDD-HHMMSS/` snapshots, 0700 with 0600 files, each with a `manifest.json` of SHA-256s, the origin and the retention in force (`keepCount`/`keepDays`), which a pass falls back to when `settings.json` lacks a retention key), `certs/` (self-signed TLS for `--https`), `.env` (CODEMAN_USERNAME/PASSWORD fallback for the `codeman attach` CLI), `install.log` (installer step output, written by `install.sh`'s `run_step`) and `tailscale-rename` (the node name before `install.sh` renamed it, so uninstall can offer it back; both installer-route only). Transient: `self-update-runner.sh`. Multi-user case spaces live OUTSIDE the data dir at `~/codeman-users//cases` (shared across instances like `~/codeman-cases`, override `CODEMAN_USER_SPACES_DIR`). +All in `~/.codeman/`: `state.json` (sessions, settings, respawn, orchestrator, cron jobs/runs, owner tab layouts, case default groups), `mux-sessions.json` (tmux recovery), `settings.json` (user prefs), `push-keys.json` + `push-subscriptions.json`, `session-lifecycle.jsonl` (audit log), `update-status.json` (self-updater progress, polled across the service restart), `docker-env-applied.json` (Compose deployment only: sha256 of the Dockerfile + compose file the running container was built from, written by `Start-Codeman.sh`, read by the self-updater's environment gate), `docker-build-source.json` (Compose deployment only: the checkout's HEAD commit and `package-lock.json` hash the `codeman-node-modules`/`codeman-dist` volumes currently reflect, written by both `Start-Codeman.sh` and a successful in-place self-update, compared to detect and refresh a volume left stale by an externally-triggered rebuild), `linked-cases.json`, `webviews.json` (saved web-tab dashboard URLs), `remote-hosts.json` + `remote-cases.json`, `docker-hosts.json` + `docker-cases.json` + `docker-exports/`, `subagent-window-states.json` + `subagent-parents.json` (subagent window layout, GET/PUT `/api/subagent-window-states`/`-parents`), `hook-secret` (per-instance), `users.json` (multi-user, mode 0600) + `admin-audit.jsonl`, `intents.json` (Read My Mind intent profiles, mode 0600), `agent-inbox.json` (agent inbox snapshot, only while `agentInboxPersistEnabled` is on, mode 0600), `webhook.json` (webhook notifications: enabled/service/scope plus the ntfy/Slack/Discord URL, a bearer secret, so mode 0600, kept out of `settings.json` and never returned by `/api/webhook`, which is admin-only in multi-user mode), `config-backup-id` (the random id config backups record as each manifest's `origin`, so instances sharing a backup folder keep to their own snapshots; mode 0600, never tracked or restored, and deleting it unlists this instance's earlier snapshots), `backups/config/` (the default config-backup folder, moved by `configBackupDir`: `cfg-YYYYMMDD-HHMMSS/` snapshots, 0700 with 0600 files, each with a `manifest.json` of SHA-256s, the origin and the retention in force (`keepCount`/`keepDays`), which a pass falls back to when `settings.json` lacks a retention key), `certs/` (self-signed TLS for `--https`), `.env` (CODEMAN_USERNAME/PASSWORD fallback for the `codeman attach` CLI), `install.log` (installer step output, written by `install.sh`'s `run_step`) and `tailscale-rename` (the node name before `install.sh` renamed it, so uninstall can offer it back; both installer-route only). Transient: `self-update-runner.sh`. Multi-user case spaces live OUTSIDE the data dir at `~/codeman-users//cases` (shared across instances like `~/codeman-cases`, override `CODEMAN_USER_SPACES_DIR`). **Generated top-level dirs** (all gitignored — don't edit or commit): `dist/` (esbuild output), `out/`, `coverage/`, `test-results/`, `tmp/`, `screenshots-echo-diag/`. The committed gesture bundle (`src/web/public/gesture/gesture-codeman.js`) IS tracked, but its runtime wasm/model assets (`src/web/public/gesture/wasm/`, `*.task`) are fetched and gitignored. diff --git a/README.md b/README.md index b3fe7e7e..4ccdcfba 100644 --- a/README.md +++ b/README.md @@ -988,6 +988,9 @@ codeman agent read "$SID" --tail 3000 # terminal tail, ANSI codeman agent send "$SID" 'run the tests, then print WORKDONE followed by _4711' # hook-less modes (opencode, pi, …): ask for the marker in halves … codeman agent wait "$SID" --match WORKDONE_4711 # … and wait on the joined form, which the prompt's echo never contains codeman agent interrupt "$SID" # a bare ESC, conversation intact +codeman agent post "$SID" 'context for later, not a prompt' # stored in its inbox, not typed into the pane +codeman agent inbox --wait --timeout 300000 # your own inbox: blocks while nothing is new +codeman agent ack # remove what you handled (read never removes) codeman agent rm "$SID" # any session except this one ``` @@ -1067,6 +1070,7 @@ REST over Fastify — **~230 handlers across 25 route modules**, plus an SSE str | `GET` | `/api/sessions/:id/last-response` | The last answer as clean text, read from the transcript (claude, codex, deepseek) | | `GET` | `/api/sessions/:id/wait` | Block until a signal fires (`?until=stop,idle,exit&timeout=&fresh=`); a timeout is a `200` | | `GET` | `/api/sessions/:id/wait-output` | Block until a literal string appears (`?match=&nocase=&from=now\|buffer&timeout=`) | +| `POST` / `GET` | `/api/sessions/:id/inbox` | Agent inbox: post `{text}` / read every pending message (`?wait=` blocks while nothing is new); `POST …/inbox/ack {ids}` removes them, receiver only | | `GET` | `/api/sessions/unified` | Unified live + history list (Session Manager) — `?q=&limit=` | | `POST` | `/api/sessions/:id/pin` | Pin/unpin in the Session Manager (`{pinned}`) | | `PUT` | `/api/session-order` | Sync tab order across devices (`{order: [ids]}`) | diff --git a/docs/api-reference.md b/docs/api-reference.md index 32774fbf..8ac38050 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -591,10 +591,93 @@ request returns. The capture runs synchronous tmux calls on the server; the ## The `codeman agent` CLI (client over these endpoints) -`codeman agent ls|spawn|send|wait|read|interrupt|rm` (`src/cli-agent.ts`) is the command-line client for the endpoints above, for agents the skill does not reach (a pi or shell session, a machine without `jq`). It adds no route: `spawn` is `POST /api/v1/quick-start` (+ `wait-output` on the mode's `capabilities.composerReadyMark` from the CLI registry, where it declares one), `send` is `POST …/input` with `clientId`+`seq` (and `wait`/`waitTimeout` for `--wait` / `--until `; `delivered:false` without `duplicate` and `wait.ended` both exit 3 — the CLI never reports a dead worker as done), `wait` is `GET …/wait` (`--until`) or `GET …/wait-output` (`--match`, `from=buffer` by default), `read` is `GET …/last-response` or `GET …/terminal?tail=`, `interrupt` is `POST …/input` with a bare `\u001b`, `rm` is `DELETE …/sessions/:id`. A fire-and-forget `send` to a sleeping wake-on-LAN host reads the route's `buffered` (own line, exit 0) and `dropped` (exit 1: the chunk is gone). An id may be the 8-character form `ls` prints, resolved through `GET /api/v1/sessions`; anything shorter refuses before any request, the same floor as `PARENT_SESSION_ID_MIN_PREFIX`. Every call carries `X-Codeman-Parent-Session`; only `spawn`'s quick-start carries `X-Codeman-Agent-Origin: codeman-agent-cli` (the agent-scratch label must never reach a request that cannot create the case directory). Basic auth comes from `CODEMAN_PASSWORD` or the data dir's `.env`. Server-side error codes are shown verbatim (`INVALID_INPUT: until=stop …` on a hook-less mode is not hidden); exit codes are `0` ok, `1` error, `2` timeout, `3` the session exited, `4` refused by a client-side guard. See the README section "`codeman agent`" for the guards and `test/cli-agent.test.ts` for the pinned behaviour. +`codeman agent ls|spawn|send|wait|read|interrupt|rm|post|inbox|ack` (`src/cli-agent.ts`) is the command-line client for the endpoints above, for agents the skill does not reach (a pi or shell session, a machine without `jq`). It adds no route: `spawn` is `POST /api/v1/quick-start` (+ `wait-output` on the mode's `capabilities.composerReadyMark` from the CLI registry, where it declares one), `send` is `POST …/input` with `clientId`+`seq` (and `wait`/`waitTimeout` for `--wait` / `--until `; `delivered:false` without `duplicate` and `wait.ended` both exit 3 — the CLI never reports a dead worker as done), `wait` is `GET …/wait` (`--until`) or `GET …/wait-output` (`--match`, `from=buffer` by default), `read` is `GET …/last-response` or `GET …/terminal?tail=`, `interrupt` is `POST …/input` with a bare `\u001b`, `rm` is `DELETE …/sessions/:id`, and the inbox verbs map one to one onto the [agent inbox](#agent-inbox) routes (`post` → `POST …/inbox`, `inbox [id] [--wait]` → `GET …/inbox[?wait=]`, own inbox by default, `ack ` → `POST …/inbox/ack`); `ls` adds an `INBOX` column from `GET /api/v1/agent-inbox/summary` and drops it when that route does not answer. A fire-and-forget `send` to a sleeping wake-on-LAN host reads the route's `buffered` (own line, exit 0) and `dropped` (exit 1: the chunk is gone). An id may be the 8-character form `ls` prints, resolved through `GET /api/v1/sessions`; anything shorter refuses before any request, the same floor as `PARENT_SESSION_ID_MIN_PREFIX`. Every call carries `X-Codeman-Parent-Session`; only `spawn`'s quick-start carries `X-Codeman-Agent-Origin: codeman-agent-cli` (the agent-scratch label must never reach a request that cannot create the case directory). Basic auth comes from `CODEMAN_PASSWORD` or the data dir's `.env`. Server-side error codes are shown verbatim (`INVALID_INPUT: until=stop …` on a hook-less mode is not hidden); exit codes are `0` ok, `1` error, `2` timeout, `3` the session exited, `4` refused by a client-side guard. See the README section "`codeman agent`" for the guards and `test/cli-agent.test.ts` for the pinned behaviour. `codeman mcp` (`src/mcp-server.ts`) is a Model Context Protocol server on stdio over the same endpoints, for MCP clients: `list_sessions` (`GET /api/v1/sessions`), `start_session` (`POST /api/v1/quick-start` with `X-Codeman-Agent-Origin: codeman-mcp`, then `wait-output` on the mode's ready mark), `send_prompt` (`POST …/input` with `wait`/`waitTimeout`), `wait_for_session` (`GET …/wait` or `…/wait-output`), `read_output` (`GET …/last-response` or `…/terminal?tail=`), `interrupt_session` and `delete_session`. It adds no route. The URL comes from `CODEMAN_API_URL` or `--url`, never a guess; prompting, interrupting and deleting are limited to sessions the process started (or the calling session's own children) unless it runs with `--allow-any-session`. See the README section "MCP server" and `test/mcp-server.test.ts`. +## Agent inbox + +A per-session mailbox: another agent (or the user) posts a message, and the +receiving agent reads it when it chooses to. Nothing is typed into the +receiver's pane, which is the difference from `POST …/input`: a post does not +start or interrupt a turn, and the text may span several lines. Design and the +reasons behind each rule: [discussion #558](https://github.com/Ark0N/Codeman/discussions/558). + +| Method | Route | Body / query | Answer (`data`) | +|---|---|---|---| +| `POST` | `/api/v1/sessions/:id/inbox` | `{ text }` | `{ sessionId, message, pending, unseen }` | +| `GET` | `/api/v1/sessions/:id/inbox` | `?wait=` (optional) | `{ sessionId, receiver, messages, pending, unseen, wait? }` | +| `POST` | `/api/v1/sessions/:id/inbox/ack` | `{ ids: string[] }` (1 to 200) | `{ sessionId, removed, pending, unseen }` | +| `GET` | `/api/v1/agent-inbox/summary` | | `{ sessions: { [id]: { pending, unseen, oldestUnseenAt, parked } } }` | + +A message is `{ id, from, text, createdAt, seen }`. `from` is a session id or +`null` (the user); `seen` says whether the receiver had read it before this read. + +**Who is calling.** The server resolves the caller from the +`X-Codeman-Parent-Session` header, the one `codeman agent` sends on every +request, with the same resolver as session lineage: a live session the caller +can see, with the same owner as the receiving session, matched by exact id or a +unique prefix of at least 8 characters. Anything else counts as no session. +The body cannot name a sender (`InboxPostSchema` is strict). In multi-user mode +that means a caller cannot claim to be another owner's session. In single-user +mode every local process holds the same credentials, so the header is a hint the +client gives about itself, not proof. + +**Read until ack.** A read returns every pending message, oldest first, and +removes nothing. Only `POST …/ack` removes, by id. Acking an id that is already +gone is a success, so a retry after a lost response does not fail. + +**Seen is the receiver's.** A read marks messages seen only when the resolved +caller is the receiving session itself (`receiver: true`). Any other read, a +coordinator checking a worker or a request without the header, is a peek and +changes nothing. `ack` from anyone but the receiver is `403 FORBIDDEN`. + +**`?wait=`** blocks while the inbox holds nothing **unseen**. A message that +was read but not acked does not end the wait, so a receiver that keeps one +pending does not spin. The value is clamped to the agent-wait bounds like +`timeout` on `/wait`, a timeout is a `200` with `wait.timedOut: true`, and the +waiter counts against the same per-session, per-owner and total caps (`409 +SESSION_BUSY` / `429 RATE_LIMITED`). `wait` has the shape of the other wait +results plus `posted` (a post released it). A session deleted during the wait +answers `wait.ended: true` with an empty inbox. + +**Limits.** 200 messages, 16384 bytes (UTF-8) per message, 512 KiB of text per +inbox. A message over the per-message cap is `400 INVALID_INPUT`. A post that +would cross either inbox cap is `409 CONFLICT` and nothing is stored; the oldest +message is never dropped to make room. Messages expire 12 hours after their +`createdAt`, read or not. + +**Summary.** `GET /api/v1/agent-inbox/summary` lists every session the caller +can see that has mail or a parked receiver: `pending`, `unseen`, +`oldestUnseenAt` (epoch ms, `null` when nothing is unseen) and `parked` (the +receiver itself is waiting on `?wait=` right now). Counts only, never text. It +is not part of the session object, so a post does not broadcast +`session:updated`. + +**Lifetime.** An inbox is deleted with its session (`DELETE +/api/v1/sessions/:id` and every other delete path), not when a session is +detached. By default inboxes live in memory and a server restart empties them. +The synced setting `agentInboxPersistEnabled` (default off; Settings → Cross-session features) +writes them to `agent-inbox.json` in the data dir (mode `0600`, tmp file + +rename) and restores them at boot. Inboxes of sessions that did not come back +are pruned only after the session restore succeeded. Switching the setting off +deletes the file and keeps what is in memory. The file is never read by +`/api/search`, and the file viewer refuses it. + +```bash +# coordinator: leave a task for a worker +curl -s -X POST "$API/api/v1/sessions/$WORKER/inbox" \ + -H "X-Codeman-Parent-Session: $CODEMAN_SESSION_ID" -H 'Content-Type: application/json' \ + -d "$(jq -n --arg t "$(cat task.md)" '{text: $t}')" + +# worker: wait for new mail, handle it, then ack by id +curl -s "$API/api/v1/sessions/$CODEMAN_SESSION_ID/inbox?wait=300000" \ + -H "X-Codeman-Parent-Session: $CODEMAN_SESSION_ID" | jq '.data.messages' +curl -s -X POST "$API/api/v1/sessions/$CODEMAN_SESSION_ID/inbox/ack" \ + -H "X-Codeman-Parent-Session: $CODEMAN_SESSION_ID" -H 'Content-Type: application/json' \ + -d '{"ids":[""]}' +``` + ## Prompt uploads (`POST /api/v1/sessions/:id/paste-image`) A `multipart/form-data` body with one `image` part. The file is written into the diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..6cde5a00 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -688,6 +688,20 @@ Either way the actual launch (`runCustomModelEntry`) routes through `run()` itse Frontend `readmymind-ui.js`: header 🧠 marker-hidden (`btn-readmymind--hidden`) until the setting is ON; phones hide it in mobile.css and get a keyboard-accessory 🧠 key instead (ships in BOTH bar templates, revealed by the `rmm-enabled` class on the BAR element — setMode() rebuilds button innerHTML, so per-key state would be wiped; synced at init + every `applyHeaderVisibilitySettings()`). Alternate suggestions render as tappable rows that swap into the editable field without losing edits; Rethink rejects the whole shown set and carries the optional steer note (`#readMyMindSteer`, sent as `steer`, shown in ready + empty-result phases, cleared on each open). Suggestions render via value/`textContent` ONLY and Send/Insert go through `POST /input` (server-side, so the sendEnterKey/local-echo trap does not apply) — nothing auto-sends, ever. User guide: `docs/readmymind.md`. +### Agent inbox + +**Agent inbox** (discussion #558): a per-session mailbox one agent posts to and another reads, so a coordinator can hand a worker text without typing into its pane. Store `web/agent-inbox.ts` (pure, `sessionWaits`-style singleton, no IO), routes `routes/inbox-routes.ts`, CLI verbs `codeman agent post|inbox|ack`, opt-in snapshot `web/agent-inbox-persistence.ts`. Nothing in it writes to a pane, and it is kept out of `toState()` so a post does not fan out a `session:updated` to every browser; the counts live behind `GET /api/agent-inbox/summary` (owner-scoped, counts only, never text), which `codeman agent ls` merges into its INBOX column. + +⚠️ **Read until ack.** A read removes nothing; the same messages come back until the receiver acks them BY ID (`POST …/inbox/ack`, idempotent: an id already gone is not an error). Acking by id rather than "everything I read" is what keeps a message posted between the read and the ack from being lost. + +⚠️ **Seen and ack belong to the receiver.** The caller is whatever `resolveParentSessionId()` makes of `X-Codeman-Parent-Session` (live session, same owner as the receiver), never a body field (`InboxPostSchema` is strict). Only a read whose caller IS the receiver marks messages seen; anyone else's read is a peek. An ack from anyone else is a 403, not a silent no-op, so a coordinator never believes it cleared a worker's mail. In multi-user mode this stops a caller from naming another owner's session; in single-user mode every local process holds the same credentials, so the header is the client's own claim about itself, which is all the seen/peek split needs. + +⚠️ **`?wait=` parks while nothing is UNSEEN**, not while nothing is pending: a receiver holding a seen-but-unacked message would otherwise get an immediate answer on every loop and spin. Inbox waiters are the registry's third waiter kind (`waitForInbox`/`notifyInbox`), so they count against the same per-session/owner/total caps and are ended by `cancelAll` like signal and output waiters. `notifyInbox` runs AFTER the store, so a woken reader finds the message. `parked` in the summary counts receiver waiters only. + +⚠️ **Caps refuse, they never drop.** 16 KiB per message (UTF-8 bytes), 512 KiB and 200 messages per inbox; a full inbox is 409 CONFLICT and nothing is stored. Dropping the oldest would lose exactly the message a slow receiver has not handled yet. TTL is 12 h from each message's own `createdAt`, swept lazily, and a restore keeps `createdAt`, so a restart does not restart the clock. + +⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. + ### Voice dictation via Claude **Voice dictation via Claude** (`claudeVoiceEnabled`, SYNCED, default OFF): the mic button can transcribe through this machine's Claude Code login instead of a Deepgram key, using the same speech-to-text service the CLI's own `/voice` mode uses. diff --git a/plugins/codeman/skills/codeman/SKILL.md b/plugins/codeman/skills/codeman/SKILL.md index d48aaa00..50133e8c 100644 --- a/plugins/codeman/skills/codeman/SKILL.md +++ b/plugins/codeman/skills/codeman/SKILL.md @@ -35,7 +35,8 @@ This skill works the same whichever CLI you are: Codeman puts it where each CLI for skills (`.claude/skills/` for Claude Code, `.agents/skills/` for Codex, Gemini CLI, OpenCode, Antigravity, Grok, DeepSeek and OMP). A **pi** or **shell** worker never gets it, but has the same environment: tell it *"other sessions: `codeman agent --help`"*. -That bundled CLI (`ls`, `spawn`, `send`, `wait`, `read`, `interrupt`, `rm`) is the same +That bundled CLI (`ls`, `spawn`, `send`, `wait`, `read`, `interrupt`, `rm`, and the inbox +verbs `post`, `inbox`, `ack`) is the same verbs over the same endpoints, with the guards below (no control bytes, no self-delete, no guessed URL) enforced in code, and it is also your own way through when this machine has no `jq` for the preamble below. @@ -119,9 +120,9 @@ fi AUTH=(); [ -n "${CODEMAN_PASSWORD:-}" ] && AUTH=(-u "${CODEMAN_USERNAME:-admin}:$CODEMAN_PASSWORD") # -k: harmless on http, required on https (self-signed cert). # X-Codeman-Parent-Session: tags workers YOU spawn as your children, so the web UI can -# draw the lineage. Set once here and every present and future create call carries it; -# it is ignored on every other endpoint. Purely cosmetic (see §5.1) and it can never -# fail a spawn, so there is no case where you would want to leave it off. +# draw the lineage. Set once here and every present and future create call carries it. +# The agent inbox reads it too, as "who is calling" (§5.15); every other endpoint ignores +# it. It can never fail a spawn, so there is no case where you would want to leave it off. # X-Codeman-Agent-Origin: marks a case directory a spawn CREATES as agent scratch, so the # user can find and delete it long after your workers are gone (§5.14). Same deal: set # once, cosmetic, never fails a spawn, and it labels only directories Codeman creates. @@ -602,6 +603,7 @@ One row per job. Acting on this table alone is correct; the §5 links are the de | find yourself, list what exists | `GET /api/v1/sessions`, match your `$SELF` by **prefix** | [§5.11](reference/verbs.md#511-list-and-find-yourself) | | read or record what the user wants | `GET/PUT .../intent`, and `POST .../readmymind` to predict | [§5.12](reference/verbs.md#512-read-my-mind) | | talk to a claude worker directly | `ListAgents` / `SendMessage`, when the feature is on at both ends | [§5.13](reference/verbs.md#513-messaging-claude-workers) | +| leave a message a session reads when it is ready, any mode, multi-line | `codeman agent post "…"` (`POST .../inbox`). Nothing is typed into its pane and no turn starts; the receiver reads with `codeman agent inbox --wait` and removes with `codeman agent ack `. An idle receiver never looks on its own: `codeman agent ls` shows unread mail, then `send` it one line | [§5.15](reference/verbs.md#515-agent-inbox) | | clean up | `delete_session "$SID"` per id you created. Case directories and git worktrees are **not** removed with it; `GET /api/v1/cases/agent-created` lists the scratch case dirs your spawns left behind, for you to report | [§5.14](reference/verbs.md#514-clean-up) | ## 3. Rules digest @@ -705,6 +707,7 @@ these**; open the one row you actually hit. | [5.12 Read My Mind](reference/verbs.md#512-read-my-mind) | read or record what the user wants for a case | | [5.13 Messaging claude workers](reference/verbs.md#513-messaging-claude-workers) | `ListAgents` / `SendMessage` instead of the HTTP path | | [5.14 Clean up](reference/verbs.md#514-clean-up) | what deleting a session does **not** remove, and how to list the case dirs you left | +| [5.15 Agent inbox](reference/verbs.md#515-agent-inbox) | messages that wait for the receiver instead of being typed: post, read until ack, who counts as the receiver | ## 6. Setup and auth diff --git a/plugins/codeman/skills/codeman/preamble.sh b/plugins/codeman/skills/codeman/preamble.sh index 5de248af..52f4707d 100644 --- a/plugins/codeman/skills/codeman/preamble.sh +++ b/plugins/codeman/skills/codeman/preamble.sh @@ -18,9 +18,9 @@ fi AUTH=(); [ -n "${CODEMAN_PASSWORD:-}" ] && AUTH=(-u "${CODEMAN_USERNAME:-admin}:$CODEMAN_PASSWORD") # -k: harmless on http, required on https (self-signed cert). # X-Codeman-Parent-Session: tags workers YOU spawn as your children, so the web UI can -# draw the lineage. Set once here and every present and future create call carries it; -# it is ignored on every other endpoint. Purely cosmetic (see §5.1) and it can never -# fail a spawn, so there is no case where you would want to leave it off. +# draw the lineage. Set once here and every present and future create call carries it. +# The agent inbox reads it too, as "who is calling" (§5.15); every other endpoint ignores +# it. It can never fail a spawn, so there is no case where you would want to leave it off. # X-Codeman-Agent-Origin: marks a case directory a spawn CREATES as agent scratch, so the # user can find and delete it long after your workers are gone (§5.14). Same deal: set # once, cosmetic, never fails a spawn, and it labels only directories Codeman creates. diff --git a/plugins/codeman/skills/codeman/reference/endpoints.md b/plugins/codeman/skills/codeman/reference/endpoints.md index 3edf4f70..70143316 100644 --- a/plugins/codeman/skills/codeman/reference/endpoints.md +++ b/plugins/codeman/skills/codeman/reference/endpoints.md @@ -608,6 +608,22 @@ Approving a permission dialog on your own is exactly the laundering this skill f can access, which includes the user's own working sessions. An approval belonging to one of those is something you **report**, never something you answer. +### Agent inbox: hand a worker text without typing into its pane + +Stored, not typed: the receiver reads it when it asks. Semantics, CLI form and the +receiver loop: `verbs.md` §5.15. + +| Task | Call | +|------|------| +| post to a session's inbox | `POST /api/v1/sessions/:id/inbox` body `{"text":"…"}` → `.data.message.{id,from}`; 409 = inbox full, nothing stored | +| read (yours marks seen, anyone else's is a peek) | `GET /api/v1/sessions/:id/inbox` → `.data.{receiver,pending,unseen,messages[]}` | +| block until something unseen arrives | `GET /api/v1/sessions/:id/inbox?wait=60000` → `.data.wait.{posted,timedOut,ended}` | +| remove what you handled (receiver only, idempotent) | `POST /api/v1/sessions/:id/inbox/ack` body `{"ids":["…"]}`; 403 from anyone else | +| counts per session, never text | `GET /api/v1/agent-inbox/summary` → `.data.sessions[id].{pending,unseen,oldestUnseenAt,parked}` | + +`from` and "who is the receiver" both come from the `X-Codeman-Parent-Session` header the +preamble's `CURL` sends, never from the body. + ### The wait primitives Three bounded long-polls. Shared semantics: diff --git a/plugins/codeman/skills/codeman/reference/verbs.md b/plugins/codeman/skills/codeman/reference/verbs.md index 14d37133..5896d8b0 100644 --- a/plugins/codeman/skills/codeman/reference/verbs.md +++ b/plugins/codeman/skills/codeman/reference/verbs.md @@ -764,3 +764,46 @@ Confirm cleanup with `GET /api/v1/sessions`, never with `/api/v1/sessions/unifie (that one folds in transcript history from the whole machine and will keep showing your worker forever). +### 5.15 Agent inbox + +Every session has an inbox. A message posted there is stored, not typed: the receiver's +pane is untouched, no turn starts, and the text may span several lines. The receiver +reads when it is ready. Use it for results a worker hands back, for a follow-up task a +busy worker should pick up after its current one, and for anything longer than one +composer line. Use `send` when you want a turn to start now. + +```bash +codeman agent post "$WORKER" "$(cat task.md)" # ONE quoted argument; prints pending/unseen +codeman agent inbox --wait --timeout 300000 # your own inbox; blocks while nothing is new +codeman agent ack […] # remove what you handled, by the full id inbox printed +codeman agent inbox "$WORKER" # peek at someone else's: marks nothing +codeman agent ls # INBOX column: pending (new), parked +``` + +The same over HTTP: `POST /api/v1/sessions/:id/inbox {"text":…}`, `GET +/api/v1/sessions/:id/inbox?wait=`, `POST /api/v1/sessions/:id/inbox/ack {"ids":[…]}`, +`GET /api/v1/agent-inbox/summary`. The preamble's `CURL` already sends +`X-Codeman-Parent-Session`, which is how the server knows who you are. + +Rules that matter: + +- **Reading removes nothing.** Every read returns every pending message until you ack + it, so a crash between reading and doing loses nothing. Ack once the work is done, + not when you have read it: a sender looking at `ls` sees unacked mail as not done. +- **Only the receiver's own read counts.** Your read of your own inbox marks messages + seen (`seen: true` on the next read); reading another session's inbox is a peek and + changes nothing for it. Only the receiver can ack (403 otherwise). +- **`--wait` waits for something new.** It returns at once when there is unseen mail + and otherwise blocks until a post arrives; seen but unacked messages come back with it + and do not end the wait. Timeout is exit 2, the session vanishing is exit 3. +- **The sender is resolved, not claimed.** `from` is the session the header resolves to, + or `null` (the user). In single-user mode that is a hint, since every local process + holds the same credentials. +- **Full means refused.** 200 messages or 512 KiB per inbox, 16 KiB per message. A full + inbox answers 409 and stores nothing; the oldest is never dropped. Messages expire 12 + hours after they were posted. +- **Nobody is woken.** An idle receiver does not look at its inbox by itself, and the + server never types into its pane. When `ls` shows a worker idle with unread mail and + not parked, `send` it one line ("read your inbox: `codeman agent inbox`"). +- **Lifetime.** An inbox goes with its session on delete. A server restart empties all + inboxes unless the user turned on *Keep agent inbox across restarts* in Settings. diff --git a/skills/codeman/SKILL.md b/skills/codeman/SKILL.md index d48aaa00..50133e8c 100644 --- a/skills/codeman/SKILL.md +++ b/skills/codeman/SKILL.md @@ -35,7 +35,8 @@ This skill works the same whichever CLI you are: Codeman puts it where each CLI for skills (`.claude/skills/` for Claude Code, `.agents/skills/` for Codex, Gemini CLI, OpenCode, Antigravity, Grok, DeepSeek and OMP). A **pi** or **shell** worker never gets it, but has the same environment: tell it *"other sessions: `codeman agent --help`"*. -That bundled CLI (`ls`, `spawn`, `send`, `wait`, `read`, `interrupt`, `rm`) is the same +That bundled CLI (`ls`, `spawn`, `send`, `wait`, `read`, `interrupt`, `rm`, and the inbox +verbs `post`, `inbox`, `ack`) is the same verbs over the same endpoints, with the guards below (no control bytes, no self-delete, no guessed URL) enforced in code, and it is also your own way through when this machine has no `jq` for the preamble below. @@ -119,9 +120,9 @@ fi AUTH=(); [ -n "${CODEMAN_PASSWORD:-}" ] && AUTH=(-u "${CODEMAN_USERNAME:-admin}:$CODEMAN_PASSWORD") # -k: harmless on http, required on https (self-signed cert). # X-Codeman-Parent-Session: tags workers YOU spawn as your children, so the web UI can -# draw the lineage. Set once here and every present and future create call carries it; -# it is ignored on every other endpoint. Purely cosmetic (see §5.1) and it can never -# fail a spawn, so there is no case where you would want to leave it off. +# draw the lineage. Set once here and every present and future create call carries it. +# The agent inbox reads it too, as "who is calling" (§5.15); every other endpoint ignores +# it. It can never fail a spawn, so there is no case where you would want to leave it off. # X-Codeman-Agent-Origin: marks a case directory a spawn CREATES as agent scratch, so the # user can find and delete it long after your workers are gone (§5.14). Same deal: set # once, cosmetic, never fails a spawn, and it labels only directories Codeman creates. @@ -602,6 +603,7 @@ One row per job. Acting on this table alone is correct; the §5 links are the de | find yourself, list what exists | `GET /api/v1/sessions`, match your `$SELF` by **prefix** | [§5.11](reference/verbs.md#511-list-and-find-yourself) | | read or record what the user wants | `GET/PUT .../intent`, and `POST .../readmymind` to predict | [§5.12](reference/verbs.md#512-read-my-mind) | | talk to a claude worker directly | `ListAgents` / `SendMessage`, when the feature is on at both ends | [§5.13](reference/verbs.md#513-messaging-claude-workers) | +| leave a message a session reads when it is ready, any mode, multi-line | `codeman agent post "…"` (`POST .../inbox`). Nothing is typed into its pane and no turn starts; the receiver reads with `codeman agent inbox --wait` and removes with `codeman agent ack `. An idle receiver never looks on its own: `codeman agent ls` shows unread mail, then `send` it one line | [§5.15](reference/verbs.md#515-agent-inbox) | | clean up | `delete_session "$SID"` per id you created. Case directories and git worktrees are **not** removed with it; `GET /api/v1/cases/agent-created` lists the scratch case dirs your spawns left behind, for you to report | [§5.14](reference/verbs.md#514-clean-up) | ## 3. Rules digest @@ -705,6 +707,7 @@ these**; open the one row you actually hit. | [5.12 Read My Mind](reference/verbs.md#512-read-my-mind) | read or record what the user wants for a case | | [5.13 Messaging claude workers](reference/verbs.md#513-messaging-claude-workers) | `ListAgents` / `SendMessage` instead of the HTTP path | | [5.14 Clean up](reference/verbs.md#514-clean-up) | what deleting a session does **not** remove, and how to list the case dirs you left | +| [5.15 Agent inbox](reference/verbs.md#515-agent-inbox) | messages that wait for the receiver instead of being typed: post, read until ack, who counts as the receiver | ## 6. Setup and auth diff --git a/skills/codeman/preamble.sh b/skills/codeman/preamble.sh index 5de248af..52f4707d 100644 --- a/skills/codeman/preamble.sh +++ b/skills/codeman/preamble.sh @@ -18,9 +18,9 @@ fi AUTH=(); [ -n "${CODEMAN_PASSWORD:-}" ] && AUTH=(-u "${CODEMAN_USERNAME:-admin}:$CODEMAN_PASSWORD") # -k: harmless on http, required on https (self-signed cert). # X-Codeman-Parent-Session: tags workers YOU spawn as your children, so the web UI can -# draw the lineage. Set once here and every present and future create call carries it; -# it is ignored on every other endpoint. Purely cosmetic (see §5.1) and it can never -# fail a spawn, so there is no case where you would want to leave it off. +# draw the lineage. Set once here and every present and future create call carries it. +# The agent inbox reads it too, as "who is calling" (§5.15); every other endpoint ignores +# it. It can never fail a spawn, so there is no case where you would want to leave it off. # X-Codeman-Agent-Origin: marks a case directory a spawn CREATES as agent scratch, so the # user can find and delete it long after your workers are gone (§5.14). Same deal: set # once, cosmetic, never fails a spawn, and it labels only directories Codeman creates. diff --git a/skills/codeman/reference/endpoints.md b/skills/codeman/reference/endpoints.md index 3edf4f70..70143316 100644 --- a/skills/codeman/reference/endpoints.md +++ b/skills/codeman/reference/endpoints.md @@ -608,6 +608,22 @@ Approving a permission dialog on your own is exactly the laundering this skill f can access, which includes the user's own working sessions. An approval belonging to one of those is something you **report**, never something you answer. +### Agent inbox: hand a worker text without typing into its pane + +Stored, not typed: the receiver reads it when it asks. Semantics, CLI form and the +receiver loop: `verbs.md` §5.15. + +| Task | Call | +|------|------| +| post to a session's inbox | `POST /api/v1/sessions/:id/inbox` body `{"text":"…"}` → `.data.message.{id,from}`; 409 = inbox full, nothing stored | +| read (yours marks seen, anyone else's is a peek) | `GET /api/v1/sessions/:id/inbox` → `.data.{receiver,pending,unseen,messages[]}` | +| block until something unseen arrives | `GET /api/v1/sessions/:id/inbox?wait=60000` → `.data.wait.{posted,timedOut,ended}` | +| remove what you handled (receiver only, idempotent) | `POST /api/v1/sessions/:id/inbox/ack` body `{"ids":["…"]}`; 403 from anyone else | +| counts per session, never text | `GET /api/v1/agent-inbox/summary` → `.data.sessions[id].{pending,unseen,oldestUnseenAt,parked}` | + +`from` and "who is the receiver" both come from the `X-Codeman-Parent-Session` header the +preamble's `CURL` sends, never from the body. + ### The wait primitives Three bounded long-polls. Shared semantics: diff --git a/skills/codeman/reference/verbs.md b/skills/codeman/reference/verbs.md index 14d37133..5896d8b0 100644 --- a/skills/codeman/reference/verbs.md +++ b/skills/codeman/reference/verbs.md @@ -764,3 +764,46 @@ Confirm cleanup with `GET /api/v1/sessions`, never with `/api/v1/sessions/unifie (that one folds in transcript history from the whole machine and will keep showing your worker forever). +### 5.15 Agent inbox + +Every session has an inbox. A message posted there is stored, not typed: the receiver's +pane is untouched, no turn starts, and the text may span several lines. The receiver +reads when it is ready. Use it for results a worker hands back, for a follow-up task a +busy worker should pick up after its current one, and for anything longer than one +composer line. Use `send` when you want a turn to start now. + +```bash +codeman agent post "$WORKER" "$(cat task.md)" # ONE quoted argument; prints pending/unseen +codeman agent inbox --wait --timeout 300000 # your own inbox; blocks while nothing is new +codeman agent ack […] # remove what you handled, by the full id inbox printed +codeman agent inbox "$WORKER" # peek at someone else's: marks nothing +codeman agent ls # INBOX column: pending (new), parked +``` + +The same over HTTP: `POST /api/v1/sessions/:id/inbox {"text":…}`, `GET +/api/v1/sessions/:id/inbox?wait=`, `POST /api/v1/sessions/:id/inbox/ack {"ids":[…]}`, +`GET /api/v1/agent-inbox/summary`. The preamble's `CURL` already sends +`X-Codeman-Parent-Session`, which is how the server knows who you are. + +Rules that matter: + +- **Reading removes nothing.** Every read returns every pending message until you ack + it, so a crash between reading and doing loses nothing. Ack once the work is done, + not when you have read it: a sender looking at `ls` sees unacked mail as not done. +- **Only the receiver's own read counts.** Your read of your own inbox marks messages + seen (`seen: true` on the next read); reading another session's inbox is a peek and + changes nothing for it. Only the receiver can ack (403 otherwise). +- **`--wait` waits for something new.** It returns at once when there is unseen mail + and otherwise blocks until a post arrives; seen but unacked messages come back with it + and do not end the wait. Timeout is exit 2, the session vanishing is exit 3. +- **The sender is resolved, not claimed.** `from` is the session the header resolves to, + or `null` (the user). In single-user mode that is a hint, since every local process + holds the same credentials. +- **Full means refused.** 200 messages or 512 KiB per inbox, 16 KiB per message. A full + inbox answers 409 and stores nothing; the oldest is never dropped. Messages expire 12 + hours after they were posted. +- **Nobody is woken.** An idle receiver does not look at its inbox by itself, and the + server never types into its pane. When `ls` shows a worker idle with unread mail and + not parked, `send` it one line ("read your inbox: `codeman agent inbox`"). +- **Lifetime.** An inbox goes with its session on delete. A server restart empties all + inboxes unless the user turned on *Keep agent inbox across restarts* in Settings. diff --git a/src/cli-agent.ts b/src/cli-agent.ts index f89de132..cf0d76fd 100644 --- a/src/cli-agent.ts +++ b/src/cli-agent.ts @@ -3,7 +3,8 @@ * inside a Codeman session, in every CLI mode. * * A thin HTTP client over endpoints that already exist (`quick-start`, `input`, - * `wait`, `wait-output`, `last-response`, `terminal`, `DELETE sessions/:id`). It + * `wait`, `wait-output`, `last-response`, `terminal`, `DELETE sessions/:id`, and the + * agent inbox: `inbox`, `inbox/ack`, `agent-inbox/summary`). It * invents no route and no transport: everything goes through `CODEMAN_API_URL`, so * auth, ownership and the per-session waiter cap apply unchanged. The behaviour is * the packaged agent skill's (`skills/codeman`), ported from shell prose into code @@ -416,15 +417,55 @@ export async function resolveSessionId( return { error: `"${id}" is ambiguous: ${matches.map((s) => s.id.slice(0, 13)).join(', ')}`, code: EXIT.error }; } -/** `agent ls` — every session the caller can see, self marked. */ +/** One session's row in `GET /api/agent-inbox/summary`. */ +export interface InboxSummaryRow { + pending: number; + unseen: number; + oldestUnseenAt: number | null; + parked: boolean; +} + +/** + * The inbox summary, or undefined when it cannot be had (a server without the route, + * a transient error). `ls` must still list sessions then, so this never fails the verb. + */ +async function fetchInboxSummary(deps: AgentDeps): Promise | undefined> { + try { + const res = await deps.request(deps.ctx, { method: 'GET', path: '/api/v1/agent-inbox/summary' }); + if (!res.json?.success) return undefined; + return (res.json.data as { sessions?: Record } | undefined)?.sessions ?? {}; + } catch { + return undefined; + } +} + +/** The INBOX cell of `ls`: `3 (1 new)`, `parked`, both, or empty. */ +export function inboxCell(row: InboxSummaryRow | undefined): string { + if (!row) return ''; + const parts: string[] = []; + if (row.pending > 0) parts.push(row.unseen > 0 ? `${row.pending} (${row.unseen} new)` : String(row.pending)); + if (row.parked) parts.push('parked'); + return parts.join(', '); +} + +/** `agent ls` — every session the caller can see, self marked, with its inbox counts. */ export async function agentLs(deps: AgentDeps): Promise { - const res = await deps.request(deps.ctx, { method: 'GET', path: '/api/v1/sessions' }); + const [res, inbox] = await Promise.all([ + deps.request(deps.ctx, { method: 'GET', path: '/api/v1/sessions' }), + fetchInboxSummary(deps), + ]); if (!res.json?.success) return fail(deps, describeFailure(res)); const sessions = (res.json.data as SessionRow[] | undefined) ?? []; if (deps.json) { emitJson( deps, - sessions.map((s) => ({ ...s, self: isSelfSession(deps.ctx.selfId, s.id) })) + sessions.map((s) => ({ + ...s, + self: isSelfSession(deps.ctx.selfId, s.id), + // null = empty inbox and not parked. The key is absent when the server did + // not answer the summary, so "no data" never reads as "no mail". + ...(inbox ? { inbox: inbox[s.id] ?? null } : {}), + })) ); return EXIT.ok; } @@ -432,17 +473,22 @@ export async function agentLs(deps: AgentDeps): Promise { deps.io.out(palette.muted('(no sessions)')); return EXIT.ok; } - const rows = sessions.map((s) => [ - isSelfSession(deps.ctx.selfId, s.id) ? '*' : ' ', - s.id.slice(0, 8), - s.mode ?? '?', - s.status ?? '?', - s.name || s.workingDir || '', - ]); - deps.io.out(table([[' ', 'ID', 'MODE', 'STATUS', 'NAME'], ...rows], { gap: 2 })); + const rows = sessions.map((s) => { + const row = [isSelfSession(deps.ctx.selfId, s.id) ? '*' : ' ', s.id.slice(0, 8), s.mode ?? '?', s.status ?? '?']; + if (inbox) row.push(inboxCell(inbox[s.id])); + row.push(s.name || s.workingDir || ''); + return row; + }); + const header = inbox ? [' ', 'ID', 'MODE', 'STATUS', 'INBOX', 'NAME'] : [' ', 'ID', 'MODE', 'STATUS', 'NAME']; + deps.io.out(table([header, ...rows], { gap: 2 })); deps.io.out( palette.muted(`* = this session (${deps.ctx.selfId.slice(0, 8)}). status is a UI hint, never a sync signal.`) ); + if (inbox) { + deps.io.out( + palette.muted('INBOX: pending (unseen), parked = the session is waiting on `agent inbox --wait` right now.') + ); + } return EXIT.ok; } @@ -790,6 +836,140 @@ export async function agentRm(deps: AgentDeps, options: { id: string }): Promise return EXIT.ok; } +// ───────────────────────────────────────────────────────────────────────────── +// Agent inbox +// ───────────────────────────────────────────────────────────────────────────── + +/** A message as `GET …/inbox` hands it out. */ +export interface InboxMessageRow { + id: string; + from: string | null; + text: string; + createdAt: number; + seen: boolean; +} + +/** + * The message `post` was given, which must be ONE argument, as with `send`: several + * would be an unquoted `$(cat notes.md)` split by the shell, and joining the pieces + * with spaces loses its newlines. Unlike `send`, newlines inside the one argument are + * fine: the inbox stores text as data and never types it. + */ +export function postTextFromArgs(words: readonly string[]): { text: string } | { error: string } { + if (words.length === 1) return { text: words[0] }; + return { error: `refusing: the message must be ONE argument, got ${words.length} — quote it` }; +} + +/** `agent post` — store a message in another session's inbox. Never types into its pane. */ +export async function agentPost(deps: AgentDeps, options: { id: string; text: string }): Promise { + if (options.text.length === 0) return fail(deps, 'refusing: empty message', EXIT.refused); + const target = await resolveSessionId(deps, options.id); + if ('error' in target) return fail(deps, target.error, target.code); + const res = await deps.request(deps.ctx, { + method: 'POST', + path: `/api/v1/sessions/${encodeURIComponent(target.id)}/inbox`, + body: { text: options.text }, + }); + // 409 = the receiver's inbox is full; nothing was stored. Passed through as an error. + if (!res.json?.success) return fail(deps, describeFailure(res)); + const data = res.json.data as { message?: InboxMessageRow; pending?: number; unseen?: number } | undefined; + if (deps.json) emitJson(deps, data ?? {}); + else + deps.io.out( + palette.ok( + `${GLYPH.ok} posted to ${target.id.slice(0, 8)} (${data?.pending ?? '?'} pending, ${data?.unseen ?? '?'} unseen)` + ) + ); + return EXIT.ok; +} + +export interface InboxReadOptions { + /** Another session's inbox (a peek); omitted = this session's own. */ + id?: string; + /** Long-poll budget in ms; the read parks while nothing is unseen. */ + waitMs?: number; +} + +/** Relative age for humans: `12s`, `4m`, `3h`. */ +function age(createdAt: number, now: number): string { + const s = Math.max(0, Math.round((now - createdAt) / 1000)); + if (s < 60) return `${s}s`; + if (s < 3600) return `${Math.round(s / 60)}m`; + return `${Math.round(s / 3600)}h`; +} + +/** + * `agent inbox` — read this session's inbox (or peek at another's). Removes nothing: + * a message stays until `agent ack `. Only reading your OWN inbox marks messages + * seen. Exit 0 with mail, 2 when a `--wait` ran out with nothing unseen, 3 when the + * session went away during the wait. + */ +export async function agentInboxRead(deps: AgentDeps, options: InboxReadOptions): Promise { + const target = await resolveSessionId(deps, options.id ?? deps.ctx.selfId); + if ('error' in target) return fail(deps, target.error, target.code); + const res = await deps.request(deps.ctx, { + method: 'GET', + path: `/api/v1/sessions/${encodeURIComponent(target.id)}/inbox`, + query: { wait: options.waitMs }, + timeoutMs: options.waitMs !== undefined ? options.waitMs + 10_000 : undefined, + }); + if (!res.json?.success) return fail(deps, describeFailure(res)); + const data = res.json.data as + | { + receiver?: boolean; + messages?: InboxMessageRow[]; + pending?: number; + unseen?: number; + wait?: { timedOut?: boolean; ended?: boolean }; + } + | undefined; + const messages = data?.messages ?? []; + const code = data?.wait?.ended ? EXIT.dead : data?.wait?.timedOut ? EXIT.timeout : EXIT.ok; + if (deps.json) { + emitJson(deps, data ?? {}); + return code; + } + const now = (deps.now ?? Date.now)(); + for (const m of messages) { + const sender = m.from ? m.from.slice(0, 8) : 'user'; + const tag = m.seen ? palette.muted('seen') : palette.emph('new'); + deps.io.out(palette.muted(`── ${m.id} from ${sender}, ${age(m.createdAt, now)} ago, `) + tag); + deps.io.out(m.text); + } + if (data?.wait?.ended) { + deps.io.err(palette.err(`${GLYPH.fail} the session went away during the wait`)); + } else if (messages.length === 0) { + deps.io.err(palette.muted(data?.wait?.timedOut ? '(no mail; wait timed out, exit 2)' : '(no mail)')); + } else if (data?.receiver) { + deps.io.err( + palette.muted( + `${messages.length} pending. Nothing is removed by reading: when done, \`codeman agent ack …\` (full ids above).` + ) + ); + } else { + deps.io.err(palette.muted(`${messages.length} pending (peek: nothing was marked seen).`)); + } + return code; +} + +/** `agent ack` — remove messages from THIS session's inbox by id. Idempotent. */ +export async function agentAck(deps: AgentDeps, options: { ids: string[] }): Promise { + const ids = options.ids.map((id) => id.trim()).filter(Boolean); + if (ids.length === 0) return fail(deps, 'refusing: no message ids', EXIT.refused); + const self = await resolveSessionId(deps, deps.ctx.selfId); + if ('error' in self) return fail(deps, self.error, self.code); + const res = await deps.request(deps.ctx, { + method: 'POST', + path: `/api/v1/sessions/${encodeURIComponent(self.id)}/inbox/ack`, + body: { ids }, + }); + if (!res.json?.success) return fail(deps, describeFailure(res)); + const data = res.json.data as { removed?: number; pending?: number } | undefined; + if (deps.json) emitJson(deps, data ?? {}); + else deps.io.out(palette.ok(`${GLYPH.ok} acked ${data?.removed ?? 0} (${data?.pending ?? '?'} still pending)`)); + return EXIT.ok; +} + // ───────────────────────────────────────────────────────────────────────────── // Commander wiring // ───────────────────────────────────────────────────────────────────────────── @@ -833,7 +1013,9 @@ async function run(json: boolean, verb: (deps: AgentDeps) => Promise): P export function registerAgentCommands(program: Command): Command { const agent = program .command('agent') - .description('Talk to other sessions from inside one (any CLI mode): list, spawn, send, wait, read, interrupt, rm'); + .description( + 'Talk to other sessions from inside one (any CLI mode): list, spawn, send, wait, read, interrupt, rm, and the inbox: post, inbox, ack' + ); agent .command('ls') @@ -977,6 +1159,45 @@ export function registerAgentCommands(program: Command): Command { run(Boolean(options.json), (deps) => agentInterrupt(deps, { id })) ); + agent + .command('post ') + .description( + "Leave a message in another session's inbox (ONE quoted argument; multi-line is fine). Nothing is typed into its pane: it reads with `agent inbox`" + ) + .option('--json', 'Machine-readable output') + .action((id: string, words: string[], options: { json?: boolean }) => + run(Boolean(options.json), (deps) => { + const message = postTextFromArgs(words); + if ('error' in message) return Promise.resolve(fail(deps, message.error, EXIT.refused)); + return agentPost(deps, { id, text: message.text }); + }) + ); + + agent + .command('inbox [id]') + .description( + "Read this session's inbox (or peek at another's with [id]). Reading removes nothing; `agent ack` does. --wait blocks while nothing is new: exit 2 on timeout" + ) + .option('-w, --wait', 'Block until a new (unseen) message arrives') + .option('-t, --timeout ', 'Wait budget in ms (with --wait)', String(DEFAULT_WAIT_MS)) + .option('--json', 'Machine-readable output') + .action((id: string | undefined, options: { wait?: boolean; timeout?: string; json?: boolean }) => + run(Boolean(options.json), (deps) => + agentInboxRead(deps, { + id, + waitMs: options.wait ? parsePositiveInt(options.timeout, DEFAULT_WAIT_MS) : undefined, + }) + ) + ); + + agent + .command('ack ') + .description("Remove messages from this session's inbox by id, once handled (unknown ids are not an error)") + .option('--json', 'Machine-readable output') + .action((ids: string[], options: { json?: boolean }) => + run(Boolean(options.json), (deps) => agentAck(deps, { ids })) + ); + agent .command('rm ') .description('Delete any session except this one (refuses your own id)') diff --git a/src/web/agent-inbox-persistence.ts b/src/web/agent-inbox-persistence.ts new file mode 100644 index 00000000..604a1b0f --- /dev/null +++ b/src/web/agent-inbox-persistence.ts @@ -0,0 +1,138 @@ +/** + * @fileoverview Opt-in snapshot of the agent inbox, so pending mail between agents + * survives a server restart (the agents themselves keep running in tmux across it). + * + * Off by default: an install that never turns on `agentInboxPersistEnabled` writes + * nothing new to disk. The rules (discussion #558): + * - Off means no file. Switching it off deletes the snapshot; the in-memory inboxes + * stay as they are. + * - Its own file under `dataPath()`, written 0600 through tmp + rename like + * `intents.json`, since messages carry the same kind of text prompts do. It is never + * read by `/api/search` and `sensitive-path.ts` keeps the file viewer off it. + * - Writes are debounced (a burst of posts is one write) and flushed on shutdown. + * - Boot pruning is NOT done here: the server calls `agentInbox.pruneExcept()` only + * after the session restore succeeded, so a failed restore leaves the mail alone. + * + * @module web/agent-inbox-persistence + */ + +import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname } from 'node:path'; +import { dataPath } from '../config/instance.js'; +import { agentInbox, type AgentInbox } from './agent-inbox.js'; + +/** A burst of posts (a coordinator fanning out to five workers) becomes one write. */ +const WRITE_DEBOUNCE_MS = 250; + +export class AgentInboxPersistence { + private enabled = false; + private timer: NodeJS.Timeout | null = null; + + constructor( + private readonly inbox: AgentInbox = agentInbox, + /** Resolved per call: test HOMEs change between files. */ + private readonly filePath: () => string = () => dataPath('agent-inbox.json'), + private readonly debounceMs: number = WRITE_DEBOUNCE_MS + ) { + this.inbox.onChange = () => this.schedule(); + } + + get isEnabled(): boolean { + return this.enabled; + } + + /** + * Boot: apply the setting and, when it is on, load the snapshot into the store. + * Returns how many messages were restored. A missing or unreadable file is an + * empty inbox, never a failed boot. + */ + load(enabled: boolean): number { + this.enabled = enabled; + if (!enabled) { + // A file left from an earlier "on" is removed, so off really means no file. + this.removeFile(); + return 0; + } + const path = this.filePath(); + if (!existsSync(path)) return 0; + try { + return this.inbox.restore(JSON.parse(readFileSync(path, 'utf-8'))); + } catch (err) { + console.warn(`[AgentInbox] Failed to load ${path}, starting empty:`, err); + return 0; + } + } + + /** + * The setting changed. On: write the current inboxes now. Off: drop any pending + * write and delete the snapshot; memory is untouched. + */ + setEnabled(enabled: boolean): void { + if (enabled === this.enabled) return; + this.enabled = enabled; + if (enabled) { + this.writeNow(); + } else { + this.cancel(); + this.removeFile(); + } + } + + /** Shutdown: write a pending change now instead of losing it with the timer. */ + flush(): void { + if (!this.timer) return; + this.cancel(); + this.writeNow(); + } + + /** Test hook: drop a pending write and detach from the store. */ + dispose(): void { + this.cancel(); + if (this.inbox.onChange) this.inbox.onChange = undefined; + } + + private schedule(): void { + if (!this.enabled || this.timer) return; + this.timer = setTimeout(() => { + this.timer = null; + this.writeNow(); + }, this.debounceMs); + // A pending write must not hold the process open; shutdown flushes it explicitly. + this.timer.unref(); + } + + private cancel(): void { + if (this.timer) clearTimeout(this.timer); + this.timer = null; + } + + private writeNow(): void { + if (!this.enabled) return; + const path = this.filePath(); + const tmpPath = `${path}.tmp`; + try { + mkdirSync(dirname(path), { recursive: true }); + // 0600: agent messages can carry anything a prompt can, secrets included. + // `mode` applies only when the file is created, and the rename makes every + // write a fresh file, so the final file is always 0600. + rmSync(tmpPath, { force: true }); + writeFileSync(tmpPath, JSON.stringify(this.inbox.snapshot()), { mode: 0o600 }); + renameSync(tmpPath, path); + } catch (err) { + console.warn(`[AgentInbox] Failed to persist ${path}:`, err); + } + } + + private removeFile(): void { + const path = this.filePath(); + try { + rmSync(path, { force: true }); + rmSync(`${path}.tmp`, { force: true }); + } catch (err) { + console.warn(`[AgentInbox] Failed to remove ${path}:`, err); + } + } +} + +/** Process-wide instance wired to `agentInbox`; the server and the settings route use it. */ +export const agentInboxPersistence = new AgentInboxPersistence(); diff --git a/src/web/agent-inbox.ts b/src/web/agent-inbox.ts new file mode 100644 index 00000000..7b5822e2 --- /dev/null +++ b/src/web/agent-inbox.ts @@ -0,0 +1,302 @@ +/** + * @fileoverview Agent inbox: a per-session mailbox other agents (or the user) post to, + * and the receiving agent reads when it chooses to. + * + * The other agent-to-agent channel, `POST /api/sessions/:id/input`, types into the + * receiver's composer. That is right for a prompt and wrong for a message: every + * delivery starts or interrupts a turn, the receiver cannot defer it, and it is one + * line only. The inbox stores instead of typing. Nothing here ever reaches a pane. + * + * Invariants (each pinned in `test/agent-inbox.test.ts`): + * - **Read until ack.** A read hands out every pending message and removes nothing. + * A message goes only when the receiver acknowledges its id. "Read and dropped" + * and "done" must not look the same to the sender (discussion #558, the 09-23 case). + * - **Seen is the receiver's alone.** `read(…, { receiver: true })` marks what it + * handed out as seen; any other read is a peek and changes nothing. The route + * decides who the receiver is; this module only takes the flag. + * - **Bounded, and full refuses.** `MAX_MESSAGES_PER_INBOX`, `MAX_MESSAGE_BYTES` and + * `MAX_INBOX_BYTES`. A post that would cross any of them is refused so the sender + * learns. The oldest message is never dropped to make room. + * - **TTL from the message's own timestamp** (`MESSAGE_TTL_MS`). A restore keeps + * `createdAt`, so a restart does not restart the clock. Expiry is lazy: every + * accessor sweeps the inbox it touches, `summary()` and `snapshot()` sweep all. + * - **No IO, no `Session` import.** Persistence (`agent-inbox-persistence.ts`) hooks + * `onChange`; long-poll waiters live in `session-wait-registry.ts`. + * + * @consumedby web/routes/inbox-routes, web/agent-inbox-persistence, web/server + * @module web/agent-inbox + */ + +import { randomUUID } from 'node:crypto'; + +// ─── Bounds ────────────────────────────────────────────────────────────────── + +/** Messages one inbox holds before posts are refused. */ +export const MAX_MESSAGES_PER_INBOX = 200; +/** UTF-8 bytes per message. A bigger payload belongs in the workspace; post its path. */ +export const MAX_MESSAGE_BYTES = 16_384; +/** UTF-8 bytes of message text one inbox holds before posts are refused. */ +export const MAX_INBOX_BYTES = 512 * 1024; +/** A message older than this is gone, read or not (same span as the approvals inbox). */ +export const MESSAGE_TTL_MS = 12 * 60 * 60 * 1000; + +// ─── Types ─────────────────────────────────────────────────────────────────── + +/** A message as it is stored (and persisted). */ +interface StoredMessage { + id: string; + /** The posting session's full id, or null when no session could be resolved (the user). */ + from: string | null; + text: string; + /** Epoch ms. The TTL counts from here. */ + createdAt: number; + /** Epoch ms of the first receiver read that handed this message out; null while unseen. */ + seenAt: number | null; +} + +/** A message as a read hands it out. */ +export interface InboxMessage { + id: string; + from: string | null; + text: string; + createdAt: number; + /** True when a receiver read had handed this message out BEFORE this read. */ + seen: boolean; +} + +export interface InboxReadResult { + /** Every pending message, oldest first: unseen and seen-but-unacked alike. */ + messages: InboxMessage[]; + pending: number; + /** Unseen after this read (0 after a receiver read; unchanged by a peek). */ + unseen: number; +} + +export type PostResult = + | { ok: true; message: InboxMessage; pending: number; unseen: number } + | { ok: false; reason: 'empty' | 'message-too-large' | 'inbox-full-count' | 'inbox-full-bytes' }; + +/** What `summary()` reports for one session. Counts and times only, never text. */ +export interface InboxCounts { + pending: number; + unseen: number; + /** `createdAt` of the oldest unseen message, null when nothing is unseen. */ + oldestUnseenAt: number | null; +} + +/** On-disk shape. `version` lets a later format refuse an older file instead of misreading it. */ +export interface InboxSnapshot { + version: 1; + inboxes: Record; +} + +// ─── Store ─────────────────────────────────────────────────────────────────── + +export class AgentInbox { + private readonly inboxes = new Map(); + + /** Any mutation (post, seen, ack, expiry, drop, prune, restore excluded). Persistence hooks this. */ + onChange: (() => void) | undefined; + + constructor(private readonly now: () => number = Date.now) {} + + /** + * Store a message for `sessionId`. `from` is the RESOLVED sender (the route's job), + * never a client-supplied label. + */ + post(sessionId: string, from: string | null, text: string): PostResult { + if (text.length === 0) return { ok: false, reason: 'empty' }; + const bytes = Buffer.byteLength(text, 'utf8'); + if (bytes > MAX_MESSAGE_BYTES) return { ok: false, reason: 'message-too-large' }; + const queue = this.live(sessionId); + if (queue.length >= MAX_MESSAGES_PER_INBOX) return { ok: false, reason: 'inbox-full-count' }; + if (inboxBytes(queue) + bytes > MAX_INBOX_BYTES) return { ok: false, reason: 'inbox-full-bytes' }; + const stored: StoredMessage = { id: randomUUID(), from, text, createdAt: this.now(), seenAt: null }; + queue.push(stored); + this.inboxes.set(sessionId, queue); + this.onChange?.(); + return { ok: true, message: view(stored), pending: queue.length, unseen: countUnseen(queue) }; + } + + /** + * Every pending message, oldest first. Removes nothing. With `receiver`, the + * messages handed out are marked seen (each keeps its first `seenAt`); without it + * this is a peek and changes nothing at all. + */ + read(sessionId: string, options: { receiver: boolean }): InboxReadResult { + const queue = this.live(sessionId); + const messages = queue.map(view); + if (options.receiver) { + let marked = false; + const at = this.now(); + for (const m of queue) { + if (m.seenAt !== null) continue; + m.seenAt = at; + marked = true; + } + if (marked) this.onChange?.(); + } + return { messages, pending: queue.length, unseen: countUnseen(queue) }; + } + + /** Messages no receiver read has handed out yet. A `?wait=` read parks while this is 0. */ + unseenCount(sessionId: string): number { + return countUnseen(this.live(sessionId)); + } + + pendingCount(sessionId: string): number { + return this.live(sessionId).length; + } + + /** + * Remove the given messages. Idempotent: an id that is unknown or already gone is + * not an error, so a retry after a lost response succeeds. Returns how many went. + */ + ack(sessionId: string, ids: readonly string[]): number { + const queue = this.live(sessionId); + if (queue.length === 0 || ids.length === 0) return 0; + const drop = new Set(ids); + const kept = queue.filter((m) => !drop.has(m.id)); + const removed = queue.length - kept.length; + if (removed === 0) return 0; + this.store(sessionId, kept); + this.onChange?.(); + return removed; + } + + /** Per-session counts for every inbox that holds anything. Never blocks. */ + summary(): Map { + const out = new Map(); + for (const sessionId of [...this.inboxes.keys()]) { + const queue = this.live(sessionId); + if (queue.length === 0) continue; + const unseen = queue.filter((m) => m.seenAt === null); + out.set(sessionId, { + pending: queue.length, + unseen: unseen.length, + oldestUnseenAt: unseen.length > 0 ? unseen[0].createdAt : null, + }); + } + return out; + } + + /** The session was DELETED: its mail goes with it. Not called on detach or shutdown. */ + drop(sessionId: string): void { + if (this.inboxes.delete(sessionId)) this.onChange?.(); + } + + /** + * Boot-time sweep, run only after the session restore succeeded: discard inboxes + * whose session did not come back. Every route needs a live session, so an orphan + * could otherwise never be removed. Returns how many inboxes were dropped. + */ + pruneExcept(keep: Iterable): number { + const alive = new Set(keep); + let dropped = 0; + for (const sessionId of [...this.inboxes.keys()]) { + if (alive.has(sessionId)) continue; + this.inboxes.delete(sessionId); + dropped++; + } + if (dropped > 0) this.onChange?.(); + return dropped; + } + + /** Whole-store snapshot for persistence, expired messages already swept. */ + snapshot(): InboxSnapshot { + const inboxes: Record = {}; + for (const sessionId of [...this.inboxes.keys()]) { + const queue = this.live(sessionId); + if (queue.length > 0) inboxes[sessionId] = queue.map((m) => ({ ...m })); + } + return { version: 1, inboxes }; + } + + /** + * Replace the store from a snapshot (server boot). Malformed input is skipped entry + * by entry rather than thrown: a corrupt file must not stop the server. Expired + * messages are not restored, and the caps hold for what is. Does not fire + * `onChange`: writing back what was just read would be a no-op at best. + */ + restore(snapshot: unknown): number { + const data = snapshot as Partial | null; + this.inboxes.clear(); + if (!data || data.version !== 1 || typeof data.inboxes !== 'object' || data.inboxes === null) return 0; + const cutoff = this.now() - MESSAGE_TTL_MS; + let restored = 0; + for (const [sessionId, raw] of Object.entries(data.inboxes)) { + if (!Array.isArray(raw)) continue; + const queue: StoredMessage[] = []; + let bytes = 0; + for (const entry of raw) { + if (!isStoredMessage(entry) || entry.createdAt <= cutoff) continue; + const size = Buffer.byteLength(entry.text, 'utf8'); + if (queue.length >= MAX_MESSAGES_PER_INBOX || bytes + size > MAX_INBOX_BYTES) break; + queue.push({ ...entry }); + bytes += size; + } + if (queue.length === 0) continue; + this.inboxes.set(sessionId, queue); + restored += queue.length; + } + return restored; + } + + /** Test hook: back to an empty store with no listener. */ + resetForTests(): void { + this.inboxes.clear(); + this.onChange = undefined; + } + + /** The inbox with expired messages swept out (fires `onChange` when it swept any). */ + private live(sessionId: string): StoredMessage[] { + const queue = this.inboxes.get(sessionId); + if (!queue) return []; + const cutoff = this.now() - MESSAGE_TTL_MS; + if (queue.length === 0 || queue[0].createdAt > cutoff) return queue; + // Oldest first, so the expired ones are a prefix. + const kept = queue.filter((m) => m.createdAt > cutoff); + this.store(sessionId, kept); + this.onChange?.(); + return kept; + } + + private store(sessionId: string, queue: StoredMessage[]): void { + if (queue.length === 0) this.inboxes.delete(sessionId); + else this.inboxes.set(sessionId, queue); + } +} + +function view(m: StoredMessage): InboxMessage { + return { id: m.id, from: m.from, text: m.text, createdAt: m.createdAt, seen: m.seenAt !== null }; +} + +function countUnseen(queue: readonly StoredMessage[]): number { + let n = 0; + for (const m of queue) if (m.seenAt === null) n++; + return n; +} + +function inboxBytes(queue: readonly StoredMessage[]): number { + let total = 0; + for (const m of queue) total += Buffer.byteLength(m.text, 'utf8'); + return total; +} + +function isStoredMessage(value: unknown): value is StoredMessage { + const m = value as Partial | null; + return ( + !!m && + typeof m.id === 'string' && + (m.from === null || typeof m.from === 'string') && + typeof m.text === 'string' && + m.text.length > 0 && + Buffer.byteLength(m.text, 'utf8') <= MAX_MESSAGE_BYTES && + typeof m.createdAt === 'number' && + Number.isFinite(m.createdAt) && + (m.seenAt === null || typeof m.seenAt === 'number') + ); +} + +/** Process-wide store used by the routes and the server wiring. Tests construct their own. */ +export const agentInbox = new AgentInbox(); diff --git a/src/web/public/index.html b/src/web/public/index.html index f3a2b2a2..1ddee901 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2145,6 +2145,13 @@ +
+
+ Keep agent inbox across restarts + Saves pending codeman agent post messages to agent-inbox.json in the data dir (readable by you only), so they survive a server restart. Off by default; turning it off deletes the file. Messages expire after 12 hours either way. +
+ +
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 5504719c..7484f487 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -536,6 +536,8 @@ Object.assign(CodemanApp.prototype, { document.getElementById('appSettingsShowUltracodeAgents').checked = settings.showUltracodeAgents ?? defaults.showUltracodeAgents ?? false; // Approvals Inbox: synced, default OFF (opt-in; only an explicit true enables). document.getElementById('appSettingsApprovalsInbox').checked = settings.approvalsInboxEnabled === true; + // Agent inbox snapshot on disk: synced, default OFF. + document.getElementById('appSettingsAgentInboxPersist').checked = settings.agentInboxPersistEnabled === true; // Custom Model Endpoint Profiles: synced, default OFF. The toggle governs both // the Run-menu picker's generated entries and this settings panel's visibility; // the endpoint list itself is server state, loaded on demand below. @@ -2921,6 +2923,7 @@ Object.assign(CodemanApp.prototype, { configBackupEnabled: document.getElementById('appSettingsConfigBackup').checked, configBackupDir: document.getElementById('appSettingsConfigBackupDir').value.trim(), ...this._configBackupRetentionPayload(_prev), + agentInboxPersistEnabled: document.getElementById('appSettingsAgentInboxPersist').checked, readMyMindEnabled: document.getElementById('appSettingsReadMyMind').checked, ultracodeFloatingWindows: document.getElementById('appSettingsUltracodeFloatingWindows').checked, showMultiMonitorButton: document.getElementById('appSettingsShowMultiMonitorButton').checked, diff --git a/src/web/route-helpers.ts b/src/web/route-helpers.ts index 75a92fcb..c2361305 100644 --- a/src/web/route-helpers.ts +++ b/src/web/route-helpers.ts @@ -11,7 +11,8 @@ import fs from 'node:fs/promises'; import type { z } from 'zod'; import type { FastifyReply, FastifyRequest } from 'fastify'; import { Session } from '../session.js'; -import { ApiErrorCode, createErrorResponse, type AuthUser, type SessionState } from '../types.js'; +import { ApiErrorCode, createErrorResponse, type ApiResponse, type AuthUser, type SessionState } from '../types.js'; +import type { WaitCapacityError } from './session-wait-registry.js'; import { MAX_CONCURRENT_SESSIONS } from '../config/map-limits.js'; import { parseRalphLoopConfig, extractCompletionPhrase } from '../ralph-config.js'; import { SseEvent } from './sse-events.js'; @@ -627,3 +628,50 @@ export function autoConfigureRalph(session: Session, workingDir: string, ctx: Ev }); } } + +// ========== Long-poll helpers (shared by the wait routes and the inbox routes) ========== + +/** + * Map a waiter-cap rejection to the code that tells the caller the truth. + * + * The two caps mean different things and warrant different recovery: `session` is + * genuinely about THIS session, while `owner` and `total` are process-wide budgets + * that say nothing about it. Reporting a global cap as `SESSION_BUSY` (409, + * documented as "Session is busy") sent an agent off to a different session to hit + * the identical error. `RATE_LIMITED` is the code whose whole meaning is "come back + * later", and clients and proxies already treat 429 that way. + */ +export function waitCapacityResponse(err: WaitCapacityError): ApiResponse { + const code = err.scope === 'session' ? ApiErrorCode.SESSION_BUSY : ApiErrorCode.RATE_LIMITED; + // The registry's message already names the scope and the limit; passing it through + // verbatim keeps the wording in one place. + return createErrorResponse(code, err.message); +} + +/** + * An `AbortController` that fires when the CLIENT goes away, and only then. + * + * Freeing an abandoned waiter matters because the documented pattern is a loop of + * short waits: `curl --max-time 30 ".../wait?timeout=600000"` abandons a live waiter + * every iteration until the cap is hit and an innocent session reports busy. Same for + * any proxy that cuts the connection. + * + * ⚠️ **It must listen on the RESPONSE, not the request.** `req.raw` emits `'close'` + * as soon as the request body has finished streaming, which on a POST is BEFORE the + * handler ever blocks — measured at +1ms with `aborted: false`, indistinguishable + * from a real hang-up at +0ms. Wiring the abort there cancels every send-and-wait + * instantly and silently kills the feature (it survives on GET only because a GET has + * no body to finish). `reply.raw` emits `'close'` both when the response completes + * and when the socket dies, and `writableFinished` is what tells those apart: true + * only if the response actually went out. The guard is load-bearing, not defensive. + * + * `app.inject()` never emits `'close'` at all, so this is only observable over real + * HTTP — which is why the regression test for it binds a port. + */ +export function abortOnClientHangUp(reply: FastifyReply): AbortController { + const controller = new AbortController(); + reply.raw.on('close', () => { + if (!reply.raw.writableFinished) controller.abort(); + }); + return controller; +} diff --git a/src/web/routes/inbox-routes.ts b/src/web/routes/inbox-routes.ts new file mode 100644 index 00000000..4d19c67e --- /dev/null +++ b/src/web/routes/inbox-routes.ts @@ -0,0 +1,181 @@ +/** + * @fileoverview Agent inbox routes: post, read (optionally long-polling), ack, and an + * owner-scoped summary of counts. Store semantics live in `agent-inbox.ts`. + * + * Who is calling decides what a read or an ack does, so it is resolved here and never + * taken from the body. `resolveParentSessionId()` reads the `X-Codeman-Parent-Session` + * header `codeman agent` sends on every request and accepts it only for a live session + * the caller can see that has the same owner as the receiver (exact id, or a unique + * prefix of at least 8). Anything else is "no session": a post is from the user, and a + * read is a peek. What that buys: in multi-user mode a caller cannot name another + * owner's session. In single-user mode every local process holds the same + * credentials, so the header is a hint the client sets about itself. + * + * - `POST /api/sessions/:id/inbox` stores a message. `from` is the resolved caller or + * null. A full inbox is 409 CONFLICT; the oldest message is never dropped. + * - `GET /api/sessions/:id/inbox[?wait=ms]` returns every pending message. Only the + * receiver's own read marks them seen. With `wait`, it parks while nothing is + * UNSEEN (a seen-but-unacked message does not end the wait, or a receiver holding + * one would spin). + * - `POST /api/sessions/:id/inbox/ack` removes messages by id, idempotently, and only + * for the receiver itself (403 otherwise). + * - `GET /api/agent-inbox/summary` reports `pending`, `unseen`, `oldestUnseenAt` and + * `parked` per session the caller can see. Counts only, never text, and kept out of + * `toState()` so a post does not fan out a `session:updated` to every browser. + * + * Nothing here writes to a pane. + * + * @module web/routes/inbox-routes + */ + +import { FastifyInstance } from 'fastify'; +import { ApiErrorCode, createErrorResponse } from '../../types.js'; +import { InboxAckSchema, InboxPostSchema, InboxReadQuerySchema } from '../schemas.js'; +import { + abortOnClientHangUp, + canAccessOwned, + findSessionOrFail, + getAuthUser, + ownerFor, + parseBody, + resolveParentSessionId, + waitCapacityResponse, +} from '../route-helpers.js'; +import { agentInbox, MAX_INBOX_BYTES, MAX_MESSAGE_BYTES, MAX_MESSAGES_PER_INBOX } from '../agent-inbox.js'; +import { sessionWaits, WaitCapacityError, type InboxWaitResult } from '../session-wait-registry.js'; +import { clampWaitMs } from '../../config/agent-wait.js'; +import type { SessionPort } from '../ports/index.js'; + +/** Per-session counts the summary route reports. */ +export interface InboxSessionSummary { + pending: number; + unseen: number; + oldestUnseenAt: number | null; + /** The receiver itself is parked on `GET …/inbox?wait=` right now. */ + parked: boolean; +} + +export function registerInboxRoutes(app: FastifyInstance, ctx: SessionPort): void { + /** The resolved caller is the receiving session itself. */ + const callerIsReceiver = (req: Parameters[1], id: string, owner?: string) => + resolveParentSessionId(ctx, req, undefined, owner) === id; + + app.post('/api/sessions/:id/inbox', async (req) => { + const { id } = req.params as { id: string }; + const session = findSessionOrFail(ctx, id, req); + const { text } = parseBody(InboxPostSchema, req.body, 'Invalid inbox message'); + const from = resolveParentSessionId(ctx, req, undefined, session.owner) ?? null; + + const result = agentInbox.post(id, from, text); + if (!result.ok) { + switch (result.reason) { + case 'empty': + return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'text must not be empty'); + case 'message-too-large': + return createErrorResponse( + ApiErrorCode.INVALID_INPUT, + `Message is larger than ${MAX_MESSAGE_BYTES} bytes (UTF-8). Write it to a file in the workspace and post the path.` + ); + case 'inbox-full-count': + return createErrorResponse( + ApiErrorCode.CONFLICT, + `Inbox of ${id} is full (${MAX_MESSAGES_PER_INBOX} messages). Nothing was stored; the receiver has to ack before more can be posted.` + ); + case 'inbox-full-bytes': + return createErrorResponse( + ApiErrorCode.CONFLICT, + `Inbox of ${id} is full (${MAX_INBOX_BYTES} bytes of pending text). Nothing was stored; the receiver has to ack before more can be posted.` + ); + } + } + // After the store, so a parked reader that wakes finds the message. + sessionWaits.notifyInbox(id); + return { + success: true, + data: { sessionId: id, message: result.message, pending: result.pending, unseen: result.unseen }, + }; + }); + + app.get('/api/sessions/:id/inbox', async (req, reply) => { + const { id } = req.params as { id: string }; + const session = findSessionOrFail(ctx, id, req); + const query = parseBody(InboxReadQuerySchema, req.query); + const receiver = callerIsReceiver(req, id, session.owner); + // A receiver loops on this URL with identical parameters; a cached 200 would turn + // the loop into a busy spin (same reason as GET …/wait). + reply.header('Cache-Control', 'no-store'); + + let wait: InboxWaitResult | undefined; + if (query.wait !== undefined) { + const timeoutMs = clampWaitMs(query.wait); + if (agentInbox.unseenCount(id) > 0) { + wait = { + posted: false, + timedOut: false, + immediate: true, + ended: false, + aborted: false, + waitedMs: 0, + timeoutMs, + }; + } else { + const abort = abortOnClientHangUp(reply); + try { + wait = await sessionWaits.waitForInbox(id, { + timeoutMs, + owner: ownerFor(req), + abortSignal: abort.signal, + receiver, + }); + } catch (err) { + if (err instanceof WaitCapacityError) return waitCapacityResponse(err); + throw err; + } + } + } + + // Read AFTER the wait: this is what the caller gets, so it must be current. A wait + // that ended because the session was deleted reads an empty inbox. + const read = agentInbox.read(id, { receiver }); + return { success: true, data: { sessionId: id, receiver, ...read, ...(wait ? { wait } : {}) } }; + }); + + app.post('/api/sessions/:id/inbox/ack', async (req) => { + const { id } = req.params as { id: string }; + const session = findSessionOrFail(ctx, id, req); + const { ids } = parseBody(InboxAckSchema, req.body); + if (!callerIsReceiver(req, id, session.owner)) { + // A no-op here would let a coordinator believe it cleared a worker's mail, and + // would hide a client that forgot the header. Refusing says both out loud. + return createErrorResponse( + ApiErrorCode.FORBIDDEN, + `Only ${id} itself can acknowledge its inbox (X-Codeman-Parent-Session must resolve to it). Reading as anyone else is a peek.` + ); + } + const removed = agentInbox.ack(id, ids); + return { + success: true, + data: { sessionId: id, removed, pending: agentInbox.pendingCount(id), unseen: agentInbox.unseenCount(id) }, + }; + }); + + app.get('/api/agent-inbox/summary', async (req, reply) => { + reply.header('Cache-Control', 'no-store'); + const user = getAuthUser(req); + const counts = agentInbox.summary(); + const sessions: Record = {}; + for (const session of ctx.sessions.values()) { + if (!canAccessOwned(user, session.owner)) continue; + const c = counts.get(session.id); + const parked = sessionWaits.inboxWaiterCount(session.id, true) > 0; + if (!c && !parked) continue; + sessions[session.id] = { + pending: c?.pending ?? 0, + unseen: c?.unseen ?? 0, + oldestUnseenAt: c?.oldestUnseenAt ?? null, + parked, + }; + } + return { success: true, data: { sessions } }; + }); +} diff --git a/src/web/routes/index.ts b/src/web/routes/index.ts index ad34f34d..ec5e0e80 100644 --- a/src/web/routes/index.ts +++ b/src/web/routes/index.ts @@ -12,6 +12,7 @@ export { registerCronRoutes } from './cron-routes.js'; export { registerSystemRoutes } from './system-routes.js'; export { registerHookEventRoutes } from './hook-event-routes.js'; export { registerApprovalRoutes } from './approval-routes.js'; +export { registerInboxRoutes } from './inbox-routes.js'; export { registerRebootRestoreRoutes } from './reboot-restore-routes.js'; export { registerReadMyMindRoutes } from './readmymind-routes.js'; export { registerGitStatusRoutes } from './git-status-routes.js'; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 3982f4ea..dfa81fea 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -4,7 +4,7 @@ * auto-clear, auto-compact, image watcher, flicker filter, and logout. */ -import { FastifyInstance, type FastifyReply, type FastifyRequest } from 'fastify'; +import { FastifyInstance, type FastifyRequest } from 'fastify'; import { z } from 'zod'; import { join, dirname, extname, basename, resolve as resolvePath } from 'node:path'; import { homedir } from 'node:os'; @@ -115,6 +115,8 @@ import { sessionCapacityMessage, SETTINGS_PATH, validatePathWithinBase, + waitCapacityResponse, + abortOnClientHangUp, } from '../route-helpers.js'; import { buildAgentCaseMarker, writeAgentCaseMarker } from '../../agent-case-marker.js'; import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js'; @@ -738,23 +740,6 @@ function parseWaitQuery(schema: z.ZodType, query: unknown, label: string): }); } -/** - * Map a waiter-cap rejection to the code that tells the caller the truth. - * - * The two caps mean different things and warrant different recovery: `session` is - * genuinely about THIS session, while `owner` and `total` are process-wide budgets - * that say nothing about it. Reporting a global cap as `SESSION_BUSY` (409, - * documented as "Session is busy") sent an agent off to a different session to hit - * the identical error. `RATE_LIMITED` is the code whose whole meaning is "come back - * later", and clients and proxies already treat 429 that way. - */ -function waitCapacityResponse(err: WaitCapacityError): ApiResponse { - const code = err.scope === 'session' ? ApiErrorCode.SESSION_BUSY : ApiErrorCode.RATE_LIMITED; - // The registry's message already names the scope and the limit; passing it through - // verbatim keeps the wording in one place. - return createErrorResponse(code, err.message); -} - /** * The signal a session is ALREADY emitting, corrected for liveness. * @@ -931,34 +916,6 @@ export function _paneDeathWatcherCount(): number { return paneDeathWatchers.size; } -/** - * An `AbortController` that fires when the CLIENT goes away, and only then. - * - * Freeing an abandoned waiter matters because the documented pattern is a loop of - * short waits: `curl --max-time 30 ".../wait?timeout=600000"` abandons a live waiter - * every iteration until the cap is hit and an innocent session reports busy. Same for - * any proxy that cuts the connection. - * - * ⚠️ **It must listen on the RESPONSE, not the request.** `req.raw` emits `'close'` - * as soon as the request body has finished streaming, which on a POST is BEFORE the - * handler ever blocks — measured at +1ms with `aborted: false`, indistinguishable - * from a real hang-up at +0ms. Wiring the abort there cancels every send-and-wait - * instantly and silently kills the feature (it survives on GET only because a GET has - * no body to finish). `reply.raw` emits `'close'` both when the response completes - * and when the socket dies, and `writableFinished` is what tells those apart: true - * only if the response actually went out. The guard is load-bearing, not defensive. - * - * `app.inject()` never emits `'close'` at all, so this is only observable over real - * HTTP — which is why the regression test for it binds a port. - */ -function abortOnClientHangUp(reply: FastifyReply): AbortController { - const controller = new AbortController(); - reply.raw.on('close', () => { - if (!reply.raw.writableFinished) controller.abort(); - }); - return controller; -} - /** * Inject the agent skill into a case on create, surfacing only the REFUSALS. * diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index c7a37c10..9f90b1cc 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -64,6 +64,7 @@ import { AUTH_COOKIE_NAME } from '../middleware/auth.js'; import { QR_AUTH_FAILURE_MAX } from '../../config/tunnel-config.js'; import { AUTH_SESSION_TTL_MS } from '../../config/auth-config.js'; import { resolveTerminalHistoryConfig } from '../../config/terminal-history.js'; +import { agentInboxPersistence } from '../agent-inbox-persistence.js'; /** * Defaults for `POST /api/deepseek/install-profile`. @@ -1129,6 +1130,10 @@ export function registerSystemRoutes( 'Workflow run watcher' ); + // Agent inbox snapshot (default OFF). From `merged` like the toggles above: a + // partial PUT that omits the key must not switch it off and delete the file. + agentInboxPersistence.setEnabled(merged.agentInboxPersistEnabled === true); + // Handle image watcher toggle dynamically toggleService((merged.imageWatcherEnabled as boolean) ?? false, imageWatcher, 'Image watcher', () => { // Re-watch all active sessions that have image watcher enabled diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 86aa3f25..2a815163 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -1537,6 +1537,12 @@ export const SettingsUpdateSchema = z configBackupKeepCount: z.number().int().min(2).max(500).optional(), /** Also drop snapshots older than this many days; 0 = no age limit. */ configBackupKeepDays: z.number().int().min(0).max(3650).optional(), + /** + * Agent inbox snapshot on disk (src/web/agent-inbox-persistence.ts). SYNCED, default OFF: + * off means no file is written, and switching it off deletes the snapshot (the in-memory + * inboxes stay). On, pending messages between agents survive a server restart. + */ + agentInboxPersistEnabled: z.boolean().optional(), /** * Read My Mind predictor model override. Empty/absent = the AI-checker * default (opus: prediction quality is the product and it runs only on an @@ -1836,6 +1842,32 @@ export const SessionWaitOutputQuerySchema = z.object({ timeout: z.coerce.number().int().positive().optional(), }); +// ========== Agent Inbox Routes ========== + +/** + * Body for `POST /api/sessions/:id/inbox`. The byte cap (`MAX_MESSAGE_BYTES`) is the + * store's, checked there in UTF-8 bytes; this bound is only the outer fence so a + * multi-megabyte body never reaches it. Multi-line text is fine: it is stored and + * handed back as data, never typed. + */ +export const InboxPostSchema = z + .object({ + text: z.string().min(1, 'text must not be empty').max(16_384), + }) + .strict(); + +/** Query for `GET /api/sessions/:id/inbox`. `wait` follows the wait routes' `timeout`: clamped, never a 400 for size. */ +export const InboxReadQuerySchema = z.object({ + wait: z.coerce.number().int().positive().optional(), +}); + +/** Body for `POST /api/sessions/:id/inbox/ack`: the ids a read handed out. */ +export const InboxAckSchema = z + .object({ + ids: z.array(z.string().min(1).max(64)).min(1).max(200), + }) + .strict(); + // ========== Session Mutation Routes ========== /** PUT /api/sessions/:id/name */ diff --git a/src/web/sensitive-path.ts b/src/web/sensitive-path.ts index ecaaa1aa..a9499629 100644 --- a/src/web/sensitive-path.ts +++ b/src/web/sensitive-path.ts @@ -100,6 +100,8 @@ const SENSITIVE_PATTERNS: RegExp[] = [ /\/\.codeman[^/]*\/settings\.json$/, /\/\.codeman[^/]*\/push-keys\.json$/, /\/\.codeman[^/]*\/intents\.json$/, + // Pending agent-inbox messages (opt-in snapshot): same text as prompts, same 0600. + /\/\.codeman[^/]*\/agent-inbox\.json$/, ]; /** diff --git a/src/web/server.ts b/src/web/server.ts index b3f537e9..6b6408bc 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -116,6 +116,8 @@ import { } from './session-listener-wiring.js'; import { sessionWaits } from './session-wait-registry.js'; import { intentStore } from '../intent-store.js'; +import { agentInbox } from './agent-inbox.js'; +import { agentInboxPersistence } from './agent-inbox-persistence.js'; import { AI_CHECK_MODEL } from '../config/ai-defaults.js'; import { approvalInbox } from './approval-inbox.js'; import { SCREEN_BLOCKED_SOURCE } from './screen-blocked.js'; @@ -201,6 +203,7 @@ import { registerScheduledRoutes, registerHookEventRoutes, registerApprovalRoutes, + registerInboxRoutes, registerRebootRestoreRoutes, registerReadMyMindRoutes, registerGitStatusRoutes, @@ -1229,6 +1232,7 @@ export class WebServer extends EventEmitter { registerScheduledRoutes(this.app, ctx); registerHookEventRoutes(this.app, ctx); registerApprovalRoutes(this.app, ctx); + registerInboxRoutes(this.app, ctx); registerRebootRestoreRoutes(this.app, ctx); registerReadMyMindRoutes(this.app, ctx); registerGitStatusRoutes(this.app, ctx); @@ -1686,6 +1690,10 @@ export class WebServer extends EventEmitter { // The 'exit' here is NOT redundant with the PTY-exit listener: listeners are // detached a few lines above, before `session.stop()`, so on a delete the // session's own exit event never reaches the registry. + // The agent inbox goes with a DELETED session only. killMux=false is shutdown or + // detach, and a detached session comes back to the mail posted while it was away. + // Before cancelAll, so a reader released by it reads an empty inbox. + if (killMux) agentInbox.drop(sessionId); sessionWaits.notifySignal(sessionId, 'exit'); sessionWaits.cancelAll(sessionId); approvalInbox.resolveForSession(sessionId, 'session_ended'); @@ -2211,6 +2219,13 @@ export class WebServer extends EventEmitter { return settings.agentSkillEnabled === true; } + // Whether pending agent-inbox mail is snapshotted to disk (synced + // `agentInboxPersistEnabled` setting). Default OFF: an absent key writes nothing. + private async getAgentInboxPersistEnabled(): Promise { + const settings = await this.readSettings(); + return settings.agentInboxPersistEnabled === true; + } + // Whether a Claude session installs Codeman's hooks block into its workspace // (synced `workspaceHooksEnabled` setting). Default ON — an absent key means a // user who has never seen this setting, and OFF for them would mean no tab @@ -3041,6 +3056,10 @@ export class WebServer extends EventEmitter { // This prevents race conditions where clients connect before state is ready // CRITICAL: Skip in test mode to prevent tests from picking up user sessions if (!this.testMode) { + // Pending agent mail first (opt-in snapshot), so it is in place before any + // route can answer. Pruned against the restored sessions in finalizeRestoredState. + const restoredMail = agentInboxPersistence.load(await this.getAgentInboxPersistEnabled()); + if (restoredMail > 0) console.log(`[AgentInbox] restored ${restoredMail} pending message(s)`); const restored = await this.restoreMuxSessions(); await this.finalizeRestoredState(restored); @@ -4013,6 +4032,13 @@ export class WebServer extends EventEmitter { this.tabLayouts.markRestorationComplete(); await this.cleanupStaleSessions(); await this.tabLayouts.reconcileAfterRestoration(); + // Same gate as the tab layouts: only a restore that succeeded may decide which + // inboxes are orphans. A failed one returns above and leaves every message alone. + // Kept: live sessions plus every record state.json still holds (a session that + // was not re-adopted this boot but is kept for recovery keeps its mail). + const keep = new Set([...this.sessions.keys(), ...Object.keys(this.store.getSessions())]); + const pruned = agentInbox.pruneExcept(keep); + if (pruned > 0) console.log(`[AgentInbox] dropped ${pruned} inbox(es) of sessions that did not come back`); } /** @@ -4189,6 +4215,8 @@ export class WebServer extends EventEmitter { sessionWaits.cancelEverything(); approvalInbox.stop(); this.keepWarmService.stop(); + // A post within the debounce window before shutdown would otherwise not reach disk. + agentInboxPersistence.flush(); // Same reason as `cancelEverything` above: an in-flight wake is awaited by a request, // and `app.close()` (the last line of this method) does not abort in-flight requests — // so without this a restart during a wake waits out the readiness poll. diff --git a/src/web/session-wait-registry.ts b/src/web/session-wait-registry.ts index 3a3b9b83..fbdbc246 100644 --- a/src/web/session-wait-registry.ts +++ b/src/web/session-wait-registry.ts @@ -7,11 +7,16 @@ * lets a request *block* until something happens instead, so an orchestrating * agent can `curl` and wait. Plan: `docs/agent-control-plan.md`. * - * Two waiter kinds: + * Three waiter kinds: * - **signal waiters** resolve on the first of a requested set of lifecycle * signals (`idle`, `working`, `stop`, `blocked`, `exit`). * - **output waiters** resolve when a literal string appears in a session's * ANSI-stripped output. + * - **inbox waiters** resolve when a message is posted to the session's agent + * inbox (`agent-inbox.ts`). The registry knows nothing about messages: the + * route decides whether to park (nothing unseen) and `notifyInbox()` is the + * post's "something arrived". Kept here so `GET .../inbox?wait=` is charged to + * the same per-session / per-owner / total caps as the other two. * * ## What "ANSI-stripped output" means here * @@ -390,6 +395,12 @@ export interface OutputWaitResult extends WaitResultBase { snippet: string | null; } +/** Outcome of an inbox wait. */ +export interface InboxWaitResult extends WaitResultBase { + /** True when a post released the wait (the caller should read the inbox now). */ + posted: boolean; +} + /** * Thrown when a waiter would exceed a configured cap. * @@ -458,6 +469,15 @@ export interface OutputWaitOptions extends WaitOptionsBase { initialText?: string; } +export interface InboxWaitOptions extends WaitOptionsBase { + /** + * The caller IS the receiving session (as resolved by the route), not a coordinator + * peeking. Counted separately so the inbox summary's `parked` means "the receiver is + * listening", which is what a sender deciding whether to nudge needs to know. + */ + receiver?: boolean; +} + export interface SessionWaitRegistryOptions { maxWaitersPerSession?: number; maxWaitersPerOwner?: number; @@ -483,6 +503,11 @@ interface SignalWaiter extends WaiterBase { settle: (result: SignalWaitResult) => void; } +interface InboxWaiter extends WaiterBase { + receiver: boolean; + settle: (result: InboxWaitResult) => void; +} + interface OutputWaiter extends WaiterBase { /** Original-case needle, kept for reporting. */ needle: string; @@ -505,6 +530,7 @@ interface OutputWaiter extends WaiterBase { export class SessionWaitRegistry { private readonly signalWaiters = new Map>(); private readonly outputWaiters = new Map>(); + private readonly inboxWaiters = new Map>(); /** * Live waiter count per owner. A derived count would mean walking every waiter in * the process on each registration, so it is maintained incrementally instead: @@ -544,20 +570,34 @@ export class SessionWaitRegistry { return this.outputWaiters.get(sessionId)?.size ?? 0; } - /** Pending waiters of both kinds for a session. */ - waiterCount(sessionId: string): number { - return this.signalWaiterCount(sessionId) + this.outputWaiterCount(sessionId); + /** + * Pending inbox waiters for a session; with `receiverOnly`, only those whose caller + * is the receiving session itself (the inbox summary's `parked`). + */ + inboxWaiterCount(sessionId: string, receiverOnly = false): number { + const set = this.inboxWaiters.get(sessionId); + if (!set) return 0; + if (!receiverOnly) return set.size; + let count = 0; + for (const waiter of set) if (waiter.receiver) count++; + return count; } - /** Pending waiters of both kinds across every session. */ + /** Pending waiters of every kind for a session. */ + waiterCount(sessionId: string): number { + return this.signalWaiterCount(sessionId) + this.outputWaiterCount(sessionId) + this.inboxWaiterCount(sessionId); + } + + /** Pending waiters of every kind across every session. */ totalWaiterCount(): number { let total = 0; for (const set of this.signalWaiters.values()) total += set.size; for (const set of this.outputWaiters.values()) total += set.size; + for (const set of this.inboxWaiters.values()) total += set.size; return total; } - /** Pending waiters of both kinds charged to one owner. */ + /** Pending waiters of every kind charged to one owner. */ ownerWaiterCount(owner: string): number { return this.ownerCounts.get(owner) ?? 0; } @@ -920,6 +960,105 @@ export class SessionWaitRegistry { } } + // ── Inbox waits ── + + /** + * Block until a message is posted to this session's inbox, the timeout elapses, or + * the session ends. + * + * There is no "already satisfied" shortcut here: whether the inbox holds unseen mail + * is the store's business, so the route checks that first and only calls this when + * there is nothing to hand out. A post that lands between that check and this call + * is not lost either: both run in the same tick, and `notifyInbox` is called by the + * post route after the message is stored. + * + * @throws {WaitCapacityError} when a waiter cap is already reached. + */ + waitForInbox(sessionId: string, options: InboxWaitOptions): Promise { + const base = { posted: false, timedOut: false, immediate: false, waitedMs: 0, timeoutMs: options.timeoutMs }; + if (options.abortSignal?.aborted) return Promise.resolve({ ...base, ended: true, aborted: true }); + if (this.stopped) return Promise.resolve({ ...base, ended: true, aborted: false }); + + this.assertCapacity(sessionId, options.owner); + + return new Promise((resolve) => { + let set = this.inboxWaiters.get(sessionId); + if (!set) { + set = new Set(); + this.inboxWaiters.set(sessionId, set); + } + + const waiter: InboxWaiter = { + receiver: options.receiver === true, + startedAt: Date.now(), + timeoutMs: options.timeoutMs, + owner: options.owner, + timer: setTimeout(() => { + this.removeInboxWaiter(sessionId, waiter); + resolve({ + posted: false, + timedOut: true, + immediate: false, + ended: false, + aborted: false, + waitedMs: Date.now() - waiter.startedAt, + timeoutMs: waiter.timeoutMs, + }); + }, options.timeoutMs), + settle: resolve, + }; + set.add(waiter); + this.chargeOwner(options.owner); + waiter.detachAbort = this.attachAbort(options.abortSignal, () => { + this.removeInboxWaiter(sessionId, waiter); + resolve({ + posted: false, + timedOut: false, + immediate: false, + ended: true, + aborted: true, + waitedMs: Date.now() - waiter.startedAt, + timeoutMs: waiter.timeoutMs, + }); + }); + }); + } + + /** + * A message was posted to this session's inbox: release every inbox waiter on it. + * + * @returns how many waiters were resolved. + */ + notifyInbox(sessionId: string): number { + const set = this.inboxWaiters.get(sessionId); + if (!set || set.size === 0) return 0; + let resolved = 0; + for (const waiter of [...set]) { + this.removeInboxWaiter(sessionId, waiter); + waiter.settle({ + posted: true, + timedOut: false, + immediate: false, + ended: false, + aborted: false, + waitedMs: Date.now() - waiter.startedAt, + timeoutMs: waiter.timeoutMs, + }); + resolved++; + } + return resolved; + } + + private removeInboxWaiter(sessionId: string, waiter: InboxWaiter): void { + clearTimeout(waiter.timer); + waiter.detachAbort?.(); + const set = this.inboxWaiters.get(sessionId); + if (!set) return; + if (!set.delete(waiter)) return; + this.releaseOwner(waiter.owner); + if (set.size === 0) this.inboxWaiters.delete(sessionId); + } + // ── Teardown ── /** @@ -969,6 +1108,23 @@ export class SessionWaitRegistry { } } + const inbox = this.inboxWaiters.get(sessionId); + if (inbox) { + for (const waiter of [...inbox]) { + this.removeInboxWaiter(sessionId, waiter); + waiter.settle({ + posted: false, + timedOut: false, + immediate: false, + ended: true, + aborted: false, + waitedMs: Date.now() - waiter.startedAt, + timeoutMs: waiter.timeoutMs, + }); + resolved++; + } + } + this.pendingAnsi.delete(sessionId); return resolved; } @@ -999,7 +1155,7 @@ export class SessionWaitRegistry { cancelEverything(): number { this.stopped = true; let resolved = 0; - const ids = new Set([...this.signalWaiters.keys(), ...this.outputWaiters.keys()]); + const ids = new Set([...this.signalWaiters.keys(), ...this.outputWaiters.keys(), ...this.inboxWaiters.keys()]); for (const id of ids) resolved += this.cancelAll(id); return resolved; } diff --git a/test/agent-inbox-persistence.test.ts b/test/agent-inbox-persistence.test.ts new file mode 100644 index 00000000..b0f1f753 --- /dev/null +++ b/test/agent-inbox-persistence.test.ts @@ -0,0 +1,104 @@ +/** + * @fileoverview Opt-in agent inbox snapshot (src/web/agent-inbox-persistence.ts). + * + * Off writes nothing, switching off deletes the file and keeps memory, on writes 0600 + * through tmp + rename, and a boot with the setting on restores what was pending. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { AgentInbox } from '../src/web/agent-inbox.js'; +import { AgentInboxPersistence } from '../src/web/agent-inbox-persistence.js'; + +describe('AgentInboxPersistence', () => { + let dir: string; + let file: string; + let inbox: AgentInbox; + let persistence: AgentInboxPersistence; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'codeman-inbox-')); + file = join(dir, 'agent-inbox.json'); + inbox = new AgentInbox(); + // Debounce 0 so a test can wait one macrotask instead of sleeping. + persistence = new AgentInboxPersistence(inbox, () => file, 0); + }); + + afterEach(() => { + persistence.dispose(); + rmSync(dir, { recursive: true, force: true }); + }); + + const tick = () => new Promise((r) => setTimeout(r, 5)); + + it('writes nothing while off (the default)', async () => { + persistence.load(false); + inbox.post('s', null, 'hello'); + await tick(); + expect(existsSync(file)).toBe(false); + }); + + it('writes a 0600 snapshot when on, and no tmp file is left behind', async () => { + persistence.load(true); + inbox.post('s', null, 'hello'); + await tick(); + expect(JSON.parse(readFileSync(file, 'utf-8')).inboxes.s[0].text).toBe('hello'); + expect(statSync(file).mode & 0o777).toBe(0o600); + expect(existsSync(`${file}.tmp`)).toBe(false); + }); + + it('switching on writes the current inboxes at once', () => { + persistence.load(false); + inbox.post('s', null, 'posted while off'); + persistence.setEnabled(true); + expect(JSON.parse(readFileSync(file, 'utf-8')).inboxes.s).toHaveLength(1); + }); + + it('switching off deletes the snapshot and keeps the in-memory inboxes', async () => { + persistence.load(true); + inbox.post('s', null, 'hello'); + await tick(); + expect(existsSync(file)).toBe(true); + persistence.setEnabled(false); + expect(existsSync(file)).toBe(false); + expect(inbox.pendingCount('s')).toBe(1); + inbox.post('s', null, 'after off'); + await tick(); + expect(existsSync(file)).toBe(false); + }); + + it('a boot with the setting on restores pending mail', async () => { + persistence.load(true); + inbox.post('s', 'sender', 'survives'); + await tick(); + persistence.dispose(); + + const next = new AgentInbox(); + const nextPersistence = new AgentInboxPersistence(next, () => file, 0); + expect(nextPersistence.load(true)).toBe(1); + expect(next.read('s', { receiver: false }).messages[0]).toMatchObject({ from: 'sender', text: 'survives' }); + nextPersistence.dispose(); + }); + + it('a boot with the setting off removes a file left from earlier and restores nothing', () => { + writeFileSync(file, JSON.stringify({ version: 1, inboxes: { s: [] } })); + expect(persistence.load(false)).toBe(0); + expect(existsSync(file)).toBe(false); + }); + + it('an unreadable file is an empty inbox, not a failed boot', () => { + writeFileSync(file, '{not json'); + expect(persistence.load(true)).toBe(0); + }); + + it('flush writes a pending change before shutdown', () => { + const slow = new AgentInboxPersistence(inbox, () => file, 60_000); + slow.load(true); + inbox.post('s', null, 'just before shutdown'); + expect(existsSync(file)).toBe(false); + slow.flush(); + expect(JSON.parse(readFileSync(file, 'utf-8')).inboxes.s).toHaveLength(1); + slow.dispose(); + }); +}); diff --git a/test/agent-inbox-server-wiring.test.ts b/test/agent-inbox-server-wiring.test.ts new file mode 100644 index 00000000..44674248 --- /dev/null +++ b/test/agent-inbox-server-wiring.test.ts @@ -0,0 +1,80 @@ +/** + * @fileoverview How server.ts treats the agent inbox across a session's lifecycle. + * + * - Delete (`killMux=true`) drops the inbox; detach and shutdown (`killMux=false`) + * keep it, because a detached session comes back. + * - Boot pruning of orphaned inboxes runs only after a successful restore, and keeps + * every session state.json still holds. A failed restore leaves all mail alone. + * + * Uses a bare `WebServer.prototype` object with just the fields these paths touch, + * like tab-layout-restore-gate.test.ts. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { WebServer } from '../src/web/server.js'; +import { agentInbox } from '../src/web/agent-inbox.js'; + +type CleanupServer = { + _doCleanupSession(sessionId: string, killMux: boolean, reason?: string): Promise; + finalizeRestoredState(restored: boolean): Promise; + [key: string]: unknown; +}; + +function bareServer(live: string[] = [], persisted: string[] = []): CleanupServer { + const server = Object.create(WebServer.prototype) as CleanupServer; + Object.assign(server, { + sessions: new Map(live.map((id) => [id, {}])), + keepWarmService: { remove: () => {} }, + store: { + getSession: () => null, + getSessions: () => Object.fromEntries(persisted.map((id) => [id, {}])), + removeRalphState: () => {}, + }, + tabLayouts: { + markRestorationComplete: () => {}, + markRestorationFailed: () => {}, + reconcileAfterRestoration: async () => {}, + }, + cleanupStaleSessions: async () => 0, + _customModelDisplacedNotified: new Set(), + respawnControllers: new Map(), + respawnTimers: new Map(), + pendingRespawnStarts: new Map(), + runSummaryTrackers: new Map(), + stopTranscriptWatcher: () => {}, + persistDeb: { cancelKey: () => {} }, + sse: { cleanupSessionBatches: () => {} }, + broadcast: () => {}, + }); + return server; +} + +describe('agent inbox server wiring', () => { + beforeEach(() => agentInbox.resetForTests()); + afterEach(() => agentInbox.resetForTests()); + + it('a delete drops the inbox; a detach (killMux=false) keeps it', async () => { + const server = bareServer(); + agentInbox.post('detached', null, 'still here after detach'); + agentInbox.post('deleted', null, 'gone with the session'); + await server._doCleanupSession('detached', false, 'test'); + await server._doCleanupSession('deleted', true, 'test'); + expect(agentInbox.pendingCount('detached')).toBe(1); + expect(agentInbox.pendingCount('deleted')).toBe(0); + }); + + it('a failed restore prunes nothing', async () => { + const server = bareServer(); + agentInbox.post('not-restored', null, 'm'); + await server.finalizeRestoredState(false); + expect(agentInbox.pendingCount('not-restored')).toBe(1); + }); + + it('a successful restore prunes orphans and keeps live and persisted sessions', async () => { + const server = bareServer(['live'], ['kept-for-recovery']); + for (const id of ['live', 'kept-for-recovery', 'orphan']) agentInbox.post(id, null, 'm'); + await server.finalizeRestoredState(true); + expect(agentInbox.pendingCount('live')).toBe(1); + expect(agentInbox.pendingCount('kept-for-recovery')).toBe(1); + expect(agentInbox.pendingCount('orphan')).toBe(0); + }); +}); diff --git a/test/agent-inbox.test.ts b/test/agent-inbox.test.ts new file mode 100644 index 00000000..a611a37f --- /dev/null +++ b/test/agent-inbox.test.ts @@ -0,0 +1,263 @@ +/** + * @fileoverview The agent inbox store (src/web/agent-inbox.ts), pure: no routes, no IO. + * + * Each block pins one rule from discussion #558: read until ack, seen only for the + * receiver, byte and count caps that refuse instead of dropping, a TTL counted from + * each message's own timestamp, and a snapshot that survives malformed input. + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { + AgentInbox, + MAX_INBOX_BYTES, + MAX_MESSAGE_BYTES, + MAX_MESSAGES_PER_INBOX, + MESSAGE_TTL_MS, +} from '../src/web/agent-inbox.js'; + +const RX = 'receiver-session'; + +describe('AgentInbox', () => { + let now: number; + let inbox: AgentInbox; + + beforeEach(() => { + now = 1_000_000; + inbox = new AgentInbox(() => now); + }); + + const post = (text: string, from: string | null = 'sender') => { + const r = inbox.post(RX, from, text); + if (!r.ok) throw new Error(`post refused: ${r.reason}`); + return r.message; + }; + + describe('read until ack', () => { + it('a read removes nothing; the same messages come back until acked', () => { + const a = post('first'); + const b = post('second'); + expect(inbox.read(RX, { receiver: true }).messages.map((m) => m.id)).toEqual([a.id, b.id]); + expect(inbox.read(RX, { receiver: true }).messages.map((m) => m.id)).toEqual([a.id, b.id]); + expect(inbox.pendingCount(RX)).toBe(2); + expect(inbox.ack(RX, [a.id])).toBe(1); + expect(inbox.read(RX, { receiver: true }).messages.map((m) => m.id)).toEqual([b.id]); + }); + + it('ack is idempotent: an id that is already gone or never existed is not an error', () => { + const a = post('x'); + expect(inbox.ack(RX, [a.id])).toBe(1); + expect(inbox.ack(RX, [a.id])).toBe(0); + expect(inbox.ack(RX, ['no-such-id'])).toBe(0); + expect(inbox.ack('no-such-session', [a.id])).toBe(0); + }); + + it('ack removes only the given ids, never a message posted after the read', () => { + const a = post('read me'); + inbox.read(RX, { receiver: true }); + const late = post('arrived after the read'); + inbox.ack(RX, [a.id]); + expect(inbox.read(RX, { receiver: false }).messages.map((m) => m.id)).toEqual([late.id]); + }); + + it('keeps multi-line text byte for byte', () => { + const text = 'line one\nline two\r\n\ttabbed\n'; + post(text); + expect(inbox.read(RX, { receiver: true }).messages[0].text).toBe(text); + }); + }); + + describe('seen belongs to the receiver', () => { + it('a receiver read marks seen; the flag on each message is the state BEFORE this read', () => { + post('m'); + const first = inbox.read(RX, { receiver: true }); + expect(first.messages[0].seen).toBe(false); + expect(first.unseen).toBe(0); + const second = inbox.read(RX, { receiver: true }); + expect(second.messages[0].seen).toBe(true); + }); + + it('a peek changes nothing: unseen stays unseen', () => { + post('m'); + const peek = inbox.read(RX, { receiver: false }); + expect(peek.messages[0].seen).toBe(false); + expect(peek.unseen).toBe(1); + expect(inbox.unseenCount(RX)).toBe(1); + expect(inbox.summary().get(RX)).toMatchObject({ pending: 1, unseen: 1 }); + }); + + it('a message posted after a receiver read is unseen again', () => { + post('a'); + inbox.read(RX, { receiver: true }); + expect(inbox.unseenCount(RX)).toBe(0); + post('b'); + expect(inbox.unseenCount(RX)).toBe(1); + }); + }); + + describe('caps refuse, never drop the oldest', () => { + it(`refuses message ${MAX_MESSAGES_PER_INBOX + 1} and keeps the first one`, () => { + const first = post('0'); + for (let i = 1; i < MAX_MESSAGES_PER_INBOX; i++) post(String(i)); + expect(inbox.post(RX, null, 'one too many')).toEqual({ ok: false, reason: 'inbox-full-count' }); + const ids = inbox.read(RX, { receiver: false }).messages.map((m) => m.id); + expect(ids).toHaveLength(MAX_MESSAGES_PER_INBOX); + expect(ids[0]).toBe(first.id); + }); + + it('counts the per-message cap in UTF-8 bytes, not characters', () => { + // 'é' is 2 bytes: half the byte budget in characters already fills it. + expect(inbox.post(RX, null, 'é'.repeat(MAX_MESSAGE_BYTES / 2)).ok).toBe(true); + expect(inbox.post(RX, null, 'é'.repeat(MAX_MESSAGE_BYTES / 2 + 1))).toEqual({ + ok: false, + reason: 'message-too-large', + }); + }); + + it('refuses a post that would cross the per-inbox byte cap', () => { + const chunk = 'x'.repeat(MAX_MESSAGE_BYTES); + const fit = Math.floor(MAX_INBOX_BYTES / MAX_MESSAGE_BYTES); + for (let i = 0; i < fit; i++) post(chunk); + expect(inbox.post(RX, null, 'x'.repeat(MAX_INBOX_BYTES - fit * MAX_MESSAGE_BYTES + 1))).toEqual({ + ok: false, + reason: 'inbox-full-bytes', + }); + expect(inbox.pendingCount(RX)).toBe(fit); + }); + + it('an ack makes room again', () => { + const ids: string[] = []; + for (let i = 0; i < MAX_MESSAGES_PER_INBOX; i++) ids.push(post(String(i)).id); + inbox.ack(RX, [ids[0]]); + expect(inbox.post(RX, null, 'now it fits').ok).toBe(true); + }); + + it('refuses empty text', () => { + expect(inbox.post(RX, null, '')).toEqual({ ok: false, reason: 'empty' }); + }); + }); + + describe('TTL from the message timestamp', () => { + it('a message older than the TTL is gone, read or not', () => { + post('old'); + now += MESSAGE_TTL_MS / 2; + const fresh = post('fresh'); + now += MESSAGE_TTL_MS / 2 + 1; + expect(inbox.read(RX, { receiver: false }).messages.map((m) => m.id)).toEqual([fresh.id]); + }); + + it('a restore keeps createdAt, so a restart does not restart the clock', () => { + post('m'); + const snap = JSON.parse(JSON.stringify(inbox.snapshot())); + now += MESSAGE_TTL_MS - 1000; + const restarted = new AgentInbox(() => now); + expect(restarted.restore(snap)).toBe(1); + now += 1001; + expect(restarted.pendingCount(RX)).toBe(0); + }); + + it('does not restore a message that expired while the server was down', () => { + post('m'); + const snap = inbox.snapshot(); + now += MESSAGE_TTL_MS + 1; + expect(new AgentInbox(() => now).restore(snap)).toBe(0); + }); + + it('expiry fires onChange so the snapshot catches up', () => { + post('m'); + let changes = 0; + inbox.onChange = () => changes++; + now += MESSAGE_TTL_MS + 1; + inbox.summary(); + expect(changes).toBe(1); + }); + }); + + describe('summary', () => { + it('reports counts and the oldest unseen time, never text', () => { + const a = post('secret one'); + now += 10; + post('secret two'); + inbox.read(RX, { receiver: true }); + now += 10; + const c = post('secret three'); + const row = inbox.summary().get(RX); + expect(row).toEqual({ pending: 3, unseen: 1, oldestUnseenAt: c.createdAt }); + expect(JSON.stringify(row)).not.toContain('secret'); + expect(a.createdAt).toBeLessThan(c.createdAt); + }); + + it('leaves out sessions with an empty inbox', () => { + const a = post('m'); + inbox.ack(RX, [a.id]); + expect(inbox.summary().has(RX)).toBe(false); + }); + }); + + describe('lifecycle', () => { + it('drop discards the inbox and fires onChange', () => { + post('m'); + let changes = 0; + inbox.onChange = () => changes++; + inbox.drop(RX); + expect(inbox.pendingCount(RX)).toBe(0); + expect(changes).toBe(1); + }); + + it('pruneExcept drops only inboxes of sessions not kept', () => { + inbox.post('keep-me', null, 'a'); + inbox.post('gone', null, 'b'); + expect(inbox.pruneExcept(['keep-me'])).toBe(1); + expect(inbox.pendingCount('keep-me')).toBe(1); + expect(inbox.pendingCount('gone')).toBe(0); + }); + }); + + describe('snapshot / restore', () => { + it('round-trips messages, senders and the seen state', () => { + const a = post('from a session', 'sender-1'); + post('from the user', null); + inbox.read(RX, { receiver: true }); + const c = post('unseen'); + const restored = new AgentInbox(() => now); + expect(restored.restore(JSON.parse(JSON.stringify(inbox.snapshot())))).toBe(3); + const read = restored.read(RX, { receiver: false }); + expect(read.messages.map((m) => [m.id, m.from, m.seen])).toEqual([ + [a.id, 'sender-1', true], + [expect.any(String), null, true], + [c.id, 'sender', false], + ]); + }); + + it('skips malformed entries and files instead of throwing', () => { + const restored = new AgentInbox(() => now); + expect(restored.restore(null)).toBe(0); + expect(restored.restore({ version: 2, inboxes: {} })).toBe(0); + expect(restored.restore({ version: 1, inboxes: { s: 'nope' } })).toBe(0); + const good = { id: 'g', from: null, text: 'ok', createdAt: now, seenAt: null }; + expect( + restored.restore({ + version: 1, + inboxes: { + s: [ + good, + { id: 1 }, + { ...good, id: 'big', text: 'x'.repeat(MAX_MESSAGE_BYTES + 1) }, + { ...good, text: '' }, + ], + }, + }) + ).toBe(1); + }); + + it('holds the count cap on restore', () => { + const many = Array.from({ length: MAX_MESSAGES_PER_INBOX + 5 }, (_, i) => ({ + id: `m${i}`, + from: null, + text: 'x', + createdAt: now, + seenAt: null, + })); + const restored = new AgentInbox(() => now); + expect(restored.restore({ version: 1, inboxes: { s: many } })).toBe(MAX_MESSAGES_PER_INBOX); + }); + }); +}); diff --git a/test/cli-agent.test.ts b/test/cli-agent.test.ts index 7d420123..55ad117e 100644 --- a/test/cli-agent.test.ts +++ b/test/cli-agent.test.ts @@ -13,8 +13,11 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { AgentGuardError, EXIT, + agentAck, + agentInboxRead, agentInterrupt, agentLs, + agentPost, agentRead, agentRm, agentSend, @@ -28,10 +31,12 @@ import { deleteRefusal, describeFailure, httpRequest, + inboxCell, inputRefusal, isSelfSession, MIN_ID_PREFIX_LENGTH, parsePositiveInt, + postTextFromArgs, registerAgentCommands, resolveAgentContext, stripAnsi, @@ -607,6 +612,147 @@ describe('session id prefixes', () => { }); }); +describe('agent inbox verbs', () => { + const MSG = { + id: 'm-1', + from: OTHER, + text: 'line one\nline two', + createdAt: 1_700_000_000_000 - 90_000, + seen: false, + }; + + it('ls merges the inbox summary into an INBOX column', async () => { + const deps = fakeDeps((o) => + o.path === '/api/v1/agent-inbox/summary' + ? ok({ + sessions: { + [SELF]: { pending: 2, unseen: 1, oldestUnseenAt: 1, parked: false }, + [OTHER]: { pending: 0, unseen: 0, oldestUnseenAt: null, parked: true }, + }, + }) + : ok([ + { id: SELF, mode: 'claude', status: 'busy', name: 'me' }, + { id: OTHER, mode: 'codex', status: 'idle', name: 'worker' }, + ]) + ); + expect(await agentLs(deps)).toBe(EXIT.ok); + const text = deps.out.join('\n'); + expect(text).toMatch(/ID\s+MODE\s+STATUS\s+INBOX\s+NAME/); + expect(text).toMatch(/058ee7b5\s+claude\s+busy\s+2 \(1 new\)\s+me/); + expect(text).toMatch(/94990c6d\s+codex\s+idle\s+parked\s+worker/); + }); + + it('ls --json carries each session’s inbox row (null when it has none)', async () => { + const deps = fakeDeps( + (o) => + o.path === '/api/v1/agent-inbox/summary' + ? ok({ sessions: { [OTHER]: { pending: 1, unseen: 1, oldestUnseenAt: 5, parked: false } } }) + : ok([{ id: SELF }, { id: OTHER }]), + true + ); + await agentLs(deps); + const parsed = JSON.parse(deps.out.join('')) as Array<{ id: string; inbox: unknown }>; + expect(parsed.map((s) => s.inbox)).toEqual([null, { pending: 1, unseen: 1, oldestUnseenAt: 5, parked: false }]); + }); + + it('ls still lists sessions when the server has no summary route', async () => { + const deps = fakeDeps((o) => + o.path === '/api/v1/agent-inbox/summary' ? apiError(404, 'NOT_FOUND', 'Route not found') : ok([{ id: OTHER }]) + ); + expect(await agentLs(deps)).toBe(EXIT.ok); + expect(deps.out.join('\n')).not.toContain('INBOX'); + }); + + it('inboxCell', () => { + expect(inboxCell(undefined)).toBe(''); + expect(inboxCell({ pending: 3, unseen: 0, oldestUnseenAt: null, parked: false })).toBe('3'); + expect(inboxCell({ pending: 3, unseen: 2, oldestUnseenAt: 1, parked: true })).toBe('3 (2 new), parked'); + }); + + it('post sends the text as data to the resolved session and never to /input', async () => { + const deps = fakeDeps([ok({ message: MSG, pending: 1, unseen: 1 })]); + expect(await agentPost(deps, { id: OTHER, text: 'line one\nline two' })).toBe(EXIT.ok); + expect(deps.calls).toEqual([ + expect.objectContaining({ + method: 'POST', + path: `/api/v1/sessions/${OTHER}/inbox`, + body: { text: 'line one\nline two' }, + }), + ]); + expect(deps.out.join('')).toMatch(/posted to 94990c6d \(1 pending, 1 unseen\)/); + }); + + it('post passes a full inbox (409) through as an error', async () => { + const deps = fakeDeps([apiError(409, 'CONFLICT', 'Inbox of x is full (200 messages).')]); + expect(await agentPost(deps, { id: OTHER, text: 'x' })).toBe(EXIT.error); + expect(deps.err.join('')).toMatch(/CONFLICT: Inbox of x is full/); + }); + + it('post takes ONE argument and refuses an empty one before any request', async () => { + expect(postTextFromArgs(['a\nb'])).toEqual({ text: 'a\nb' }); + expect(postTextFromArgs(['a', 'b'])).toEqual({ error: expect.stringMatching(/ONE argument, got 2/) }); + const deps = fakeDeps([]); + expect(await agentPost(deps, { id: OTHER, text: '' })).toBe(EXIT.refused); + expect(deps.calls).toEqual([]); + }); + + it('inbox reads this session’s own inbox by default and prints full ids plus the ack hint', async () => { + const deps = fakeDeps([ok({ receiver: true, messages: [MSG], pending: 1, unseen: 0 })]); + expect(await agentInboxRead(deps, {})).toBe(EXIT.ok); + expect(deps.calls[0]).toMatchObject({ method: 'GET', path: `/api/v1/sessions/${SELF}/inbox` }); + expect(deps.calls[0].query).toEqual({ wait: undefined }); + const text = deps.out.join('\n'); + expect(text).toMatch(/── m-1 from 94990c6d, 2m ago, new/); + expect(text).toContain('line one\nline two'); + expect(deps.err.join('')).toMatch(/codeman agent ack /); + }); + + it('inbox is a peek and says so', async () => { + const deps = fakeDeps([ok([{ id: OTHER }]), ok({ receiver: false, messages: [MSG], pending: 1, unseen: 1 })]); + expect(await agentInboxRead(deps, { id: OTHER.slice(0, 8) })).toBe(EXIT.ok); + expect(deps.calls[1].path).toBe(`/api/v1/sessions/${OTHER}/inbox`); + expect(deps.err.join('')).toMatch(/peek: nothing was marked seen/); + }); + + it('inbox --wait passes the budget and maps timeout to exit 2, a vanished session to exit 3', async () => { + const timedOut = fakeDeps([ok({ receiver: true, messages: [], pending: 0, unseen: 0, wait: { timedOut: true } })]); + expect(await agentInboxRead(timedOut, { waitMs: 30_000 })).toBe(EXIT.timeout); + expect(timedOut.calls[0]).toMatchObject({ query: { wait: 30_000 }, timeoutMs: 40_000 }); + const ended = fakeDeps([ok({ receiver: true, messages: [], pending: 0, unseen: 0, wait: { ended: true } })]); + expect(await agentInboxRead(ended, { waitMs: 30_000 })).toBe(EXIT.dead); + }); + + it('ack posts the ids to this session’s own inbox', async () => { + const deps = fakeDeps([ok({ removed: 2, pending: 0 })]); + expect(await agentAck(deps, { ids: ['m-1', ' m-2 '] })).toBe(EXIT.ok); + expect(deps.calls[0]).toMatchObject({ + method: 'POST', + path: `/api/v1/sessions/${SELF}/inbox/ack`, + body: { ids: ['m-1', 'm-2'] }, + }); + expect(deps.out.join('')).toMatch(/acked 2 \(0 still pending\)/); + }); + + it('ack refuses without ids and passes a 403 through', async () => { + const none = fakeDeps([]); + expect(await agentAck(none, { ids: [' '] })).toBe(EXIT.refused); + expect(none.calls).toEqual([]); + const forbidden = fakeDeps([apiError(403, 'FORBIDDEN', 'Only x itself can acknowledge its inbox')]); + expect(await agentAck(forbidden, { ids: ['m-1'] })).toBe(EXIT.error); + }); + + it('none of the inbox verbs writes to a pane', async () => { + const deps = fakeDeps((o) => + o.path === '/api/v1/sessions' ? ok([{ id: OTHER }]) : ok({ messages: [], pending: 0, unseen: 0 }) + ); + await agentPost(deps, { id: OTHER, text: 'x' }); + await agentInboxRead(deps, {}); + await agentInboxRead(deps, { id: OTHER, waitMs: 1000 }); + await agentAck(deps, { ids: ['m'] }); + for (const call of deps.calls) expect(call.path).not.toMatch(/\/input$/); + }); +}); + describe('help texts carry the traps the skill documents', () => { const agent = registerAgentCommands(new Command()); const sub = (name: string) => agent.commands.find((c) => c.name() === name)!; diff --git a/test/routes/inbox-routes.test.ts b/test/routes/inbox-routes.test.ts new file mode 100644 index 00000000..b37b3b1f --- /dev/null +++ b/test/routes/inbox-routes.test.ts @@ -0,0 +1,255 @@ +/** + * @fileoverview Agent inbox routes (src/web/routes/inbox-routes.ts) via app.inject(). + * + * The caller is whoever `X-Codeman-Parent-Session` resolves to (live session, same + * owner as the receiver). These tests pin what that decides: who a post is from, + * whether a read marks seen or is a peek, and who may ack. Plus the wait rule (parks + * while nothing is UNSEEN), the full-inbox 409 and the owner-scoped summary. + * + * The routes use the process-wide `agentInbox` and `sessionWaits`; every test resets + * the inbox and cancels its waiters. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; +import { registerInboxRoutes } from '../../src/web/routes/inbox-routes.js'; +import { agentInbox, MAX_MESSAGES_PER_INBOX } from '../../src/web/agent-inbox.js'; +import { sessionWaits } from '../../src/web/session-wait-registry.js'; +import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; +import { httpStatusForErrorCode, type ApiErrorCode, type AuthUser } from '../../src/types.js'; +import { createMockRouteContext, createMockSession, type MockRouteContext } from '../mocks/index.js'; + +const RX = 'aaaaaaaa-1111-4111-8111-111111111111'; +const TX = 'bbbbbbbb-2222-4222-8222-222222222222'; +const OTHER = 'cccccccc-3333-4333-8333-333333333333'; + +interface Harness { + app: FastifyInstance; + ctx: MockRouteContext; +} + +let currentUser: AuthUser | undefined; + +async function createHarness(): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + const ctx = createMockRouteContext({ sessionId: RX }); + for (const id of [TX, OTHER]) ctx.sessions.set(id, createMockSession(id) as never); + app.decorateRequest('authUser', undefined); + app.addHook('onRequest', async (req) => { + req.authUser = currentUser; + }); + registerInboxRoutes(app, ctx as never); + // server.ts's envelope hook: an error envelope carries its 4xx. + app.addHook('preSerialization', (req, reply, payload: unknown, done) => { + const p = payload as { success?: unknown; errorCode?: unknown } | null; + if (p && p.success === false && reply.statusCode === 200 && typeof p.errorCode === 'string') { + reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode)); + } + done(null, payload); + }); + installRouteErrorHandler(app); + await app.ready(); + return { app, ctx }; +} + +const as = (caller?: string) => (caller ? { 'x-codeman-parent-session': caller } : {}); + +describe('agent inbox routes', () => { + let h: Harness; + + const post = (to: string, text: string, caller?: string) => + h.app.inject({ method: 'POST', url: `/api/sessions/${to}/inbox`, payload: { text }, headers: as(caller) }); + const read = (of: string, caller?: string, query = '') => + h.app.inject({ method: 'GET', url: `/api/sessions/${of}/inbox${query}`, headers: as(caller) }); + const ack = (of: string, ids: string[], caller?: string) => + h.app.inject({ method: 'POST', url: `/api/sessions/${of}/inbox/ack`, payload: { ids }, headers: as(caller) }); + const data = (res: { body: string }) => JSON.parse(res.body).data; + + beforeEach(async () => { + agentInbox.resetForTests(); + currentUser = undefined; + delete process.env.CODEMAN_MULTIUSER; + h = await createHarness(); + }); + + afterEach(async () => { + for (const id of [RX, TX, OTHER]) sessionWaits.cancelAll(id); + await h.app.close(); + agentInbox.resetForTests(); + delete process.env.CODEMAN_MULTIUSER; + }); + + describe('from', () => { + it('is the resolved caller session (exact id or 8-char prefix)', async () => { + expect(data(await post(RX, 'hi', TX)).message.from).toBe(TX); + expect(data(await post(RX, 'hi', TX.slice(0, 8))).message.from).toBe(TX); + }); + + it('is null (the user) without a header, for an unknown id, or for a short prefix', async () => { + expect(data(await post(RX, 'a')).message.from).toBeNull(); + expect(data(await post(RX, 'b', 'dddddddd-0000-4000-8000-000000000000')).message.from).toBeNull(); + expect(data(await post(RX, 'c', TX.slice(0, 7))).message.from).toBeNull(); + }); + + it('a body field cannot set it: the schema rejects unknown keys', async () => { + const res = await h.app.inject({ + method: 'POST', + url: `/api/sessions/${RX}/inbox`, + payload: { text: 'x', from: TX }, + }); + expect(res.statusCode).toBe(400); + }); + + it('multi-user: a session of another owner never resolves as the sender', async () => { + process.env.CODEMAN_MULTIUSER = '1'; + currentUser = { username: 'alice', role: 'admin' }; + (h.ctx.sessions.get(RX) as unknown as { owner: string }).owner = 'alice'; + (h.ctx.sessions.get(TX) as unknown as { owner: string }).owner = 'bob'; + (h.ctx.sessions.get(OTHER) as unknown as { owner: string }).owner = 'alice'; + expect(data(await post(RX, 'x', TX)).message.from).toBeNull(); + expect(data(await post(RX, 'y', OTHER)).message.from).toBe(OTHER); + }); + }); + + describe('post', () => { + it('keeps multi-line text as data', async () => { + await post(RX, 'one\ntwo\n', TX); + expect(data(await read(RX, RX)).messages[0].text).toBe('one\ntwo\n'); + }); + + it('a full inbox is 409 CONFLICT with a clear message, and the oldest stays', async () => { + const first = data(await post(RX, 'first')).message.id; + for (let i = 1; i < MAX_MESSAGES_PER_INBOX; i++) agentInbox.post(RX, null, String(i)); + const res = await post(RX, 'one too many'); + expect(res.statusCode).toBe(409); + expect(JSON.parse(res.body)).toMatchObject({ errorCode: 'CONFLICT' }); + expect(JSON.parse(res.body).error).toMatch(/full .*Nothing was stored/); + expect(data(await read(RX)).messages[0].id).toBe(first); + }); + + it('a message over the byte cap is 400', async () => { + expect((await post(RX, 'é'.repeat(8193))).statusCode).toBe(400); + }); + + it('404 for an unknown session', async () => { + expect((await post('nope', 'x')).statusCode).toBe(404); + }); + }); + + describe('read', () => { + it('only the receiver marks seen; a coordinator read is a peek', async () => { + await post(RX, 'task', TX); + const peek = data(await read(RX, TX)); + expect(peek).toMatchObject({ receiver: false, pending: 1, unseen: 1 }); + expect(data(await read(RX)).unseen).toBe(1); + const own = data(await read(RX, RX)); + expect(own).toMatchObject({ receiver: true, pending: 1, unseen: 0 }); + expect(own.messages[0].seen).toBe(false); + expect(data(await read(RX, RX)).messages[0].seen).toBe(true); + }); + + it('?wait= parks while nothing is unseen, even with a seen-but-unacked message', async () => { + await post(RX, 'already seen', TX); + await read(RX, RX); + const parked = read(RX, RX, '?wait=5000'); + await new Promise((r) => setTimeout(r, 30)); + expect(sessionWaits.inboxWaiterCount(RX, true)).toBe(1); + await post(RX, 'new', TX); + const res = data(await parked); + expect(res.wait).toMatchObject({ posted: true, timedOut: false }); + expect(res.messages.map((m: { text: string; seen: boolean }) => [m.text, m.seen])).toEqual([ + ['already seen', true], + ['new', false], + ]); + }); + + it('?wait= answers at once when something is unseen', async () => { + await post(RX, 'm', TX); + const res = data(await read(RX, RX, '?wait=60000')); + expect(res.wait).toMatchObject({ immediate: true, waitedMs: 0, timeoutMs: 60000 }); + }); + + it('?wait= runs out as a 200 with timedOut, clamped to the wait floor', async () => { + const res = await read(RX, RX, '?wait=1'); + expect(res.statusCode).toBe(200); + expect(data(res).wait).toMatchObject({ timedOut: true, timeoutMs: 1000 }); + expect(res.headers['cache-control']).toBe('no-store'); + }); + + it('?wait= rejects junk', async () => { + expect((await read(RX, RX, '?wait=-5')).statusCode).toBe(400); + expect((await read(RX, RX, '?wait=abc')).statusCode).toBe(400); + }); + + it('a parked read on a session that goes away answers ended, with an empty inbox', async () => { + await post(RX, 'x'); + await read(RX, RX); + const parked = read(RX, RX, '?wait=5000'); + await new Promise((r) => setTimeout(r, 30)); + // What server.ts does on delete: drop the inbox, then cancel the waiters. + agentInbox.drop(RX); + sessionWaits.cancelAll(RX); + const res = data(await parked); + expect(res.wait).toMatchObject({ ended: true }); + expect(res.messages).toEqual([]); + }); + }); + + describe('ack', () => { + it('removes by id for the receiver, idempotently', async () => { + const id = data(await post(RX, 'm', TX)).message.id; + expect(data(await ack(RX, [id], RX))).toMatchObject({ removed: 1, pending: 0 }); + const again = await ack(RX, [id, 'never-existed'], RX); + expect(again.statusCode).toBe(200); + expect(data(again)).toMatchObject({ removed: 0, pending: 0 }); + }); + + it('is 403 for anyone but the receiver, and removes nothing', async () => { + const id = data(await post(RX, 'm', TX)).message.id; + for (const caller of [TX, undefined]) { + const res = await ack(RX, [id], caller); + expect(res.statusCode).toBe(403); + expect(JSON.parse(res.body).errorCode).toBe('FORBIDDEN'); + } + expect(agentInbox.pendingCount(RX)).toBe(1); + }); + + it('rejects an empty id list', async () => { + expect((await ack(RX, [], RX)).statusCode).toBe(400); + }); + }); + + describe('summary', () => { + it('reports counts and parked per session, never text', async () => { + await post(RX, 'secret', TX); + await post(TX, 'other secret', RX); + await read(TX, TX); + const parked = read(OTHER, OTHER, '?wait=5000'); + // A coordinator peeking with ?wait= is not the receiver: not "parked". + const peeking = read(RX, TX, '?wait=5000'); + await new Promise((r) => setTimeout(r, 30)); + const res = await h.app.inject({ method: 'GET', url: '/api/agent-inbox/summary' }); + expect(res.body).not.toContain('secret'); + expect(data(res).sessions).toEqual({ + [RX]: { pending: 1, unseen: 1, oldestUnseenAt: expect.any(Number), parked: false }, + [TX]: { pending: 1, unseen: 0, oldestUnseenAt: null, parked: false }, + [OTHER]: { pending: 0, unseen: 0, oldestUnseenAt: null, parked: true }, + }); + sessionWaits.cancelAll(OTHER); + sessionWaits.cancelAll(RX); + await Promise.all([parked, peeking]); + }); + + it('multi-user: lists only sessions the caller can see', async () => { + process.env.CODEMAN_MULTIUSER = '1'; + currentUser = { username: 'alice', role: 'user' }; + (h.ctx.sessions.get(RX) as unknown as { owner: string }).owner = 'alice'; + (h.ctx.sessions.get(TX) as unknown as { owner: string }).owner = 'bob'; + agentInbox.post(RX, null, 'mine'); + agentInbox.post(TX, null, 'bob’s'); + const res = await h.app.inject({ method: 'GET', url: '/api/agent-inbox/summary' }); + expect(Object.keys(data(res).sessions)).toEqual([RX]); + }); + }); +}); diff --git a/test/sensitive-path.test.ts b/test/sensitive-path.test.ts index a9cca5ee..90b61b10 100644 --- a/test/sensitive-path.test.ts +++ b/test/sensitive-path.test.ts @@ -88,6 +88,7 @@ describe('isSensitivePath', () => { ['codeman push keys (VAPID private)', `${HOME}/.codeman/push-keys.json`], ['codeman intent profiles', `${HOME}/.codeman/intents.json`], ['codeman intents on a named instance', `${HOME}/.codeman-beta/intents.json`], + ['codeman agent inbox snapshot', `${HOME}/.codeman/agent-inbox.json`], ]; it.each(blocked)('blocks the %s', (_label, path) => { diff --git a/test/session-wait-registry.test.ts b/test/session-wait-registry.test.ts index 63a7b9ee..4c8ea74a 100644 --- a/test/session-wait-registry.test.ts +++ b/test/session-wait-registry.test.ts @@ -1088,3 +1088,66 @@ describe('SessionWaitRegistry: teardown and leak prevention', () => { expect(reg.totalWaiterCount()).toBe(0); }); }); + +describe('inbox waiters', () => { + it('resolve with posted:true on notifyInbox, and only for that session', async () => { + const reg = new SessionWaitRegistry(); + const a = reg.waitForInbox('a', { timeoutMs: 5_000 }); + const b = reg.waitForInbox('b', { timeoutMs: 50 }); + expect(reg.notifyInbox('a')).toBe(1); + await expect(a).resolves.toMatchObject({ posted: true, timedOut: false, ended: false }); + await expect(b).resolves.toMatchObject({ posted: false, timedOut: true }); + expect(reg.totalWaiterCount()).toBe(0); + }); + + it('count toward the per-session, per-owner and total caps like the other kinds', async () => { + const reg = new SessionWaitRegistry({ maxWaitersPerSession: 2, maxWaitersPerOwner: 3, maxWaitersTotal: 4 }); + const pending = [ + reg.waitForSignal('s', { until: ['stop'], timeoutMs: 5_000, owner: 'alice' }), + reg.waitForInbox('s', { timeoutMs: 5_000, owner: 'alice' }), + ]; + expect(() => reg.assertCapacity('s', 'alice')).toThrow(WaitCapacityError); + expect(() => reg.waitForInbox('s', { timeoutMs: 5_000 })).toThrow(/scope: session/); + pending.push(reg.waitForInbox('t', { timeoutMs: 5_000, owner: 'alice' })); + expect(() => reg.waitForInbox('u', { timeoutMs: 5_000, owner: 'alice' })).toThrow(/scope: owner/); + pending.push(reg.waitForInbox('u', { timeoutMs: 5_000, owner: 'bob' })); + expect(() => reg.waitForInbox('v', { timeoutMs: 5_000 })).toThrow(/scope: total/); + expect(reg.waiterCount('s')).toBe(2); + reg.stop(); + await Promise.all(pending); + expect(reg.ownerWaiterCount('alice')).toBe(0); + }); + + it('counts receiver waiters separately (the summary’s parked)', () => { + const reg = new SessionWaitRegistry(); + void reg.waitForInbox('s', { timeoutMs: 5_000, receiver: false }); + expect(reg.inboxWaiterCount('s')).toBe(1); + expect(reg.inboxWaiterCount('s', true)).toBe(0); + void reg.waitForInbox('s', { timeoutMs: 5_000, receiver: true }); + expect(reg.inboxWaiterCount('s', true)).toBe(1); + reg.stop(); + }); + + it('cancelAll ends them with ended:true, and an abort frees the slot at once', async () => { + const reg = new SessionWaitRegistry(); + const ended = reg.waitForInbox('s', { timeoutMs: 5_000 }); + expect(reg.cancelAll('s')).toBe(1); + await expect(ended).resolves.toMatchObject({ ended: true, aborted: false, posted: false }); + + const controller = new AbortController(); + const aborted = reg.waitForInbox('s', { timeoutMs: 5_000, abortSignal: controller.signal }); + expect(reg.inboxWaiterCount('s')).toBe(1); + controller.abort(); + await expect(aborted).resolves.toMatchObject({ ended: true, aborted: true }); + expect(reg.inboxWaiterCount('s')).toBe(0); + }); + + it('after stop() a new inbox wait resolves at once with ended:true', async () => { + const reg = new SessionWaitRegistry(); + const parked = reg.waitForInbox('s', { timeoutMs: 5_000 }); + reg.stop(); + await expect(parked).resolves.toMatchObject({ ended: true }); + await expect(reg.waitForInbox('s', { timeoutMs: 5_000 })).resolves.toMatchObject({ ended: true, posted: false }); + expect(reg.totalWaiterCount()).toBe(0); + }); +}); diff --git a/test/tab-layout-restore-gate.test.ts b/test/tab-layout-restore-gate.test.ts index 117e29e3..47fcd46d 100644 --- a/test/tab-layout-restore-gate.test.ts +++ b/test/tab-layout-restore-gate.test.ts @@ -45,7 +45,12 @@ describe('tab layout restore gate', () => { }; cleanupStaleSessions(): Promise; finalizeRestoredState(restored: boolean): Promise; + sessions: Map; + store: { getSessions(): Record }; }; + // Read by the agent-inbox prune at the end of a successful restore. + server.sessions = new Map(); + server.store = { getSessions: () => ({}) }; server.tabLayouts = { markRestorationComplete: () => order.push('complete'), markRestorationFailed: () => order.push('failed'), From a1a33cf0502efc8045f4090d7467e7d23903868d Mon Sep 17 00:00:00 2001 From: Randalix Date: Thu, 15 Oct 2026 12:17:45 +0200 Subject: [PATCH 2/4] fix(agent-inbox): review round 1 - codeman agent post: accept text that starts with "-" (markdown from a file), --json and -h stay options. - DELETE /api/sessions/:id on a persisted-only row drops the inbox a detach kept. - inbox --wait: wait.ended is exit 3 only when the session list no longer has the session; a restart or detach ending the wait is exit 2 (wait again). - A read whose client hung up during ?wait= marks nothing seen. - InboxPostSchema bound is 4x the byte cap, so every oversize message gets the store's "post a file path" answer. - Store: restore sorts oldest first; the TTL sweep checks every message, not only the first. - Docs: admins can act as any session of the receiver's owner (now tested for admin and non-admin); a peek with ?wait= does not block; exit codes. - Tests: partial PUT /api/settings keeps persistence on. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/api-reference.md | 14 ++++-- docs/architecture-invariants.md | 4 +- .../codeman/skills/codeman/reference/verbs.md | 8 +++- skills/codeman/reference/verbs.md | 8 +++- src/cli-agent.ts | 30 ++++++++++-- src/web/agent-inbox.ts | 13 ++++-- src/web/routes/inbox-routes.ts | 11 +++-- src/web/routes/session-routes.ts | 4 ++ src/web/schemas.ts | 9 +++- src/web/server.ts | 5 +- test/agent-inbox.test.ts | 16 +++++++ test/cli-agent.test.ts | 46 +++++++++++++++++-- test/routes/inbox-routes.test.ts | 30 +++++++++++- test/routes/session-routes.test.ts | 12 +++++ ...system-routes-settings-partial-put.test.ts | 30 +++++++++++- 15 files changed, 209 insertions(+), 31 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 8ac38050..1c335683 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -591,7 +591,7 @@ request returns. The capture runs synchronous tmux calls on the server; the ## The `codeman agent` CLI (client over these endpoints) -`codeman agent ls|spawn|send|wait|read|interrupt|rm|post|inbox|ack` (`src/cli-agent.ts`) is the command-line client for the endpoints above, for agents the skill does not reach (a pi or shell session, a machine without `jq`). It adds no route: `spawn` is `POST /api/v1/quick-start` (+ `wait-output` on the mode's `capabilities.composerReadyMark` from the CLI registry, where it declares one), `send` is `POST …/input` with `clientId`+`seq` (and `wait`/`waitTimeout` for `--wait` / `--until `; `delivered:false` without `duplicate` and `wait.ended` both exit 3 — the CLI never reports a dead worker as done), `wait` is `GET …/wait` (`--until`) or `GET …/wait-output` (`--match`, `from=buffer` by default), `read` is `GET …/last-response` or `GET …/terminal?tail=`, `interrupt` is `POST …/input` with a bare `\u001b`, `rm` is `DELETE …/sessions/:id`, and the inbox verbs map one to one onto the [agent inbox](#agent-inbox) routes (`post` → `POST …/inbox`, `inbox [id] [--wait]` → `GET …/inbox[?wait=]`, own inbox by default, `ack ` → `POST …/inbox/ack`); `ls` adds an `INBOX` column from `GET /api/v1/agent-inbox/summary` and drops it when that route does not answer. A fire-and-forget `send` to a sleeping wake-on-LAN host reads the route's `buffered` (own line, exit 0) and `dropped` (exit 1: the chunk is gone). An id may be the 8-character form `ls` prints, resolved through `GET /api/v1/sessions`; anything shorter refuses before any request, the same floor as `PARENT_SESSION_ID_MIN_PREFIX`. Every call carries `X-Codeman-Parent-Session`; only `spawn`'s quick-start carries `X-Codeman-Agent-Origin: codeman-agent-cli` (the agent-scratch label must never reach a request that cannot create the case directory). Basic auth comes from `CODEMAN_PASSWORD` or the data dir's `.env`. Server-side error codes are shown verbatim (`INVALID_INPUT: until=stop …` on a hook-less mode is not hidden); exit codes are `0` ok, `1` error, `2` timeout, `3` the session exited, `4` refused by a client-side guard. See the README section "`codeman agent`" for the guards and `test/cli-agent.test.ts` for the pinned behaviour. +`codeman agent ls|spawn|send|wait|read|interrupt|rm|post|inbox|ack` (`src/cli-agent.ts`) is the command-line client for the endpoints above, for agents the skill does not reach (a pi or shell session, a machine without `jq`). It adds no route: `spawn` is `POST /api/v1/quick-start` (+ `wait-output` on the mode's `capabilities.composerReadyMark` from the CLI registry, where it declares one), `send` is `POST …/input` with `clientId`+`seq` (and `wait`/`waitTimeout` for `--wait` / `--until `; `delivered:false` without `duplicate` and `wait.ended` both exit 3 — the CLI never reports a dead worker as done), `wait` is `GET …/wait` (`--until`) or `GET …/wait-output` (`--match`, `from=buffer` by default), `read` is `GET …/last-response` or `GET …/terminal?tail=`, `interrupt` is `POST …/input` with a bare `\u001b`, `rm` is `DELETE …/sessions/:id`, and the inbox verbs map one to one onto the [agent inbox](#agent-inbox) routes (`post` → `POST …/inbox`, `inbox [id] [--wait]` → `GET …/inbox[?wait=]`, own inbox by default, `ack ` → `POST …/inbox/ack`); `ls` adds an `INBOX` column from `GET /api/v1/agent-inbox/summary` and drops it when that route does not answer. A fire-and-forget `send` to a sleeping wake-on-LAN host reads the route's `buffered` (own line, exit 0) and `dropped` (exit 1: the chunk is gone). An id may be the 8-character form `ls` prints, resolved through `GET /api/v1/sessions`; anything shorter refuses before any request, the same floor as `PARENT_SESSION_ID_MIN_PREFIX`. Every call carries `X-Codeman-Parent-Session`; only `spawn`'s quick-start carries `X-Codeman-Agent-Origin: codeman-agent-cli` (the agent-scratch label must never reach a request that cannot create the case directory). Basic auth comes from `CODEMAN_PASSWORD` or the data dir's `.env`. Server-side error codes are shown verbatim (`INVALID_INPUT: until=stop …` on a hook-less mode is not hidden); exit codes are `0` ok, `1` error, `2` timeout, `3` the session exited, `4` refused by a client-side guard (for `inbox --wait`, `3` only when `GET /api/v1/sessions` no longer lists the session; a wait the server ended otherwise, on a restart or a detach, is `2`). `post` takes text that starts with `-` as it is (a markdown file often does); text that is exactly an option (`--json`, `-h`) goes after `--`. See the README section "`codeman agent`" for the guards and `test/cli-agent.test.ts` for the pinned behaviour. `codeman mcp` (`src/mcp-server.ts`) is a Model Context Protocol server on stdio over the same endpoints, for MCP clients: `list_sessions` (`GET /api/v1/sessions`), `start_session` (`POST /api/v1/quick-start` with `X-Codeman-Agent-Origin: codeman-mcp`, then `wait-output` on the mode's ready mark), `send_prompt` (`POST …/input` with `wait`/`waitTimeout`), `wait_for_session` (`GET …/wait` or `…/wait-output`), `read_output` (`GET …/last-response` or `…/terminal?tail=`), `interrupt_session` and `delete_session`. It adds no route. The URL comes from `CODEMAN_API_URL` or `--url`, never a guess; prompting, interrupting and deleting are limited to sessions the process started (or the calling session's own children) unless it runs with `--allow-any-session`. See the README section "MCP server" and `test/mcp-server.test.ts`. @@ -619,7 +619,9 @@ request, with the same resolver as session lineage: a live session the caller can see, with the same owner as the receiving session, matched by exact id or a unique prefix of at least 8 characters. Anything else counts as no session. The body cannot name a sender (`InboxPostSchema` is strict). In multi-user mode -that means a caller cannot claim to be another owner's session. In single-user +that means a caller cannot claim to be another owner's session. Admins are the +exception, as everywhere else: an admin can see every session, so an admin's +request can act as any session of the receiver's owner. In single-user mode every local process holds the same credentials, so the header is a hint the client gives about itself, not proof. @@ -639,7 +641,13 @@ pending does not spin. The value is clamped to the agent-wait bounds like waiter counts against the same per-session, per-owner and total caps (`409 SESSION_BUSY` / `429 RATE_LIMITED`). `wait` has the shape of the other wait results plus `posted` (a post released it). A session deleted during the wait -answers `wait.ended: true` with an empty inbox. +answers `wait.ended: true` with an empty inbox. A detach or a server shutdown +ends the wait with `ended: true` as well, so check the session list before +treating the session as gone (`codeman agent inbox --wait` does: exit 3 only +when the list lacks it, exit 2 otherwise). A client that hangs up during the wait +marks nothing seen. A **peek** with `?wait=` returns at once for as long as the +receiver has unseen mail, since only the receiver's read clears it; to watch +whether a worker has read its mail, poll the summary instead. **Limits.** 200 messages, 16384 bytes (UTF-8) per message, 512 KiB of text per inbox. A message over the per-message cap is `400 INVALID_INPUT`. A post that diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 6cde5a00..b1d062fa 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -694,13 +694,13 @@ Frontend `readmymind-ui.js`: header 🧠 marker-hidden (`btn-readmymind--hidden` ⚠️ **Read until ack.** A read removes nothing; the same messages come back until the receiver acks them BY ID (`POST …/inbox/ack`, idempotent: an id already gone is not an error). Acking by id rather than "everything I read" is what keeps a message posted between the read and the ack from being lost. -⚠️ **Seen and ack belong to the receiver.** The caller is whatever `resolveParentSessionId()` makes of `X-Codeman-Parent-Session` (live session, same owner as the receiver), never a body field (`InboxPostSchema` is strict). Only a read whose caller IS the receiver marks messages seen; anyone else's read is a peek. An ack from anyone else is a 403, not a silent no-op, so a coordinator never believes it cleared a worker's mail. In multi-user mode this stops a caller from naming another owner's session; in single-user mode every local process holds the same credentials, so the header is the client's own claim about itself, which is all the seen/peek split needs. +⚠️ **Seen and ack belong to the receiver.** The caller is whatever `resolveParentSessionId()` makes of `X-Codeman-Parent-Session` (live session, same owner as the receiver), never a body field (`InboxPostSchema` is strict). Only a read whose caller IS the receiver marks messages seen; anyone else's read is a peek. An ack from anyone else is a 403, not a silent no-op, so a coordinator never believes it cleared a worker's mail. In multi-user mode this stops a non-admin caller from naming another owner's session (an admin passes `canAccessOwned` for every session, so an admin can act as any session of the receiver's owner); in single-user mode every local process holds the same credentials, so the header is the client's own claim about itself, which is all the seen/peek split needs. ⚠️ **`?wait=` parks while nothing is UNSEEN**, not while nothing is pending: a receiver holding a seen-but-unacked message would otherwise get an immediate answer on every loop and spin. Inbox waiters are the registry's third waiter kind (`waitForInbox`/`notifyInbox`), so they count against the same per-session/owner/total caps and are ended by `cancelAll` like signal and output waiters. `notifyInbox` runs AFTER the store, so a woken reader finds the message. `parked` in the summary counts receiver waiters only. ⚠️ **Caps refuse, they never drop.** 16 KiB per message (UTF-8 bytes), 512 KiB and 200 messages per inbox; a full inbox is 409 CONFLICT and nothing is stored. Dropping the oldest would lose exactly the message a slow receiver has not handled yet. TTL is 12 h from each message's own `createdAt`, swept lazily, and a restore keeps `createdAt`, so a restart does not restart the clock. -⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. +⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it, and `DELETE /api/sessions/:id` on a row that is no longer live (the persisted-only branch in `session-routes.ts`) drops what the detach kept. A detach also ends a parked read with `ended: true`, which is why `codeman agent inbox --wait` asks the session list before it exits 3. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. ### Voice dictation via Claude diff --git a/plugins/codeman/skills/codeman/reference/verbs.md b/plugins/codeman/skills/codeman/reference/verbs.md index 5896d8b0..65af5c65 100644 --- a/plugins/codeman/skills/codeman/reference/verbs.md +++ b/plugins/codeman/skills/codeman/reference/verbs.md @@ -795,10 +795,14 @@ Rules that matter: changes nothing for it. Only the receiver can ack (403 otherwise). - **`--wait` waits for something new.** It returns at once when there is unseen mail and otherwise blocks until a post arrives; seen but unacked messages come back with it - and do not end the wait. Timeout is exit 2, the session vanishing is exit 3. + and do not end the wait. Timeout is exit 2, and so is a wait the server cut short + (a restart): loop again. Exit 3 means the session list no longer has the session. + `--wait` on someone else's inbox returns at once while they have unseen mail (only + their read clears it); check `ls` instead. - **The sender is resolved, not claimed.** `from` is the session the header resolves to, or `null` (the user). In single-user mode that is a hint, since every local process - holds the same credentials. + holds the same credentials. In multi-user mode it is checked against the owner; + an admin can still act as any session of the receiver's owner. - **Full means refused.** 200 messages or 512 KiB per inbox, 16 KiB per message. A full inbox answers 409 and stores nothing; the oldest is never dropped. Messages expire 12 hours after they were posted. diff --git a/skills/codeman/reference/verbs.md b/skills/codeman/reference/verbs.md index 5896d8b0..65af5c65 100644 --- a/skills/codeman/reference/verbs.md +++ b/skills/codeman/reference/verbs.md @@ -795,10 +795,14 @@ Rules that matter: changes nothing for it. Only the receiver can ack (403 otherwise). - **`--wait` waits for something new.** It returns at once when there is unseen mail and otherwise blocks until a post arrives; seen but unacked messages come back with it - and do not end the wait. Timeout is exit 2, the session vanishing is exit 3. + and do not end the wait. Timeout is exit 2, and so is a wait the server cut short + (a restart): loop again. Exit 3 means the session list no longer has the session. + `--wait` on someone else's inbox returns at once while they have unseen mail (only + their read clears it); check `ls` instead. - **The sender is resolved, not claimed.** `from` is the session the header resolves to, or `null` (the user). In single-user mode that is a hint, since every local process - holds the same credentials. + holds the same credentials. In multi-user mode it is checked against the owner; + an admin can still act as any session of the receiver's owner. - **Full means refused.** 200 messages or 512 KiB per inbox, 16 KiB per message. A full inbox answers 409 and stores nothing; the oldest is never dropped. Messages expire 12 hours after they were posted. diff --git a/src/cli-agent.ts b/src/cli-agent.ts index cf0d76fd..8bd51e96 100644 --- a/src/cli-agent.ts +++ b/src/cli-agent.ts @@ -890,6 +890,17 @@ export interface InboxReadOptions { waitMs?: number; } +/** True only when the session list was read and does not contain `id`. */ +async function sessionIsGone(deps: AgentDeps, id: string): Promise { + try { + const res = await deps.request(deps.ctx, { method: 'GET', path: '/api/v1/sessions' }); + if (!res.json?.success) return false; + return !((res.json.data as SessionRow[] | undefined) ?? []).some((s) => s.id === id); + } catch { + return false; + } +} + /** Relative age for humans: `12s`, `4m`, `3h`. */ function age(createdAt: number, now: number): string { const s = Math.max(0, Math.round((now - createdAt) / 1000)); @@ -901,8 +912,8 @@ function age(createdAt: number, now: number): string { /** * `agent inbox` — read this session's inbox (or peek at another's). Removes nothing: * a message stays until `agent ack `. Only reading your OWN inbox marks messages - * seen. Exit 0 with mail, 2 when a `--wait` ran out with nothing unseen, 3 when the - * session went away during the wait. + * seen. Exit 0 with mail, 2 when a `--wait` ran out with nothing unseen or the server + * cut it short (restart, detach), 3 when the session is gone from the list. */ export async function agentInboxRead(deps: AgentDeps, options: InboxReadOptions): Promise { const target = await resolveSessionId(deps, options.id ?? deps.ctx.selfId); @@ -924,7 +935,12 @@ export async function agentInboxRead(deps: AgentDeps, options: InboxReadOptions) } | undefined; const messages = data?.messages ?? []; - const code = data?.wait?.ended ? EXIT.dead : data?.wait?.timedOut ? EXIT.timeout : EXIT.ok; + // `ended` is also what a server restart or a detach answers. Exit 3 tells a receiver + // loop to stop, so it needs the session list to confirm the session is gone; while + // it is still listed (or the list cannot be had, as during a restart) the wait was + // only cut short, which is exit 2: loop again. + const gone = data?.wait?.ended ? await sessionIsGone(deps, target.id) : false; + const code = gone ? EXIT.dead : data?.wait?.timedOut || data?.wait?.ended ? EXIT.timeout : EXIT.ok; if (deps.json) { emitJson(deps, data ?? {}); return code; @@ -936,8 +952,10 @@ export async function agentInboxRead(deps: AgentDeps, options: InboxReadOptions) deps.io.out(palette.muted(`── ${m.id} from ${sender}, ${age(m.createdAt, now)} ago, `) + tag); deps.io.out(m.text); } - if (data?.wait?.ended) { + if (gone) { deps.io.err(palette.err(`${GLYPH.fail} the session went away during the wait`)); + } else if (data?.wait?.ended) { + deps.io.err(palette.muted('(the server ended the wait early, e.g. a restart; exit 2, wait again)')); } else if (messages.length === 0) { deps.io.err(palette.muted(data?.wait?.timedOut ? '(no mail; wait timed out, exit 2)' : '(no mail)')); } else if (data?.receiver) { @@ -1165,6 +1183,10 @@ export function registerAgentCommands(program: Command): Command { "Leave a message in another session's inbox (ONE quoted argument; multi-line is fine). Nothing is typed into its pane: it reads with `agent inbox`" ) .option('--json', 'Machine-readable output') + // A message is often a file (`"$(cat task.md)"`) and markdown starts with `- ` or + // `---` often enough; without this commander takes it for an unknown option. + // `--json` and `-h` are still options: text that is exactly one goes after `--`. + .allowUnknownOption() .action((id: string, words: string[], options: { json?: boolean }) => run(Boolean(options.json), (deps) => { const message = postTextFromArgs(words); diff --git a/src/web/agent-inbox.ts b/src/web/agent-inbox.ts index 7b5822e2..ae0013db 100644 --- a/src/web/agent-inbox.ts +++ b/src/web/agent-inbox.ts @@ -228,8 +228,12 @@ export class AgentInbox { if (!Array.isArray(raw)) continue; const queue: StoredMessage[] = []; let bytes = 0; - for (const entry of raw) { - if (!isStoredMessage(entry) || entry.createdAt <= cutoff) continue; + // Oldest first whatever order the file has, so the caps keep the oldest mail, + // which is the rule for posts too. + const entries = raw + .filter((entry): entry is StoredMessage => isStoredMessage(entry) && entry.createdAt > cutoff) + .sort((a, b) => a.createdAt - b.createdAt); + for (const entry of entries) { const size = Buffer.byteLength(entry.text, 'utf8'); if (queue.length >= MAX_MESSAGES_PER_INBOX || bytes + size > MAX_INBOX_BYTES) break; queue.push({ ...entry }); @@ -253,8 +257,9 @@ export class AgentInbox { const queue = this.inboxes.get(sessionId); if (!queue) return []; const cutoff = this.now() - MESSAGE_TTL_MS; - if (queue.length === 0 || queue[0].createdAt > cutoff) return queue; - // Oldest first, so the expired ones are a prefix. + // Every message, not only queue[0]: a clock stepping back can put an older + // createdAt behind a newer one. At most 200 entries, so the scan is cheap. + if (!queue.some((m) => m.createdAt <= cutoff)) return queue; const kept = queue.filter((m) => m.createdAt > cutoff); this.store(sessionId, kept); this.onChange?.(); diff --git a/src/web/routes/inbox-routes.ts b/src/web/routes/inbox-routes.ts index 4d19c67e..ff5285e5 100644 --- a/src/web/routes/inbox-routes.ts +++ b/src/web/routes/inbox-routes.ts @@ -8,8 +8,10 @@ * the caller can see that has the same owner as the receiver (exact id, or a unique * prefix of at least 8). Anything else is "no session": a post is from the user, and a * read is a peek. What that buys: in multi-user mode a caller cannot name another - * owner's session. In single-user mode every local process holds the same - * credentials, so the header is a hint the client sets about itself. + * owner's session (an admin, who can see every session, can act as any session of + * the receiver's owner, as admins can everywhere else). In single-user mode every + * local process holds the same credentials, so the header is a hint the client sets + * about itself. * * - `POST /api/sessions/:id/inbox` stores a message. `from` is the resolved caller or * null. A full inbox is 409 CONFLICT; the oldest message is never dropped. @@ -135,8 +137,9 @@ export function registerInboxRoutes(app: FastifyInstance, ctx: SessionPort): voi } // Read AFTER the wait: this is what the caller gets, so it must be current. A wait - // that ended because the session was deleted reads an empty inbox. - const read = agentInbox.read(id, { receiver }); + // that ended because the session was deleted reads an empty inbox. A client that + // hung up never sees this answer, so its read must not mark anything seen. + const read = agentInbox.read(id, { receiver: receiver && !wait?.aborted }); return { success: true, data: { sessionId: id, receiver, ...read, ...(wait ? { wait } : {}) } }; }); diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index dfa81fea..21c9a02f 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -119,6 +119,7 @@ import { abortOnClientHangUp, } from '../route-helpers.js'; import { buildAgentCaseMarker, writeAgentCaseMarker } from '../../agent-case-marker.js'; +import { agentInbox } from '../agent-inbox.js'; import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js'; import { clampEnvOverridesForOwner } from '../../session-env-clamp.js'; import { enabledClis, getCli } from '../../config/cli-registry/registry.js'; @@ -1680,6 +1681,9 @@ export function registerSessionRoutes( // below re-looks-up the record by id, so the returned SessionState is unused here. findPersistedSessionOrFail(ctx.store, id, req); ctx.store.demoteOrRemoveSession(id); + // A detached session kept its agent inbox (server.ts _doCleanupSession drops it + // only on killMux); retiring the row is the delete, so the mail goes now. + agentInbox.drop(id); // Mirrors the broadcast at the tail of the live-session cleanup path // (_doCleanupSession in server.ts) — without it, other open tabs keep // showing the retired row until their next unrelated fetch. diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 2a815163..a240e51e 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -1847,12 +1847,17 @@ export const SessionWaitOutputQuerySchema = z.object({ /** * Body for `POST /api/sessions/:id/inbox`. The byte cap (`MAX_MESSAGE_BYTES`) is the * store's, checked there in UTF-8 bytes; this bound is only the outer fence so a - * multi-megabyte body never reaches it. Multi-line text is fine: it is stored and + * multi-megabyte body never reaches it. It is 4× the byte cap in characters (a UTF-8 + * character is at most 4 bytes), so every over-size message gets the store's answer, + * which tells the sender what to do instead. Multi-line text is fine: it is stored and * handed back as data, never typed. */ export const InboxPostSchema = z .object({ - text: z.string().min(1, 'text must not be empty').max(16_384), + text: z + .string() + .min(1, 'text must not be empty') + .max(4 * 16_384), }) .strict(); diff --git a/src/web/server.ts b/src/web/server.ts index 6b6408bc..bad6b2f6 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1691,8 +1691,9 @@ export class WebServer extends EventEmitter { // detached a few lines above, before `session.stop()`, so on a delete the // session's own exit event never reaches the registry. // The agent inbox goes with a DELETED session only. killMux=false is shutdown or - // detach, and a detached session comes back to the mail posted while it was away. - // Before cancelAll, so a reader released by it reads an empty inbox. + // detach: mail posted before that is still there when the session is re-attached + // (while detached, the routes answer 404, so nothing new arrives). Before + // cancelAll, so a reader released by it reads an empty inbox. if (killMux) agentInbox.drop(sessionId); sessionWaits.notifySignal(sessionId, 'exit'); sessionWaits.cancelAll(sessionId); diff --git a/test/agent-inbox.test.ts b/test/agent-inbox.test.ts index a611a37f..8fa576b5 100644 --- a/test/agent-inbox.test.ts +++ b/test/agent-inbox.test.ts @@ -161,6 +161,15 @@ describe('AgentInbox', () => { expect(new AgentInbox(() => now).restore(snap)).toBe(0); }); + it('sweeps an expired message that is not first (clock stepped back)', () => { + const late = post('posted first, stamped later'); + now -= 1000; // the clock steps back + const early = post('posted second, stamped earlier'); + now += MESSAGE_TTL_MS + 1000 - (late.createdAt - early.createdAt) / 2; + // `early` is past the TTL, `late` is not, and `early` sits behind `late`. + expect(inbox.read(RX, { receiver: false }).messages.map((m) => m.id)).toEqual([late.id]); + }); + it('expiry fires onChange so the snapshot catches up', () => { post('m'); let changes = 0; @@ -248,6 +257,13 @@ describe('AgentInbox', () => { ).toBe(1); }); + it('restores oldest first whatever the file order, so the caps keep the oldest', () => { + const msg = (id: string, createdAt: number) => ({ id, from: null, text: 'x', createdAt, seenAt: null }); + const restored = new AgentInbox(() => now); + restored.restore({ version: 1, inboxes: { s: [msg('new', now - 10), msg('old', now - 20)] } }); + expect(restored.read('s', { receiver: false }).messages.map((m) => m.id)).toEqual(['old', 'new']); + }); + it('holds the count cap on restore', () => { const many = Array.from({ length: MAX_MESSAGES_PER_INBOX + 5 }, (_, i) => ({ id: `m${i}`, diff --git a/test/cli-agent.test.ts b/test/cli-agent.test.ts index 55ad117e..5ec13474 100644 --- a/test/cli-agent.test.ts +++ b/test/cli-agent.test.ts @@ -714,12 +714,32 @@ describe('agent inbox verbs', () => { expect(deps.err.join('')).toMatch(/peek: nothing was marked seen/); }); - it('inbox --wait passes the budget and maps timeout to exit 2, a vanished session to exit 3', async () => { + it('inbox --wait passes the budget and maps timeout to exit 2', async () => { const timedOut = fakeDeps([ok({ receiver: true, messages: [], pending: 0, unseen: 0, wait: { timedOut: true } })]); expect(await agentInboxRead(timedOut, { waitMs: 30_000 })).toBe(EXIT.timeout); expect(timedOut.calls[0]).toMatchObject({ query: { wait: 30_000 }, timeoutMs: 40_000 }); - const ended = fakeDeps([ok({ receiver: true, messages: [], pending: 0, unseen: 0, wait: { ended: true } })]); - expect(await agentInboxRead(ended, { waitMs: 30_000 })).toBe(EXIT.dead); + }); + + describe('a wait the server ended', () => { + const ended = ok({ receiver: true, messages: [], pending: 0, unseen: 0, wait: { ended: true } }); + + it('is exit 3 only when the session list confirms the session is gone', async () => { + const deps = fakeDeps([ended, ok([{ id: OTHER }])]); + expect(await agentInboxRead(deps, { waitMs: 30_000 })).toBe(EXIT.dead); + expect(deps.calls[1]).toMatchObject({ method: 'GET', path: '/api/v1/sessions' }); + expect(deps.err.join('')).toMatch(/went away/); + }); + + it('is exit 2 (wait again) while the session is still listed: a restart or detach', async () => { + const deps = fakeDeps([ended, ok([{ id: SELF }])]); + expect(await agentInboxRead(deps, { waitMs: 30_000 })).toBe(EXIT.timeout); + expect(deps.err.join('')).toMatch(/ended the wait early/); + }); + + it('is exit 2 when the list cannot be had (the server is still coming back)', async () => { + const deps = fakeDeps([ended, apiError(503, 'INTERNAL_ERROR', 'restarting')]); + expect(await agentInboxRead(deps, { waitMs: 30_000 })).toBe(EXIT.timeout); + }); }); it('ack posts the ids to this session’s own inbox', async () => { @@ -767,6 +787,26 @@ describe('help texts carry the traps the skill documents', () => { expect(sub('send').description()).toContain('send -- "- fix the bug"'); }); + it('post takes text that starts with "-" (markdown from a file) without --', async () => { + const program = new Command(); + program.exitOverride(); + const post = registerAgentCommands(program).commands.find((c) => c.name() === 'post')!; + const got: Array<{ words: string[]; json: boolean }> = []; + post.action((_id: string, words: string[], o: { json?: boolean }) => { + got.push({ words, json: Boolean(o.json) }); + }); + for (const text of ['- [ ] item\nnext', '---\ntitle: a', '-v is great']) { + await program.parseAsync(['node', 'codeman', 'agent', 'post', 'abcdefgh', text]); + } + await program.parseAsync(['node', 'codeman', 'agent', 'post', 'abcdefgh', 'hi', '--json']); + expect(got).toEqual([ + { words: ['- [ ] item\nnext'], json: false }, + { words: ['---\ntitle: a'], json: false }, + { words: ['-v is great'], json: false }, + { words: ['hi'], json: true }, + ]); + }); + it('rm does not claim a lineage check it does not make', () => { expect(sub('rm').description()).toMatch(/^Delete any session except this one/); }); diff --git a/test/routes/inbox-routes.test.ts b/test/routes/inbox-routes.test.ts index b37b3b1f..4661ef1e 100644 --- a/test/routes/inbox-routes.test.ts +++ b/test/routes/inbox-routes.test.ts @@ -101,6 +101,28 @@ describe('agent inbox routes', () => { expect(res.statusCode).toBe(400); }); + it('multi-user, non-admin: a session the caller cannot see never resolves as the sender', async () => { + process.env.CODEMAN_MULTIUSER = '1'; + currentUser = { username: 'alice', role: 'user' }; + (h.ctx.sessions.get(RX) as unknown as { owner: string }).owner = 'alice'; + (h.ctx.sessions.get(TX) as unknown as { owner: string }).owner = 'bob'; + (h.ctx.sessions.get(OTHER) as unknown as { owner: string }).owner = 'alice'; + expect(data(await post(RX, 'x', TX)).message.from).toBeNull(); + expect(data(await post(RX, 'y', OTHER)).message.from).toBe(OTHER); + // Nor can alice read as bob's session or ack for it: bob's inbox is a 404 to her. + expect((await read(TX, TX)).statusCode).toBe(404); + expect((await ack(TX, ['m'], TX)).statusCode).toBe(404); + }); + + it('multi-user, admin: may act as any session of the receiver’s owner (admins see everything)', async () => { + process.env.CODEMAN_MULTIUSER = '1'; + currentUser = { username: 'root', role: 'admin' }; + (h.ctx.sessions.get(RX) as unknown as { owner: string }).owner = 'bob'; + (h.ctx.sessions.get(TX) as unknown as { owner: string }).owner = 'bob'; + expect(data(await post(RX, 'x', TX)).message.from).toBe(TX); + expect(data(await read(RX, RX)).receiver).toBe(true); + }); + it('multi-user: a session of another owner never resolves as the sender', async () => { process.env.CODEMAN_MULTIUSER = '1'; currentUser = { username: 'alice', role: 'admin' }; @@ -128,8 +150,12 @@ describe('agent inbox routes', () => { expect(data(await read(RX)).messages[0].id).toBe(first); }); - it('a message over the byte cap is 400', async () => { - expect((await post(RX, 'é'.repeat(8193))).statusCode).toBe(400); + it('a message over the byte cap is 400 with the store’s hint, multi-byte or not', async () => { + for (const text of ['é'.repeat(8193), 'x'.repeat(16_385)]) { + const res = await post(RX, text); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/Write it to a file in the workspace and post the path/); + } }); it('404 for an unknown session', async () => { diff --git a/test/routes/session-routes.test.ts b/test/routes/session-routes.test.ts index 1d6eaf8b..c9777769 100644 --- a/test/routes/session-routes.test.ts +++ b/test/routes/session-routes.test.ts @@ -26,6 +26,7 @@ import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; import { Session } from '../../src/session.js'; +import { agentInbox } from '../../src/web/agent-inbox.js'; // Mock execFile so the send-key route's `tmux` invocation is observable (not run for real). const { execFile } = vi.hoisted(() => ({ execFile: vi.fn() })); @@ -985,6 +986,17 @@ describe('session-routes', () => { expect(harness.ctx.broadcast).toHaveBeenCalledWith('session:deleted', { id: 'ghost-session' }); }); + it('a persisted-only delete drops the agent inbox the detach kept', async () => { + // Detach (killMux=false) keeps the inbox; retiring the row afterwards is the delete. + agentInbox.resetForTests(); + agentInbox.post('ghost-session', null, 'mail from before the detach'); + vi.mocked(harness.ctx.store.getSession).mockReturnValueOnce({ id: 'ghost-session', owner: undefined } as never); + const res = await harness.app.inject({ method: 'DELETE', url: '/api/sessions/ghost-session' }); + expect(res.statusCode).toBe(200); + expect(agentInbox.pendingCount('ghost-session')).toBe(0); + agentInbox.resetForTests(); + }); + it('404s a persisted-only session id the state store does not recognize either', async () => { vi.mocked(harness.ctx.store.getSession).mockReturnValueOnce(null); const res = await harness.app.inject({ diff --git a/test/routes/system-routes-settings-partial-put.test.ts b/test/routes/system-routes-settings-partial-put.test.ts index b6876843..416b1d64 100644 --- a/test/routes/system-routes-settings-partial-put.test.ts +++ b/test/routes/system-routes-settings-partial-put.test.ts @@ -47,13 +47,15 @@ const { EXISTING_SETTINGS, subagentWatcher, imageWatcher, workflowRunWatcher } = }; }; return { - // Persisted settings.json for these tests: two watchers ON, subagent tracking OFF. + // Persisted settings.json for these tests: two watchers ON, subagent tracking OFF, + // agent inbox snapshot ON. EXISTING_SETTINGS: { subagentTrackingEnabled: false, imageWatcherEnabled: true, showUltracodeAgents: true, promptCacheKeepWarm: false, promptCacheKeepWarmMaxHours: 2, + agentInboxPersistEnabled: true, }, subagentWatcher: makeWatcher(false), imageWatcher: makeWatcher(true), @@ -61,6 +63,9 @@ const { EXISTING_SETTINGS, subagentWatcher, imageWatcher, workflowRunWatcher } = }; }); +// setEnabled(false) deletes the snapshot file, so a wrong default here loses mail. +const { agentInboxPersistence } = vi.hoisted(() => ({ agentInboxPersistence: { setEnabled: vi.fn() } })); + vi.mock('node:fs/promises', () => ({ default: { readFile: vi.fn(async () => JSON.stringify(EXISTING_SETTINGS)), @@ -76,6 +81,7 @@ vi.mock('node:fs', async (importOriginal) => { vi.mock('../../src/subagent-watcher.js', () => ({ subagentWatcher })); vi.mock('../../src/image-watcher.js', () => ({ imageWatcher })); vi.mock('../../src/workflow-run-watcher.js', () => ({ workflowRunWatcher })); +vi.mock('../../src/web/agent-inbox-persistence.js', () => ({ agentInboxPersistence })); describe('PUT /api/settings — partial body must not reset service toggles', () => { let harness: RouteTestHarness; @@ -87,6 +93,7 @@ describe('PUT /api/settings — partial body must not reset service toggles', () w.stop.mockClear(); w.__resetRunning(); // running state, not just call records — see makeWatcher } + agentInboxPersistence.setEnabled.mockClear(); }); afterEach(async () => { @@ -167,4 +174,25 @@ describe('PUT /api/settings — partial body must not reset service toggles', () expect((await put({ promptCacheKeepWarm: true })).statusCode).toBe(200); expect(harness.ctx.keepWarm.rescheduleAll).toHaveBeenCalledTimes(2); }); + + it('keeps the agent inbox snapshot on when the body omits its key', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + payload: { showPlanUsageLimits: true }, + }); + expect(res.statusCode).toBe(200); + expect(agentInboxPersistence.setEnabled).toHaveBeenCalledWith(true); + expect(agentInboxPersistence.setEnabled).not.toHaveBeenCalledWith(false); + }); + + it('switches the agent inbox snapshot off when the body says so', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + payload: { agentInboxPersistEnabled: false }, + }); + expect(res.statusCode).toBe(200); + expect(agentInboxPersistence.setEnabled).toHaveBeenCalledWith(false); + }); }); From 9ba24a02dfb9cd8369c204934ae4896d49c4c6b9 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 14:15:49 +0200 Subject: [PATCH 3/4] fix: apply the review items for #690 at merge Boot prune keeps the inboxes of sessions on the reboot-restore offer (their records are already pruned and a rebuild reuses the id), with a case in test/agent-inbox-server-wiring.test.ts. docs/wiki: post, inbox and ack in the codeman agent command block of Driving-Codeman-From-An-Agent, and "Keep agent inbox across restarts" in Settings-Reference. codeman agent inbox prints message text through terminalSafeText (ANSI stripped, other C0, DEL and C1 controls shown as \xHH); --json stays as stored. Shutdown flushes the inbox snapshot again after app.close(), for a post that lands after the first flush. POST /api/cleanup-state drops the inboxes of the records it removes. InboxPostSchema derives its fence from MAX_MESSAGE_BYTES instead of the literal. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/api-reference.md | 3 +- docs/architecture-invariants.md | 2 +- docs/wiki/Driving-Codeman-From-An-Agent.md | 11 +++++++ docs/wiki/Settings-Reference.md | 7 +++++ src/cli-agent.ts | 18 ++++++++++- src/web/routes/system-routes.ts | 4 +++ src/web/schemas.ts | 3 +- src/web/server.ts | 22 ++++++++++--- test/agent-inbox-server-wiring.test.ts | 36 ++++++++++++++++++++-- test/cli-agent.test.ts | 30 ++++++++++++++++++ test/routes/system-routes.test.ts | 24 +++++++++++++++ 11 files changed, 150 insertions(+), 10 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 1c335683..324d4207 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -668,7 +668,8 @@ detached. By default inboxes live in memory and a server restart empties them. The synced setting `agentInboxPersistEnabled` (default off; Settings → Cross-session features) writes them to `agent-inbox.json` in the data dir (mode `0600`, tmp file + rename) and restores them at boot. Inboxes of sessions that did not come back -are pruned only after the session restore succeeded. Switching the setting off +are pruned only after the session restore succeeded; a session on the +reboot-restore offer keeps its inbox, so a rebuild finds its mail. Switching the setting off deletes the file and keeps what is in memory. The file is never read by `/api/search`, and the file viewer refuses it. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index b1d062fa..eea27701 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -700,7 +700,7 @@ Frontend `readmymind-ui.js`: header 🧠 marker-hidden (`btn-readmymind--hidden` ⚠️ **Caps refuse, they never drop.** 16 KiB per message (UTF-8 bytes), 512 KiB and 200 messages per inbox; a full inbox is 409 CONFLICT and nothing is stored. Dropping the oldest would lose exactly the message a slow receiver has not handled yet. TTL is 12 h from each message's own `createdAt`, swept lazily, and a restore keeps `createdAt`, so a restart does not restart the clock. -⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it, and `DELETE /api/sessions/:id` on a row that is no longer live (the persisted-only branch in `session-routes.ts`) drops what the detach kept. A detach also ends a parked read with `ended: true`, which is why `codeman agent inbox --wait` asks the session list before it exits 3. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. +⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it, and `DELETE /api/sessions/:id` on a row that is no longer live (the persisted-only branch in `session-routes.ts`) drops what the detach kept. A detach also ends a parked read with `ended: true`, which is why `codeman agent inbox --wait` asks the session list before it exits 3. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. The keep set is the live sessions, the records `state.json` still holds AND the sessions on the reboot-restore offer (`rebootRestoreRegistry`): `cleanupStaleSessions()` has already pruned an offered session's record by then, and a rebuild reuses its id, so leaving the offer out returned every rebuilt session with an empty inbox (the 12 h TTL bounds what is never rebuilt). `POST /api/cleanup-state` drops the inboxes of the records it removes, since it is a delete for a detached row. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. ### Voice dictation via Claude diff --git a/docs/wiki/Driving-Codeman-From-An-Agent.md b/docs/wiki/Driving-Codeman-From-An-Agent.md index 3d976294..6b2b337f 100644 --- a/docs/wiki/Driving-Codeman-From-An-Agent.md +++ b/docs/wiki/Driving-Codeman-From-An-Agent.md @@ -47,6 +47,9 @@ codeman agent read "$SID" --tail 3000 # terminal tail, ANSI codeman agent send "$SID" 'run the tests, then print WORKDONE followed by _4711' # hook-less modes: the marker in halves … codeman agent wait "$SID" --match WORKDONE_4711 # … and the wait on the joined form codeman agent interrupt "$SID" # a bare ESC, conversation intact +codeman agent post "$SID" 'context for later, not a prompt' # stored in its inbox, not typed into the pane +codeman agent inbox --wait --timeout 300000 # your own inbox: blocks while nothing is new +codeman agent ack # remove what you handled (read never removes) codeman agent rm "$SID" # any session except this one ``` @@ -56,6 +59,14 @@ codeman agent rm "$SID" # any session except t starts with `-` goes after `--`: `codeman agent send "$SID" -- "- fix the bug"`. - **Markers** follow [the split-marker trick](#the-split-marker-trick): the echo of your own prompt is output too, so ask for the marker in halves and wait on the joined form. +- **`post`, `inbox`, `ack`** are the agent inbox: a message for another session that starts + no turn. `post` stores ONE quoted argument (multi-line is fine, so `"$(cat task.md)"` works) + and types nothing into the pane. `inbox` reads this session's mail, or peeks at another's + with an id, and removes nothing; `ack …` removes what you handled. `inbox --wait` + blocks while nothing is new, so mail you read but did not ack does not wake it again. + `ls` shows each session's mail in an INBOX column. Mail lives in memory unless + **Keep agent inbox across restarts** is on (see [Settings Reference](Settings-Reference)), + and goes when its session is deleted. - **Exit codes** are the same for every verb: `0` done, `1` error, `2` timeout, `3` the worker exited, `4` refused. `--json` prints the response's `data`. diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 89e09246..27a2244b 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -89,6 +89,13 @@ way; it also enables the `Ctrl+Shift+G` grid toggle on this device. See This section also holds background-agent tracking, including whether to track agents for every session or only the active tab. +**Keep agent inbox across restarts** (under **Cross-session features**, synced, off by +default) saves the pending `codeman agent post` messages to `agent-inbox.json` in the data +dir, readable by you only, so they survive a server restart. Off, the agent inbox lives in +memory and a restart empties it; switching it off deletes the file and keeps what is in +memory. Messages expire 12 hours after they were posted either way. See +[Driving Codeman From An Agent](Driving-Codeman-From-An-Agent#the-codeman-agent-commands). + ### Appearance | Setting | Notes | diff --git a/src/cli-agent.ts b/src/cli-agent.ts index 8bd51e96..6a5fad47 100644 --- a/src/cli-agent.ts +++ b/src/cli-agent.ts @@ -191,6 +191,22 @@ export function stripAnsi(text: string): string { return stripAnsiSequences(text).replace(/\x1b[()][AB0]/g, ''); } +/** + * Inbox message text for a terminal. Another session wrote it, so an escape sequence + * in it would otherwise reach the reader's terminal (retitle it, or move the cursor + * over the `from` line above it). ANSI sequences go as in `read --tail`, and every + * control character left (C0 other than newline and tab, DEL, C1) is shown as `\xHH`, + * so a bare `\r` or backspace cannot overwrite the header either. A CRLF line end + * stays a line end. `--json` prints the text as stored. + */ +export function terminalSafeText(text: string): string { + return ( + stripAnsi(text.replace(/\r\n/g, '\n')) + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x08\x0b-\x1f\x7f-\x9f]/g, (c) => `\\x${c.charCodeAt(0).toString(16).padStart(2, '0')}`) + ); +} + /** Parse a positive-integer option (`--timeout` ms, `--tail` bytes); the server rejects anything else. */ export function parsePositiveInt(raw: string | undefined, fallback: number, flag = '--timeout'): number { if (raw === undefined) return fallback; @@ -950,7 +966,7 @@ export async function agentInboxRead(deps: AgentDeps, options: InboxReadOptions) const sender = m.from ? m.from.slice(0, 8) : 'user'; const tag = m.seen ? palette.muted('seen') : palette.emph('new'); deps.io.out(palette.muted(`── ${m.id} from ${sender}, ${age(m.createdAt, now)} ago, `) + tag); - deps.io.out(m.text); + deps.io.out(terminalSafeText(m.text)); } if (gone) { deps.io.err(palette.err(`${GLYPH.fail} the session went away during the wait`)); diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 9f90b1cc..45a4d24f 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -65,6 +65,7 @@ import { QR_AUTH_FAILURE_MAX } from '../../config/tunnel-config.js'; import { AUTH_SESSION_TTL_MS } from '../../config/auth-config.js'; import { resolveTerminalHistoryConfig } from '../../config/terminal-history.js'; import { agentInboxPersistence } from '../agent-inbox-persistence.js'; +import { agentInbox } from '../agent-inbox.js'; /** * Defaults for `POST /api/deepseek/install-profile`. @@ -802,6 +803,9 @@ export function registerSystemRoutes( const lifecycleLog = getLifecycleLog(); for (const s of result.cleaned) { lifecycleLog.log({ event: 'stale_cleaned', sessionId: s.id, name: s.name }); + // A detached session kept its agent inbox; removing its record is the delete, + // so the mail goes with it (as on the persisted-only DELETE /api/sessions/:id). + agentInbox.drop(s.id); } return { cleanedSessions: result.count }; }); diff --git a/src/web/schemas.ts b/src/web/schemas.ts index a240e51e..b8db099e 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -23,6 +23,7 @@ import { WEBHOOK_KINDS, WEBHOOK_SCOPES } from '../types/push.js'; import { MIN_MATCH_LENGTH, MAX_MATCH_LENGTH } from '../config/agent-wait.js'; import { MAX_WAKE_MACS } from '../config/remote-wake-limits.js'; import { MAX_INPUT_LENGTH } from '../config/terminal-limits.js'; +import { MAX_MESSAGE_BYTES } from './agent-inbox.js'; import { enabledCliIds, enabledClis } from '../config/cli-registry/registry.js'; import type { SessionMode } from '../types.js'; import { isAdvisorModel } from '../types/session.js'; @@ -1857,7 +1858,7 @@ export const InboxPostSchema = z text: z .string() .min(1, 'text must not be empty') - .max(4 * 16_384), + .max(4 * MAX_MESSAGE_BYTES), }) .strict(); diff --git a/src/web/server.ts b/src/web/server.ts index bad6b2f6..d9b45404 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -4036,8 +4036,15 @@ export class WebServer extends EventEmitter { // Same gate as the tab layouts: only a restore that succeeded may decide which // inboxes are orphans. A failed one returns above and leaves every message alone. // Kept: live sessions plus every record state.json still holds (a session that - // was not re-adopted this boot but is kept for recovery keeps its mail). - const keep = new Set([...this.sessions.keys(), ...Object.keys(this.store.getSessions())]); + // was not re-adopted this boot but is kept for recovery keeps its mail), plus the + // sessions on the reboot-restore offer. `cleanupStaleSessions()` above has already + // pruned their records, and a rebuild reuses the old id, so without them a restored + // session would come back to an empty inbox. The 12 h TTL bounds what is never rebuilt. + const keep = new Set([ + ...this.sessions.keys(), + ...Object.keys(this.store.getSessions()), + ...rebootRestoreRegistry.list(() => true).map((entry) => entry.sessionId), + ]); const pruned = agentInbox.pruneExcept(keep); if (pruned > 0) console.log(`[AgentInbox] dropped ${pruned} inbox(es) of sessions that did not come back`); } @@ -4217,9 +4224,10 @@ export class WebServer extends EventEmitter { approvalInbox.stop(); this.keepWarmService.stop(); // A post within the debounce window before shutdown would otherwise not reach disk. + // Flushed again after `app.close()` below, for a post that lands in between. agentInboxPersistence.flush(); // Same reason as `cancelEverything` above: an in-flight wake is awaited by a request, - // and `app.close()` (the last line of this method) does not abort in-flight requests — + // and `app.close()` (at the end of this method) does not abort in-flight requests, // so without this a restart during a wake waits out the readiness poll. this.remoteWake?.stop(); @@ -4360,7 +4368,13 @@ export class WebServer extends EventEmitter { // Dispose push store (flush pending saves) this.pushStore.dispose(); - await this.app.close(); + try { + await this.app.close(); + } finally { + // The routes answer until `app.close()` returns, so a post after the flush above + // would otherwise sit in an unref'd debounce timer and never reach disk. + agentInboxPersistence.flush(); + } } } diff --git a/test/agent-inbox-server-wiring.test.ts b/test/agent-inbox-server-wiring.test.ts index 44674248..6cb416cd 100644 --- a/test/agent-inbox-server-wiring.test.ts +++ b/test/agent-inbox-server-wiring.test.ts @@ -4,7 +4,9 @@ * - Delete (`killMux=true`) drops the inbox; detach and shutdown (`killMux=false`) * keep it, because a detached session comes back. * - Boot pruning of orphaned inboxes runs only after a successful restore, and keeps - * every session state.json still holds. A failed restore leaves all mail alone. + * every session state.json still holds plus every session on the reboot-restore + * offer (its record is already pruned, and a rebuild reuses the id). A failed + * restore leaves all mail alone. * * Uses a bare `WebServer.prototype` object with just the fields these paths touch, * like tab-layout-restore-gate.test.ts. @@ -12,6 +14,9 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { WebServer } from '../src/web/server.js'; import { agentInbox } from '../src/web/agent-inbox.js'; +import { rebootRestoreRegistry } from '../src/web/reboot-restore-registry.js'; +import type { RebootRestoreEntry } from '../src/reboot-restore.js'; +import type { SessionState } from '../src/types.js'; type CleanupServer = { _doCleanupSession(sessionId: string, killMux: boolean, reason?: string): Promise; @@ -48,9 +53,23 @@ function bareServer(live: string[] = [], persisted: string[] = []): CleanupServe return server; } +/** A reboot-restore offer entry; the prune reads only its `sessionId`. */ +function offerEntry(sessionId: string): RebootRestoreEntry { + return { + sessionId, + workingDir: `/tmp/codeman-agent-inbox-offer/${sessionId}`, + mode: 'claude', + resumeConversationId: `conv-${sessionId}`, + state: { id: sessionId, workingDir: `/tmp/codeman-agent-inbox-offer/${sessionId}` } as SessionState, + }; +} + describe('agent inbox server wiring', () => { beforeEach(() => agentInbox.resetForTests()); - afterEach(() => agentInbox.resetForTests()); + afterEach(() => { + agentInbox.resetForTests(); + rebootRestoreRegistry.reset(); + }); it('a delete drops the inbox; a detach (killMux=false) keeps it', async () => { const server = bareServer(); @@ -77,4 +96,17 @@ describe('agent inbox server wiring', () => { expect(agentInbox.pendingCount('kept-for-recovery')).toBe(1); expect(agentInbox.pendingCount('orphan')).toBe(0); }); + + it('a successful restore keeps the inbox of a session on the reboot-restore offer', async () => { + // After a host reboot the dead session is in neither `this.sessions` nor state.json + // (cleanupStaleSessions already pruned its record), only in the offer, and the + // rebuild reuses its id, so its mail must still be there when it comes back. + const server = bareServer(); + rebootRestoreRegistry.set([offerEntry('on-offer')]); + for (const id of ['on-offer', 'orphan']) agentInbox.post(id, null, 'm'); + await server.finalizeRestoredState(true); + expect(rebootRestoreRegistry.list(() => true).map((e) => e.sessionId)).toEqual(['on-offer']); + expect(agentInbox.pendingCount('on-offer')).toBe(1); + expect(agentInbox.pendingCount('orphan')).toBe(0); + }); }); diff --git a/test/cli-agent.test.ts b/test/cli-agent.test.ts index 5ec13474..c6331aad 100644 --- a/test/cli-agent.test.ts +++ b/test/cli-agent.test.ts @@ -40,6 +40,7 @@ import { registerAgentCommands, resolveAgentContext, stripAnsi, + terminalSafeText, waitExitCode, type AgentContext, type AgentDeps, @@ -707,6 +708,35 @@ describe('agent inbox verbs', () => { expect(deps.err.join('')).toMatch(/codeman agent ack /); }); + it('inbox prints message text with escape sequences and control characters neutralized', async () => { + const hostile = { + ...MSG, + text: 'title \x1b]0;pwned\x07ok\r\nred \x1b[31mtext\x1b[0m\rover\bwrite\ttab\x9b2J\nend', + }; + const deps = fakeDeps([ok({ receiver: true, messages: [hostile], pending: 1, unseen: 0 })]); + expect(await agentInboxRead(deps, {})).toBe(EXIT.ok); + // out[0] is the `── from …` header (styled by the palette), out[1] the text. + expect(deps.out).toHaveLength(2); + expect(deps.out[1]).not.toMatch(/[\x00-\x08\x0b-\x1f\x7f-\x9f]/); + expect(deps.out[1]).toBe('title ok\nred text\\x0dover\\x08write\ttab\\x9b2J\nend'); + }); + + it('terminalSafeText keeps newline and tab and escapes every other control', () => { + expect(terminalSafeText('a\nb\tc')).toBe('a\nb\tc'); + expect(terminalSafeText('a\r\nb')).toBe('a\nb'); + expect(terminalSafeText('\x1b[2J\x1bc\x00\x7f')).toBe('\\x1bc\\x00\\x7f'); + expect(terminalSafeText('ünïcödé ✓')).toBe('ünïcödé ✓'); + }); + + it('inbox --json hands the text out as stored', async () => { + const raw = { ...MSG, text: 'red \x1b[31mtext' }; + const deps = fakeDeps([ok({ receiver: true, messages: [raw], pending: 1, unseen: 0 })], true); + await agentInboxRead(deps, {}); + expect((JSON.parse(deps.out.join('')) as { messages: Array<{ text: string }> }).messages[0].text).toBe( + 'red \x1b[31mtext' + ); + }); + it('inbox is a peek and says so', async () => { const deps = fakeDeps([ok([{ id: OTHER }]), ok({ receiver: false, messages: [MSG], pending: 1, unseen: 1 })]); expect(await agentInboxRead(deps, { id: OTHER.slice(0, 8) })).toBe(EXIT.ok); diff --git a/test/routes/system-routes.test.ts b/test/routes/system-routes.test.ts index 0cd44d47..3ac9baca 100644 --- a/test/routes/system-routes.test.ts +++ b/test/routes/system-routes.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; import { registerSystemRoutes } from '../../src/web/routes/system-routes.js'; +import { agentInbox } from '../../src/web/agent-inbox.js'; // ── Mocks ────────────────────────────────────────────────────────── @@ -343,6 +344,29 @@ describe('system-routes', () => { expect(harness.ctx.store.cleanupSessionsByIds).toHaveBeenCalledWith(new Set()); expect(runStaleSessionCleanup).toHaveBeenCalledOnce(); }); + + it('drops the agent inbox of every record it removes, and only those', async () => { + agentInbox.resetForTests(); + agentInbox.post('stale-detached', null, 'mail for a detached session'); + agentInbox.post('still-kept', null, 'mail for a session that stays'); + harness.ctx.tabLayouts.runStaleSessionCleanup = vi.fn( + async (_activeIds: Set, action: (ids: ReadonlySet) => unknown) => + action(new Set(['stale-detached'])) + ); + vi.mocked(harness.ctx.store.cleanupSessionsByIds).mockReturnValueOnce({ + count: 1, + cleaned: [{ id: 'stale-detached', name: 'w9-gone' }], + }); + try { + const res = await harness.app.inject({ method: 'POST', url: '/api/cleanup-state' }); + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).cleanedSessions).toBe(1); + expect(agentInbox.pendingCount('stale-detached')).toBe(0); + expect(agentInbox.pendingCount('still-kept')).toBe(1); + } finally { + agentInbox.resetForTests(); + } + }); }); // ========== GET /api/subagents ========== From db0742177b70e807fcaf034d5a0b194a3471fbae Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 14:48:05 +0200 Subject: [PATCH 4/4] fix: address the release-prep review of #690 cleanup-state keeps the agent inbox of a removed record whose tmux session the multiplexer still lists (a detached pane the next boot re-adopts under the same id); test and architecture-invariants note added. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/architecture-invariants.md | 2 +- src/web/routes/system-routes.ts | 10 +++++++--- test/routes/system-routes.test.ts | 31 +++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index eea27701..f9dd62d8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -700,7 +700,7 @@ Frontend `readmymind-ui.js`: header 🧠 marker-hidden (`btn-readmymind--hidden` ⚠️ **Caps refuse, they never drop.** 16 KiB per message (UTF-8 bytes), 512 KiB and 200 messages per inbox; a full inbox is 409 CONFLICT and nothing is stored. Dropping the oldest would lose exactly the message a slow receiver has not handled yet. TTL is 12 h from each message's own `createdAt`, swept lazily, and a restore keeps `createdAt`, so a restart does not restart the clock. -⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it, and `DELETE /api/sessions/:id` on a row that is no longer live (the persisted-only branch in `session-routes.ts`) drops what the detach kept. A detach also ends a parked read with `ended: true`, which is why `codeman agent inbox --wait` asks the session list before it exits 3. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. The keep set is the live sessions, the records `state.json` still holds AND the sessions on the reboot-restore offer (`rebootRestoreRegistry`): `cleanupStaleSessions()` has already pruned an offered session's record by then, and a rebuild reuses its id, so leaving the offer out returned every rebuilt session with an empty inbox (the 12 h TTL bounds what is never rebuilt). `POST /api/cleanup-state` drops the inboxes of the records it removes, since it is a delete for a detached row. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. +⚠️ **Lifetime follows the session, not the pane.** `_doCleanupSession` drops the inbox only when `killMux` is set (a real delete); a detach or server shutdown keeps it, and `DELETE /api/sessions/:id` on a row that is no longer live (the persisted-only branch in `session-routes.ts`) drops what the detach kept. A detach also ends a parked read with `ended: true`, which is why `codeman agent inbox --wait` asks the session list before it exits 3. Persistence (`agentInboxPersistEnabled`, SYNCED, default OFF) writes `agent-inbox.json` via `dataPath()`, 0600 through tmp+rename, debounced and flushed on stop; off means no file, and switching it off deletes the snapshot while memory stays. It is never fed to `/api/search`, and `sensitive-path.ts` keeps the file viewer off it. Boot loads the snapshot before `restoreMuxSessions`, but prunes inboxes of sessions that no longer exist only in `finalizeRestoredState`'s SUCCESS path: a failed restore must not delete mail for sessions that are about to come back. The keep set is the live sessions, the records `state.json` still holds AND the sessions on the reboot-restore offer (`rebootRestoreRegistry`): `cleanupStaleSessions()` has already pruned an offered session's record by then, and a rebuild reuses its id, so leaving the offer out returned every rebuilt session with an empty inbox (the 12 h TTL bounds what is never rebuilt). `POST /api/cleanup-state` drops the inboxes of the records it removes, since it is a delete for a detached row, except a record whose tmux session the multiplexer still lists (`mux.getSession`): a detach keeps that pane, and the next boot re-adopts it under the same id even without a `state.json` record. Tests: `test/agent-inbox.test.ts`, `test/agent-inbox-persistence.test.ts`, `test/routes/inbox-routes.test.ts`, `test/agent-inbox-server-wiring.test.ts`, the inbox block in `test/session-wait-registry.test.ts`. ### Voice dictation via Claude diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 45a4d24f..7dc35a47 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -803,9 +803,13 @@ export function registerSystemRoutes( const lifecycleLog = getLifecycleLog(); for (const s of result.cleaned) { lifecycleLog.log({ event: 'stale_cleaned', sessionId: s.id, name: s.name }); - // A detached session kept its agent inbox; removing its record is the delete, - // so the mail goes with it (as on the persisted-only DELETE /api/sessions/:id). - agentInbox.drop(s.id); + // A detached session kept its agent inbox. Its record going is the delete only + // when no tmux session stands behind it: a detached pane the multiplexer still + // lists is re-adopted under the same id at the next boot (restoreMuxSessions, + // with or without a state.json record), and its mail must come back with it. + // Keeping a pane that died while detached errs toward the 12 h TTL and the boot + // prune, the same trade the boot keep set makes. + if (!ctx.mux.getSession(s.id)) agentInbox.drop(s.id); } return { cleanedSessions: result.count }; }); diff --git a/test/routes/system-routes.test.ts b/test/routes/system-routes.test.ts index 3ac9baca..f96ed393 100644 --- a/test/routes/system-routes.test.ts +++ b/test/routes/system-routes.test.ts @@ -367,6 +367,37 @@ describe('system-routes', () => { agentInbox.resetForTests(); } }); + + it('keeps the agent inbox of a removed record whose tmux session the multiplexer still lists', async () => { + // Detach keeps the tmux session and its mux entry; the next boot re-adopts it + // under the same id even without a state.json record, so its mail must survive. + agentInbox.resetForTests(); + agentInbox.post('stale-detached', null, 'mail for a session nothing can bring back'); + agentInbox.post('pane-still-alive', null, 'mail for a detached pane that boot re-adopts'); + harness.ctx.tabLayouts.runStaleSessionCleanup = vi.fn( + async (_activeIds: Set, action: (ids: ReadonlySet) => unknown) => + action(new Set(['stale-detached', 'pane-still-alive'])) + ); + vi.mocked(harness.ctx.store.cleanupSessionsByIds).mockReturnValueOnce({ + count: 2, + cleaned: [ + { id: 'stale-detached', name: 'w9-gone' }, + { id: 'pane-still-alive', name: 'w8-detached' }, + ], + }); + // The harness is rebuilt per test, so this mock does not leak into the next one. + vi.mocked(harness.ctx.mux.getSession).mockImplementation(((id: string) => + id === 'pane-still-alive' ? { sessionId: id } : null) as unknown as typeof harness.ctx.mux.getSession); + try { + const res = await harness.app.inject({ method: 'POST', url: '/api/cleanup-state' }); + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).cleanedSessions).toBe(2); + expect(agentInbox.pendingCount('stale-detached')).toBe(0); + expect(agentInbox.pendingCount('pane-still-alive')).toBe(1); + } finally { + agentInbox.resetForTests(); + } + }); }); // ========== GET /api/subagents ==========