diff --git a/CLAUDE.md b/CLAUDE.md index 5681d688..b58005f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -182,7 +182,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Input**: `session.writeViaMux()` for programmatic/curl input via tmux `send-keys -l` + `send-keys Enter`, single-line only. Interactive **browser** input goes through a durable **exactly-once** layer: a stable `clientId` + monotonic per-session `seq` persisted to localStorage until the server ACKs, so a dropped link cannot lose or double-deliver a prompt. `ws-connection-registry.ts` supersedes only same-TAB reconnects, so two tabs on one session coexist. → [architecture-invariants#input-delivery-and-ws-resilience](docs/architecture-invariants.md#input-delivery-and-ws-resilience) -**Agent wait primitives**: bounded long-polls so an agent driving Codeman from a shell can block instead of poll: `GET /api/sessions/:id/wait` (lifecycle signal), `GET /api/sessions/:id/wait-output` (literal substring, **never** regex) and `wait`/`waitTimeout` on `POST /api/sessions/:id/input`. Registry in `session-wait-registry.ts` (pure, no `Session` reference), bounds in `config/agent-wait.ts`. ⚠️ **A timeout is a 200** (`wait.timedOut`), never an error, so callers loop over short waits. ⚠️ `stop`/`blocked` come from Claude Code hooks and therefore fire for **`claude` mode ONLY** (`shell` installs none either); asking for one explicitly on another mode is a 400, the default set silently drops them. ⚠️ Send-and-wait registers the waiter BEFORE the write (a separate POST-then-wait races and reports the PREVIOUS turn), and both teardown paths must `notifySignal('exit')` BEFORE `cancelAll()`. ⚠️ Client-hangup abort listens on **`reply.raw`** guarded by `writableFinished`: on `req.raw`, `close` fires when the request BODY ends, which on a POST killed every send-and-wait instantly and no `app.inject()` test could see it. ⚠️ Worker liveness cannot come from `session.pid` — for a tmux session that is the local attach client, which outlives a worker dying inside its pane — so it is probed at the mux layer (`isPaneDead`, ~750 ms cache) on blocking waits only, never on the input hot path. ⚠️ Signals are edge-triggered with no history: one that fires with no waiter registered is unobservable afterwards, so gather fan-outs with send-and-wait or latched `wait-output` markers, never fire-and-forget-then-sequential-signal-waits. → [architecture-invariants#agent-wait-primitives](docs/architecture-invariants.md#agent-wait-primitives), `docs/api-reference.md` +**Agent wait primitives**: bounded long-polls so an agent driving Codeman from a shell can block instead of poll: `GET /api/sessions/:id/wait` (lifecycle signal), `GET /api/sessions/:id/wait-output` (literal substring, **never** regex) and `wait`/`waitTimeout` on `POST /api/sessions/:id/input`. Registry in `session-wait-registry.ts` (pure, no `Session` reference), bounds in `config/agent-wait.ts`. ⚠️ **A timeout is a 200** (`wait.timedOut`), never an error, so callers loop over short waits. ⚠️ `stop`/`blocked` come from Claude Code hooks and therefore fire for **`claude` mode ONLY** (`shell` installs none either); asking for one explicitly on another mode is a 400, the default set silently drops them. ⚠️ Send-and-wait registers the waiter BEFORE the write (a separate POST-then-wait races and reports the PREVIOUS turn), and both teardown paths must `notifySignal('exit')` BEFORE `cancelAll()`. ⚠️ Client-hangup abort listens on **`reply.raw`** guarded by `writableFinished`: on `req.raw`, `close` fires when the request BODY ends, which on a POST killed every send-and-wait instantly and no `app.inject()` test could see it. ⚠️ Worker liveness cannot come from `session.pid` — for a tmux session that is the local attach client, which outlives a worker dying inside its pane — so it is probed at the mux layer (`isPaneDead`, ~750 ms cache) on blocking waits only, never on the input hot path. ⚠️ Signals are edge-triggered with no history: one that fires with no waiter registered is unobservable afterwards, so gather fan-outs with send-and-wait or latched `wait-output` markers, never fire-and-forget-then-sequential-signal-waits. The primitives are packaged as the **`skills/codeman` agent skill**: installable via `codeman skill install [--case ]` / `skill uninstall`, or auto-injected into a case's `.claude/skills/` on Claude session create behind `agentSkillEnabled` (SYNCED, default OFF). Injection is ADD-ONLY at create, marker-owned (`applyAgentSkill` in `hooks-config.ts` never touches an unmarked user copy) and refuses symlinks (this repo's own `.claude/skills/codeman` is a symlink to the source, which the injector must never write through). → [architecture-invariants#agent-wait-primitives](docs/architecture-invariants.md#agent-wait-primitives), `docs/api-reference.md` **Idle detection**: Multi-layer (completion message → AI check → output silence → token stability). See `docs/respawn-state-machine.md`. diff --git a/README.md b/README.md index 9082f347..8bed0368 100644 --- a/README.md +++ b/README.md @@ -691,6 +691,18 @@ Single-digit selection (1-9), color-coded status, token counts, auto-refresh. De For AI agents and automation that control Codeman without a browser: an agent that spins up worker sessions, a CI bot, or **Claude Code running _inside_ a Codeman session orchestrating other sessions**. Everything the UI does is HTTP + a CLI, so an agent can do it too. +> **Shortcut: install the packaged agent skill.** Everything below (plus worked multi-worker recipes) ships as a Claude Code skill in [`skills/codeman`](skills/codeman/SKILL.md), so an agent inside a session can drive Codeman without you pasting docs into the prompt. Three ways to get it: +> +> - `npx skills add Ark0N/Codeman --skill codeman -g`: global, works for any skills-aware agent +> - `codeman skill install` (global) or `codeman skill install --case `: for npm installs that never cloned the repo; `codeman skill uninstall` reverses it +> - **App Settings → Agent Skill** (`agentSkillEnabled`, default off): Codeman then injects the skill into each case on Claude session create; a user-authored `skills/codeman` in the case is never overwritten +> +> A global install (`codeman skill install`, or `npx skills add`) is picked up by **every new Claude Code session on the machine**, inside Codeman or not. The skill self-gates: outside a Codeman session (`CODEMAN_MUX` unset) it refuses to act, so a global install costs an idle session nothing. +> +> ⚠️ Turning `agentSkillEnabled` back off **does not remove already-injected copies** (a create-time sweep would yank the skill out from under other live sessions sharing that `.claude/` dir). Remove them per case with `codeman skill uninstall --case `. + + + ### Detect that you're inside Codeman When a CLI runs in a Codeman-managed session, these environment variables are set — read them instead of hardcoding anything: diff --git a/docs/agent-control-plan.md b/docs/agent-control-plan.md index 1e1ae474..0ccdc00d 100644 --- a/docs/agent-control-plan.md +++ b/docs/agent-control-plan.md @@ -1,8 +1,9 @@ # Agent Control Plan: skill packaging + wait primitives -**Status**: steps 1 to 5 IMPLEMENTED and multi-round verified, uncommitted as of 2026-08-08. -Step 6 (CLI install command + per-case injection + `agentSkillEnabled`) is not built. -See [§7 Build log](#7-build-log-what-actually-happened) for what shipped, what each +**Status**: steps 1 to 6 IMPLEMENTED, uncommitted as of 2026-08-09. Steps 1 to 5 were +multi-round verified on 2026-08-08; step 6 (CLI install command + per-case injection + +`agentSkillEnabled`) was built 2026-08-09; see the step-6 entry at the end of +[§7 Build log](#7-build-log-what-actually-happened) for what shipped, what each verification round found, and what is still open. **Date**: 2026-08-08 @@ -522,8 +523,8 @@ Bundled manifests plus local override only, no network. | 2 ✅ | `GET .../wait` + wiring in listener-wiring, hook-event-routes, server teardown | 15 route tests green; live-verified on an isolated `CODEMAN_INSTANCE=waittest` instance (immediate resolve, 400 on a bad signal, 200+`timedOut` on timeout, hook `stop` and `permission_prompt`→`blocked` waking an in-flight wait, delete delivering `exit`, SIGTERM not blocked); full `test:ci` sweep green | | 3 ✅ | `GET .../wait-output` | 16 route tests green; live-verified on real PTY bytes (`echo MARKER` waking a blocked request in ~1s, `from=buffer` immediate hit, never-seen marker timing out at exactly 2001ms, nocase, `regex` refused with a 400); full `test:ci` sweep green | | 4 ✅ | `wait` field on `POST .../input`, non-wait path proven unchanged | 16 route tests green; live-verified (no-wait returns in 26ms with the historical bare body; an idle session did NOT satisfy a `wait` request, blocking the full 2001ms, which is the race the endpoint exists to close; the stop hook resolved a send-and-wait at 1510ms and the input was confirmed in the tmux pane; `wait:null` accepted) | -| 5 | `skills/codeman/SKILL.md` + reference files + `.claude/skills` symlink | live dogfood: a real session orchestrates a worker end to end | -| 6 | `codeman skill install` CLI + `applyAgentSkill()` + `agentSkillEnabled` setting | settings partial-PUT test, case-creation test | +| 5 ✅ | `skills/codeman/SKILL.md` + reference files + `.claude/skills` symlink | live dogfood: a real session orchestrates a worker end to end | +| 6 ✅ | `codeman skill install` CLI + `applyAgentSkill()` + `agentSkillEnabled` setting | 10 unit tests (`test/agent-skill.test.ts`) + real-server case-creation tests (`test/quick-start.test.ts`, incl. the settings PUT accepting the key) green; CLI verified live (install/uninstall, global + `--case`, foreign/symlink refusals) | | 7 | Docs: api-reference, extending-codeman, README | | | 8 | COM (minor bump: new endpoints, new setting, new optional fields) | both CI and Release workflows green | @@ -532,12 +533,14 @@ without the wait endpoints, so the wait work goes first. ## 6. Open questions for the owner -1. `skills/` at the repo root, accepted despite the short-root rule? (Recommended yes, the - install one-liner depends on it.) -2. `agentSkillEnabled` default: OFF for the first release then flip, or ON immediately? -3. Auto-inject the skill into every case's `.claude/skills/`, or global install only? +1. ✅ `skills/` at the repo root: accepted (built that way; the install one-liner depends on it). +2. ✅ `agentSkillEnabled` default: **OFF** for the first release, per §2.2's rationale (skills + cost context on every turn; measure before defaulting on). Flip later if dogfooding earns it. +3. ✅ Both: global install via `npx skills add` / `codeman skill install`, AND per-case + auto-injection behind the (default-off) setting. Injection is add-only at session create and + marker-guarded, so a user-authored copy is never touched. 4. Is `X-Codeman-Caller-Session` self-protection worth the 10 lines, given it is a footgun guard - and not a security boundary? + and not a security boundary? (Still open, not built with step 6.) 5. Regex support in `wait-output`: confirm literal-only for v1. --- @@ -654,9 +657,47 @@ success without running its task. Two traps recurred often enough to name: - **Release checklist**: `package.json` `files` includes `skills`, which is still untracked. `git add skills/` must be part of the release commit, or npm publishes a tarball without the skill (a `files` entry that does not exist is silently - ignored, so nothing fails). + ignored, so nothing fails). `test/agent-skill.test.ts` reads the packaged source, + so CI at least fails loudly if the directory goes missing from a checkout. - The 1.13.0 changeset is written under `.changeset/`; consuming it (COM flow), the release commit, and the deploy remain. - Deferred with Part 3: the latched last-signal-per-turn. Nice-to-haves from the reviews: N2 (create the death-watcher inside its `try`) and converting timeout-shaped test detections into fast assertions. +- §2.4's `X-Codeman-Caller-Session` footgun guard: still not built (open question 4). + +### Step 6 (2026-08-09): install command, per-case injection, the setting + +Built to the §2.6 file list, mirroring the statusLine mechanism throughout: + +| Piece | Where | +| ----- | ----- | +| `applyAgentSkill(casePath, enabled)` + `installAgentSkillInto` / `removeAgentSkillFrom` | `src/hooks-config.ts` | +| `codeman skill install` / `skill uninstall` (`--global` default, `--case `) | `src/cli.ts` | +| `agentSkillEnabled` (SYNCED, default OFF) | `schemas.ts` (`SettingsUpdateSchema`), `getAgentSkillEnabled()` on `ConfigPort`/`server.ts`, checkbox in `index.html` + `settings-ui.js` | +| Injection call sites (Claude mode only) | `POST /api/sessions` next to `refreshStaleCodemanHooks`; `POST /api/quick-start` after the case-create/self-heal blocks (local + docker cases; remote skipped, its path lives on another host) | +| Tests | `test/agent-skill.test.ts` (10 unit), `test/quick-start.test.ts` (real server: default-off, PUT accepts key, injection on create, shell-mode skipped) | + +Decisions worth keeping: + +- **Ownership marker, prefix-matched.** The injected SKILL.md ends with + ``; install/refresh/remove all refuse a copy + without the marker (a user's own skill) and match on the PREFIX so a wording change + cannot disown older injected copies (the `BACKGROUND_WAKE_MARKER_PREFIX` pattern). +- **Symlink refusal.** This repo's own dogfooding layout + (`.claude/skills/codeman -> ../../skills/codeman`) means the injector must `lstat` + the skill dir AND its `skills/` parent and bail on a symlink, or enabling the + setting in the Codeman repo itself would overwrite the skill source through the link. +- **ADD-ONLY at session create**, same shared-`.claude` rationale as the statusLine: + a create while the setting is off must not yank the skill out from under other live + sessions in the repo. The remove path exists (CLI `skill uninstall`, tests); no + automatic sweep removes on toggle-off. +- **Removal is manifest-based, never `rm -rf`**: only files the packaged source would + have written are deleted, directories are pruned bottom-up only if they emptied, so + a user's extra notes in `reference/` survive an uninstall. +- **Source resolution**: `join(moduleDir, '..', 'skills', 'codeman')` works from + `src/` (tsx), `dist/` (tsc build), and the npm tarball alike, because all three sit + one level below the package root and `files` ships `skills/`. +- **Nothing acts on the setting at PUT time**: injection reads the merged persisted + settings at session create (`readSettings`, ~2s cache), so the partial-PUT invariant + (`toggleService` reading `merged`) is untouched by construction. diff --git a/docs/extending-codeman.md b/docs/extending-codeman.md index c68a2f46..3f0b56ba 100644 --- a/docs/extending-codeman.md +++ b/docs/extending-codeman.md @@ -160,6 +160,13 @@ Around 200 handlers across 21 route files cover sessions, cases, files, cron, respawn, Ralph, the orchestrator, search, and admin. Each route module carries an `@fileoverview` describing its endpoints. +If the caller is an agent running _inside_ a Codeman session, install the packaged +agent skill instead of teaching it these calls by hand: `skills/codeman` in the repo +(`npx skills add Ark0N/Codeman --skill codeman -g`, or `codeman skill install +[--case ]`, or the synced `agentSkillEnabled` App Setting for automatic +per-case injection on Claude session create). The skill carries the guard, the +safety rules, and verified wait/orchestration recipes. + The common ones: ```bash diff --git a/skills/codeman/SKILL.md b/skills/codeman/SKILL.md index 7ff25f68..f4d3a995 100644 --- a/skills/codeman/SKILL.md +++ b/skills/codeman/SKILL.md @@ -17,7 +17,24 @@ sessions. Every recipe below was verified live. Full endpoint tables and troubleshooting: [reference/endpoints.md](reference/endpoints.md). Worked multi-worker flows: [reference/recipes.md](reference/recipes.md). -## 0. Guard — run this before anything else +## 0. Guard, and the one thing that breaks every recipe below + +⚠️ **Your shell state does not survive between tool calls.** Each Bash call starts a +fresh shell, so `$API`, `$SELF`, the `CURL` array and `delete_session` are all gone by +the next call, and `$$` is a different pid. Three consequences, all of which have +teeth: + +- **Re-run this entire preamble at the top of every Bash call that touches the API.** + Running it once and assuming it stuck is the single most likely way to break a run. +- **Never re-paste only half of it.** The delete guard below is written so that a + missing definition deletes nothing, but that only holds if you never hand-roll a + `DELETE` of your own. +- **Never put `$$` in a `clientId`.** It changes per call, so the "resend the identical + request" loop in §3 would stop being a duplicate and would **retype the prompt**, + submitting the turn twice. Use a fixed literal (`codeman-agent-1` below). + +Only real environment variables (`CODEMAN_*`) survive, which is why this preamble +rebuilds everything else from them. ```bash test "${CODEMAN_MUX:-}" = 1 || { echo "Not inside a Codeman-managed session; refusing to act."; exit 1; } @@ -39,13 +56,35 @@ if [ -z "${CODEMAN_PASSWORD:-}" ]; then # stock installs: install.sh puts it UNIT="$HOME/.config/systemd/user/codeman-web.service" PLIST="$HOME/Library/LaunchAgents/com.codeman.web.plist" if [ -f "$UNIT" ]; then - CODEMAN_PASSWORD=$(sed -n 's/^Environment="CODEMAN_PASSWORD=\(.*\)"$/\1/p' "$UNIT" | head -1) + # install.sh backslash-escapes " and \ in the unit value; undo it or a password + # containing either recovers wrong and auth fails. + CODEMAN_PASSWORD=$(sed -n 's/^Environment="CODEMAN_PASSWORD=\(.*\)"$/\1/p' "$UNIT" | head -1 | sed 's/\\\(["\\]\)/\1/g') elif [ -f "$PLIST" ]; then - CODEMAN_PASSWORD=$(awk '/CODEMAN_PASSWORD<\/key>/{getline; print}' "$PLIST" | sed -n 's/.*\(.*\)<\/string>.*/\1/p') + # install.sh XML-escapes the plist value; undo it (& LAST, mirroring escape order). + CODEMAN_PASSWORD=$(awk '/CODEMAN_PASSWORD<\/key>/{getline; print}' "$PLIST" | sed -n 's/.*\(.*\)<\/string>.*/\1/p' \ + | sed -e 's/<//g' -e 's/&/\&/g') fi fi AUTH=(); [ -n "${CODEMAN_PASSWORD:-}" ] && AUTH=(-u "${CODEMAN_USERNAME:-admin}:$CODEMAN_PASSWORD") CURL=(curl -sk "${AUTH[@]}") # -k: harmless on http, required on https (self-signed cert) + +# Fail-CLOSED session delete. The DELETE lives INSIDE the guard on purpose: the older +# `is_self "$SID" || curl -X DELETE ...` shape failed OPEN, because an undefined +# is_self exits 127 and the `||` branch then ran the delete completely unguarded. +# Undefined delete_session is "command not found", which deletes nothing. +delete_session() { + local id="${1:-}" + [ -n "$id" ] || { echo "refusing: empty session id"; return 1; } + [ "${#SELF}" -ge 8 ] || { echo "refusing: \$SELF unset or too short to prove this is not me"; return 1; } + # ids appear in full AND 8-char form (Docker exports a truncated $SELF; mux names and + # UI surfaces carry 8-char ids), so compare by prefix in BOTH directions. Equality or + # a one-directional check each miss a real combination, and the miss deletes you. + case "$id" in "$SELF"*) echo "refusing: $id is me"; return 1 ;; esac + case "$SELF" in "$id"*) echo "refusing: $id is me"; return 1 ;; esac + "${CURL[@]}" -X DELETE "$API/api/v1/sessions/$id" +} + +CID=codeman-agent-1 # FIXED literal, never "agent-$$" (see §0) ``` - If `CODEMAN_MUX` is not `1`, **stop and say so**. Do not guess an API URL; a server @@ -67,20 +106,16 @@ CURL=(curl -sk "${AUTH[@]}") # -k: harmless on http, required on https (self-s You are yourself a session on this server, and the API has **no undo**. -- **Never act on your own session — and know that this check is the ONLY guard.** +- **Never act on your own session, and know that `delete_session` is the ONLY guard.** The server has no self-protection: a session that DELETEs its own id succeeds and - dies silently (verified live). Session ids appear in both full and 8-character - forms (Docker cases export a truncated `$SELF`; mux names and UI surfaces carry - 8-char ids), so compare by prefix **in both directions**, never by equality: - - ```bash - is_self() { case "$1" in "$SELF"*) return 0 ;; esac; case "$SELF" in "$1"*) return 0 ;; esac; return 1; } - ``` - - One-directional or equality checks each miss a real combination (full `$SELF` vs - a target you transcribed in 8-char form, or truncated `$SELF` vs a full target) - and the miss deletes you. Check `is_self` before every `DELETE`, kill, respawn, - or input call. + dies silently (verified live). **Always delete through `delete_session "$SID"` from + §0; never write a bare `curl -X DELETE` and never reintroduce the + `is_self … || curl -X DELETE …` shape.** That older form failed open: with the + function undefined (a half-re-pasted preamble, see §0) bash returns 127, the `||` + branch fires, and the delete runs with no self-check at all. Wrapping the request + inside the guard is what makes a lost preamble delete nothing instead of deleting + you. Apply the same prefix-both-directions reasoning before any kill, respawn, or + input call you write by hand. - **Mutating calls you may make unprompted** (this is an allowlist): `POST /api/v1/quick-start`, `POST /api/v1/sessions/:id/input`, and `DELETE /api/v1/sessions/:id` **only** for a session you created in this @@ -162,15 +197,25 @@ the composer is not) and always pays it in full before the fallback runs — the budget belongs to stage 3, after the dialog is answered: ```bash -SID=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ - -d '{"caseName":"worker-1","mode":"claude"}' | jq -r '.data.sessionId') +# ALWAYS check .success: on failure `.data.sessionId` is null, jq -r prints the string +# "null", and the flow below then burns its full readiness budget against +# /api/v1/sessions/null before reporting jq noise instead of the actual cause. +Q=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ + -d '{"caseName":"worker-1","mode":"claude"}') +SID=$(jq -r 'if .success then .data.sessionId else empty end' <<<"$Q") +if [ -z "$SID" ]; then + # SESSION_BUSY here is the 50-session cap, not the waiter cap; FORBIDDEN/CONFLICT/ + # OPERATION_FAILED/INVALID_INPUT are the others. None are retryable in a loop. + jq -c '{error, errorCode}' <<<"$Q"; echo "quick-start failed; stopping." + exit 1 +fi for _ in $(seq 1 30); do # bounded: a bad SID would otherwise poll forever [ "$("${CURL[@]}" "$API/api/v1/sessions/$SID" | jq '.data.pid')" != null ] && break; sleep 1 done # ⚠️ pid != null proves STARTUP only, never life: a worker that later dies inside # its pane keeps status "idle" and a pid (the local tmux attach client, not the # worker). The death check is wait?until=exit, below. -CID="agent-$$"; SEQ=1 +SEQ=1 # $CID came from the §0 preamble; do NOT rebuild it from $$ # the composer's status bar ("bypass permissions on") is the ready marker — Codeman # spawns claude in bypass mode. Single-token matches only: TUI text is space-less. R=$("${CURL[@]}" -G "$API/api/v1/sessions/$SID/wait-output" \ @@ -245,15 +290,40 @@ SEQ=$((SEQ+1)) The typed line shows `${M}_…`, the real output shows `DONE_… rc=`, and the snippet carries the exit code back to you. -**Read a worker's output** — the terminal buffer, tail in **bytes** (`textOutput` in -`GET .../output` stays empty for interactive sessions; don't use it): +**Read a worker's answer.** For `claude` and `codex` workers this is the read path: +`last-response` returns the agent's final message as clean text, taken from the +transcript rather than the screen, so it carries none of the TUI's box-drawing or +repaint noise. + +```bash +for _ in $(seq 1 10); do # the transcript write LAGS the stop signal + TXT=$("${CURL[@]}" "$API/api/v1/sessions/$SID/last-response" | jq -r '.data.text') + [ -n "$TXT" ] && break; sleep 1 +done +printf '%s\n' "$TXT" +``` + +`.data` is `{text, timestamp}`. ⚠️ **Poll it, do not read it once.** `text` is written +from the transcript file, which is flushed slightly *after* the `stop` hook fires, so a +single read taken the instant send-and-wait returns comes back `""` even though the +turn finished (verified live: empty on the first call, full text seconds later). `text` +is also `""` before the worker's first completed turn, and always `""` for modes with +no transcript (`shell`, `opencode`, `gemini`, `antigravity`, verified live), which is +why the loop above is bounded rather than open-ended. Fall back to the terminal buffer there, tail in **bytes** +(`textOutput` in `GET .../output` stays empty for interactive sessions; don't use it): ```bash "${CURL[@]}" "$API/api/v1/sessions/$SID/terminal?tail=3000" | jq -r '.data.terminalBuffer' \ | sed -e 's/\x1b\[[0-9;?]*[a-zA-Z]//g' -e 's/\x1b([B0]//g' | grep -v '^[[:space:]]*$' | tail -30 ``` -Avoid `?full=1` (entire tmux scrollback, a context bomb) unless doing a post-mortem. +⚠️ Do not use that pipeline to read a **claude/codex** answer. A full-screen TUI draws +with cursor moves, so the stripped buffer is largely one long line: `tail -30` has +almost nothing to split on and you get a wall of repaint noise with the answer buried +in it (verified live, side by side with `last-response` returning the exact prose). +The terminal buffer is for *diagnosis* (is my prompt sitting unsubmitted?), not for +reading answers. Avoid `?full=1` (entire tmux scrollback, a context bomb) unless doing +a post-mortem. **Detect a dead worker cheaply**: `GET .../wait?until=exit&timeout=60000` answers immediately (`signal:"exit"`, `immediate:true`) if the PTY is gone — including a @@ -262,10 +332,10 @@ as `status:"idle"` with a pid (that pid is the local tmux attach client, not the worker). The wait routes are the only liveness check; a worker dying while a wait is parked resolves it within ~3 s. A session deleted mid-wait resolves in ~1 s. -**Clean up** — only ids you created, `is_self`-checked, one at a time: +**Clean up** — only ids you created, one at a time, always through the §0 helper: ```bash -is_self "$SID" || "${CURL[@]}" -X DELETE "$API/api/v1/sessions/$SID" +delete_session "$SID" ``` Everything else (endpoint tables, per-mode signal table, error codes, capacity diff --git a/skills/codeman/reference/endpoints.md b/skills/codeman/reference/endpoints.md index 5d96e668..c62c6f43 100644 --- a/skills/codeman/reference/endpoints.md +++ b/skills/codeman/reference/endpoints.md @@ -14,7 +14,7 @@ Every JSON response: `{"success":true,"data":…}` or | `INVALID_INPUT` | 400 | malformed request; the message names the bad field | | `UNAUTHORIZED` | 401 | auth required or failed (send `-u user:password`). ⚠️ The 401 body is plain text, NOT this envelope — `jq` dies with a parse error, see the guard in SKILL.md | | `NOT_FOUND` | 404 | no such session, or one this caller does not own | -| `SESSION_BUSY` | 409 | this session's waiter cap (16, combined signal+output) is full | +| `SESSION_BUSY` | 409 | on a **wait**: this session's waiter cap (16, combined signal+output) is full. On **quick-start**: the 50-session cap is full, so clean up before starting more | | `CONFLICT` / `ALREADY_EXISTS` | 409 | conflicts with current state | | `OPERATION_FAILED` | 422 | well-formed but could not be completed | | `RATE_LIMITED` | 429 | per-owner or process-wide waiter pool is full — back off; switching sessions will not help | @@ -32,16 +32,19 @@ Every JSON response: `{"success":true,"data":…}` or | unified list incl. history | `GET /api/v1/sessions/unified` → `.data.sessions[]` (NOT `.data[]`), and it folds in transcript history from the whole machine — never use it to verify cleanup; `GET /api/v1/sessions` is the cleanup check | | start case + session in one call | `POST /api/v1/quick-start` | | send input | `POST /api/v1/sessions/:id/input` | -| read terminal (tail is in **BYTES**, raw ANSI) | `GET /api/v1/sessions/:id/terminal?tail=3000` → `.data.terminalBuffer` | +| **read a worker's answer** (claude/codex) | `GET /api/v1/sessions/:id/last-response` → `.data.{text,timestamp}` — clean transcript text, no TUI noise. ⚠️ **Poll it**: the transcript flush lags the `stop` signal, so a read taken the instant send-and-wait returns is `""` (verified live). Also `""` before the first completed turn, and always `""` for `shell`/`opencode`/`gemini`/`antigravity` (no transcript) | +| read terminal (tail is in **BYTES**, raw ANSI) | `GET /api/v1/sessions/:id/terminal?tail=3000` → `.data.terminalBuffer` — for *diagnosis* (unsubmitted prompt?), not for reading answers | | full tmux scrollback (context bomb; post-mortems only) | `GET /api/v1/sessions/:id/terminal?full=1` | -| background agents of a session | `GET /api/v1/subagents` | +| background agents, one session | `GET /api/v1/sessions/:id/subagents` | +| background agents, global list | `GET /api/v1/subagents` (admin-only in multi-user mode) | | server status / version | `GET /api/v1/status` → `.data.version` | -| delete one session (yours, `is_self`-checked) | `DELETE /api/v1/sessions/:id` | +| delete one session (yours only, via `delete_session`) | `DELETE /api/v1/sessions/:id` — never call it bare; the fail-closed helper in SKILL.md §0 is the only self-protection that exists | ⚠️ `GET /api/v1/sessions/:id/output` → `.data.textOutput` looks like the obvious read but stays **empty for interactive tmux-backed sessions** (it is fed only by the legacy -JSON-stream path). Verified empty on live claude and shell sessions. Read -`terminal?tail=` instead and strip ANSI: +JSON-stream path). Verified empty on live claude and shell sessions. Use +`last-response` for claude/codex answers; only fall back to `terminal?tail=` for +hook-less modes, or to diagnose a prompt that was never submitted, and strip ANSI: ```bash … | jq -r '.data.terminalBuffer' | sed -e 's/\x1b\[[0-9;?]*[a-zA-Z]//g' -e 's/\x1b([B0]//g' @@ -53,6 +56,18 @@ JSON-stream path). Verified empty on live claude and shell sessions. Read `.data.{sessionId, caseName, casePath}`. Creates the case directory (a real directory on the user's disk) if missing — do not retry it in a loop, and remember the name. +⚠️ **Branch on `.success` before reading `.data.sessionId`.** On any failure the field +is absent, `jq -r` prints the literal string `null`, and every later call then targets +`/api/v1/sessions/null`, burning the full readiness budget and reporting jq noise +instead of the real cause. Failure modes here are `SESSION_BUSY` (the **50-session +cap**, not the waiter cap), `FORBIDDEN`, `CONFLICT`, `OPERATION_FAILED` and +`INVALID_INPUT`; none of them are retryable in a loop. + +⚠️ `caseName` resolves through the linked-cases registry first, so a name that happens +to match a case the user linked in lands in that **real repo**, not a fresh scratch +directory. Pick distinctive scratch names, and use a linked name deliberately when you +do want a worker in an existing checkout. + `POST /api/v1/sessions/:id/input` body: `{"input":"one line\r","useMux":true,"clientId":"agent-1","seq":1}` plus optionally `"wait"` / `"waitTimeout"` (below). diff --git a/skills/codeman/reference/recipes.md b/skills/codeman/reference/recipes.md index 4bbb2fa5..148ddd49 100644 --- a/skills/codeman/reference/recipes.md +++ b/skills/codeman/reference/recipes.md @@ -1,10 +1,17 @@ # Worked orchestration flows -Loaded on demand from the `codeman` skill. Every flow assumes the guard preamble from -SKILL.md ran (`$API`, `$SELF`, `"${CURL[@]}"`, `is_self`). Track every session id you -create; delete them (and only them) when done. Remember the two silent killers: -**every input ends with `\r`**, and **markers must be split** so the typed-line echo -does not match them. +Loaded on demand from the `codeman` skill. Every flow assumes the SKILL.md §0 preamble +is in scope (`$API`, `$SELF`, `$CID`, `"${CURL[@]}"`, `delete_session`). + +⚠️ **That preamble does not survive between tool calls**, so re-run it at the top of +every Bash call that uses these flows, in full. Re-pasting only part of it is the +failure mode the fail-closed `delete_session` exists to contain, and a `clientId` you +rebuild from `$$` changes per call, which turns the duplicate-resend loop in Flow 1 +into a second typed prompt. + +Track every session id you create; delete them (and only them) when done. The two +silent killers: **every input ends with `\r`**, and **markers must be split** so the +typed-line echo does not match them. ## Flow 1: claude worker, end to end @@ -13,11 +20,15 @@ the turn to finish, read the answer, clean up. Verified live: the stop hook reso the send-and-wait within seconds of the turn ending. ```bash -# 1. start (returns before the CLI inside is ready) -SID=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ - -d '{"caseName":"worker-tests","mode":"claude"}' | jq -r '.data.sessionId') +# 1. start (returns before the CLI inside is ready). ALWAYS check .success: on failure +# .data.sessionId is null, jq -r yields the string "null", and every step below +# then runs against /api/v1/sessions/null and reports jq noise, not the cause. +Q=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ + -d '{"caseName":"worker-tests","mode":"claude"}') +SID=$(jq -r 'if .success then .data.sessionId else empty end' <<<"$Q") +[ -n "$SID" ] || { jq -c '{error, errorCode}' <<<"$Q"; echo "quick-start failed"; exit 1; } CREATED+=("$SID") # the cleanup list -CID="agent-$$"; SEQ=1 +SEQ=1 # $CID is the fixed literal from §0; never rebuild it from $$ # 2. readiness. "wait for idle" or "wait for ❯" is NOT readiness: a fresh session # reports idle before anything spawned, and the first-run trust dialog contains ❯. @@ -84,13 +95,23 @@ case "$(jq -r '.data.wait.signal' <<<"$R")" in null) jq -e '.data.wait.ended' <<<"$R" >/dev/null && echo "worker deleted mid-wait" ;; esac -# 5. read the answer: terminal tail (BYTES), ANSI-stripped. textOutput stays empty -# for interactive sessions; terminal?full=1 is a context bomb. -"${CURL[@]}" "$API/api/v1/sessions/$SID/terminal?tail=4000" | jq -r '.data.terminalBuffer' \ - | sed -e 's/\x1b\[[0-9;?]*[a-zA-Z]//g' -e 's/\x1b([B0]//g' | grep -v '^[[:space:]]*$' | tail -30 +# 5. read the answer. For a claude worker this is last-response: clean transcript text, +# no TUI repaint noise. Do NOT scrape the terminal for this — a full-screen TUI +# draws with cursor moves, so the stripped buffer is nearly one long line and the +# answer arrives buried in redraw garbage. +# POLL it: the transcript flush lags the stop signal, so a single read taken the +# instant step 3 returned comes back "" even though the turn finished (verified live). +for _ in $(seq 1 10); do + TXT=$("${CURL[@]}" "$API/api/v1/sessions/$SID/last-response" | jq -r '.data.text') + [ -n "$TXT" ] && break; sleep 1 +done +printf '%s\n' "$TXT" +# (.data is {text,timestamp}; text is also "" before the first completed turn and +# always "" for shell/opencode/gemini/antigravity, which have no transcript — use +# the terminal tail there, and here only to diagnose an unsubmitted prompt.) -# 6. clean up — exact id, own list only, self-check -is_self "$SID" || "${CURL[@]}" -X DELETE "$API/api/v1/sessions/$SID" +# 6. clean up — exact id, own list only, through the fail-closed §0 helper +delete_session "$SID" ``` Increment `SEQ` for every *new* input to the same worker. Reuse the same `SEQ` only to @@ -104,8 +125,10 @@ live), so send-and-wait can burn its whole timeout. The reliable pattern is a sp unique marker plus `wait-output from=buffer`: ```bash -SID=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ - -d '{"caseName":"builder","mode":"shell"}' | jq -r '.data.sessionId') +Q=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ + -d '{"caseName":"builder","mode":"shell"}') +SID=$(jq -r 'if .success then .data.sessionId else empty end' <<<"$Q") +[ -n "$SID" ] || { jq -c '{error, errorCode}' <<<"$Q"; echo "quick-start failed"; exit 1; } CREATED+=("$SID") for _ in $(seq 1 30); do [ "$("${CURL[@]}" "$API/api/v1/sessions/$SID" | jq '.data.pid')" != null ] && break; sleep 1 @@ -115,7 +138,7 @@ done # An unsplit marker matches the echo of your own keystrokes before the build runs. N="${RANDOM}_$$"; MARK="DONE_$N" "${CURL[@]}" -X POST "$API/api/v1/sessions/$SID/input" -H 'Content-Type: application/json' \ - -d '{"input":"M=DONE; npm run build; echo ${M}_'"$N"' rc=$?\r","useMux":true,"clientId":"build-'$$'","seq":1}' + -d '{"input":"M=DONE; npm run build; echo ${M}_'"$N"' rc=$?\r","useMux":true,"clientId":"codeman-build-1","seq":1}' for TRY in $(seq 1 30); do # BOUNDED (30 min): a \r-less send makes an uncapped loop infinite R=$("${CURL[@]}" -G "$API/api/v1/sessions/$SID/wait-output" \ @@ -136,8 +159,10 @@ waiter cap is 16 and abandoned concurrent waits pile up against it. ```bash declare -A WORKER MARKS for task in lint typecheck unit; do - SID=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ - -d '{"caseName":"fan-'"$task"'","mode":"shell"}' | jq -r '.data.sessionId') + Q=$("${CURL[@]}" -X POST "$API/api/v1/quick-start" -H 'Content-Type: application/json' \ + -d '{"caseName":"fan-'"$task"'","mode":"shell"}') + SID=$(jq -r 'if .success then .data.sessionId else empty end' <<<"$Q") + [ -n "$SID" ] || { jq -c '{error, errorCode}' <<<"$Q"; echo "$task: spawn failed"; continue; } WORKER[$task]=$SID; CREATED+=("$SID") done for task in "${!WORKER[@]}"; do @@ -147,7 +172,7 @@ for task in "${!WORKER[@]}"; do done N="${task}_${RANDOM}"; MARKS[$task]="DONE_$N" "${CURL[@]}" -X POST "$API/api/v1/sessions/$SID/input" -H 'Content-Type: application/json' \ - -d '{"input":"M=DONE; npm run '"$task"'; echo ${M}_'"$N"' rc=$?\r","useMux":true,"clientId":"fan-'$$'","seq":1}' + -d '{"input":"M=DONE; npm run '"$task"'; echo ${M}_'"$N"' rc=$?\r","useMux":true,"clientId":"codeman-fan-'"$task"'","seq":1}' done for task in "${!WORKER[@]}"; do # sequential gather; each wait blocks until that worker is done for TRY in $(seq 1 30); do # BOUNDED per worker, same reasoning as Flow 2 @@ -171,8 +196,8 @@ other was still running): ```bash sendwait() { # $1=sid $2=prompt $3=seq — assumes the worker passed Flow 1's readiness - local body; body=$(jq -n --arg p "$2" --argjson s "$3" \ - '{input:($p+"\r"),useMux:true,clientId:"fan-'$$'",seq:$s,wait:true,waitTimeout:600000}') + local body; body=$(jq -n --arg p "$2" --argjson s "$3" --arg c "codeman-fan-$1" \ + '{input:($p+"\r"),useMux:true,clientId:$c,seq:$s,wait:true,waitTimeout:600000}') "${CURL[@]}" -X POST "$API/api/v1/sessions/$1/input" \ -H 'Content-Type: application/json' --data-binary "$body" > "/tmp/fan-$1.json" } @@ -200,7 +225,7 @@ declare -A TOK for i in 1 2; do TOK[$i]="${RANDOM}_$i" BODY=$(jq -n --arg p "do task $i; when completely done print the word WORKDONE immediately followed by _${TOK[$i]}" \ - --arg c "fan-$$" --argjson s 2 '{input:($p+"\r"),useMux:true,clientId:$c,seq:$s}') + --arg c "codeman-fan-$i" --argjson s 2 '{input:($p+"\r"),useMux:true,clientId:$c,seq:$s}') "${CURL[@]}" -X POST "$API/api/v1/sessions/${SIDS[$i]}/input" \ -H 'Content-Type: application/json' --data-binary "$BODY" done @@ -237,12 +262,17 @@ At the end of the conversation (or on abort), delete exactly what you created: ```bash for id in "${CREATED[@]}"; do - is_self "$id" || "${CURL[@]}" -X DELETE "$API/api/v1/sessions/$id" + delete_session "$id" done ``` - Only ids from your own `CREATED` list. Never enumerate `/api/v1/sessions` and delete by pattern; other sessions belong to the user. +- Always go through `delete_session`. It refuses an empty id, refuses when `$SELF` is + unset or too short to prove the target is not you, and prefix-checks in both + directions. A hand-written `curl -X DELETE`, or the old + `is_self "$id" || curl -X DELETE …`, has none of that: an undefined `is_self` exits + 127 and the `||` branch deletes unguarded. - If you created a *case* purely as scratch and the user confirmed it is disposable, `DELETE /api/v1/cases/:name` removes it — but that recursively deletes the directory from disk, so never do it without the user's explicit go-ahead for that diff --git a/src/cli.ts b/src/cli.ts index 522989bc..db3b0865 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,9 +12,11 @@ import chalk from 'chalk'; import { createRequire } from 'module'; import http from 'node:http'; import https from 'node:https'; -import { readFileSync } from 'node:fs'; -import { isAbsolute } from 'node:path'; +import { existsSync, readFileSync } from 'node:fs'; +import { isAbsolute, join } from 'node:path'; +import { homedir } from 'node:os'; import { dataPath } from './config/instance.js'; +import { installAgentSkillInto, removeAgentSkillFrom, type AgentSkillApplyResult } from './hooks-config.js'; import { getSessionManager } from './session-manager.js'; import { getTaskQueue } from './task-queue.js'; import { getRalphLoop } from './ralph-loop.js'; @@ -119,6 +121,112 @@ program console.log(makeAttachmentMagicLink(filePath)); }); +// ============ Skill Commands ============ + +/** Same registry the server resolves case names through (mirrors `case-routes.ts`). */ +const LINKED_CASES_FILE = dataPath('linked-cases.json'); + +/** + * Case name to directory, checking `linked-cases.json` FIRST and falling back to the + * shared single-user cases dir. Mirrors `resolveCasePath()` in `case-routes.ts`, which + * is what the web UI and `quick-start` use. Without the linked-cases lookup this + * command rejected every case linked in from outside `~/codeman-cases` with + * "Case not found", even though the server resolved the same name fine. + * + * Sync and tolerant on purpose: a missing or malformed registry means "no linked + * cases", never a crash. + */ +function resolveCliCasePath(name: string): string { + try { + const linked = JSON.parse(readFileSync(LINKED_CASES_FILE, 'utf-8')) as Record; + const target = linked?.[name]; + if (typeof target === 'string' && target) return target; + } catch { + // no registry yet, or unreadable/invalid JSON: fall through to the cases dir + } + return join(homedir(), 'codeman-cases', name); +} + +/** + * Resolve where `skill install` / `skill uninstall` operate. Global is + * `~/.claude/skills/codeman` (Claude Code's user-scope skill dir, read by every new + * session); `--case ` targets `/.claude/skills/codeman`, resolved through + * `resolveCliCasePath()` above. The web server's automatic per-case injection + * (`agentSkillEnabled`) covers multi-user spaces; this CLI is a local operator tool + * and stays single-user. + */ +function resolveSkillTarget(options: { case?: string }): string { + if (options.case) { + const casePath = resolveCliCasePath(options.case); + if (!existsSync(casePath)) { + console.error(chalk.red(`✗ Case not found: ${casePath}`)); + process.exit(1); + } + return join(casePath, '.claude', 'skills', 'codeman'); + } + return join(homedir(), '.claude', 'skills', 'codeman'); +} + +/** Print an AgentSkillApplyResult for humans; exit non-zero when nothing was done. */ +function reportSkillResult(result: AgentSkillApplyResult, target: string): void { + const messages: Record = { + installed: { ok: true, text: `Agent skill installed: ${target}` }, + refreshed: { ok: true, text: `Agent skill refreshed (was stale): ${target}` }, + unchanged: { ok: true, text: `Agent skill already up to date: ${target}` }, + removed: { ok: true, text: `Agent skill removed: ${target}` }, + absent: { ok: true, text: `Nothing to remove at ${target}` }, + foreign: { + ok: false, + text: `${target} exists but is not Codeman-managed (no marker), refusing to touch it. Remove it yourself if you want the packaged skill there.`, + }, + symlink: { + ok: false, + text: `${target} (or its parent) is a symlink, refusing to write through it.`, + }, + }; + const message = messages[result]; + if (message.ok) { + console.log(chalk.green(`✓ ${message.text}`)); + } else { + console.error(chalk.red(`✗ ${message.text}`)); + process.exit(1); + } +} + +const skillCmd = program + .command('skill') + .description('Manage the Codeman agent skill (lets an agent inside a session drive the API)'); + +skillCmd + .command('install') + .description('Install the agent skill globally (~/.claude/skills/codeman) or into one case') + .option('-g, --global', 'Install into ~/.claude/skills/codeman, picked up by every new session (the default)') + .option('-c, --case ', 'Install into /.claude/skills/codeman instead (linked cases resolve too)') + .action(async (options: { global?: boolean; case?: string }) => { + try { + const target = resolveSkillTarget(options); + reportSkillResult(await installAgentSkillInto(target), target); + } catch (err) { + console.error(chalk.red(`✗ Failed to install agent skill: ${getErrorMessage(err)}`)); + process.exit(1); + } + }); + +skillCmd + .command('uninstall') + .description('Remove a Codeman-managed agent skill copy (never touches a user-authored one)') + .option('-g, --global', 'Remove from ~/.claude/skills/codeman (the default)') + .option('-c, --case ', 'Remove from /.claude/skills/codeman instead (linked cases resolve too)') + .action(async (options: { global?: boolean; case?: string }) => { + try { + const target = resolveSkillTarget(options); + reportSkillResult(await removeAgentSkillFrom(target), target); + } catch (err) { + console.error(chalk.red(`✗ Failed to remove agent skill: ${getErrorMessage(err)}`)); + process.exit(1); + } + }); + // ============ Session Commands ============ const sessionCmd = program.command('session').alias('s').description('Manage Claude sessions'); diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 715fa360..251929ea 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -27,8 +27,9 @@ */ import { existsSync } from 'node:fs'; -import { readFile, writeFile, mkdir } from 'node:fs/promises'; -import { join } from 'node:path'; +import { readFile, writeFile, mkdir, lstat, readdir, unlink, rmdir } from 'node:fs/promises'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; import type { HookEventType } from './types.js'; import { HOOK_TIMEOUT_SECONDS } from './config/auth-config.js'; @@ -720,3 +721,171 @@ export async function applyStatusLineConfig(casePath: string, enabled: boolean): await writeFile(settingsPath, JSON.stringify(existing, null, 2) + '\n'); }); } + +// ─── Agent skill injection ─────────────────────────────────────────────────── + +/** + * Version-agnostic ownership prefix for the injected agent skill, same pattern as + * `BACKGROUND_WAKE_MARKER_PREFIX`: ownership is decided on the prefix so a wording + * change in the full marker cannot disown every previously injected copy. + */ +const AGENT_SKILL_MARKER_PREFIX = '`; + +/** + * Packaged source of the skill: `skills/codeman/` at the package root. Resolved + * relative to this module so it works from `src/` (tsx dev), `dist/` (tsc build), + * and an npm install (`files` includes `skills`), all of which sit one level below + * the package root. + */ +function agentSkillSourceDir(): string { + return join(dirname(fileURLToPath(import.meta.url)), '..', 'skills', 'codeman'); +} + +interface AgentSkillFile { + /** Path relative to the target skill dir (e.g. `reference/endpoints.md`). */ + relPath: string; + content: string; +} + +/** + * Read the packaged skill: SKILL.md (marker appended) plus every markdown file + * under `reference/`. Enumerated from disk rather than a hardcoded manifest so a + * new reference file ships without touching this module. + */ +async function readAgentSkillSource(): Promise { + const src = agentSkillSourceDir(); + const skill = await readFile(join(src, 'SKILL.md'), 'utf-8'); + const files: AgentSkillFile[] = [{ relPath: 'SKILL.md', content: `${skill.trimEnd()}\n\n${AGENT_SKILL_MARKER}\n` }]; + let referenceNames: string[] = []; + try { + referenceNames = (await readdir(join(src, 'reference'))).filter((name) => name.endsWith('.md')).sort(); + } catch { + // no reference dir in the source; SKILL.md alone is still a valid skill + } + for (const name of referenceNames) { + files.push({ relPath: join('reference', name), content: await readFile(join(src, 'reference', name), 'utf-8') }); + } + return files; +} + +async function isSymlink(path: string): Promise { + try { + return (await lstat(path)).isSymbolicLink(); + } catch { + return false; + } +} + +/** What an install/remove actually did, so callers (CLI, logs) can say so. */ +export type AgentSkillApplyResult = + | 'installed' // fresh copy written + | 'refreshed' // our copy was stale and got rewritten + | 'unchanged' // our copy already matches the packaged source + | 'removed' // our copy deleted + | 'absent' // nothing there to remove + | 'foreign' // a copy exists but is not ours; left untouched + | 'symlink'; // the skill dir (or its parent) is a symlink; left untouched + +/** + * Install or refresh the Codeman agent skill into `skillDir` (a `.../codeman` + * directory, e.g. `/.claude/skills/codeman` or `~/.claude/skills/codeman`). + * + * Refuses two shapes rather than writing through them: + * - a SYMLINK at the skill dir or its `skills/` parent: this repo's own dogfooding + * layout (`.claude/skills/codeman -> ../../skills/codeman`) would otherwise have + * the injector overwrite the repo source through the link; + * - a FOREIGN copy (SKILL.md present without our marker): that is the user's own + * skill, and per the statusLine rule we never clobber what we did not write. + * + * Idempotent and cheap: unchanged files are not rewritten, so calling on every + * session create causes no mtime churn. + */ +export async function installAgentSkillInto(skillDir: string): Promise { + if ((await isSymlink(dirname(skillDir))) || (await isSymlink(skillDir))) return 'symlink'; + + let existing: string | null = null; + try { + existing = await readFile(join(skillDir, 'SKILL.md'), 'utf-8'); + } catch { + // absent: fresh install + } + if (existing !== null && !existing.includes(AGENT_SKILL_MARKER_PREFIX)) return 'foreign'; + + const files = await readAgentSkillSource(); + let changed = false; + for (const file of files) { + const target = join(skillDir, file.relPath); + let current: string | null = null; + try { + current = await readFile(target, 'utf-8'); + } catch { + // missing: will be written + } + if (current === file.content) continue; + await mkdir(dirname(target), { recursive: true }); + await writeFile(target, file.content); + changed = true; + } + if (!changed) return 'unchanged'; + return existing === null ? 'installed' : 'refreshed'; +} + +/** + * Remove a Codeman-managed skill copy from `skillDir`. Same ownership and symlink + * refusals as the install path. Deletes only files the packaged source would have + * written (never `rm -rf`, so a user's extra files in the directory survive), then + * prunes the directories bottom-up if they emptied. + */ +export async function removeAgentSkillFrom(skillDir: string): Promise { + if ((await isSymlink(dirname(skillDir))) || (await isSymlink(skillDir))) return 'symlink'; + + let existing: string | null = null; + try { + existing = await readFile(join(skillDir, 'SKILL.md'), 'utf-8'); + } catch { + return 'absent'; + } + if (!existing.includes(AGENT_SKILL_MARKER_PREFIX)) return 'foreign'; + + // Manifest-based, with SKILL.md as the fallback when the packaged source is + // unreadable: removal must still work on an install whose skills/ dir went missing. + const files = await readAgentSkillSource().catch((): AgentSkillFile[] => [{ relPath: 'SKILL.md', content: '' }]); + for (const file of files) { + await unlink(join(skillDir, file.relPath)).catch(() => {}); + } + await rmdir(join(skillDir, 'reference')).catch(() => {}); // fails when non-empty, fine + await rmdir(skillDir).catch(() => {}); + await rmdir(dirname(skillDir)).catch(() => {}); // prune `.claude/skills` if now empty + return 'removed'; +} + +/** + * Add or remove the Codeman agent skill in `/.claude/skills/codeman`, + * mirroring `applyStatusLineConfig`'s shape. Gated by the synced `agentSkillEnabled` + * app setting (default OFF); callers gate on Claude mode, since the skill is discovered + * via `.claude/skills/`, which only Claude Code reads. + * + * Call-site policy is ADD-ONLY on session create (callers pass `enabled: true` or + * skip the call), for the statusLine reason: sessions in a repo share one `.claude/` + * dir, so a single create while the setting is off must not yank the skill out from + * under other live sessions. + * + * ⚠️ Consequence: turning `agentSkillEnabled` OFF sweeps nothing. There is deliberately + * no server-side toggle-off sweep (it would have to walk every case, including ones + * with live sessions, and would hit exactly the shared-`.claude/` hazard above), so + * already-injected copies stay on disk until removed per case with + * `codeman skill uninstall --case `. The `enabled: false` branch here backs that + * CLI and the tests; it has no server call site. Keep the README's Agent Skill note in + * sync if this ever changes. + */ +export async function applyAgentSkill(casePath: string, enabled: boolean): Promise { + const skillDir = join(casePath, '.claude', 'skills', 'codeman'); + return enabled ? installAgentSkillInto(skillDir) : removeAgentSkillFrom(skillDir); +} diff --git a/src/web/ports/config-port.ts b/src/web/ports/config-port.ts index bff5e9331..a0a0bdcd 100644 --- a/src/web/ports/config-port.ts +++ b/src/web/ports/config-port.ts @@ -17,6 +17,8 @@ export interface ConfigPort { getModelConfig(): Promise<{ defaultModel?: string; agentTypeOverrides?: Record } | null>; getClaudeModeConfig(): Promise<{ claudeMode?: ClaudeMode; allowedTools?: string }>; getTerminalHistoryConfig(): Promise; + /** Synced `agentSkillEnabled` app setting (default OFF); gates per-case agent-skill injection. */ + getAgentSkillEnabled(): Promise; getDefaultClaudeMdPath(): Promise; getLightState(identity?: { username: string; role: 'admin' | 'user' }): unknown; getLightSessionsState(): unknown[]; diff --git a/src/web/public/index.html b/src/web/public/index.html index 98142965..aca5bac5 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1637,6 +1637,14 @@ Enable experimental Agent Teams for all new Claude sessions (disabled by default) +
+ + + Give new Claude sessions the Codeman skill (start workers, send prompts, wait for results via the API) +