From 0a52a99ca918cf8341446672eaa22b2f655fd413 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Thu, 24 Sep 2026 07:48:26 +0800 Subject: [PATCH] feat(cli-registry): CLI management write API + Settings UI (Phases 1-6) (#476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli-registry): add cliManagementEnabled flag and GET /api/clis Phases 1-2 of docs/cli-enable-disable-plan.md ("PR C" from the #343 review): a synced, default-OFF master flag gating the upcoming CLI management surface, plus a read-only GET /api/clis endpoint listing every registry entry (stock + custom, enabled or not) for the Settings UI. Non-admins in multi-user mode see an empty list rather than a 403. Write endpoints, auto-install, custom entry CRUD and the Settings UI list itself land in later phases. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * feat(cli-registry): Phases 3-6 - write API + custom entries + Settings UI Completes docs/cli-enable-disable-plan.md ("PR C" from the #343 review). Phase 3: PUT /api/clis/:id toggles enabled for any EXISTING entry (stock or custom) via a shallow merge onto its clis.json override; shell/claude are structurally un-disableable (Decision 4), an unknown id 404s rather than becoming a creation backdoor. Phase 4: POST /api/clis/:id/install runs a STOCK entry's already-vetted install command (shell:true, bounded by timeout, process-group killed on expiry, output captured, audit-logged). A custom entry's id is refused outright, independent of anything Phase 5 does (Decision 3: a custom entry's install text is display-only, never executed). Phase 5: POST /api/clis (create) / PUT /api/clis/custom/:id (update) / DELETE /api/clis/:id (custom only) — a deliberately minimal request shape (id/label/shortBadge/binaries/a simple launch variant), assembled into a full CliEntry with conservative capability defaults and re-validated through CliEntrySchema before writing, never a relaxed path for UI-originated entries. Stock-id collisions, duplicate custom ids, and edits/deletes against a stock id are all rejected explicitly. Phase 6: the Settings UI section (App Settings -> Agents & CLIs), gated independently on cliManagementEnabled AND admin-in-multi-user-mode (Decision 5), fetching/rendering GET /api/clis and wiring every write endpoint above. Every write endpoint answers the same way when the feature is off: 403 FORBIDDEN via one shared requireCliManagementGate() (Phase 1's own checklist item). registry-writer.ts is a new, deliberately separate write module so registry.ts itself stays import-side-effect-free, same tmp+ rename+0600 shape as custom-model-hosts.ts. 27 new/updated route tests covering every gate, collision, and cleanup path; full CI gate green (415/416 files, 7854 tests). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * fix(cli-registry): toggling a CLI off in Settings never hid it anywhere else window.__codemanCliAvailable — the flag isCliAvailable() reads client-side to gate the welcome-screen buttons, the Run-menu dropdown and the mobile overview — was built purely from each CLI's own installed-on-PATH resolver (isClaudeAvailable() etc.), with no reference to the registry's `enabled` flag at all. So disabling a CLI via the new Settings UI (or a hand-edited clis.json) updated the settings row and nothing else: every launch surface kept offering it, both live and after a full page reload, since even a fresh render never consulted the registry. Fixed in two places: - server.ts: after building `available`, intersect the nine real SessionMode ids against `enabledClis()`. git/cloudflared (utility binaries, not CLI registry entries) and deepseekBinary (a secondary installed-only flag for the "add a profile" affordance) are deliberately left alone. - settings-ui.js: `toggleCliEnabled()` now patches `window.__codemanCliAvailable` in place and refreshes the welcome screen, the mobile overview and an already-open Run menu, mirroring the existing `installDeepSeekProfile()` pattern for the same "injected once, needs an explicit patch" reason — without this half, the server-side fix alone still left every surface stale until the next reload. New test in test/render-index-html.test.ts: an installed-but-disabled CLI (codex, forced via clis.json + reloadCliRegistry()) reads as unavailable, while an installed-and-enabled one (claude) is unaffected by the override. Verified on the Debian devbox (codeman-devbox, real tmux — this sandbox has none and WebServer's constructor hard-requires it): typecheck clean, the new test passes (17/17 in render-index-html.test.ts), the CLI-registry suites pass (86/86), and the full CI gate is green (415 test files, 7855 tests, 0 failures). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD * docs(cli-registry): update the CLI-management plan with status, gotchas, and the Run-menu gap Phases 1-6 were implemented across two commits (da07b38c, db4557d9) with no corresponding update to the plan doc itself — every checklist still read Status: TODO and every box unchecked. Brings the doc in line with the tree: - A new "Status as of 2026-09-22" section up top: what's actually implemented (verified by grepping the routes/schema/UI, not just trusting the commit messages), the availability-flag staleness bug found and fixed in this session (commit 0c77dd0a) with its devbox verification record, and one real outstanding gap. - The outstanding gap: a custom CLI created via Phase 5's write API has no way to actually be launched. The Run menu is static per-mode markup with no consumer of window.__codemanCliCatalog, so Phase 6's own "create a custom entry, confirm it can be launched" verify step was never actually exercised against this. Documented with two candidate fixes, neither started. - Each phase's checklist flipped to [x] where confirmed present in the tree, Status lines updated from TODO to DONE, and the two originally-open questions (Phase 2's installed source, Phase 5's PUT endpoint shape) marked resolved against what actually shipped. No code changes in this commit — documentation only, so a future session (or the one already mid-flight on a separate checkout of this same branch) picks up accurate status instead of a stale plan. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD * docs: add the CLI-registry deployment plan and the parked Copilot plan Both were sitting as untracked scratch files in the master checkout, never committed to any branch. Moving them here rather than leaving them loose: - DEPLOYMENT_PLAN.md is the live tracker for the CLI-registry follow-up series (PR A #347 merged, PR B #380 merged, PR B2 merged as #458) and is where PR C (this branch's own CLI-management work) belongs. - docs/copilot-integration-plan.md is explicitly PARKED, referenced by name in docs/cli-enable-disable-plan.md's own header as a sibling plan tracked separately — kept for continuity, not active on this branch. The other scratch files found alongside these (PRA.md, PRB.md, PR-B2.md and their review-response counterparts) described PR A/B/B2, all now merged — deleted from the master checkout as stale rather than committed anywhere, since their content is superseded by the real merged PRs. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD * fix(cli-registry): render enabled CLIs in launch surfaces * test(cli-registry): update frontend branch guard * fix(test): isolate suite from deployment environment * fix(cli-registry): revise Decision 4 - claude is toggleable, shell stays permanent shell/claude were both structurally un-disableable in the original plan (Decision 4). Revised: shell keeps the hard backend guarantee (it is the one non-agent mode several code paths assume always exists as a raw- terminal fallback), but claude is now a normal toggleable entry like any other CLI. Safe to do because internal session creation (tmux-manager.ts, session.ts, Ralph, plan-orchestrator) resolves a CLI via getCli(), which does not check `enabled` at all - only the Run menu and the HTTP-facing sessionModeSchema() (new session requests through the normal API) key off it. Disabling claude therefore behaves identically in kind to disabling any other CLI: no internal fallback path breaks, it just stops being offered for new sessions until re-enabled. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * fix(cli-registry): hide shell's toggle entirely instead of greying it out A permanently-disabled switch next to every other row's working toggle read as broken rather than intentional. shell now renders no switch at all - a plain "Always available" label - so there is nothing to click that could look like it should work but doesn't. Backend guard is unchanged (UNDISABLEABLE_IDS still refuses shell unconditionally); this is UI-only. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * fix(cli-registry): sort the Installed CLIs list, installed-first then alphabetical renderCliList() previously rendered in registry order (each entry's fixed order field). Now sorts installed CLIs first, then not-installed, each group alphabetical by label - matches how a user actually scans the list (what's ready to use, then what needs installing). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * style: prettier fixes from the master merge * fix(cli-registry): install/edit take effect immediately, confirm before install, phone labels Four gaps found verifying #476 against the #343 review trail: - Installed or edited CLIs kept reading as missing/stale. Every binary lookup (the nine per-CLI resolvers and the generic registry one) caches in its own closure, with a negative-cache backoff of up to 5 minutes, and nothing cleared them. invalidateCliExecutableResolvers(binaries) now drops those caches per binary; install (success or failure), create, edit and delete call it plus invalidateCliResolverCache(id). Before this, a CLI installed from Settings could fail to launch for minutes, and an edited custom entry kept launching its old binary until a restart. - The Settings "installed" badge for a custom entry used a private `which`, ignoring the entry's searchDirs and the login-shell lookup that spawn and the Run menu use; it now asks the same generic resolver they do. - Install ran on a single click. The #343 review asked for auto-install to sit behind an explicit confirm; the confirm now names the exact command, which GET /api/clis returns for stock entries only (installCommand). - The phone Run button showed the two-letter tab badge ("CC", "CX") instead of the word ("Claude", "Codex"). It uses the registry label again, which is identical to the old static table for every stock CLI (now pinned). 14 new tests; 9 of them fail against the previous head and pass here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * fix(cli-registry): address #476 review — safe serialized writes, no id branches, docs Must-fix: - registry-writer: start fresh only on ENOENT; refuse (409) a clis.json that does not parse or has group/world permission bits instead of overwriting it (isUnsafePermissions now exported from registry.ts) - mutateRegistryFile(): one promise chain for every mutation, with the existence/duplicate checks inside the serialized step, plus a unique tmp name per write - docs: CLAUDE.md, architecture-invariants, cli-registry (new Settings section) and api-reference (the six /api/clis routes) - drop DEPLOYMENT_PLAN.md and docs/copilot-integration-plan.md Smaller: - PUT /api/clis/custom/:id keeps the entry's current enabled state when the body omits it - runMode setter falls back to the first enabled catalogue entry, not 'claude' - shell guard keyed on kind === 'shell' (routes + Settings list); stock probe map shared with server.ts via utils/cli-installed-probes.ts - stock claude label is now 'Claude Code', so the Run menu / phone overview label rewrites are gone (doctor row keeps "Claude CLI" via its override) - welcome buttons are translatable again and read "Run Claude Code" / "Run Shell"; zh-CN gains "Run Codex" / "Run OMP" - install: per-id in-flight guard (409) and CODEMAN_* stripped from its env - fileoverview / CliEnableSchema comments no longer say stock-only - test-env isolation changes moved to their own PR Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n * test(cli-registry): pin the #343/#347 findings #476 makes reachable A CLI toggled or created through the routes is accepted or rejected by CreateSessionSchema with no restart (#343 finding 2), and a custom CLI created through the API renders a real local, remote and docker launch command (#347 finding 5: no more `cd && undefined`). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n --------- Co-authored-by: Claude Sonnet 5 --- CLAUDE.md | 2 +- config/clis.stock.json | 2 +- docs/api-reference.md | 13 + docs/architecture-invariants.md | 4 +- docs/cli-enable-disable-plan.md | 314 +++++++++ docs/cli-registry.md | 14 +- install.sh | 2 +- src/config/cli-registry/registry-writer.ts | 115 +++ src/config/cli-registry/registry.ts | 15 +- src/config/cli-registry/stock.ts | 2 +- src/config/cli-registry/types.ts | 10 +- src/config/dependency-registry.ts | 4 +- src/utils/cli-executable-resolver.ts | 31 + src/utils/cli-installed-probes.ts | 69 ++ src/web/public/i18n.js | 2 + src/web/public/index.html | 119 ++-- src/web/public/mobile-overview.js | 24 +- src/web/public/session-ui.js | 88 ++- src/web/public/settings-ui.js | 348 +++++++++- src/web/public/styles.css | 36 + src/web/routes/cli-registry-routes.ts | 553 +++++++++++++++ src/web/routes/index.ts | 1 + src/web/schemas.ts | 42 ++ src/web/server.ts | 88 +-- test/cli-executable-resolver.test.ts | 44 ++ test/cli-management-settings-ui.test.ts | 88 +++ test/cli-registry-no-id-branching.test.ts | 1 - test/frontend-cli-no-id-branching.test.ts | 33 +- test/mobile-overview.test.ts | 53 ++ test/render-index-html.test.ts | 58 +- test/routes/cli-registry-routes.test.ts | 771 +++++++++++++++++++++ test/run-mode-dispatch.test.ts | 17 +- test/run-mode-ui.test.ts | 181 +++-- test/server-index-title.test.ts | 7 +- 34 files changed, 2898 insertions(+), 253 deletions(-) create mode 100644 docs/cli-enable-disable-plan.md create mode 100644 src/config/cli-registry/registry-writer.ts create mode 100644 src/utils/cli-installed-probes.ts create mode 100644 src/web/routes/cli-registry-routes.ts create mode 100644 test/cli-management-settings-ui.test.ts create mode 100644 test/routes/cli-registry-routes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 3e27add7..da788221 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -229,7 +229,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Docker Compose deployment** (`docker/`): Codeman runs in a container and spawns Docker cases as **SIBLING** containers via the host socket, never nested. `resolveDockerDaemonMountSource()` maps HOME bind sources into the daemon's namespace (`CODEMAN_DOCKER_HOST_HOME`); `CODEMAN_CASES_PATH` makes workspaces resolve to the same absolute path on both sides. ⚠️ `CODEMAN_CASES_PATH` must move every consumer: resolve it only via `config/cases-dir.ts`. ⚠️ `.dockerignore` matches whole paths: keep `**/.env` or `docker/.env` secrets ship in the image. ⚠️ Long-form binds create missing sources ROOT-OWNED: `Start-Codeman.sh` pre-creates them, and `docker/entrypoint.sh` (root, `cap_add: [CHOWN, DAC_OVERRIDE, KILL, SETGID, SETUID]` against `cap_drop: ALL`, `KILL` for tini; pinned by the test) fixes ownership then drops to `PUID:PGID` via `setpriv`, never re-owning foreign dirs. ⚠️ Append `/opt/codeman-cli` to `PATH`, never prepend. ⚠️ `server.Dockerfile`, the compose file and `.env.example` feed the self-updater's environment gate (`docs/docker-self-update.md`). → [architecture-invariants#docker-compose-deployment](docs/architecture-invariants.md#docker-compose-deployment), `docs/docker-compose.md` -**CLI registry** (`src/config/cli-registry/`): every run mode is a `CliEntry` (discovery, launch argv template, env handling, `capabilities`, and the `overlays` behind remote/docker pane commands). **No code outside `stock.ts` may branch on a CLI id**: use a capability field or a NAMED PROFILE (`profiles.ts`); `test/cli-registry-no-id-branching.test.ts` and `test/frontend-cli-no-id-branching.test.ts` enforce it. ⚠️ Config holds typed argv tokens, never shell text; literals are validated at LOAD time and a bad one rejects the whole entry. ⚠️ Keep `external`, `hooks` and `altScreen` independent; never derive one from another. ⚠️ Config regexes (`discovery.version.regex`, `capabilities.workDetect.workingLine`, `capabilities.workDetect.watchingLine`) must compile through `compileVersionRegex()`. ⚠️ `privilegedParams[].param` names a LAUNCH PARAM, not the legacy `Config` field (bridged only by `launch.legacyConfigAliases`); a wrong name silently clamps nothing. ⚠️ Resolve the registry AT CALL TIME, never in a module-level const. ⚠️ Remote claude/omp arms of `buildRemoteLaunchCommand` are not covered by the pane-command golden. `~/.codeman/clis.json` overrides entries (read-only). → [architecture-invariants#cli-registry](docs/architecture-invariants.md#cli-registry), `docs/cli-registry.md` +**CLI registry** (`src/config/cli-registry/`): every run mode is a `CliEntry` (discovery, launch argv template, env handling, `capabilities`, and the `overlays` behind remote/docker pane commands). **No code outside `stock.ts` may branch on a CLI id**: use a capability field or a NAMED PROFILE (`profiles.ts`); `test/cli-registry-no-id-branching.test.ts` and `test/frontend-cli-no-id-branching.test.ts` enforce it. ⚠️ Config holds typed argv tokens, never shell text; literals are validated at LOAD time and a bad one rejects the whole entry. ⚠️ Keep `external`, `hooks` and `altScreen` independent; never derive one from another. ⚠️ Config regexes (`discovery.version.regex`, `capabilities.workDetect.workingLine`, `capabilities.workDetect.watchingLine`) must compile through `compileVersionRegex()`. ⚠️ `privilegedParams[].param` names a LAUNCH PARAM, not the legacy `Config` field (bridged only by `launch.legacyConfigAliases`); a wrong name silently clamps nothing. ⚠️ Resolve the registry AT CALL TIME, never in a module-level const. ⚠️ Remote claude/omp arms of `buildRemoteLaunchCommand` are not covered by the pane-command golden. `~/.codeman/clis.json` overrides entries. It is WRITTEN only by the opt-in CLI management routes (`cliManagementEnabled`, default OFF; `/api/clis`, `cli-registry-routes.ts`), and only through `mutateRegistryFile()` in `registry-writer.ts`, which serializes mutations and refuses (409) a file that does not parse or has group/world permission bits rather than overwriting it. Importing the registry still writes nothing. → [architecture-invariants#cli-registry](docs/architecture-invariants.md#cli-registry), `docs/cli-registry.md` **External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP)**: `isExternalCliMode()` in `session.ts` gates Claude-specific behavior off (Ralph tracker, BashToolParser, token parsing, ❯ readiness; readiness is output stabilization); work detection is per-CLI `capabilities.workDetect` data, not this gate. All eight **require tmux, no direct PTY fallback** (secrets go via socket-scoped `tmux setenv`, never the command line). ⚠️ `run*()` in `session-ui.js` MUST unwrap the `{success,data}` envelope. ⚠️ **Codex uses predictive write-through echo, never the buffer overlay**: `_predictHookOnData` must never `return` (wire path stays byte-identical), and flushed text and a bracketed paste must go out as separate delayed writes. ⚠️ **Pi**: no bypass flag, never invent one; `approveProjectTrust` executes repo code, so it is in the clamp's **materialize** branch; never wire `--api-key`. ⚠️ **Grok**: `alwaysApprove` is stripped for non-granted owners (only-if-sent). ⚠️ **DeepSeek**: the agent is a PROFILE (Run gates on `isDeepSeekRunnable()`); the permission switch is the `DSH_PERMISSION_MODE` env var, so `clampEnvOverridesForOwner()` must DROP `DSH_PERMISSION_MODE`, `DSH_HOME` and `DEEPSEEK_BASE_URL` for non-granted owners; `hooksAvailableForMode()` is per-SESSION for it (pass `sessionHookOptions(session)`) and is never a stand-in for `mode === 'claude'`; answers come from `deepseek-transcript.ts`, paired by header `cwd` + boot window, never newest-mtime. ⚠️ **OMP**: `OMP_AUTH_BROKER_URL`/`_TOKEN` are clamped the same way. → [architecture-invariants#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp](docs/architecture-invariants.md#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp) diff --git a/config/clis.stock.json b/config/clis.stock.json index 89982217..5a86af0e 100644 --- a/config/clis.stock.json +++ b/config/clis.stock.json @@ -1,7 +1,7 @@ [ { "id": "claude", - "label": "Claude", + "label": "Claude Code", "shortBadge": "CC", "enabled": true, "order": 0, diff --git a/docs/api-reference.md b/docs/api-reference.md index c85b4ff7..bc520de2 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -691,6 +691,19 @@ normal `caseName`/`mode`/etc. body) jarring than a full relaunch, and folding it into the one-shot path is separate work — see `docs/custom-model-endpoints-plan.md`). +## CLI management + +Read and write the CLI registry (`docs/cli-registry.md`). Every **write** route answers `403 FORBIDDEN` while `cliManagementEnabled` is off (the default), and for a non-admin in multi-user mode. A write that would overwrite a `clis.json` which does not parse, or which has group/world permission bits, is refused with `409 CONFLICT` and a message naming the fix; the file is left untouched. + +| Method | Path | Body | Notes | +| -------- | ----------------------------- | ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `GET` | `/api/clis` | none | Every entry, disabled ones included: `id`, `label`, `shortBadge`, `order`, `kind`, `enabled`, `stock`, `installed`, and `installCommand` for a stock entry. Not gated; a non-admin in multi-user mode gets `[]`. | +| `PUT` | `/api/clis/:id` | `{ enabled }` | Toggle an existing entry, stock or custom. `404` for an unknown id; `400 INVALID_INPUT` when disabling a `kind: 'shell'` entry. | +| `POST` | `/api/clis/:id/install` | none | Run a **stock** entry's install command (never a custom one: `400`). `409 CONFLICT` while an install for the same id is running; `422 OPERATION_FAILED` with the output tail when it fails. Never enables the entry. | +| `POST` | `/api/clis` | `{ id, label, shortBadge, binaries, argv, enabled? }` | Create a custom entry. `409 ALREADY_EXISTS` for a stock id or an existing custom id. `enabled` defaults to `true`. | +| `PUT` | `/api/clis/custom/:id` | `{ label, shortBadge, binaries, argv, enabled? }` | Replace an existing custom entry. An absent `enabled` keeps the entry's current state. `400` for a stock id, `404` for an unknown one. | +| `DELETE` | `/api/clis/:id` | none | Delete a custom entry. `400` for a stock id, `404` for an unknown one. | + ## Voice dictation Browser dictation transcribed through this server's Claude Code login, i.e. the diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 02412f19..e05dbe11 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -109,11 +109,11 @@ Further detail: ⚠️ An ADOPTED container may back SEVERAL cases at different ⚠️ **`param` is TWO namespaces.** `launch.params` keys, `env.configSetenv[].fromParam` and `capabilities.privilegedParams[].param` all name a LAUNCH PARAM; the legacy `Config` wire field is a separate namespace, bridged only by `launch.legacyConfigAliases`. Getting `privilegedParams[].param` wrong is SILENT — it is the multi-user bypass clamp's only handle on a CLI's privilege switch, and a wrong name clamps nothing with no load error and no failing test — so `schema.ts` rejects an entry naming a param it never declared. Codex is the entry where the two names differ (`bypassApprovals` vs `dangerouslyBypassApprovals`) and therefore the one that catches a regression. -⚠️ Six fields are DECLARED-FOR-LATER and read by nothing (`shortBadge`, `accent`, `capabilities.echo`/`wheelForward`/`keyboardAccessory`/`maxFrameBytes`): all frontend behaviour, transcribed rather than measured (except `accent`, measured against styles.css on 2026-09-21), so re-measure before wiring one up; the list is pinned so it cannot quietly grow. +⚠️ Five fields are DECLARED-FOR-LATER and read by nothing (`accent`, `capabilities.echo`/`wheelForward`/`keyboardAccessory`/`maxFrameBytes`): all frontend behaviour, transcribed rather than measured (except `accent`, measured against styles.css on 2026-09-21), so re-measure before wiring one up; the list is pinned so it cannot quietly grow. (`shortBadge` left the list when the Settings CLI management list started showing it.) Spawn commands are pinned as literal strings in `test/cli-registry-spawn-golden.test.ts`, remote/docker pane commands in `test/location-overlay-commands.test.ts`. ⚠️ That second golden no longer covers **remote claude or remote omp**: both now have their own arm in `buildRemoteLaunchCommand` (a `--session-id || --resume` pair, and `--continue`, so a respawn continues the same conversation) and never reach `defaultRemoteCommandForMode`, which is what that test asserts. Their real pins are `toContain` substrings in `test/tmux-manager.test.ts` and `test/remote-shared-sessions.test.ts`; changing either arm will NOT fail the golden. -⚠️ Anything reading the registry resolves it AT CALL TIME (`sessionModeSchema()`, `allowedEnvPrefixes()`, `dependencyRegistry()`, the resolvers' `searchDirs` thunks) — a module-level const freezes at first import, so a CLI enabled while the server ran moved the run menu but not that surface. `~/.codeman/clis.json` overrides any entry (read-only in this release; nothing writes it, so importing the registry has no filesystem side effects). User guide: `docs/cli-registry.md`. +⚠️ Anything reading the registry resolves it AT CALL TIME (`sessionModeSchema()`, `allowedEnvPrefixes()`, `dependencyRegistry()`, the resolvers' `searchDirs` thunks) — a module-level const freezes at first import, so a CLI enabled while the server ran moved the run menu but not that surface. `~/.codeman/clis.json` overrides any entry. Importing the registry still has no filesystem side effects: the only writer is `registry-writer.ts`, imported only by `cli-registry-routes.ts` (the opt-in CLI management routes, `cliManagementEnabled`, default OFF). ⚠️ Every write goes through `mutateRegistryFile()`: mutations run one at a time on a single promise chain, each with a unique temp file (three parallel toggles once lost two and 500'd on a shared temp name), and a file the READER would ignore (group/world permission bits) or quarantine (unparseable) is REFUSED with a 409 rather than rewritten, so one Settings click cannot replace a hand-edit or turn a refused file into trusted 0600 config. Only a missing file starts fresh. After a write the registry is reloaded, so changes apply without a restart. User guide: `docs/cli-registry.md`. ## Session data and lifecycle diff --git a/docs/cli-enable-disable-plan.md b/docs/cli-enable-disable-plan.md new file mode 100644 index 00000000..1ff7f01a --- /dev/null +++ b/docs/cli-enable-disable-plan.md @@ -0,0 +1,314 @@ +# CLI management Settings UI + write API — plan + +> Tracked separately from `DEPLOYMENT_PLAN.md` (PR B2, merged) and `docs/copilot-integration-plan.md` +> (parked). This is "PR C" from the original #343 review: *"settings UI + write endpoints + +> auto-install, once we've settled the trust model... I want to make that call on its own, not +> inside a 100-file diff."* +> +> **Phase 0 is CLOSED as of 2026-09-21** — all three original pieces are IN SCOPE (expanded from +> this plan's first draft, which recommended #2/#3 as separate/out-of-scope; the user chose full +> scope instead, with the risk called out explicitly for #3 before confirming). See "Decisions" +> below for the full record. + +## Status as of 2026-09-22 + +**Phases 1–6 are ALL IMPLEMENTED** (commits `da07b38c` "add cliManagementEnabled flag and GET +/api/clis" and `db4557d9` "Phases 3-6 - write API + custom entries + Settings UI", both on this +branch, `feat/cli-management`). Confirmed present in the tree: `cliManagementEnabled` in +`SettingsUpdateSchema`; `GET /api/clis`, `PUT /api/clis/:id`, `POST /api/clis/:id/install`, +`POST /api/clis`, `PUT /api/clis/custom/:id`, `DELETE /api/clis/:id` in +`src/web/routes/cli-registry-routes.ts`; the `shell`/`claude` `UNDISABLEABLE_IDS` backend guard; +`isAdmin(req)` gating on both the list and write routes; `appendAdminAudit` wired into the install +route; tmp+rename+`0o600` writes in `registry-writer.ts`; the full Settings UI (row list, toggle, +Install button, custom-entry create/edit/delete form) in `settings-ui.js` + `index.html`. +`test/routes/cli-registry-routes.test.ts` (425 lines) and `test/cli-registry-no-id-branching.test.ts` +cover it. This status section, plus the fix and gap below, is the one piece of that work done in +a *different* session from the one that wrote Phases 1–6 — reviewed by reading the diff and +verifying each claim against the actual routes/tests, not by re-implementing anything. + +### Gotcha found and fixed (commit `0c77dd0a`) + +**Toggling a CLI off in Settings had no effect anywhere except the Settings row itself.** +`window.__codemanCliAvailable` — the flag `isCliAvailable()` reads client-side to gate the +welcome-screen buttons, the Run-menu dropdown and the mobile overview — is injected **once**, at +initial page render (`server.ts`), built purely from each CLI's own installed-on-PATH resolver +(`isClaudeAvailable()` etc.), with **no reference to the registry's `enabled` flag at all**. So +disabling a CLI here updated its own row and nothing else — every launch surface kept offering it, +both live and after a full page reload, since even a *fresh* render never consulted the registry. +Root-caused and reported by the user testing the live feature ("toggle those off, they still +appear in that menu and on the front main screen"). + +Fixed two places: +- `server.ts`: after building `available`, intersect the nine real `SessionMode` ids against + `enabledClis()`. `git`/`cloudflared` (utility binaries, not CLI registry entries) and + `deepseekBinary` (a secondary installed-only flag for the "add a profile" affordance) are + deliberately left alone — they were never registry-gated to begin with. +- `settings-ui.js`: `toggleCliEnabled()` now patches `window.__codemanCliAvailable` in place and + refreshes the welcome screen, the mobile overview and an already-open Run menu, mirroring the + existing `installDeepSeekProfile()` pattern for the same "injected once, needs an explicit + patch" reason — the server-side fix alone still left every surface stale until the next reload. + +New test in `test/render-index-html.test.ts`: an installed-but-disabled CLI (codex, forced via +`clis.json` + `reloadCliRegistry()`) reads as unavailable, while an installed-and-enabled one +(claude) is unaffected by the override. + +**Verified on the Debian devbox** (`codeman-devbox`, real tmux — this sandbox has none and +`WebServer`'s constructor hard-requires it): typecheck clean, the new test passes (17/17 in +`render-index-html.test.ts`), the CLI-registry suites pass (86/86), and the **full CI gate is +green — 415 test files, 7855 tests, 0 failures**. + +### Launch-surface registry integration — completed + +The welcome screen, desktop Run menu and mobile Run picker now use the same injected CLI catalog. +Every enabled registry entry is rendered; unavailable binaries remain hidden as before. Settings +updates the catalog and availability flags in place after enable/disable, create, edit or delete, +so the launch surfaces update without a page reload. A custom entry uses the generic quick-start +path, while stock entries retain their existing per-CLI launch settings. + +Not otherwise re-verified line-by-line against every Phase 1–6 checklist item below (e.g. the +exact wording of toasts, the "same PR" sequencing notes) — the checklists are left as originally +written; treat the **Status** section above as authoritative for what exists. + +--- + +## Background + +`src/config/cli-registry/registry.ts` is READ-ONLY today, and says so in its own header comment: + +> "⚠️ READ-ONLY. Nothing in this module writes, creates or migrates the file... there is no +> settings UI and no write API yet... A `seededStockIds` ratchet belongs with the write API that +> needs it." + +Confirmed on `master` (2026-09-21): no `/api/clis` route exists at all (read or write); +`~/.codeman/clis.json` is hand-edit-only; `resolveInstallCommandForPlatform()` is documented +"Display text only — never executed" — nothing runs an install command server-side today. The +original #343 review flagged the opposite (`spawn(command, {shell: true})`, `env.allowedPrefixes` +contributed from a write) as needing its own trust-model decision; that decision was never made +after the split, just dropped. This plan makes it. + +**Closest existing precedent, and the template this plan follows for the read/write API**: +`src/web/routes/custom-model-routes.ts` + `src/custom-model-hosts.ts` (#393/#430/#459) — a small +per-item JSON store, Settings-UI-driven, admin-gated in multi-user mode, tmp+rename+0600 writes. + +**Precedent for the new master feature flag (Phase 1)**: `customModelEndpointsEnabled` — +`z.boolean().optional()` in `SettingsUpdateSchema` (`schemas.ts:1319`), a checkbox read/written by +id in `openAppSettings()`/`saveAppSettings()` (`settings-ui.js:401`/`:2120`). SYNCED, not +per-device (present in the schema, absent from `displayKeys`), default OFF. + +**Spec refs for the whole plan:** +- `src/config/cli-registry/registry.ts` — the read path; `resolveRegistry()`'s merge semantics + (`deepMerge`, `UNMERGEABLE_KEYS`) apply unchanged to whatever this plan writes +- `docs/cli-registry.md` — registry shape, "The override file", "Arg-template safety" (the four + layers Phase 5's custom-entry validation must not weaken), "Adding a CLI" (the 5-step recipe a + custom entry does NOT get to skip just because it arrives via UI instead of a stock.ts edit) +- `src/web/routes/custom-model-routes.ts` + `src/custom-model-hosts.ts` — read/write API template +- `docs/multi-user-plan.md`, `docs/security-architecture.md` — admin-gating conventions +- `CLAUDE.md` §Multi-user mode, §"Settings surface", §"Per-device vs synced settings" + +--- + +## Decisions (Phase 0, closed 2026-09-21) + +1. **Enable/disable a stock CLI's `enabled` flag** — IN SCOPE. Plus a **master feature flag** + (`cliManagementEnabled`, synced, default OFF) gating the whole Settings UI section's visibility, + matching this codebase's standing convention for new admin-facing surfaces. +2. **Auto-install** (stock CLIs' already-shipped, already-vetted install commands) — IN SCOPE, + same PR. +3. **Custom CLI entries via the UI** — IN SCOPE, **typed-argv only**: a custom entry goes through + the exact same schema/argv-safety path stock entries do (named token patterns, no raw shell-text + field). Its install command stays **display-only text**, same as every stock entry today — Phase + 4's auto-install NEVER executes a custom entry's install command, only a stock one's. This is + the one place scope was deliberately narrowed relative to what was agreed in principle, because + `docs/cli-registry.md`'s arg-template-safety section exists specifically to keep config free of + shell text, and a free-text install command for a user-defined entry would reopen exactly that. +4. **`shell`/`claude` un-disableable** — enforced at the **backend**, not just the UI (a + frontend-only guard is bypassable with curl). +5. **Non-admin visibility in multi-user mode** — the CLI-management Settings section is **hidden + entirely** for a non-admin, not shown-empty. +6. **`seededStockIds` ratchet** — not needed. `deepMerge()` only overrides a key the file actually + sets, so a CLI absent from `clis.json.clis` always falls through to its stock `enabled` value + with no special-casing. (Carried over from the first draft, not re-litigated.) + +--- + +## Phase 1 — Master feature flag: `cliManagementEnabled` + +**Status:** DONE (commit `da07b38c`) — verified present in `SettingsUpdateSchema`, `index.html`, +`openAppSettings()`/`saveAppSettings()`. + +**Spec refs:** +- `schemas.ts:1319` (`customModelEndpointsEnabled`) — the exact pattern to mirror: `z.boolean().optional()` + in `SettingsUpdateSchema` +- `settings-ui.js:401`/`:2120` — checkbox read/write by id in `openAppSettings()`/`saveAppSettings()` +- `CLAUDE.md` §"Adding Features" → "App setting" — decide per-device vs synced FIRST (this one is + synced: a feature toggle, not a display preference) and add to `displayKeys` NEVER for a synced + setting + +**Checklist:** +- [x] Add `cliManagementEnabled: z.boolean().optional()` to `SettingsUpdateSchema` +- [x] Add the checkbox to `index.html`'s `#settings-clis` section, above where Phase 6's per-CLI + list will render — reads/writes via `openAppSettings()`/`saveAppSettings()` by id, same as + `customModelEndpointsEnabled` +- [x] `readCliManagementEnabled()` helper (mirrors `readCustomModelEndpointsEnabled()` in + `custom-model-routes.ts:609`) for the route file(s) in Phases 2-5 to gate on +- [x] When OFF: `GET /api/clis` still exists but the Settings UI section stays hidden + (`applyCliManagementVisibility()`); the write endpoints reject (see Phase 3) + +**Verify:** `npm run typecheck` passes; a unit test confirms `SettingsUpdateSchema` accepts/rejects +the field correctly; toggling it in a fresh browser profile shows/hides the Settings section with +no server restart. + +--- + +## Phase 2 — Read endpoint: `GET /api/clis` + +**Status:** DONE (commit `da07b38c`) — verified present in `src/web/routes/cli-registry-routes.ts`. + +**Spec refs:** +- `src/web/routes/custom-model-routes.ts:730` (`GET /api/model-endpoints`) — multi-user read + gating: empty list for a non-admin, never a 403 +- `src/config/cli-registry/registry.ts` — `listClis()` (every entry, including disabled stock + ones — this is an admin/settings surface, unlike `enabledClis()`) +- `window.__codemanCliAvailable`'s resolvers (`isClaudeAvailable()` etc.) — candidate `installed` + source; confirm whether to reuse directly or the response needs its own probe (Open Question 4, + carried from the first draft — still genuinely open, decide during this phase not before) + +**Checklist:** +- [x] New route file `cli-registry-routes.ts` +- [x] Response excludes `launch`/`env`/`capabilities`/`overlays`/`discovery` +- [x] `isMultiUserMode() && !isAdmin(req)` → `[]` +- [x] Unit tests in `test/routes/cli-registry-routes.test.ts` (admin/non-admin/single-user, + disabled stock CLI still present) + +**Verify:** `npm test -- test/routes/cli-registry-routes.test.ts` passes; `curl localhost:3000/api/clis | jq` +shows every stock CLI including disabled ones. + +--- + +## Phase 3 — Write endpoint: `PUT /api/clis/:id` (stock enable/disable) + +**Status:** DONE (commit `db4557d9`) — `UNDISABLEABLE_IDS`, admin gate, tmp+rename+0600 all +confirmed present. + +**Spec refs:** +- `src/web/routes/custom-model-routes.ts:753` + `src/custom-model-hosts.ts:91` — write-path + template: `adminOnly` gate, read-modify-write the WHOLE file, tmp+rename+0600 +- `registry.ts:47` (`filePath()` = `dataPath(...)`) and `reloadCliRegistry()` — write to the same + resolved path, invalidate the cache on every successful write or the change is invisible until + restart + +**Checklist:** +- [x] Body: `{ enabled: boolean }`. Zod schema in `schemas.ts` +- [x] Gate order: `cliManagementEnabled` → `adminOnly` → shell/claude guard → stock-only guard +- [x] Rejects disabling `shell` or `claude` (`UNDISABLEABLE_IDS`) +- [x] Rejects a write for an id that isn't a stock CLI +- [x] Deep-merges `{ clis: { [id]: { enabled } } }`, preserving other override keys +- [x] tmp+rename+0600 write, `reloadCliRegistry()` on success +- [x] Unit tests (`test/routes/cli-registry-routes.test.ts`) + +**Verify:** `npm test` full gate green; `curl -X PUT localhost:3000/api/clis/grok -d '{"enabled":false}'` +then `GET /api/clis` shows the change with no restart; same against `shell`/`claude` returns an +error and changes nothing; `ls -la ~/.codeman/clis.json` shows mode 0600. + +--- + +## Phase 4 — Auto-install: `POST /api/clis/:id/install` (stock CLIs only) + +**Status:** DONE (commit `db4557d9`) — route present, `appendAdminAudit` wired in. + +**Spec refs:** +- `registry.ts:231` (`resolveInstallCommandForPlatform`) — currently "Display text only — never + executed"; this phase is what changes that, for stock entries only, with Decision 2's sign-off +- Original #343 review's exact concern re: `env.allowedPrefixes` contributed from a write — stays + out of scope; this phase only ever runs a command, never touches the env allowlist + +**Checklist:** +- [x] Separate endpoint from Phase 3's toggle +- [x] Gate order: `cliManagementEnabled` → `adminOnly` → stock-entry-only guard +- [x] `resolveInstallCommandForPlatform(entry)` for the target +- [x] Bounded execution (timeout, captured stdout/stderr) +- [x] Does NOT auto-enable on successful install +- [x] Audit-logged via `appendAdminAudit` +- [x] Unit tests + +**Verify:** a real install triggered via the endpoint against a CLI not currently installed, +`GET /api/clis`'s `installed` field flips true with no restart; audit log entry present; attempting +install against a custom entry's id fails with a clear error; full CI gate green. + +--- + +## Phase 5 — Custom CLI entries: create / update / delete via API + +**Status:** DONE (commit `db4557d9`) — `POST /api/clis`, `PUT /api/clis/custom/:id`, +`DELETE /api/clis/:id` all present. Open Question 2 resolved: a **separate** endpoint +(`PUT /api/clis/custom/:id`), not Phase 3's `PUT /api/clis/:id` widened. + +**Spec refs:** +- `docs/cli-registry.md` §"Arg-template safety" (all four layers), §"Adding a CLI" (the 5-step + recipe) — a custom entry created via this API must satisfy the SAME schema (`CliEntrySchema`) + every stock entry does; there is no relaxed path for UI-originated entries +- `registry.ts`'s `resolveRegistry()` — the custom-entry branch (`stock: false`, dropped with a + warning on validation failure, never falls back silently) already exists and is unchanged by + this phase; this phase only adds a way to WRITE what that branch reads + +**Checklist:** +- [x] `POST /api/clis` (create), full `CliEntrySchema` validation +- [x] `PUT /api/clis/custom/:id` (update) — separate endpoint from Phase 3's stock toggle +- [x] `DELETE /api/clis/:id` refuses for any stock id +- [x] `id` collision check against existing stock ids +- [x] `discovery.install.command` on a custom entry stays DISPLAY-ONLY +- [x] Same tmp+rename+0600 write pattern, `reloadCliRegistry()` on every successful mutation +- [x] Unit tests + +**Verify:** `npm test` full gate green; create a custom entry via curl, confirm it appears in +`GET /api/clis` — **confirm it appears in the Run menu is UNVERIFIED and currently FALSE, see +"Outstanding" above**; delete it, confirm it's gone and `clis.json` no longer references it. + +--- + +## Phase 6 — Settings UI + +**Status:** DONE (commit `db4557d9`) — `#cliListGroup`, row rendering, toggle, Install button, +custom-entry create/edit/delete form all present in `settings-ui.js`/`index.html`. Manual browser +verification per the phase's own "Verify" step (flag on/off, non-admin hidden, toggle stops the +Run menu offering a CLI, create/enable/launch a custom entry, delete it, shell/claude undisableable) +has **not** been re-run in this session — the toggle→Run-menu leg specifically was BROKEN until the +gotcha fix above, and the create→launch leg for a custom entry is the confirmed gap in +"Outstanding". + +**Spec refs:** +- `index.html:2357` (`#settings-clis`) — the existing home; Phase 1's master toggle at the top, + then the per-CLI list, then (if `cliManagementEnabled`) a "custom CLI" creation form, all above + the existing Codex-only groups +- `CLAUDE.md` §"Settings surface" — App Settings scrolls, it does not tab-switch +- `admin-ui.js` — pattern for an admin-only-VISIBLE section (not just admin-only-writable), + needed here per Decision 5 + +**Checklist:** +- [x] Whole section hidden when `cliManagementEnabled` is OFF, and separately hidden for a + non-admin in multi-user mode (`_applyCliManagementAdminGate`) +- [x] Fetches `GET /api/clis` when the section becomes visible; renders one row per CLI +- [x] Stock rows: enabled toggle only; `shell`/`claude` rows show the toggle disabled/greyed +- [x] Custom rows: enabled toggle plus edit/delete affordances +- [x] "Add custom CLI" form (id/label/badge/binary/argv) +- [x] Toggle/edit/delete update the row in place + +**Verify:** manual browser test per `CLAUDE.md`'s "Always Test Before Deploying" rule — **not yet +re-run end-to-end in this session**; do this before considering the feature ready to ship, and +expect the custom-entry-launch step to fail until the Outstanding gap above is closed. + +--- + +## Remaining Open Questions + +1. **Phase 2's `installed` source** — resolved: reuses `window.__codemanCliAvailable`'s existing + resolvers via `GET /api/clis`'s own probe (confirmed by reading the route). +2. **Phase 5's `PUT` endpoint shape** — resolved: a **separate** endpoint + (`PUT /api/clis/custom/:id`), not Phase 3's toggle route widened. +3. **Sequencing against the parked Copilot plan** — unchanged, still not blocking. +4. **NEW: custom-CLI Run-menu integration** — see "Outstanding" above. Not decided or started. + +--- + +Implementation is underway (see Status above); this line is left for history rather than removed — +the plan was originally approved before Phases 1–6 landed. diff --git a/docs/cli-registry.md b/docs/cli-registry.md index a66c080c..48cfee37 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -18,7 +18,17 @@ Every run mode Codeman can launch — Claude Code, Terminal/Shell, OpenCode, Cod ## The override file -`~/.codeman/clis.json` (instance-scoped through `dataPath()`) holds overrides and custom entries only, never a copy of the stock catalog: `{ "clis": { "": { ...partial entry... } } }`. Objects merge key-wise onto the stock entry, arrays replace wholesale. **The file must be mode 0600**; the loader refuses any group/world permission bit, read bits included, so a file created with a normal umask (0644) is ignored until you `chmod 600` it. Every reason a file was ignored or an entry dropped is logged once, prefixed `[cli-registry]`, on the first load. A stock entry whose override fails validation falls back to the shipped definition; a custom entry that fails is dropped. The file is read once per process and re-read only on restart. +`~/.codeman/clis.json` (instance-scoped through `dataPath()`) holds overrides and custom entries only, never a copy of the stock catalog: `{ "clis": { "": { ...partial entry... } } }`. Objects merge key-wise onto the stock entry, arrays replace wholesale. **The file must be mode 0600**; the loader refuses any group/world permission bit, read bits included, so a file created with a normal umask (0644) is ignored until you `chmod 600` it. Every reason a file was ignored or an entry dropped is logged once, prefixed `[cli-registry]`, on the first load. A stock entry whose override fails validation falls back to the shipped definition; a custom entry that fails is dropped. The file is read once per process and re-read after a change made through CLI management (below). + +## Managing CLIs from Settings + +App Settings → Agents & CLIs → **CLI management** (`cliManagementEnabled`, default OFF; admin-only in multi-user mode) lists every entry with an installed/not-installed badge and: + +- toggles any entry on or off. A `kind: 'shell'` entry cannot be disabled, and the row shows no switch for it. A disabled CLI disappears from the Run menu, the welcome screen and the phone overview, and new session requests for it are rejected. +- installs a missing **stock** CLI by running its shipped install command, after a confirm that names the exact command. Only one install per CLI runs at a time, and the command runs without any `CODEMAN_*` variable in its environment. A custom entry's install command is never executed. +- adds, edits and deletes **custom** entries (id, label, badge, binaries, launch argv). The server re-validates the whole assembled entry through `CliEntrySchema`, so the form cannot bypass the load-time rules. + +These are the only writes to `clis.json`. They are serialized, and a file that does not parse or has unsafe permissions is refused rather than overwritten; fix it (or `chmod 600` it) and retry. The HTTP routes are listed in `docs/api-reference.md` under *CLI management*. ## The shape of an entry @@ -149,7 +159,7 @@ This matters because it is invisible when it is wrong. `capabilities.privilegedP ## Fields declared for later -`shortBadge`, `accent`, `capabilities.echo`, `capabilities.wheelForward`, `capabilities.keyboardAccessory` and `capabilities.maxFrameBytes` are **declared but not yet read**. They all describe frontend behaviour, and the frontend is deliberately untouched here: `app.js`, `terminal-ui.js` and `styles.css` keep their own hand-authored per-CLI rules, and moving them is its own piece of work verified by a browser/mobile suite the CI gate cannot see. +`accent`, `capabilities.echo`, `capabilities.wheelForward`, `capabilities.keyboardAccessory` and `capabilities.maxFrameBytes` are **declared but not yet read**. (`shortBadge` was on this list until the CLI management list in Settings started showing it.) They all describe frontend behaviour, and the frontend is deliberately untouched here: `app.js`, `terminal-ui.js` and `styles.css` keep their own hand-authored per-CLI rules, and moving them is its own piece of work verified by a browser/mobile suite the CI gate cannot see. Treat those values as **transcribed, not authoritative** — nothing enforces that `echo.policy` matches `_updateLocalEchoState`'s fallthrough, so re-measure before wiring one up. `accent` is the one exception: it was measured against styles.css on 2026-09-21 (method in the comment above `CLAUDE` in `stock.ts`), though nothing keeps it in step with the CSS either. A field that is both wrong and unread is worse than an absent one, because the next reader trusts it; `test/cli-registry-no-id-branching.test.ts` pins the list so it cannot quietly grow, and wiring one up makes its line there fail, which is the direction you want. diff --git a/install.sh b/install.sh index 4e394c6a..71a8a4b3 100755 --- a/install.sh +++ b/install.sh @@ -169,7 +169,7 @@ export PUPPETEER_SKIP_DOWNLOAD="${PUPPETEER_SKIP_DOWNLOAD:-1}" # commit as the script itself. Nothing fetched at install time is ever executed; there is # no network refresh of these arrays. See cli_catalog_select_platform below. CLI_IDS=('claude' 'shell' 'opencode' 'codex' 'gemini' 'antigravity' 'pi' 'grok' 'deepseek' 'omp') -CLI_LABELS=('Claude' 'Shell' 'OpenCode' 'Codex' 'Gemini' 'Antigravity' 'Pi' 'Grok' 'DeepSeek' 'OMP') +CLI_LABELS=('Claude Code' 'Shell' 'OpenCode' 'Codex' 'Gemini' 'Antigravity' 'Pi' 'Grok' 'DeepSeek' 'OMP') CLI_ENABLED=(1 1 1 1 1 1 1 1 1 1) CLI_LAUNCHER_ONLY=(0 0 0 0 0 0 0 0 1 0) CLI_DOCS=('https://docs.claude.com/claude-code' '' 'https://opencode.ai/docs' 'https://developers.openai.com/codex/cli' 'https://github.com/google-gemini/gemini-cli' 'https://antigravity.google/cli' 'https://pi.dev' 'https://github.com/xai-org/grok-build' 'https://github.com/deepseek-ai/deepseek-harness' 'https://omp.sh') diff --git a/src/config/cli-registry/registry-writer.ts b/src/config/cli-registry/registry-writer.ts new file mode 100644 index 00000000..b1de73b7 --- /dev/null +++ b/src/config/cli-registry/registry-writer.ts @@ -0,0 +1,115 @@ +/** + * @fileoverview Write side of the CLI registry (docs/cli-enable-disable-plan.md, Phases 3/5). + * + * Kept deliberately SEPARATE from `registry.ts`, whose reading path does no writes on import + * (`schemas.ts` imports it, transitively). Only `cli-registry-routes.ts` imports this module, + * so that property still holds for every OTHER importer of the registry. + * + * Every mutation goes through `mutateRegistryFile()`, which does three things the #476 review + * found missing: + * + * - **Serialized.** Mutations run one at a time on a single promise chain, and each one + * reads, changes, writes and reloads before the next starts. Unserialized read-modify-write + * lost toggles when three `PUT /api/clis/:id` calls ran in parallel. + * - **Refuses a file it must not trust.** The reader ignores a `clis.json` with any + * group/world permission bit and quarantines one that does not parse. The writer used to + * treat both as "start fresh", so one Settings click replaced a hand-edited file with a + * one-key file, or rewrote a refused file as 0600 and so trusted it. It now starts fresh + * ONLY on ENOENT and otherwise throws `RegistryWriteRefusedError`, leaving the file alone. + * - **Unique temp file.** Every write gets its own tmp name before the rename, so two writes + * can never rename each other's temp file away (the ENOENT-on-rename 500s). + * + * Same tmp+rename+0600 shape as `custom-model-hosts.ts`. The file is hand-editable, so a + * write must never leave it half-written, and 0600 is the mode `isUnsafePermissions()` + * requires on the next read. + */ + +import { randomUUID } from 'node:crypto'; +import { existsSync, mkdirSync } from 'node:fs'; +import fs from 'node:fs/promises'; +import { dirname } from 'node:path'; +import { isUnsafePermissions, registryFilePath, reloadCliRegistry } from './registry.js'; +import type { CliRegistryFile } from './types.js'; + +/** A write refused because the existing `clis.json` must not be overwritten. The message is user-facing. */ +export class RegistryWriteRefusedError extends Error { + constructor(message: string) { + super(message); + this.name = 'RegistryWriteRefusedError'; + } +} + +/** + * Read the raw override file for mutation. Only a MISSING file starts fresh. A file with + * unsafe permissions, one that cannot be read, or one that does not parse is refused rather + * than overwritten, because the user's hand-edit is worth more than one toggle. + */ +export async function readRegistryFileForWrite(): Promise { + const path = registryFilePath(); + let raw: string; + try { + raw = await fs.readFile(path, 'utf-8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return { schemaVersion: 1, clis: {} }; + throw new RegistryWriteRefusedError(`Cannot read ${path} (${(err as Error).message}); not changing it.`); + } + if (isUnsafePermissions(path)) { + throw new RegistryWriteRefusedError( + `${path} has group/world permission bits, so Codeman ignores it. Run \`chmod 600 ${path}\` and check its contents before changing CLIs here.` + ); + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new RegistryWriteRefusedError( + `${path} is not valid JSON (${(err as Error).message}). Fix or remove it before changing CLIs here.` + ); + } + const clis = (parsed as { clis?: unknown } | null)?.clis; + if (typeof parsed !== 'object' || parsed === null || typeof clis !== 'object' || clis === null) { + throw new RegistryWriteRefusedError(`${path} has no "clis" object. Fix or remove it before changing CLIs here.`); + } + return parsed as CliRegistryFile; +} + +export async function writeRegistryFile(file: CliRegistryFile): Promise { + const target = registryFilePath(); + const dir = dirname(target); + if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + const tmp = `${target}.${process.pid}.${randomUUID()}.tmp`; + try { + await fs.writeFile(tmp, JSON.stringify(file, null, 2), { mode: 0o600 }); + await fs.rename(tmp, target); + } catch (err) { + await fs.rm(tmp, { force: true }).catch(() => {}); + throw err; + } +} + +let mutationChain: Promise = Promise.resolve(); + +/** + * Run one registry mutation. The chain holds exactly one at a time: `fn` receives the + * current file and returns `{ file, result }`. If `file` is set it is written and the + * registry reloaded before the next mutation starts; if not, nothing is written, which is + * how a validation failure returns early. Checks made inside `fn` (does this id exist, + * is it a duplicate) therefore see every earlier mutation's result. + * + * A failed mutation rejects its own caller only. The chain keeps going. + */ +export function mutateRegistryFile( + fn: (file: CliRegistryFile) => Promise<{ file?: CliRegistryFile; result: T }> | { file?: CliRegistryFile; result: T } +): Promise { + const run = mutationChain.then(async () => { + const current = await readRegistryFileForWrite(); + const { file, result } = await fn(current); + if (file) { + await writeRegistryFile(file); + reloadCliRegistry(); + } + return result; + }); + mutationChain = run.catch(() => {}); + return run; +} diff --git a/src/config/cli-registry/registry.ts b/src/config/cli-registry/registry.ts index cd9c590c..8f251eeb 100644 --- a/src/config/cli-registry/registry.ts +++ b/src/config/cli-registry/registry.ts @@ -48,6 +48,16 @@ function filePath(): string { return dataPath('clis.json'); } +/** + * The resolved path of `~/.codeman/clis.json`, exported for the write API + * (`cli-registry-writer.ts`, docs/cli-enable-disable-plan.md Phases 3/5) so both the read and + * write sides resolve the SAME path through the SAME instance-scoped helper — never a second + * `dataPath('clis.json')` call that could drift from this one under a future `dataPath()` change. + */ +export function registryFilePath(): string { + return filePath(); +} + /** * Keys that must never be merged out of a hand-editable JSON file. * @@ -90,8 +100,11 @@ export interface LoadResult { * as mode 0o666 there regardless of its actual ACL), so this check would flag every file on * Windows and silently ignore all user config. `win32` relies on NTFS ACLs instead, which * this check cannot see and does not attempt to. + * + * Exported for `registry-writer.ts`, which must refuse the same files: rewriting a refused + * file as 0600 would silently turn it into trusted config. */ -function isUnsafePermissions(path: string): boolean { +export function isUnsafePermissions(path: string): boolean { if (process.platform === 'win32') return false; try { const mode = statSync(path).mode & 0o777; diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index c2faf1ea..94d2e3b7 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -90,7 +90,7 @@ function agentDefaults(): Pick< // `accent` still has no reader, so nothing rendered changes because of it. const CLAUDE: CliEntry = { id: 'claude' as CliEntry['id'], - label: 'Claude', + label: 'Claude Code', shortBadge: 'CC', accent: '#3b82f6', enabled: true, diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 10e48e11..e8a20b3e 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -670,12 +670,14 @@ export interface CliOverlays { /** * ⚠️ DECLARED-FOR-LATER: fields no code reads yet. * - * `shortBadge`, `accent`, `overlays.credStore`, `capabilities.echo`, `capabilities.wheelForward`, + * `accent`, `overlays.credStore`, `capabilities.echo`, `capabilities.wheelForward`, * `capabilities.keyboardAccessory` and `capabilities.maxFrameBytes` all describe FRONTEND - * behaviour, and the frontend is deliberately untouched by the change that introduced this - * registry — `app.js`, `terminal-ui.js`, `styles.css` and friends keep their own + * behaviour, and most of the frontend is deliberately untouched by the change that introduced + * this registry — `app.js`, `terminal-ui.js`, `styles.css` and friends keep their own * hand-authored per-CLI rules, and moving them is its own piece of work with its own way of - * being verified (a mobile/browser suite the CI gate cannot see). + * being verified (a mobile/browser suite the CI gate cannot see). `shortBadge` graduated out of + * this list (docs/cli-enable-disable-plan.md, Phase 2): `GET /api/clis` reads it for the + * CLI-management Settings list. * * They are declared now because each entry should describe its CLI completely, and because * transcribing them while the hand-written source is still on screen is when the values are diff --git a/src/config/dependency-registry.ts b/src/config/dependency-registry.ts index dd589818..96512a36 100644 --- a/src/config/dependency-registry.ts +++ b/src/config/dependency-registry.ts @@ -69,7 +69,9 @@ const ALL: ProbeEnvironment[] = ['linux', 'darwin', 'wsl', 'win32']; * shown to the user and claude's does not follow the pattern. */ const DOCTOR_ROW_OVERRIDES: Record = { - claude: { usedBy: ['Claude Code sessions (default backend)'] }, + // The label override keeps the doctor row's historical "Claude CLI" spelling now that + // the registry label is the product name, "Claude Code". + claude: { label: 'Claude CLI', usedBy: ['Claude Code sessions (default backend)'] }, opencode: { usedBy: ['OpenCode sessions'] }, codex: { usedBy: ['Codex sessions'] }, gemini: { usedBy: ['Gemini sessions'] }, diff --git a/src/utils/cli-executable-resolver.ts b/src/utils/cli-executable-resolver.ts index 10bf9723..81eb78c6 100644 --- a/src/utils/cli-executable-resolver.ts +++ b/src/utils/cli-executable-resolver.ts @@ -204,6 +204,28 @@ export function createProductionCliResolverHost(options: ProductionCliResolverHo }; } +/** + * Per-binary invalidation generation, bumped by `invalidateCliExecutableResolvers()`. + * + * Every resolver instance (each per-CLI module's private one AND the generic registry + * resolver in cli-resolver.ts) is built by the factory below and caches in its own + * closure, so there is no instance to reach from outside. Keying on the BINARY name is + * what lets one call reach all of them: the CLI-management install/update routes know + * which binaries just changed, and every resolver knows its own. + */ +const binaryGenerations = new Map(); + +/** + * Forget every cached result — success and negative-cache backoff alike — for these + * binaries, so the next `resolve()` re-runs the chain immediately. For an action that + * just changed what is on disk (an install) or what a CLI's binary IS (editing a custom + * entry): without it a CLI installed from Settings kept reading as missing for up to the + * 5-minute backoff, and an edited entry kept launching its old binary until a restart. + */ +export function invalidateCliExecutableResolvers(binaries: readonly string[]): void { + for (const binary of binaries) binaryGenerations.set(binary, (binaryGenerations.get(binary) ?? 0) + 1); +} + export function createCliExecutableResolver( options: { binary: string; @@ -235,6 +257,8 @@ export function createCliExecutableResolver( let failures = 0; /** Timestamp of the most recent miss. */ let lastFailureAt = 0; + /** The invalidation generation the cached state above belongs to. */ + let generation = binaryGenerations.get(options.binary) ?? 0; const accept = (path: string | null, source: CliResolutionSource): CliResolution | null => { if (!path || !isAbsolute(path) || !host.exists(path)) return null; const validation = options.validateCandidate?.(path) ?? ({ accepted: true } as CandidateValidation); @@ -249,6 +273,13 @@ export function createCliExecutableResolver( return { resolve() { + const current = binaryGenerations.get(options.binary) ?? 0; + if (current !== generation) { + generation = current; + cached = null; + failures = 0; + lastFailureAt = 0; + } if (cached) return cached; // Negative cache: a miss is remembered and the chain — whose login-shell // tail is a synchronous 5s-bounded spawn — is not re-run until the diff --git a/src/utils/cli-installed-probes.ts b/src/utils/cli-installed-probes.ts new file mode 100644 index 00000000..888216d4 --- /dev/null +++ b/src/utils/cli-installed-probes.ts @@ -0,0 +1,69 @@ +/** + * @fileoverview One answer to "is this CLI installed here?", shared by the page render + * (`renderIndexHtml` in server.ts, which injects `window.__codemanCliAvailable` and + * `window.__codemanCliCatalog`) and `GET /api/clis` (the Settings list's badge). + * + * The two used to keep their own copies of the per-CLI probe map, so they could drift apart + * and the badge could disagree with the Run menu. + * + * Every probe is a memoized resolver, so this is cheap to call per request. Dynamic imports + * keep the nine resolvers out of any module that never asks. + */ + +import type { CliEntry } from '../config/cli-registry/types.js'; +import { isCliAvailable as isRegistryCliAvailable } from './cli-resolver.js'; + +/** + * The stock CLIs whose own resolver answers availability. It keeps the resolver's specific + * semantics (pi/grok/deepseek identity probes). DeepSeek reports RUNNABLE here, not merely + * installed: `dsh` is a profile launcher, and a dsh with no pane-capable profile would + * offer a Run button that spawns a pane which dies on arrival. + */ +export async function probeStockCliAvailability(): Promise> { + const [ + { isClaudeAvailable }, + { isOpenCodeAvailable }, + { isCodexAvailable }, + { isGeminiAvailable }, + { isAntigravityAvailable }, + { isPiAvailable }, + { isGrokAvailable }, + { isDeepSeekRunnable }, + { isOmpAvailable }, + ] = await Promise.all([ + import('./claude-cli-resolver.js'), + import('./opencode-cli-resolver.js'), + import('./codex-cli-resolver.js'), + import('./gemini-cli-resolver.js'), + import('./antigravity-cli-resolver.js'), + import('./pi-cli-resolver.js'), + import('./grok-cli-resolver.js'), + import('./deepseek-cli-resolver.js'), + import('./omp-cli-resolver.js'), + ]); + return { + claude: isClaudeAvailable(), + opencode: isOpenCodeAvailable(), + codex: isCodexAvailable(), + gemini: isGeminiAvailable(), + antigravity: isAntigravityAvailable(), + pi: isPiAvailable(), + grok: isGrokAvailable(), + deepseek: isDeepSeekRunnable(), + omp: isOmpAvailable(), + }; +} + +/** + * Is `entry` installed? A shell entry has no binary to probe, since it is the server's own + * login shell. A stock entry with a dedicated resolver uses `stockAvailability`. Anything + * else, custom entries included, uses the registry's GENERIC resolver. That is the one a + * session spawn uses, and it understands the entry's declared binaries and search dirs. + */ +export function isCliEntryInstalled(entry: CliEntry, stockAvailability: Record): boolean { + if (entry.kind === 'shell') return true; + const id = entry.id as string; + return Object.prototype.hasOwnProperty.call(stockAvailability, id) + ? stockAvailability[id] + : isRegistryCliAvailable(id); +} diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index b368fbd2..363fc253 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -111,11 +111,13 @@ Run: '运行', 'Run Claude Code': '运行 Claude Code', 'Run OpenCode': '运行 OpenCode', + 'Run Codex': '运行 Codex', 'Run Gemini': '运行 Gemini', 'Run Antigravity': '运行 Antigravity', 'Run Pi': '运行 Pi', 'Run Grok': '运行 Grok', 'Run DeepSeek': '运行 DeepSeek', + 'Run OMP': '运行 OMP', 'Run Shell': '运行 Shell', 'Select AI backend': '选择 AI 后端', 'Create New Case': '新建案例', diff --git a/src/web/public/index.html b/src/web/public/index.html index eff75353..41cd4b58 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -451,42 +451,11 @@

Codeman

Manage AI Coding tools in persistent tmux sessions.

- +
- - - - - - -
@@ -648,39 +617,13 @@
- - - - - - - - +
-