From 4b3eebd0edf7f90685fc7e929efb02a99147ae13 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 27 Oct 2026 01:48:21 +0100 Subject: [PATCH] fix: apply the review items for #708 at merge - test/cli-agent-project-spawn.test.ts:345: the doc-sync test no longer reads .changeset/project-worker-directories.md (changeset version deletes it at release); README, API reference and wiki stay. - CLAUDE.md:132 + src/cli-agent.ts @fileoverview (invariant 5): the directory spawn (POST /api/v1/sessions plus /interactive or /shell, no Agent-Origin header, so no marker) and its refusals (empty --working-dir, bare spawn from a HOME cwd); the invariants now name test/cli-agent-project-spawn.test.ts too. - test/cli-agent-project-spawn.test.ts:322: "allows a cwd below HOME" mocks the cwd to /src/app, so it covers the case the guard must leave alone. - docs/wiki/Driving-Codeman-From-An-Agent.md:59: split the sentence so exit 4 covers only the empty --working-dir; spawn "" reaches quick-start and exits 1. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- docs/wiki/Driving-Codeman-From-An-Agent.md | 7 ++++--- src/cli-agent.ts | 9 ++++++++- test/cli-agent-project-spawn.test.ts | 14 ++++++-------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bd2594ac..e395329b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,7 +129,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Agent wait primitives**: bounded long-polls: `GET /api/sessions/:id/wait`, `GET .../wait-output` (literal substring, **never** regex) and `wait`/`waitTimeout` on `POST .../input`. Registry `session-wait-registry.ts` (pure), bounds `config/agent-wait.ts`. ⚠️ A timeout is a 200 (`wait.timedOut`). ⚠️ `stop` exists for `claude` and `deepseek` ONLY (rule lives in `hooksAvailableForMode()`), `blocked` also for a CLI declaring `capabilities.workDetect.blockedLine` (codex, read off the screen; `blockedSignalAvailable()`): explicit request elsewhere is a 400. ⚠️ Send-and-wait registers the waiter BEFORE the write; teardown must `notifySignal('exit')` BEFORE `cancelAll()`; hangup abort listens on `reply.raw` (guarded by `writableFinished`), never `req.raw`; liveness comes from `isPaneDead`, never `session.pid`. ⚠️ Signals are edge-triggered with no history: gather fan-outs via send-and-wait or `wait-output` markers. Packaged as the `skills/codeman` skill (`codeman skill install`, plugin marketplace, or injection behind `agentSkillEnabled`, SYNCED, default OFF, into the directory each CLI scans: `capabilities.agentSkillDir`, `.claude/skills` for claude, `.agents/skills` for the others, none for pi): injection is add-only, marker-owned (`applyAgentSkill`), refuses symlinks, and refreshes a marker-owned user-level copy (`refreshUserAgentSkill`). → [architecture-invariants#agent-wait-primitives](docs/architecture-invariants.md#agent-wait-primitives), `docs/api-reference.md` -**Agent CLI (`codeman agent …`, `src/cli-agent.ts`)**: the command-line client for the wait primitives and the session verbs, for agents the skill does not reach (pi, shell, no `jq`; `seedAgentSessionPreamble` covers every local session whose CLI takes the skill, `capabilities.agentSkillDir`; every pane still exports `CODEMAN_MUX`/`CODEMAN_SESSION_ID`/`CODEMAN_API_URL`). It is a CLIENT: no new route, no second transport (the plan's "Socket API" non-goal stands), every call carries `X-Codeman-Parent-Session`, only `spawn`'s quick-start carries `X-Codeman-Agent-Origin` (pinned in test/cli-agent.test.ts), auth (`src/codeman-credentials.ts`, shared with `codeman attach` and the TUI) from `CODEMAN_PASSWORD` or the data dir's `.env` (the same reader `codeman attach` uses — `readCodemanEnvFile`). Verbs return exit codes (`0` ok / `1` error / `2` timeout / `3` exited / `4` refused) instead of calling `process.exit`, so `test/cli-agent.test.ts` drives every verb against a recording fake transport plus the real `httpRequest` against a local server. ⚠️ Three guards live in code, not prose, and the tests pin them: refuse without `CODEMAN_MUX=1`+`CODEMAN_API_URL` (never guess a URL); `send` transmits printable text + `\r` only (`inputRefusal`: any byte < 0x20 or DEL refuses BEFORE the transport — opencode's `Ctrl+C` is `app_exit`), ESC exists solely as `interrupt` with no `\r`; `rm` fails closed (`deleteRefusal`: empty id, self shorter than 8, prefix match in EITHER direction, re-checked on the resolved id). ⚠️ The routes accept FULL ids only — an 8-char prefix is a 404 (measured live) — while `ls` prints prefixes, so `resolveSessionId` maps a short id through `GET /api/v1/sessions` and refuses an ambiguous one. It also refuses any id under 8 characters (exit 4, on every verb, before any request), the same floor as the server's `PARENT_SESSION_ID_MIN_PREFIX`, so `rm 9` can never pick an arbitrary session. ⚠️ A 400 for `--until stop` on a hook-less mode is passed through verbatim, never papered over: the marker path is the answer there. `spawn` on a readiness timeout exits 2 and LEAVES the session for inspection (the skill's `spawn_worker` deletes it; the CLI's caller is often a human); its stdout is the id ALONE (prose on stderr) so `SID=$(…)` works. ⚠️ Two server answers that look like success and are not: `delivered:false` without `duplicate` (the bytes went into a dead pane — the field exists so a client says "restart" instead of "wait longer") and `wait.ended:true, signal:null` (the worker died during a `stop`/`idle` wait: the registry satisfies only waiters that listed `exit`, then cancels the rest) — both exit 3, checked BEFORE the happy paths in `waitExitCode`. → README "`codeman agent`", `docs/api-reference.md` +**Agent CLI (`codeman agent …`, `src/cli-agent.ts`)**: the command-line client for the wait primitives and the session verbs, for agents the skill does not reach (pi, shell, no `jq`; `seedAgentSessionPreamble` covers every local session whose CLI takes the skill, `capabilities.agentSkillDir`; every pane still exports `CODEMAN_MUX`/`CODEMAN_SESSION_ID`/`CODEMAN_API_URL`). It is a CLIENT: no new route, no second transport (the plan's "Socket API" non-goal stands), every call carries `X-Codeman-Parent-Session`, only `spawn`'s quick-start carries `X-Codeman-Agent-Origin` (pinned in test/cli-agent.test.ts), while `spawn` without a case (cwd or `--working-dir`) is `POST /api/v1/sessions` plus `/interactive` or `/shell` without that header, so no agent-case marker, and refuses (exit 4, before any request) an empty `--working-dir` and a bare spawn from a HOME cwd, since it always sends an explicit `workingDir` and the server's no-`workingDir` hooks skip cannot apply (test/cli-agent-project-spawn.test.ts), auth (`src/codeman-credentials.ts`, shared with `codeman attach` and the TUI) from `CODEMAN_PASSWORD` or the data dir's `.env` (the same reader `codeman attach` uses — `readCodemanEnvFile`). Verbs return exit codes (`0` ok / `1` error / `2` timeout / `3` exited / `4` refused) instead of calling `process.exit`, so `test/cli-agent.test.ts` drives every verb against a recording fake transport plus the real `httpRequest` against a local server. ⚠️ Three guards live in code, not prose, and the tests pin them: refuse without `CODEMAN_MUX=1`+`CODEMAN_API_URL` (never guess a URL); `send` transmits printable text + `\r` only (`inputRefusal`: any byte < 0x20 or DEL refuses BEFORE the transport — opencode's `Ctrl+C` is `app_exit`), ESC exists solely as `interrupt` with no `\r`; `rm` fails closed (`deleteRefusal`: empty id, self shorter than 8, prefix match in EITHER direction, re-checked on the resolved id). ⚠️ The routes accept FULL ids only — an 8-char prefix is a 404 (measured live) — while `ls` prints prefixes, so `resolveSessionId` maps a short id through `GET /api/v1/sessions` and refuses an ambiguous one. It also refuses any id under 8 characters (exit 4, on every verb, before any request), the same floor as the server's `PARENT_SESSION_ID_MIN_PREFIX`, so `rm 9` can never pick an arbitrary session. ⚠️ A 400 for `--until stop` on a hook-less mode is passed through verbatim, never papered over: the marker path is the answer there. `spawn` on a readiness timeout exits 2 and LEAVES the session for inspection (the skill's `spawn_worker` deletes it; the CLI's caller is often a human); its stdout is the id ALONE (prose on stderr) so `SID=$(…)` works. ⚠️ Two server answers that look like success and are not: `delivered:false` without `duplicate` (the bytes went into a dead pane — the field exists so a client says "restart" instead of "wait longer") and `wait.ended:true, signal:null` (the worker died during a `stop`/`idle` wait: the registry satisfies only waiters that listed `exit`, then cancels the rest) — both exit 3, checked BEFORE the happy paths in `waitExitCode`. → README "`codeman agent`", `docs/api-reference.md` **MCP server (`codeman mcp`, `src/mcp-server.ts`)**: a Model Context Protocol server on stdio for any MCP client, a CLIENT of the HTTP API like `codeman agent` (reuses its `httpRequest`, `inputRefusal`, `buildSendBody`; no new route). Seven tools: `list_sessions`, `start_session`, `send_prompt`, `wait_for_session`, `read_output`, `interrupt_session`, `delete_session`. ⚠️ Dual-era: answers the legacy `initialize` handshake (2024-11-05 to 2025-11-25) AND the 2026-07-28 revision (per-request `_meta` with `protocolVersion` + `clientCapabilities`, `server/discover`, `resultType: "complete"` on every modern result); a modern `tools/list`/`server/discover` MUST carry `ttlMs` + `cacheScope` (Claude Code 2.1.296 drops every tool without them: measured). ⚠️ stdout is protocol only; logs go to stderr. ⚠️ The URL comes from `--url`/`CODEMAN_API_URL`, never a guess, and unconfigured it still lists tools (registries introspect) while every call answers how to configure it. ⚠️ Prompt/interrupt/delete only sessions the PROCESS started (in-memory allowlist, so a restart fails closed) or, inside a pane, the calling session's own children, unless `--allow-any-session`; never its own session. ⚠️ `read_output` returns text ONLY, no `structuredContent` (a client that prefers it showed the model metadata instead of the output: measured). Waits default 45 s (Codex's MCP tool timeout is 60 s). Registry manifest `config/mcp-registry/server.json` (`mcpName` in package.json, version kept by `scripts/sync-plugin.mjs`). Tests: `test/mcp-server.test.ts`. diff --git a/docs/wiki/Driving-Codeman-From-An-Agent.md b/docs/wiki/Driving-Codeman-From-An-Agent.md index bab92f3f..f9fa407f 100644 --- a/docs/wiki/Driving-Codeman-From-An-Agent.md +++ b/docs/wiki/Driving-Codeman-From-An-Agent.md @@ -56,9 +56,10 @@ codeman agent rm "$SID" # any session except t - **`spawn [case]`** uses the current directory when no case is supplied. Use `codeman agent spawn --working-dir /path/to/project --mode codex` for another existing - local directory. A case and `--working-dir` cannot be combined, and an empty case - name (`spawn "$CASE"` with `CASE` unset) is refused rather than read as "no case", - as is an empty `--working-dir` (exit 4, before any request), so it never falls back to cwd. + local directory. A case and `--working-dir` cannot be combined. An empty case name + (`spawn "$CASE"` with `CASE` unset) is never read as "no case": it goes to quick-start, + which refuses it (exit 1). An empty `--working-dir` is refused before any request + (exit 4), so it never falls back to cwd. A cwd that is your home directory is refused the same way, because Codeman's hooks would land in `~/.claude`; pass `--working-dir ~` to start a worker there on purpose. Explicit case names retain quick-start behavior (creating a scratch case if missing). diff --git a/src/cli-agent.ts b/src/cli-agent.ts index ca6155cc..cb9b8b01 100644 --- a/src/cli-agent.ts +++ b/src/cli-agent.ts @@ -13,7 +13,8 @@ * with tests, so a `codex`/`opencode`/`pi` agent — which never gets the claude-only * preamble — has the same verbs from one line in its AGENTS.md. * - * Invariants (each asserted in `test/cli-agent.test.ts`): + * Invariants (each asserted in `test/cli-agent.test.ts` or + * `test/cli-agent-project-spawn.test.ts`): * 1. Refuses outside a Codeman session (`CODEMAN_MUX=1` + `CODEMAN_API_URL`); it * never guesses a URL — a server you are not part of is not yours to drive. * 2. `send` transmits printable text plus `\r` only. ESC exists solely as @@ -25,6 +26,12 @@ * 4. An id shorter than 8 characters refuses (exit 4) before any request, on every * verb. `9` would resolve to whichever session is alone with that first * character, the user's own interactive tab included. + * 5. `spawn` without a case starts the worker in an existing directory (cwd, or + * `--working-dir`) via `POST sessions` + `interactive|shell`, without the + * `X-Codeman-Agent-Origin` header, so no agent-case marker is written. It refuses + * (exit 4, before any request) an empty `--working-dir` and a bare spawn from a + * HOME cwd: it always sends an explicit `workingDir`, so the server's skip of + * workspace hooks for a request that names none cannot protect `~/.claude`. * * Commands live here as functions returning an exit code, not calling * `process.exit`, so the whole surface is unit-testable against a fake server. diff --git a/test/cli-agent-project-spawn.test.ts b/test/cli-agent-project-spawn.test.ts index 4f76aac2..68f8f767 100644 --- a/test/cli-agent-project-spawn.test.ts +++ b/test/cli-agent-project-spawn.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'; import http from 'node:http'; import { tmpdir } from 'node:os'; import { readFileSync } from 'node:fs'; @@ -321,10 +321,13 @@ describe('agent spawn never treats an implicit HOME cwd as a project', () => { }); it('allows a cwd below HOME', async () => { const home = await homeDir(); + const project = join(home, 'src', 'app'); + await mkdir(project, { recursive: true }); vi.stubEnv('HOME', home); + vi.spyOn(process, 'cwd').mockReturnValue(project); const t = deps([ok({ session: { id: OTHER } }), ok({})]); expect(await agentSpawn(t.d, base)).toBe(EXIT.ok); - expect(t.calls[0].body).toMatchObject({ workingDir: process.cwd() }); + expect(t.calls[0].body).toMatchObject({ workingDir: project }); }); it('never throws when HOME cannot be resolved, and compares the resolved paths instead', async () => { const missing = join(await homeDir(), 'absent'); @@ -340,12 +343,7 @@ describe('the directory-spawn --json shape is documented the same way everywhere // The wiki wraps lines, so whitespace is normalized before comparing. const clause = '`{ sessionId, workingDir, created: true }`, plus `ready` when a readiness wait ran, `readinessError` when that call failed, and `startupError` when the start failed'; - for (const file of [ - 'README.md', - 'docs/api-reference.md', - 'docs/wiki/Driving-Codeman-From-An-Agent.md', - '.changeset/project-worker-directories.md', - ]) { + for (const file of ['README.md', 'docs/api-reference.md', 'docs/wiki/Driving-Codeman-From-An-Agent.md']) { it(file, () => { expect(readFileSync(new URL(`../${file}`, import.meta.url), 'utf8').replace(/\s+/g, ' ')).toContain(clause); });