From 10f87428c3b98792f6b1229e41a3735a556d0427 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 23 Sep 2026 11:36:21 +0200 Subject: [PATCH] fix(cli-registry): merge-time fixes for the run-button accents (#463) - mobile.css: gemini and antigravity run/gear rules get `!important` like pi/omp/grok/deepseek, so the gear half no longer keeps the skin accent while the body takes the mode colour (two-tone button on the default skin). - test/skin-themes.test.ts: static guard that every run mode with a base `.btn-toolbar.btn-run.mode-` rule also has a resting rule inside the `html:not([data-skin="og"])` block; ids are derived from the stylesheet. - stock.ts: grok's accent comment names zinc-300 (border/badge colour); gemini's accent is #8ab4f8 to match its tab badge and run-mode dot, noted as the one exception to the border-colour method. - types.ts: "(below)" -> "(above)". - docs/cli-registry.md, CLAUDE.md: `accent` is now measured, not transcribed. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/cli-registry.md | 2 +- src/config/cli-registry/stock.ts | 14 +++++++++----- src/config/cli-registry/types.ts | 2 +- src/web/public/mobile.css | 33 ++++++++++++++++---------------- test/skin-themes.test.ts | 29 ++++++++++++++++++++++++++++ 6 files changed, 58 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 038d5fe4..b83db57f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -225,7 +225,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Docker Compose deployment** (`docker/`, contributed): Codeman itself runs in a container and spawns Docker cases as **SIBLING** containers through the mounted host socket (Docker-outside-of-Docker), never nested. That inverts one assumption the bare-host path takes for granted: the daemon no longer shares Codeman's filesystem, so a bind source valid *inside* Codeman means nothing to it. `resolveDockerDaemonMountSource()` translates sources under HOME into the daemon's namespace via `CODEMAN_DOCKER_HOST_HOME`, and `CODEMAN_CASES_PATH` points the cases dir at a host-absolute bind mount so a workspace resolves to the SAME absolute path on both sides (which is what keeps the transcript projHash matching, per Docker cases above). ⚠️ **`CODEMAN_CASES_PATH` must move every consumer or none**: it is resolved once in `config/cases-dir.ts` because `src/cli.ts` resolves case paths too, and when only the server's `CASES_DIR` learned the override, `codeman skill install --case ` reported "Case not found" on exactly the deployment the override exists for. ⚠️ **`.dockerignore` patterns match the WHOLE context-relative path**, so a bare `.env` line excludes only the ROOT file: `docker/.env` (which holds `CODEMAN_PASSWORD` and any provider keys) rode `COPY . .` into the image until `**/.env` was added — verified in both directions with a real build context. ⚠️ A Compose LONG-form bind (`type: bind`) **creates a missing host source directory ROOT-OWNED** rather than refusing. `Start-Codeman.sh` pre-creates both `CODEMAN_APPDATA_PATH` and `CODEMAN_CASES_PATH` on the host before `up`, which is what keeps the daemon from ever having to materialise either as root in the first place; the container ALSO starts as root (`cap_add: [CHOWN, DAC_OVERRIDE, KILL, SETGID, SETUID]` against the base `cap_drop: ALL`; `test/docker-entrypoint.test.ts` pins that list) so `docker/entrypoint.sh` can correct a bind source that turns up root-owned anyway (a restored backup, a cleared directory, plain `docker compose up` run without the script) before dropping to `PUID:PGID` via `setpriv` — a directory owned by neither root nor `PUID:PGID` is never re-owned, since that ownership is not this container's to reassign; it is PROBED for writability as the runtime account (`setpriv ... test -w`, so ACLs, group-writable trees and CIFS/NFS mounts pass) and refused with a message naming path, owner and PUID:PGID if that fails. ⚠️ `KILL` is in that list for tini, not the entrypoint: `init: true` keeps tini as root while the server runs as PUID, and without CAP_KILL its SIGTERM forward fails and the server is SIGKILLed on every `compose down`/`restart` instead of flushing state. ⚠️ `/opt/codeman-cli` (the runtime-owned CLI prefix) is APPENDED to `PATH`, never prepended, and the entrypoint pins its own `PATH` to the system dirs: the root part of the start resolves `setpriv` by bare name, and a prefix ahead of `/usr/bin` let a planted `setpriv` run as uid 0 (measured). `CODEMAN_DOCKER_DISABLE_SWAP_LIMIT=1` drops `--memory-swap` (and filters only that one kernel warning) for hosts without swap accounting; `--memory` still applies. ⚠️ The deployment ALSO self-updates in place (the repo bind mount at `/opt/codeman` + a restart-by-exiting supervisor) — see Self-update below and `docs/docker-self-update.md` before touching `server.Dockerfile`, the compose file or `.env.example`, since each is an input to the updater's environment gate. `docs/docker-compose.md` + `docker/README.md` (user guides) -**CLI registry** (`src/config/cli-registry/`): every run mode is a `CliEntry` — discovery (search dirs, version + identity probes), the launch argv template, env handling, the `capabilities` flags that replace per-CLI branching, and the `overlays` that back the remote/docker pane commands. **No code outside `stock.ts` may branch on a CLI id**; behaviour that genuinely differs is either a capability field or a NAMED PROFILE selected by one (`profiles.ts`), and `test/cli-registry-no-id-branching.test.ts` fails the build if an id check reappears — it matches `===`, `!==`, `case '':` and `[...].includes(mode)`, because an earlier `===`-only version let 36 negated branches survive the conversion (including a seven-mode Ralph chain whose own comment asked the next person to keep it in step with `isExternalCliMode()` by hand). A second CI-gated guard, `test/frontend-cli-no-id-branching.test.ts`, covers the two frontend files the run-menu consolidation (#458) touched, `session-ui.js` and `mobile-overview.js`: its allowlist is keyed by expression rather than line number, with an occurrence count per entry, so a new branch reusing an already-approved expression fails as a count mismatch instead of riding in on the old approval. ⚠️ Config contains no shell text: an entry declares typed argv tokens, literals are validated against a safe-word pattern at LOAD time (a bad literal rejects the whole entry — a silently dropped `--no-approve` is not cosmetic), and values resolve through patterns NAMED in code, so a user `clis.json` cannot widen its own validation. ⚠️ `external`, `hooks` and `altScreen` are three INDEPENDENT capabilities on purpose; deriving one from another shipped the `until=stop`-hangs-on-shell bug. ⚠️ Three capability fields carry a REGEX from config (`discovery.version.regex`, `capabilities.workDetect.workingLine` and `capabilities.workDetect.watchingLine`) and all three must compile through `compileVersionRegex()`, which caps length and refuses nested quantifiers; `workingLine` is the one that runs on the PTY hot path. ⚠️ **`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, so re-measure before wiring one up; the list is pinned so it cannot quietly grow. 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). → `docs/cli-registry.md` +**CLI registry** (`src/config/cli-registry/`): every run mode is a `CliEntry` — discovery (search dirs, version + identity probes), the launch argv template, env handling, the `capabilities` flags that replace per-CLI branching, and the `overlays` that back the remote/docker pane commands. **No code outside `stock.ts` may branch on a CLI id**; behaviour that genuinely differs is either a capability field or a NAMED PROFILE selected by one (`profiles.ts`), and `test/cli-registry-no-id-branching.test.ts` fails the build if an id check reappears — it matches `===`, `!==`, `case '':` and `[...].includes(mode)`, because an earlier `===`-only version let 36 negated branches survive the conversion (including a seven-mode Ralph chain whose own comment asked the next person to keep it in step with `isExternalCliMode()` by hand). A second CI-gated guard, `test/frontend-cli-no-id-branching.test.ts`, covers the two frontend files the run-menu consolidation (#458) touched, `session-ui.js` and `mobile-overview.js`: its allowlist is keyed by expression rather than line number, with an occurrence count per entry, so a new branch reusing an already-approved expression fails as a count mismatch instead of riding in on the old approval. ⚠️ Config contains no shell text: an entry declares typed argv tokens, literals are validated against a safe-word pattern at LOAD time (a bad literal rejects the whole entry — a silently dropped `--no-approve` is not cosmetic), and values resolve through patterns NAMED in code, so a user `clis.json` cannot widen its own validation. ⚠️ `external`, `hooks` and `altScreen` are three INDEPENDENT capabilities on purpose; deriving one from another shipped the `until=stop`-hangs-on-shell bug. ⚠️ Three capability fields carry a REGEX from config (`discovery.version.regex`, `capabilities.workDetect.workingLine` and `capabilities.workDetect.watchingLine`) and all three must compile through `compileVersionRegex()`, which caps length and refuses nested quantifiers; `workingLine` is the one that runs on the PTY hot path. ⚠️ **`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. 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). → `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/CLI-info parsing, ❯-prompt readiness); these CLIs render their own TUIs, so readiness is output stabilization instead. ⚠️ **Work detection is no longer part of that gate**: it is per-CLI `capabilities.workDetect` data (see the ❯ note above), so a CLI that declares its own glyph and working line gets the same screen-probed idle confirmation claude gets, and one that declares neither keeps the output-stabilization behaviour. All eight **require tmux with no direct PTY fallback**, because secrets are injected via socket-scoped `tmux setenv` and never on the spawn command line. ⚠️ `run*()` in `session-ui.js` MUST unwrap the `{success,data}` envelope; reading the raw shape silently breaks the run. ⚠️ **Codex sessions use PREDICTIVE WRITE-THROUGH echo, never the buffer overlay** (`_localEchoPolicy` in `_updateLocalEchoState`, terminal-ui.js): codex's composer reacts per keystroke ("/" pops a live-filtering picker, arrows edit server-side state, the composer grows as it wraps), so buffer-until-Enter starved it into issues #218/#219/#220/#222 and stays disabled (`_localEchoEnabled` remains false for codex). Instead, `PredictiveEchoAddon` (separate `vendor/xterm-predictive-echo.js` bundle) paints each keystroke at the predicted cell while the wire path stays BYTE-IDENTICAL: the onData hook (`_predictHookOnData`) is a plain statement with no `return`, so control always falls through into the untouched send path — pinned by vm and E2E byte-identity tests. Predictions reconcile against the parsed buffer and only while the cursor sits on the measured composer row (`isCodexComposerRow`, `/^› /`). Codex also **drops keystrokes that share a PTY read with a bracketed paste**, so flushed text and the paste sequence must go out as separate delayed writes (mirroring the Enter branch's delayed `\r`). Tests: `test/local-echo-codex-gating.test.ts`, `test/codex-predictive-echo.test.ts` (E2E vs real codex), `packages/xterm-zerolag-input/test/codex-replay.test.ts`. ⚠️ **Pi is the opposite kind of CLI and needs the opposite instincts**: it has NO permission prompts and no sandbox, so there is no bypass flag to send and Codeman must not invent one; its privileged knob is the tri-state `approveProjectTrust` (`--approve`/`--no-approve`), which makes pi EXECUTE repo-local `.pi/extensions` TypeScript, so the multi-user clamp puts pi in the **materialize** branch (an absent config still yields `--no-approve` for a non-granted owner) and `--api-key` is never wired. Pi stays OUT of `isAltScreenStripMode()` (main-screen TUI, and its 0.84.0 fullscreen mode is runtime-switchable via `/settings`, where the alt screen is load-bearing), and lands on the `'buffer'` echo policy via the `_updateLocalEchoState` fallthrough. Pi's own tests: `test/pi-mode.test.ts`, `test/routes/external-cli-bypass-clamp.test.ts`; user guide `docs/pi-integration.md`. ⚠️ **Grok is codex-shaped on permissions but opencode-shaped on rendering**: its bypass switch is `alwaysApprove` (`--always-approve`, grok's `bypassPermissions` mode — the Run button sends it `true` like antigravity's, and the clamp's only-if-sent branch strips it for non-granted owners), while its fullscreen alt-screen TUI keeps it OUT of `isAltScreenStripMode()`; the resolver version-probes `grok --version` like pi's (npm squatters exist for the name — `GET /api/grok/status` surfaces path + version), and grok lands on the `'buffer'` echo policy via the fallthrough (UNMEASURED against a live authenticated session; if its composer turns out per-keystroke-reactive like codex, flip it to the `'off'` branch). Grok's own tests: `test/grok-mode.test.ts`, `test/grok-cli-resolver.test.ts`; user guide `docs/grok-integration.md`. ⚠️ **DeepSeek breaks three of this family's assumptions, so do not pattern-match it onto its siblings.** (1) The agent is a **PROFILE, not the binary**: `dsh` is a launcher over `$DSH_HOME/profiles/` and DeepSeek ships only `web`/`headless`/`base`, so the terminal front door is ALWAYS third-party and "installed" ≠ "runnable" — the Run button gates on `isDeepSeekRunnable()` (binary AND a pane-capable profile) while `isDeepSeekAvailable()` gates the "add a profile" affordance; a `web`/`headless` profile is refused at spawn because it cannot drive a pane. (2) The permission switch is the **`DSH_PERMISSION_MODE` env export, not a flag** (`read-only`/`workspace-write`/`danger-full-access`) — the harness has none, and this is the one legitimate exception to the effort-style env-var ban because it is read with `??` as a boot-time default, so it stays soft; absent = `workspace-write`, which asks, hence the only-if-sent clamp branch, clamping to `workspace-write` (never `read-only`, which would break the workspace). ⚠️ **That clamp needs a second half no other CLI needs**, because the switch is an env var and `DSH_*` is an allowlisted `envOverrides` prefix: `applyEnvOverrides()` runs AFTER `_configureCliEnv()` in tmux-manager, so a non-granted owner sending `DSH_PERMISSION_MODE` on the SAME request would land last and hand back exactly the privilege the config clamp removed. `clampEnvOverridesForOwner()` (session-routes.ts) DROPS `DSH_PERMISSION_MODE`, `DSH_HOME` and `DEEPSEEK_BASE_URL` for a non-granted owner (the last because `_configureCliEnv()` forwards the SERVER's own `DEEPSEEK_API_KEY` into the pane, so a redirected base URL would send it to a foreign host) (dropping falls through to what `_configureCliEnv()` exports, which is the clamped value); `DSH_HOME` is there because it points the launcher at a profile tree whose plugin code runs at BOOT, before any approval row applies. Every OTHER CLI's bypass is a command-line flag reachable only through its config, which is why the config clamp alone is the whole gate for them. (3) It is the **only non-claude mode that passes `hooksAvailableForMode()`**, and for it alone that predicate is a per-SESSION question rather than a per-mode one (`deepSeekConfig.statusReporting: false` disarms the bridge, so every call site passes `sessionHookOptions(session)`; answering from the mode there re-creates the infinite-wait-dressed-as-a-timeout the guard exists to prevent). It passes because the terminal front door reports idle/working/blocked to a supervisor over a generic env-gated contract and `deepseek-status-shim.ts` makes Codeman that supervisor — real `stop`/`blocked` signals, real Approvals Inbox items, plus the `agent_working` event that clears an alert answered in the terminal. ⚠️ The resolver needs the strictest identity probe of the family (`dsh --help` must say `DeepSeek Harness`) because Debian ships an unrelated `dsh` (dancer's shell) that would pass a version probe. Model is NOT a session field (it is a profile composition entry). ⚠️ `hooksAvailableForMode()` is about hook SIGNALS and is not a stand-in for "is this a claude session": Read My Mind and intent capture read Claude's own transcript and compare `mode === 'claude'` directly, because when `deepseek` earned a yes the shared predicate silently widened both to a mode with no transcript to read (pinned by a static check in `test/deepseek-mode.test.ts`). ⚠️ **It is also the only external CLI whose answers are READ FROM DISK rather than scraped off the pane**: `deepseek-transcript.ts` reads `$DSH_HOME/sessions///session.jsonl.zstd` and backs the `last-response` route for dsh, because the pane segmenter served dsh-TUI's ASCII-art SPLASH as the worker's answer (measured), which anything polling for a first answer reads as an answer. Three traps live in that file: dsh appends **one zstd FRAME per write** and Node's `zlib` zstd decoder stops at the first (a real 56-line transcript decoded as 1 line, so the module walks frame headers itself; a Node older than 22.15 has no zstd and falls back to the pane); every turn also records a **plugin-sourced `user/message`** (the runtime-context snapshot) that must not render as the user's words; and a failed `turn/end` is surfaced as `Turn error: …` rather than as an empty string that reads as "still thinking". ⚠️ Session→transcript pairing is by the header's own `cwd` plus a ±60 s boot window, never by reproducing dsh's directory mangling (which has already changed form once) — and NEVER by newest-mtime alone, which handed a fresh worker its predecessor's answer in the same case dir. DeepSeek's own tests: `test/deepseek-mode.test.ts`, `test/deepseek-cli-resolver.test.ts`, `test/deepseek-transcript.test.ts`; user guide `docs/deepseek-integration.md`. OMP (`omp`) needs no bypass flag (the CLI's own `~/.omp` config governs trust/model routing, defaulting to `tools.approvalMode: yolo`), so its registry entry declares `privilegedParams: []` and a launch spec that only ever passes `--model`/`--resume`/`--continue` — but the multi-user clamp is NOT a no-op for it: `OMP_*` is an allowlisted `envOverrides` prefix, and the two credential-resolution keys it admits, `OMP_AUTH_BROKER_URL`/`OMP_AUTH_BROKER_TOKEN`, are clamped in `clampEnvOverridesForOwner()` for a non-granted owner, the same shape as `DEEPSEEK_BASE_URL`. Separately, `PI_*` is already allowlisted (pi needs it) and omp reads several of its knobs too (`PI_CONFIG_DIR`, `PI_CODING_AGENT_DIR`, `PI_CODING_AGENT_SESSION_DIR`, `PI_SUBPROCESS_CMD`, `PI_SHELL_PREFIX`) — a redirected `PI_CONFIG_DIR` moves the `~/.omp` tree `omp-session-resolver.ts`/`omp-transcript.ts` hardcode, silently breaking pinning/history; this is a known gap shared with pi, not fixed here. → [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/docs/cli-registry.md b/docs/cli-registry.md index dbeb24c6..a66c080c 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -151,7 +151,7 @@ This matters because it is invisible when it is wrong. `capabilities.privilegedP `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. -Treat those values as **transcribed, not authoritative** — nothing enforces that `echo.policy` matches `_updateLocalEchoState`'s fallthrough, or that `accent` matches the gradient CSS paints, so re-measure before wiring one up. 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. +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. `overlays.credStore` is in the same category, for a sharper reason: the Docker credential-seeding path still reads its own `CRED_STORES` table, because this shape allows ONE store per CLI and the live table needs two for gemini (`.gemini` for the CLI's own auth plus `.config/gcloud` for Vertex), while deepseek declares none here even though `.dsh` is seeded. Wiring it means making the field an array and correcting those two entries — a change to credential seeding, which is simultaneously the worst thing here to get wrong and the least covered by tests, since every docker IO path is no-op'd under vitest. diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index 1b96e457..beba35f0 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -83,7 +83,10 @@ function agentDefaults(): Pick< // (e.g. claude was registered as Anthropic's brand orange, `#d97757`, but the // button renders blue): `docs/cli-registry.md`'s own "transcribed, not // authoritative, re-measure before wiring one up" warning for this -// DECLARED-FOR-LATER field, taken literally. This is a data-accuracy fix only — +// DECLARED-FOR-LATER field, taken literally. The one exception is GEMINI, whose +// run-button border (#60a5fa) is the only one that disagrees with its own tab badge +// and run-mode dot (#8ab4f8); it takes the badge colour, so every accent names the +// same hex the frontend uses as that CLI's flat identity. This is a data-accuracy fix only — // `accent` still has no reader, so nothing rendered changes because of it. const CLAUDE: CliEntry = { id: 'claude' as CliEntry['id'], @@ -630,7 +633,8 @@ const GEMINI: CliEntry = { id: 'gemini' as CliEntry['id'], label: 'Gemini', shortBadge: 'GM', - accent: '#60a5fa', + // The tab badge / run-mode-dot colour, not the run-button border (see the note above CLAUDE). + accent: '#8ab4f8', enabled: true, stock: true, order: 30, @@ -916,9 +920,9 @@ const GROK: CliEntry = { shortBadge: 'GK', // Upstream hand-authored a charcoal GRADIENT across 4+ CSS spots (welcome button, tab // badge, run-mode dot, mobile skin overrides) rather than one flat colour; our registry's - // `accent` is a single hex, so this is the closest single value (the run-mode-dot colour, - // zinc-400). Nothing reads `accent` yet — the frontend is untouched in this change and - // keeps its own hand-authored CSS; the field is here so the entry is complete. + // `accent` is a single hex, so this is the closest single value (zinc-300, the run-button + // border and tab-badge colour). Nothing reads `accent` yet: the frontend keeps its own + // hand-authored CSS; the field is here so the entry is complete. accent: '#d4d4d8', enabled: true, stock: true, diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 34bb1851..10e48e11 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -695,7 +695,7 @@ export interface CliEntry { /** * Single hex colour, measured from the CLI's actual `.btn-toolbar.btn-run.mode-` * gradient in styles.css (see stock.ts's comment above `CLAUDE` for the exact - * methodology). DECLARED-FOR-LATER (below) — no code reads this yet; styles.css's + * methodology). DECLARED-FOR-LATER (above) — no code reads this yet; styles.css's * gradients are still hand-authored per id, not derived from this field via any * CSS custom property. There is no `--cli-accent` variable in the codebase. */ diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 3c528fed..f4588d21 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -949,40 +949,41 @@ html.mobile-init .file-browser-panel { border-color: rgba(16, 185, 129, 0.5); } - /* Gemini mode colors on mobile */ + /* Gemini mode colors on mobile. Same `!important` rationale as the pi block below. */ .btn-toolbar.btn-run.mode-gemini, .btn-toolbar.btn-run-gear.mode-gemini { - background: #10243f; - border-color: rgba(96, 165, 250, 0.3); - color: #dbeafe; + background: #10243f !important; + border-color: rgba(96, 165, 250, 0.3) !important; + color: #dbeafe !important; } .btn-toolbar.btn-run.mode-gemini:active, .btn-toolbar.btn-run-gear.mode-gemini:active { - background: #174ea6; - border-color: rgba(96, 165, 250, 0.5); + background: #174ea6 !important; + border-color: rgba(96, 165, 250, 0.5) !important; } - /* Antigravity mode colors on mobile */ + /* Antigravity mode colors on mobile. Same `!important` rationale as the pi block below. */ .btn-toolbar.btn-run.mode-antigravity, .btn-toolbar.btn-run-gear.mode-antigravity { - background: #0b2b33; - border-color: rgba(34, 211, 238, 0.3); - color: #cffafe; + background: #0b2b33 !important; + border-color: rgba(34, 211, 238, 0.3) !important; + color: #cffafe !important; } .btn-toolbar.btn-run.mode-antigravity:active, .btn-toolbar.btn-run-gear.mode-antigravity:active { - background: #0e7490; - border-color: rgba(34, 211, 238, 0.5); + background: #0e7490 !important; + border-color: rgba(34, 211, 238, 0.5) !important; } /* Pi mode colors on mobile. `!important` is load-bearing here, not noise: styles.css nests its skin rules - inside `html:not([data-skin="og"])`, so a bare `.btn-toolbar.btn-run` in there - resolves to (0,2,1) and outranks this (0,2,0) `.mode-pi` pair regardless of - load order. The antigravity block right above omits it and is consequently - dead on every non-og skin (i.e. on the default) — do not copy that. */ + inside `html:not([data-skin="og"])`, so a bare `.btn-toolbar.btn-run` or + `.btn-toolbar.btn-run-gear` in there outranks this `.mode-pi` pair regardless + of load order. Without it the gear half keeps the skin accent while the body + takes the nested block's mode colour, so the split button renders two-tone + (gemini and antigravity shipped that way until they got `!important` too). */ .btn-toolbar.btn-run.mode-pi, .btn-toolbar.btn-run-gear.mode-pi { background: #33121f !important; diff --git a/test/skin-themes.test.ts b/test/skin-themes.test.ts index 6608d3fe..b1f21b20 100644 --- a/test/skin-themes.test.ts +++ b/test/skin-themes.test.ts @@ -176,4 +176,33 @@ describe('Codeman light skins', () => { expect(mobileStylesSource).toContain(':is(.header, .toolbar, .keyboard-accessory-bar)'); expect(mobileStylesSource).toContain(':is(.case-settings-popover-mobile, .mobile-case-picker-sheet)'); }); + + it('re-declares every run-mode colour inside the non-og skin block', () => { + // The skin block nests under `html:not([data-skin="og"])`, so its generic + // `.btn-toolbar.btn-run` outranks a base-sheet `.mode-` pair. A mode with no + // resting rule of its own in there (a `:hover` alone does not count) renders as generic claude blue on the DEFAULT skin + // (gemini, antigravity and omp all shipped that way). Ids come from the sheet. + const css = stylesSource.replace(/\/\*[\s\S]*?\*\//g, ''); + const opener = 'html:not([data-skin="og"]) {'; + const start = css.indexOf(opener); + expect(start).toBeGreaterThan(-1); + let depth = 0; + let end = -1; + for (let i = start + opener.length - 1; i < css.length; i++) { + if (css[i] === '{') depth++; + else if (css[i] === '}' && --depth === 0) { + end = i; + break; + } + } + expect(end).toBeGreaterThan(start); + const nested = css.slice(start, end); + const base = css.slice(0, start) + css.slice(end); + const ids = (text: string) => + new Set([...text.matchAll(/\.btn-toolbar\.btn-run\.mode-([\w-]+)(?![\w-]|:)/g)].map((m) => m[1])); + const baseIds = [...ids(base)]; + expect(baseIds.length).toBeGreaterThanOrEqual(5); + const nestedIds = ids(nested); + expect(baseIds.filter((id) => !nestedIds.has(id))).toEqual([]); + }); });