From a3fbb9ae3f82cfe4a55b532b472ccd056704685f Mon Sep 17 00:00:00 2001 From: bchanot Date: Sun, 11 Oct 2026 23:39:01 +0200 Subject: [PATCH] fix(model-router): close the accepted residuals of waves 1-4 Kill switch fails closed: a present-but-unreadable override at first load or a non-boolean enabled turns the router off (logged); /route reload says when the previous config was kept. Prompt rules with nested quantifiers are dropped; a config path that is not a regular file reads as failed; caught errors log kind + code only (errWord); rejected keys are masked in logs (keyWord), alias keys match KEY_NAME, prototype names are never keys; offers capped at 256; a models alias without a fallback list is ranked at the end of the default chain; canonical matches a table id at a '-' boundary only; logCall/noteMain cannot unroute a step; route-tool failure text no longer claims nothing was routed. Repo side: mods suite probes the 'claude plugin test' usage line, doctor bounds plugin list to 60 s, update-all skips @skills-dir mods. Kit 431/431, full make test green. --- .claude/tasks/TODO.md | 18 +-- CHANGELOG.md | 1 + doctor.sh | 10 +- lib/tests/mods.test.sh | 10 +- mods/model-router/hooks/register.test.ts | 47 +++++++- mods/model-router/hooks/register.ts | 136 +++++++++++++++++------ update-all.sh | 3 + 7 files changed, 169 insertions(+), 56 deletions(-) diff --git a/.claude/tasks/TODO.md b/.claude/tasks/TODO.md index e371d3f..7b19581 100644 --- a/.claude/tasks/TODO.md +++ b/.claude/tasks/TODO.md @@ -7,14 +7,14 @@ migration of shifters/pins/model-gate in wave 2 after proof; names model-router - [x] W0 spike in dev-mods (hot reload): facts a-d established 2026-10-08 (plan file § Spike facts); e moved to W1.10 - [x] W1-A the mod in `mods/model-router/` (b721c94, contract `2026-10-08-model-router-w1a-1533`, plan r3): challenge 3 lenses + 1 confirmation (2 BLOCKER + 10 MAJOR closed by named changes), feater DONE first pass, GATE 0 MET 5/5, verifier CONFORME 6/6, security PASS (4 MEDIUM + 5 LOW reported, below) - [x] W1-A hardening (user go "oui durcis", contract criteria 7-11): `/route` composer-only; no agent model axis (effort only after spawn); pattern ≤ 200 / scan ≤ 4096 / phase keys `^[a-z][a-z0-9_-]{0,31}$` / config ≤ 64 KB / `additionalProperties: false` / `typeof e.skill`; `warnOnce` in all 14 catches + config-drop logs; `safely` around post-`next` bookkeeping. feater DONE, gap round (dead `Loop.frozen`, spawn returns `started` verbatim, tool description), GATE 0 MET 9/9, verifier CONFORME 11/11, security PASS. -- [ ] W1-A residuals (security, accepted, none exploitable from outside the user's own files): ReDoS is size-bounded only (`(a+)+$` in `~/.claude/model-router.json` + a 4 KB paste hangs the hook; fix = nested-quantifier rejection or a far lower scan cap); `stat().size` trusted (FIFO/device path in ~/.claude); unrestricted `models`/`agents`/`skills` KEYS echoed raw in logs (log flood); `agentId` read from the flat tool event (engine strip unverified); 5 closures without a kit test (no fs in the kit, LRN-206); "nothing routed" catch text after a state write. +- [x] W1-A residuals CLOSED 2026-10-11 (bugfix/model-router-residuals): nested-quantifier patterns rejected (`NESTED_QUANT`); `stat.kind !== 'file'` → layer failed; rejected keys masked in logs (`keyWord`), alias keys `KEY_NAME`; route-tool catch text says the route may be partly applied. Left (accepted): `agentId` read from the flat tool event; closures without a kit test. - [x] W1-B1 floor precedence (contract `2026-10-08-model-router-floor-1835`, 2026-10-09): `ultrathink` + typed `/effort-` = the main turn's default AND minimum (r2 after 3 lenses: a pure floor made /effort-low a no-op); one helper `mainEffort`; per-axis precedence; mid-turn prompt floors now + next turn; `"enabled": false` per machine, kept across /clear and failed reload; typed slash attested. 30 tests; verifier CONFORME 6/6 (after 1 gap round); security PASS ×2 -- [ ] W1-B1 residuals (security 2026-10-09, accepted): first-load failure of the override falls to defaults (`enabled: true`); non-boolean `enabled` drops to the default on reload; `skill.prompt` preload guard is a heuristic (`loops.size > 0`; a preload during spawn or a non-composer `/effort-*` while idle still writes the floor); marker not bound to a valid level; `/route reload` answers "config reloaded" even when the previous config was kept; transient missing override lifts a config-set off; `String(err)` of a JSON parse in the local log. Display: `/route show` folds the floor into the effort while off; model-axis text with a model-only sticky and the switch on. +- [x] W1-B1 residuals CLOSED 2026-10-11: broken override at first load → router off (fail closed, logged); non-boolean `enabled` → off; `/route reload` answers `config unreadable; previous config kept`; `errWord` (name + code only) replaces every `String(err)` log. Superseded by W2-B: `skill.prompt` preload guard, `/effort-*` marker (bridge removed). Left (accepted): transient missing override lifts a config-set off (ENOENT must mean defaults on most machines); `/route show` display items (no defect named). - [x] W1-B2 wiring (contract `2026-10-08-model-router-wiring-1835`, 2026-10-09): tracked symlink `skills/model-router` → `../mods/model-router` loads as `model-router@skills-dir` (fresh-process `claude plugin list --json` proves it), `.gitignore` `mods/*/tsconfig.json`, `lib/tests/mods.test.sh` (4 checks, capability probe, bounded, SKIP), doctor `── Mods ──` fail-soft, CLAUDE.md `## mods/`. Plan r2 from 3 lenses (5 MAJOR), feater DONE, gap round (4b label + unbounded probe), verifier CONFORME 8/8, security PASS -- [ ] W1-B2 residuals (accepted): `claude plugin --help` returns 0 → the suite's capability probe can pass on a CLI without `plugin test` and then FAIL instead of SKIP (fail-closed; fix = grep the probe output for the test usage line); doctor `claude plugin list --json` unbounded; doctor `echo -e` helpers interpolate `$_mod` (tracked folder names only); fallback timeout guard orphans grandchildren (only without coreutils timeout); `update-all.sh` runs `claude plugin update` over `@skills-dir` → one recurring warn (needs an update-all edit) +- [x] W1-B2 residuals CLOSED 2026-10-11: mods suite probe greps the `claude plugin test` usage line; doctor `plugin list --json` under `timeout 60`; update-all skips `@skills-dir` mods. Left (accepted): doctor `echo -e` helpers (tracked names only); fallback timeout guard orphans grandchildren without coreutils timeout. - [x] W1 close-out (2026-10-09): doc-sync opus audit SIGNIFICANT → user go all 10 → patched (README effort routing + Explore row + /route, USAGE, ARCHITECTURE mods/, CHANGELOG Unreleased) b22f894; hot-reload link removed from `~/.claude/dev-mods//`; BDR-115 amended (a6e2003). Pending user: `/reload-plugins` in this session (new sessions load the skills-dir copy by themselves); merge decision on feature/model-router-mod (`gitflow finish`, human signal) - [x] W1-C adaptive tiers (user 2026-10-09, contract `2026-10-09-model-router-tiers-1237`, plan r4 after 3 lenses + 2 confirmations: 6 BLOCKER + 29 MAJOR closed by named changes): phases name absolute tiers (best fable>opus>sonnet · big opus>fable>sonnet · work sonnet>opus · cheap haiku>sonnet); breaker fed by `classic.StopFailure` kinds rate_limit|overloaded|billing_error|model_not_found + `PostModelSwitch` auto (episode backoff 15→300 min, `/model` clears, `/clear` keeps, `/route reload` clears); fallback chain fable→opus→sonnet→haiku; main UPGRADE by default under `upgradeMaxTokens` 200k (fails closed on unknown usage), DOWNGRADE gated by `mainModelSwitch`; sticky `turnModel` per turn; derived `orchestrate` on background dispatches; prompt default rules (plan/reflect FR+EN, Unicode guards, skipped on `/…`, floor match, mid-turn). 58 tests; verifier CONFORME then 3 gap/hardening rounds; security PASS -- [ ] W1-C accepted-by-design (security 2026-10-09, MEDIUM, not coded around): (1) the model itself can raise the main loop to fable for the rest of a turn through the `route` tool (plan/reflect/escalate/judge) or a background dispatch (derived orchestrate = best tier) — bounded by the turn and `upgradeMaxTokens`, no sticky route is model-callable; (2) `ultrathink` and the default keyword rules now mean "best tier" (fable) at the phase's effort, so an incidental keyword in a pasted composer prompt costs a fable turn (origin composer only). Residuals: `PostModelSwitch` `auto` covers "other programmatic change" (a healthy model could be marked 15 min); strikes never decay inside a session; a `[1m]` variant's `model_not_found` marks the base model until reload; a `models` alias added by the override without a `fallback` key stays unranked (`withEveryAlias` not applied to the default chain) so `leaveDown` skips the upgrade gates for it; `canonical` prefix match has no segment boundary (`claude-sonnet-5-50` would map to sonnet); classifier deferred to W2 +- [ ] W1-C accepted-by-design (security 2026-10-09, MEDIUM, not coded around; 2026-10-11 closed: unranked `models` alias → `withEveryAlias` on the default chain too, `canonical` segment boundary `known + '-'`): (1) the model itself can raise the main loop to fable for the rest of a turn through the `route` tool (plan/reflect/escalate/judge) or a background dispatch (derived orchestrate = best tier) — bounded by the turn and `upgradeMaxTokens`, no sticky route is model-callable; (2) `ultrathink` and the default keyword rules now mean "best tier" (fable) at the phase's effort, so an incidental keyword in a pasted composer prompt costs a fable turn (origin composer only). Residuals: `PostModelSwitch` `auto` covers "other programmatic change" (a healthy model could be marked 15 min); strikes never decay inside a session; a `[1m]` variant's `model_not_found` marks the base model until reload; a `models` alias added by the override without a `fallback` key stays unranked (`withEveryAlias` not applied to the default chain) so `leaveDown` skips the upgrade gates for it; `canonical` prefix match has no segment boundary (`claude-sonnet-5-50` would map to sonnet); classifier deferred to W2 - [x] W1-C live verification part 1 (2026-10-09 after `/reload-plugins`, skills-dir copy loaded, 12 hooks): derived orchestrate on a real background Explore (main high → medium while it ran → high after its end); Explore on sonnet/medium; `route plan` → next request xhigh (engine record); `Skill(effort-low)` bridged → next request low (engine record); `/route show` resolves all 10 phases to full ids, `down: none`; steps arrive as bare `claude-fable-5-1` (no `[1m]`) so the suffix carry never fires on this session - [ ] W1-C live verification part 2 (needs a real incident): StopFailure vs turn.complete order; `PostModelSwitch` `auto` semantics and its `from_model` after a router upgrade; whether a router rewrite raises `auto`; `[1m]` carry validity on opus (only on a session whose steps carry it) - [x] W2-A mod (2026-10-09, bb56f3e on feature/model-router-w2, contract `2026-10-09-model-router-w2a-1546`, plan r4 `2026-10-09-model-router-w2-1546`): user decisions — effort-* skills deleted (W2-B), pins REWORKED not deleted (rows = phases by role; frontmatter `model:`/`effort:` kept as census-locked off-state floor after a robustness BLOCKER), orchestrators declare phases, slim model gate. Phases `write` work/high + `apply` work/low; 56 skill rows, 21 agent rows; agents' model at spawn within tier, upward only, project-defined agents skipped (agent.offer); typed slash → name-bound marker (composer|sdk|bridge) + pending slot + idle fallback; best-tier rows in a `runMain` slot surviving turn end; unrowed skill leaves the route; route answer always names the id; `null` override rows. 3 lenses + 2 confirmations (1 BLOCKER each round closed), feater + 4 gap rounds, GATE 0 MET, verifier 3× ECARTS on coverage clauses only → user accepted at the cap, security PASS (2 MEDIUM pre-existing: first-load kill switch fails open, ReDoS size-bounded). Kit 58 → 88 tests. Doc-sync skipped for A (mod-only, docs at B). @@ -22,25 +22,25 @@ migration of shifters/pins/model-gate in wave 2 after proof; names model-router - [x] W2-B repo migration (2026-10-10, 1f2d33b, contract `2026-10-10-model-router-w2b-1045`): 15 citers → `mcp__model-router__route` per phase, `effort=` on built-in judgment dispatches + SDD sonnet implementers (`effort="medium"`), `lib/effort-shift.md` + `lib/model-gate.md` rewritten (witness = route answer, `/route on`), deletions (effort-*, effort-pins.*, model-check.sh, tests, installer blocks), bridge removed from the mod, census rewritten (140 checks, drift lock rows ↔ frontmatter), analyzer xhigh, CLAUDE.global.md Design-work line. Verifier ECARTS(7) → CONFORME 10/10, security PASS, full make test green (design-tool-gate env red). Docs P1-P8 user-approved; SemVer: breaking → next release 3.0.0 (MIGRATION section at release). - [ ] (was) W2 gate A→B (user): `/reload-plugins`, then the live probe of plan § Gate (4 points: typed `/status` marker vs fallback in the verbose log; a real rowed spawn line + `step 0 agent` effort = spawn/first-step ordering; sonnet session typed `/feat` self-check + route answer; probe 1 again with a background agent alive). `typed-marker` never seen → W2-B blocked, A4 re-planned. - [x] (was) W2-B repo migration (plan § W2-B B0-B7 + STEP 6/7): bridge removal, `lib/effort-shift.md` rewrite, 15 citers → `route`, `effort=` on opus general-purpose dispatches, slim `lib/model-gate.md`, delete `model-check.sh` + `effort-pins.*` + their tests + install/update blocks, delete `skills/effort-*`, analyzer `effort: xhigh`, census rewrite (drift lock rows ↔ frontmatter), docs + registries (BDR-115 amendment, LRN typed-slash/run slot, EVAL) -- [ ] W2-A residuals (security, accepted): first-load failure of the override activates the router despite `enabled:false` (fix = treat a failed first load as off); ReDoS on a self-authored prompt pattern (size-bounded); error text in the local log; model alias keys unvalidated (PHASE_KEY would do); `offers` map uncapped; `__proto__`/`constructor` override keys untested. Known limits: `skillCalls`/`spawning` counters are global; offers keyed by name only; builtin `/effort` is not a lever inside a run. +- [x] W2-A residuals CLOSED 2026-10-11 (first load fail closed, nested quantifiers, `errWord`, alias keys, `offers` capped at 256, `__proto__`/`constructor` keys tested). Known limits (unchanged): `skillCalls`/`spawning` counters are global; offers keyed by name only; builtin `/effort` is not a lever inside a run. ## 2026-10-10 — model-router wave 3-A: first-use route confirmation + decision memory (feature/model-router-confirm) Plan `.claude/tasks/plans/2026-10-10-model-router-w3a-confirm-1201.md` r4, contract `2026-10-10-model-router-w3a-confirm-1201`. User decisions 2026-10-10: tracked routing.json reached through the plugin dir (no new link); blocking `$.ui.ask` dialog at first use; per skill row, agent row, main phase. - [x] W3-A landed (22455c0, 2026-10-11): routing.json = phases + rows + decisions; T1/T2/T3 dialogs; project exceptions `projects[]` in the tracked file (project-tree layer dropped: security); writers = dialog + `/route ask`; `/route pending`; census from the file with the `changed` WARN exemption; kit 86 → 190. Gates: 3 lenses + 1 confirmation (BLOCKER census), feater + 4 rounds, verifier CONFORME + re-verify, security BLOCK(1) credential leak fixed → PASS, full make test green (design-tool-gate env red). Live: T2 dialogs answered by the user (verifier Keep, feater Keep, security-auditor → implement then reset to verify on user go). - [x] W3-B dialog with context (2026-10-11, 604a6c4, contract `2026-10-11-model-router-w3b-dialog-1240`, plan r3): descriptions (skill frontmatter / agent.offer / phase `about`), real id + effort in the question, Later / Keep / Change, model → effort (derived from the phases of that model) → scope, rows stay phases, T3 Later/Keep. 3 lenses (2 BLOCKERs on my r1 avoided) + 1 confirmation, feater + 2 rounds, verifier CONFORME (3rd), security PASS (4 LOW: symlinked SKILL.md read, TOCTOU, raw key in a log, any plugin hooking AskUserQuestion could answer). Live decisions: security-auditor → judge (kept, floor aligned `model: opus`), plan-challenger → verify (reset on user go), orchestrate Keep (T3 live check done). -- [ ] W3-B residuals: (machine-only phase into the tracked file: CLOSED in W4-A); SKILL.md symlink follow (LOW); `descs` cap and "(custom)" label untested; the security-auditor's `changed.from = verify` entry stays in routing.json although the floor now equals the row (harmless; prune at release). +- [ ] W3-B residuals: SKILL.md symlink follow (LOW, kept: `skills/` itself is a symlink); `descs` cap and "(custom)" label untested. (security-auditor `changed` entry purged 2026-10-11.) - [x] "auto mode: use Bash/sed" block attributed to the mod: traced 2026-10-11 by reading, it is the harness's own auto-mode instruction rendered after the mod's MCP block; not in the mod source. Closed. - [x] W3-C `/route forget` (2026-10-11, cd8d72f, contract `2026-10-11-model-router-w3c-forget-1457`, plan r3): name/all/projects, guarded restore, confirmation dialog for all/projects, no-write paths, docs. 3 lenses + 1 confirmation, feater + 3 rounds (1 real defect: `ForgetPlan` typo), verifier at the cap on coverage (user accepted), security PASS. Kit 232 → 284. Live: orchestrate + escalate T3 Keeps seen. - [x] W3-C residuals closed in W4-A (68d7ea6). Live smoke of `/route forget` from the terminal still unseen. - [x] W4-A transition logs + residuals (2026-10-11, 68d7ea6, contract `2026-10-11-model-router-w4a-logs-residuals-1637`, plan r3): verbose = transitions only (main line, hand-off per dispatch, agent moves, answered-by anomaly), stream lines removed, host prefix respected; residuals of W3-A/B/C closed (a)-(g). 3 lenses (BLOCKER: agent moves decided at spawn) + 1 confirmation, feater + 4 rounds, verifier at the cap on coverage (user accepted), security PASS + 2 MEDIUM fixed. Kit 284 → 358. -- [ ] W4-A residuals (LOW, accepted): `readLayer` logs `String(err)` of a JSON parse error (a file fragment can appear in the local log); `logCall`/`noteMain` ui calls on the step path sit outside `safely` (fail-open on routing, no security impact). +- [x] W4-A residuals CLOSED 2026-10-11: `errWord`; `logCall` → `tryLog`, `noteMain` refresh under `safely`. - [ ] push-guard hotfix: the PreToolUse hook denies a read-only command whose TEXT contains the push verb next to `git` (grep pattern, heredoc); match the command's own argv only. - [x] W4-B classifier (2026-10-11, e54a2b5, contract `2026-10-11-model-router-w4b-classifier-1858`, plan r3): `$.model.classify` on plain composer prompts with no route in force → turn effort only (design→plan, analysis→reflect, coding→implement, bookkeeping→apply, chat→nothing), 3-s race, abort not counted, 3 failures pause `cooldownMinutes`, no dialog, no file write, `classify` config key, `/route classify on|off`, status + verbose line. 3 lenses (BLOCKER: unbounded await) + 1 confirmation, feater + 1 round, verifier CONFORME (2nd), security PASS. Kit 358 → 428. Deviation from the TODO line: no first-use confirmation on classified phases (noise per prompt; T3 already confirms the phase). -- [ ] W4-B residuals: `smallModelBusy` checks `cfg.models.haiku` but classify uses the engine's default small model (guard misses when they differ); abort paths judged by reading only; the 3-s tests pin the upper bound only. Live AC9 open: type a few plain prompts, read `route: classifier ` (verbose line with `/route verbose on`), drop `bookkeeping` if it misfires. +- [ ] W4-B residuals: `smallModelBusy` assumes the engine's small model is the `haiku` alias (`$.model.classify` takes no model: documented in code 2026-10-11, no fix possible); abort paths judged by reading only; the 3-s tests pin the upper bound only. Live AC9 open: type a few plain prompts, read `route: classifier ` (verbose line with `/route verbose on`), drop `bookkeeping` if it misfires. - [ ] W3-A live checks still open: T1 (typed `/feat` etc.) dialog (T3 seen live 2026-10-11: orchestrate Keep); a parallel same-agent dispatch answered after > 10 s; what an unanswered dialog resolves to on the terminal (must be Later or nothing written). Record in the contract as `[gated]` when seen. - [ ] W3-A residuals (security LOW, accepted): non-atomic cross-process write (two sessions, crash mid-write → file reads as failed, previous config kept); `host/path` of a private remote in the tracked file; a persistent write failure re-asks every use (`asked.delete` in the catch); `out.length` vs byte size at the cap; `changePatch` scope 'project' with a vanished key writes Everywhere (cwd change mid-dialog). Deferred: `/route set`, `/route confirm`, `(unconfirmed)` in show, dialog edits of phases, frontmatter auto-alignment, session-start warning when routing.json is dirty, per-machine `projects`. - [ ] routing.json ships `confirmed` for verifier/feater (user Keeps) and phases verify/implement: every clone inherits them (by design: decisions travel). Revisit at release if unwanted. -- [ ] W2 residuals: probe 4 unobserved (above); the push-guard hook denies a read-only grep (or a heredoc) whose TEXT contains the push verb next to `git` (security-auditor + orchestrator 2026-10-10, false positives, reworded); ~27 loose "sonnet pin"/"opus pin" shorthand sites kept (true by census); README config key list omits `tiers`/`fallback`/`cooldownMinutes`/`mainUpgrade`/`upgradeMaxTokens` (doc audit item 6, pre-existing); MIGRATION.md "Upgrading to 3.0.0" at release time; `skillCalls`/`spawning` counters global; offers keyed by name. +- [ ] W2 residuals: probe 4 unobserved (above); the push-guard hook denies a read-only grep (or a heredoc) whose TEXT contains the push verb next to `git` (security-auditor + orchestrator 2026-10-10, false positives, reworded); ~27 loose "sonnet pin"/"opus pin" shorthand sites kept (true by census); (README key list: already complete since W2-B docs, verified 2026-10-11); MIGRATION.md "Upgrading to 3.0.0" at release time; `skillCalls`/`spawning` counters global; offers keyed by name. - [ ] W3 optional: step heuristics, quota-aware downgrade, A/B (haiku classifier: done W4-B) ## 2026-09-30 — Higgsfield pack: CLI + skills in the install process, off by default (feature/higgsfield-pack) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8268b55..934bd29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/) and this project - The five `effort-low` … `effort-max` skills, `lib/effort-pins.txt`, `lib/effort-pins.sh` and `lib/model-check.sh`, with their tests and the effort-pin re-apply steps of `install-plugins.sh` and `update-all.sh`. The model-router rows replace them. A typed `/effort-` no longer exists: use `ultrathink` or `/route effort=max`. Breaking: the next release is 3.0.0. ### Fixed +- model-router residuals of waves 1-4 (2026-10-11): the kill switch fails closed (a `~/.claude/model-router.json` present but unreadable at first load, or an `enabled` that is not a boolean, turns the router off and says so; `/route reload` answers `config unreadable; previous config kept` instead of `config reloaded`); a prompt rule whose pattern nests quantifiers (`(a+)+$`) is dropped; a config file that is not a regular file reads as failed; every log of a caught error prints its kind and code only, never the message (a JSON parse error quotes the file); rejected table keys are masked in logs and model alias keys must be short printable names; `__proto__`, `constructor` and `prototype` are never read or written as keys; the agent offers map is capped like the descriptions; a `models` alias added without a `fallback` list is ranked at the end of the default chain (the upgrade gates now apply to it); a dated or variant id maps to its table id only at a `-` boundary (`claude-sonnet-5-50` no longer reads as sonnet); the status refresh and the main-decision log on the step path can no longer unroute a step; the route tool's failure answer says the route may be partly applied. `lib/tests/mods.test.sh` proves `claude plugin test` exists from its usage line (an unknown subcommand exits 0), `doctor.sh` bounds `claude plugin list --json` to 60 s, `update-all.sh` skips `@skills-dir` mods. Kit 428 → 431. - model-router residuals of the confirmation waves: a failed `/route forget` or `/route ask` write logs its cause and answers a generic text; forget answers clean every file-sourced name, phase and key; a restore names a frontmatter file only for a valid skill name and refuses a `__proto__` row; `/route forget` never restores `changed.phases`; the dialog's Change offers and writes shipped phases only (a machine-only phase never reaches routing.json, and an alias no shipped phase uses says so); a failure after the user answered keeps the key asked instead of asking again at every use. - `gitflow delete` (and `finish`) land on the base that contains the branch and drop the branch's upstream before `git branch -d`, so a branch whose upstream lags (manual-push mode) is deleted instead of refused by git (T18k). - An invalid `gitflow.autopush` value (not a boolean, or a config read that fails) no longer pushes. The post-commit / post-merge hooks and every push site of `lib/gitflow.sh` (`start`, `finish`, the `origin/` cleanup of `delete`) read it as auto and pushed; they now push nothing and say why. Each hook run prints `gitflow post-commit: gitflow.autopush unreadable (git rc ) — NOT pushed, treated as manual push mode; fix the value by hand` (post-merge likewise), and the lib passes through the `push-mode` verb's line, `gitflow.sh push-mode: gitflow.autopush='' is not a boolean (git rc )`. A repo with its own committed `.githooks/` (onboarded projects) keeps running its old hooks, which still push on an invalid value, until a session start runs `reconcile-hooks` and rewrites them; commit that refresh so other clones get it. Tests: `lib/gitflow-test.sh` T18q block. diff --git a/doctor.sh b/doctor.sh index 8bfe308..bd96fda 100644 --- a/doctor.sh +++ b/doctor.sh @@ -179,8 +179,12 @@ except Exception: _mods_list="" if command -v claude &>/dev/null; then - if ! _mods_list=$(claude plugin list --json 2>/dev/null); then - warn "mods: claude plugin list failed — load state not checked" + # Bounded: a hung CLI must not stall doctor (coreutils timeout; without + # it the call runs unbounded, as before). + _mods_timeout="" + command -v timeout >/dev/null 2>&1 && _mods_timeout="timeout 60" + if ! _mods_list=$($_mods_timeout claude plugin list --json 2>/dev/null); then + warn "mods: claude plugin list failed or timed out — load state not checked" _mods_list="" fi fi @@ -209,7 +213,7 @@ for _mod_manifest in "$REPO"/mods/*/.claude-plugin/plugin.json; do esac done [ "$_mods_seen" -gt 0 ] || info "no mods" -unset _mods_list _mods_seen _mod_manifest _mod _mod_link +unset _mods_list _mods_seen _mod_manifest _mod _mod_link _mods_timeout echo "" diff --git a/lib/tests/mods.test.sh b/lib/tests/mods.test.sh index dcc7f9c..74fc546 100644 --- a/lib/tests/mods.test.sh +++ b/lib/tests/mods.test.sh @@ -37,10 +37,14 @@ manifest_name() { # cli_unavailable: prints the reason and returns 0 when the capability is missing. cli_unavailable() { command -v claude >/dev/null 2>&1 || { echo "claude not found"; return 0; } - local rc=0 - bounded claude plugin test --help >/dev/null || rc=$? + local rc=0 out + out=$(bounded claude plugin test --help) || rc=$? [ "$rc" -eq 124 ] && { echo "probe timed out after ${CLI_TIMEOUT}s"; return 0; } - [ "$rc" -ne 0 ] && { echo "no 'claude plugin test' command"; return 0; } + # An unknown subcommand prints the generic `claude plugin` usage with rc 0: + # only the command's own usage line proves it exists. + if [ "$rc" -ne 0 ] || ! printf '%s' "$out" | grep -q 'claude plugin test'; then + echo "no 'claude plugin test' command"; return 0 + fi return 1 } diff --git a/mods/model-router/hooks/register.test.ts b/mods/model-router/hooks/register.test.ts index 734f838..b84a276 100644 --- a/mods/model-router/hooks/register.test.ts +++ b/mods/model-router/hooks/register.test.ts @@ -1581,13 +1581,47 @@ test('layers: show names routing.json as the config', async ($, on) => { expect(await route($, 'show')).toContain('config: routing.json') }) -test('layers: first load skips a broken override, keeps routing.json', async ( +test('layers: a broken override at first load turns the router off', async ( $, on) => { const logs = recordLogs(on) const w = newWorld({ home: 'broken' }) - const rig = await bootRun($, on, FABLE, w) - expect(await featerModel($, rig)).toBe(SONNET) + await bootRun($, on, FABLE, w) expect(logs.join('\n')).toContain('model-router.json unreadable') + expect(logs.join('\n')).toContain('router off (fail closed)') + expect(await route($, 'show')).toContain('router: off (config)') +}) + +test('layers: an override that is not a regular file fails closed', async ( + $, on) => { + const logs = recordLogs(on) + const w = newWorld({ home: { verbose: true } }) + w.kinds.set(HOME_FILE, 'other') + await bootRun($, on, FABLE, w) + expect(logs.join('\n')).toContain('model-router.json is not a regular file') + expect(await route($, 'show')).toContain('router: off (config)') +}) + +test('layers: prototype and malformed keys are dropped, named safely', async ( + $, on) => { + const logs = recordLogs(on) + const w = newWorld() + w.files.set(HOME_FILE, '{"models":{"__proto__":"claude-x-1",' + + '"constructor":"claude-x-1","bad key\\n":"claude-x-1","mini":"claude-x-1"}}') + await bootRun($, on, FABLE, w) + expect(logs).toContain('model-router: config models.constructor ignored') + expect(logs.filter(l => l === 'model-router: config models. ignored')) + .toHaveLength(2) // __proto__ and the key with a newline, both masked + expect(logs).toContain('model-router: config fallback lacks mini; appended') + const shown = await route($, 'nope') + expect(shown).toContain('mini') + expect(shown).not.toContain('constructor') +}) + +test('layers: a nested-quantifier prompt rule is dropped', async ($, on) => { + const logs = recordLogs(on) + const home = { prompt: [{ pattern: '(a+)+$', phase: 'plan' }] } + await bootRun($, on, FABLE, newWorld({ home })) + expect(logs).toContain('model-router: config prompt rule ignored') }) test('layers: a later broken override keeps the whole previous config', async ( @@ -1595,7 +1629,7 @@ test('layers: a later broken override keeps the whole previous config', async ( const w = newWorld({ home: { agents: { feater: 'mechanical' } } }) const rig = await bootRun($, on, FABLE, w) w.files.set(HOME_FILE, '{nope') - expect(await route($, 'reload')).toContain('config reloaded') + expect(await route($, 'reload')).toContain('previous config kept') expect(await featerModel($, rig)).toBe(HAIKU) }) @@ -2094,7 +2128,7 @@ test('T1: a rebuild failing after the write keeps the key asked', async ( const pending = (await route($, 'pending')).split('\n') expect(pending[1]).toBe('asked this session: skills:feat') expect(failures(logs)).toHaveLength(1) - expect(failures(logs)[0]).toContain('rebuild failed after the write') + expect(failures(logs)[0]).toContain('first-use dialog failed (Error)') }) test('T1: a Keep toasts that routing.json changed', async ($, on) => { @@ -3776,7 +3810,8 @@ test('logs: switch warnings keep their exact texts', async ($, on) => { await bootRun($, on, FABLE, newWorld({ home })) expect(logs).toContain('config "verbose" is not a boolean; ignored') expect(logs).toContain( - 'model-router: config "enabled" is not a boolean; ignored') + 'model-router: config "enabled" is not a boolean; router off (fail closed)') + expect(await route($, 'show')).toContain('router: off (config)') }) for (const bad of ['ON', 1, null]) { diff --git a/mods/model-router/hooks/register.ts b/mods/model-router/hooks/register.ts index a0c9c8c..8289ab0 100644 --- a/mods/model-router/hooks/register.ts +++ b/mods/model-router/hooks/register.ts @@ -187,6 +187,15 @@ const UNAVAILABLE: ReadonlySet = new Set([ 'rate_limit', 'overloaded', 'billing_error', 'model_not_found', ]) const PHASE_KEY = /^[a-z][a-z0-9_-]{0,31}$/ +// A model alias key, and the shape a key must have to be echoed in a log. +const KEY_NAME = /^[A-Za-z0-9][A-Za-z0-9_.:-]{0,63}$/ +// Own keys that would shadow Object.prototype: never read, never written. +const BAD_KEYS: ReadonlySet = new Set([ + '__proto__', 'constructor', 'prototype', +]) +// A quantified group holding a quantifier, the shape behind catastrophic +// backtracking ((a+)+$): rejected before compiling. +const NESTED_QUANT = /\((?:[^()\\]|\\.)*[+*}](?:[^()\\]|\\.)*\)[+*{]/ // Prompt classifier: label -> the phase whose effort the turn takes. `chat` // (and any label not here) routes nothing. const CLASSIFY_LABELS: readonly string[] = @@ -274,6 +283,26 @@ const isFullId = (v: unknown): v is string => typeof v === 'string' && MODEL_ID.test(v) const hasKey = (table: object, key: string): boolean => Object.hasOwn(table, key) +/** A row or phase key fit to read and write: never a prototype name. */ +const goodKey = (key: string): boolean => !BAD_KEYS.has(key) +/** A key in a log line: short and printable, or a placeholder. */ +const keyWord = (key: string): string => KEY_NAME.test(key) ? key : '' +/** An error in a log line: its kind only; a message may quote the file. */ +function errWord(err: unknown): string { + if (!(err instanceof Error)) return typeof err + const code = (err as { code?: unknown }).code + return typeof code === 'string' && KEY_NAME.test(code) + ? `${err.name} ${code}` + : err.name +} +/** A log on the step path: a failing ui.log must not unroute the step. */ +function tryLog($: Api, text: string): void { + try { + $.ui.log(text) + } catch { + // the step matters more than its line + } +} function isModelName(models: Record, v: unknown): v is string { return typeof v === 'string' && (hasKey(models, v) || isFullId(v)) @@ -299,15 +328,18 @@ function mergeTable( return out } for (const [key, value] of Object.entries(user)) { - const ok = key === '__proto__' ? undefined : accept(key, value) - if (ok === undefined) log(`model-router: config ${name}.${key} ignored`) + const ok = goodKey(key) ? accept(key, value) : undefined + if (ok === undefined) { + log(`model-router: config ${name}.${keyWord(key)} ignored`) + } else out[key] = ok } return out } -const acceptModel = (_key: string, v: unknown): string | undefined => - isFullId(v) ? v : undefined +/** An alias is a short printable name; its value a full id. */ +const acceptModel = (key: string, v: unknown): string | undefined => + KEY_NAME.test(key) && isFullId(v) ? v : undefined const acceptWindow = (key: string, v: unknown): number | undefined => isFullId(key) && typeof v === 'number' && Number.isInteger(v) && v > 0 @@ -363,19 +395,21 @@ function withEveryAlias( ): string[] { const missing = Object.keys(models).filter(a => !chain.includes(a)) if (missing.length > 0) { - log(`model-router: config fallback lacks ${missing.join(', ')}; appended`) + log(`model-router: config fallback lacks ${ + missing.map(keyWord).join(', ')}; appended`) } return [...chain, ...missing] } -/** The fallback chain: deduplicated aliases of `models`, else the default. */ +/** The fallback chain: deduplicated aliases of `models`, else the default; + * either way every alias ends up ranked. */ function pickFallback( user: unknown, base: string[], models: Record, log: Log, ): string[] { - if (user === undefined) return base + if (user === undefined) return withEveryAlias(base, models, log) const items = Array.isArray(user) ? (user as unknown[]) : [] const kept = items.filter( (a): a is string => typeof a === 'string' && hasKey(models, a)) @@ -421,7 +455,9 @@ function acceptRule( if (mode !== undefined && mode !== 'floor' && mode !== 'default') { return undefined } - if (pattern.length > MAX_PATTERN) return undefined + if (pattern.length > MAX_PATTERN || NESTED_QUANT.test(pattern)) { + return undefined + } return hasKey(phases, phase) && buildRegex(pattern) ? { pattern, phase, mode: mode ?? 'floor' } : undefined @@ -465,6 +501,14 @@ function pickSwitch( return pickBool(v, fallback) } +/** The kill switch fails closed: a value that is not a boolean reads off. */ +function pickEnabled(v: unknown, fallback: boolean, log: Log): boolean { + if (v === undefined) return fallback + if (typeof v === 'boolean') return v + log('model-router: config "enabled" is not a boolean; router off (fail closed)') + return false +} + type Routing = Pick @@ -507,7 +551,7 @@ function mergeRows( : user const rows = mergeTable(base, kept, name, ref, log) for (const [key, value] of entries) { - if (value === null && key !== '__proto__') delete rows[key] + if (value === null && goodKey(key)) delete rows[key] } return rows } @@ -545,9 +589,7 @@ function mergeConfig( 'model-router: config "classify" is not a boolean; ignored', user.classify, base.classify, log), spinner: pickBool(user.spinner, base.spinner), - enabled: pickSwitch( - 'model-router: config "enabled" is not a boolean; ignored', - user.enabled, base.enabled, log), + enabled: pickEnabled(user.enabled, base.enabled, log), } } @@ -559,7 +601,12 @@ async function readCapped( log: Log, ): Promise { const tooBig = `model-router: ${label} over ${MAX_CONFIG_BYTES} bytes` - if ((await $.fs.stat(path)).size > MAX_CONFIG_BYTES) { + const info = await $.fs.stat(path) + if (info.kind !== 'file') { + log(`model-router: ${label} is not a regular file`) + return undefined + } + if (info.size > MAX_CONFIG_BYTES) { log(tooBig) return undefined } @@ -587,7 +634,7 @@ async function readLayer( log(`model-router: ${label} is not an object`) return 'failed' } catch (err) { - log(`model-router: ${label} unreadable (${String(err)})`) + log(`model-router: ${label} unreadable (${errWord(err)})`) return 'failed' } } @@ -601,7 +648,7 @@ async function readOverride($: Api, log: Log): Promise { if (!home) return 'absent' return await readLayer($, `${home}/${OVERRIDE}`, OVERRIDE, log) } catch (err) { - log(`model-router: ${OVERRIDE} unreadable (${String(err)})`) + log(`model-router: ${OVERRIDE} unreadable (${errWord(err)})`) return 'failed' } } @@ -645,7 +692,7 @@ function aboutOf(phases: unknown, log: Log): Table { if (!isRecord(phases)) return out for (const [name, phase] of Object.entries(phases)) { const about = isRecord(phase) ? phase.about : undefined - if (about === undefined || name === '__proto__') continue + if (about === undefined || !goodKey(name)) continue if (typeof about === 'string' && about.length <= MAX_ABOUT) { out[name] = about } else { @@ -810,9 +857,10 @@ async function assemble( } /** - * Reads the layers. At the first load a failed layer is skipped (logged); in - * every later read any failure, or a routing.json gone missing, returns - * undefined so the caller keeps the whole previous config. + * Reads the layers. At the first load a failed routing.json is skipped + * (logged) and a failed override turns the router off (the kill switch it + * may hold is unknown); in every later read any failure, or a routing.json + * gone missing, returns undefined so the caller keeps the previous config. */ async function loadLayers( $: Api, @@ -823,7 +871,13 @@ async function loadLayers( const over = await readOverride($, log) const broken = file === 'failed' || file === 'absent' || over === 'failed' if (broken && !first) return undefined - return assemble($, file, over, log) + const loaded = await assemble($, file, over, log) + if (over === 'failed') { + log(`model-router: ${OVERRIDE} unreadable at first load; ` + + 'router off (fail closed) until a `/route reload` reads it') + loaded.cfg = { ...loaded.cfg, enabled: false } + } + return loaded } /** Compiles the prompt rules; one that fails to compile is dropped, logged. */ @@ -945,7 +999,7 @@ function canonical(cfg: Config, id: string): string { if (hasKey(cfg.models, bare)) return cfg.models[bare] ?? bare if (!bare.startsWith(PREFIX) || bare.length <= PREFIX.length) return bare const hit = Object.values(cfg.models).find( - known => bare.startsWith(known)) + known => bare === known || bare.startsWith(`${known}-`)) return hit ?? bare } @@ -1213,11 +1267,11 @@ function logCall($: Api, st: State, call: Call): void { const key = call.logKey ?? call.log if (call.log !== undefined && key !== undefined && !st.turnLogged.has(key)) { st.turnLogged.add(key) - $.ui.log(call.log) + tryLog($, call.log) } if (call.once !== undefined && !st.logged.has(call.once)) { st.logged.add(call.once) - $.ui.log(call.once) + tryLog($, call.once) } } @@ -1542,9 +1596,12 @@ async function reloadCommand($: Api, st: State): Promise { clearBreaker(st) st.classifyFails = 0 st.classifyUntil = 0 - await reloadConfig($, st) + const swapped = await reloadConfig($, st) refresh($, st) - return 'config reloaded\n' + (await show($, st)) + const head = swapped + ? 'config reloaded' + : 'config unreadable; previous config kept (see the log)' + return `${head}\n${await show($, st)}` } async function handleCommand($: Api, st: State, args: string): Promise { @@ -1724,7 +1781,12 @@ function rememberOffer( st: State, e: { agent: string; source: string; description: string }, ): void { + st.offers.delete(e.agent) st.offers.set(e.agent, e.source) + if (st.offers.size > MAX_DESCS) { + const first = st.offers.keys().next().value + if (first !== undefined) st.offers.delete(first) + } const given: unknown = e.description const line = typeof given === 'string' ? oneSentence(given.slice(0, 4096)) @@ -1887,7 +1949,7 @@ function table(parent: Rec, key: string): Rec { } function setKey(target: Rec, key: string, value: unknown): void { - if (key !== '__proto__') target[key] = value + if (goodKey(key)) target[key] = value } const confirmIn = (file: Rec, kind: Kind, name: string, phase: string) => @@ -1968,7 +2030,7 @@ function writeRouting($: Api, st: State, patch: Patch): Promise { /** A write that failed: the cause goes to the log, never to the answer. */ function failedWrite($: Api, cmd: string, err: unknown): string { - $.ui.log(`${cmd} failed: ${String(err)}`) + $.ui.log(`${cmd} failed: ${errWord(err)}`) return `${cmd} not applied, see the log` } @@ -1999,7 +2061,7 @@ async function runDialog($: Api, st: State, req: Req, text: string) { } function failOnce($: Api, st: State, err: unknown): void { - const text = `model-router: first-use dialog failed (${String(err)}); ` + + const text = `model-router: first-use dialog failed (${errWord(err)}); ` + 'default route kept' if (st.logged.has(text)) return st.logged.add(text) @@ -2208,7 +2270,7 @@ function restoreOf( entry: unknown, ): Restore | string { const { from, to } = isRecord(entry) ? entry : ({} as Rec) - if (name === '__proto__') return 'reserved name, edit routing.json' + if (!goodKey(name)) return 'reserved name, edit routing.json' if (typeof from !== 'string') return 'no phase recorded to return to' const rows = subRec(file, kind) if (typeof to !== 'string' || !hasKey(rows, name) || rows[name] !== to) { @@ -2448,7 +2510,7 @@ async function registerTool($: Api, st: State): Promise { }, }) } catch (err) { - $.ui.log(`model-router: route tool not registered: ${String(err)}`) + $.ui.log(`model-router: route tool not registered: ${errWord(err)}`) } } @@ -2465,7 +2527,7 @@ async function registerCommand($: Api): Promise { immediate: true, }) } catch (err) { - $.ui.log(`model-router: /route not registered: ${String(err)}`) + $.ui.log(`model-router: /route not registered: ${errWord(err)}`) } } @@ -2658,7 +2720,7 @@ function spawnRoute(st: State, e: SpawnIn, frozen: boolean) { /** Once per turn: a spawn whose tier has nothing at or above its head. */ function tierDownLog($: Api, st: State, agent: string, tier: string): void { - const text = `model-router: ${agent} tier ${tier} down, ` + + const text = `model-router: ${keyWord(agent)} tier ${tier} down, ` + 'frontmatter model kept' const key = `spawn-down:${agent}:${tier}` if (st.turnLogged.has(key)) return @@ -3017,7 +3079,7 @@ function withPlan(e: StepIn, plan: Plan): StepIn { function noteMain($: Api, st: State, plan: Plan): void { st.lastPlan = plan st.spinner = `${plan.model.replace(/^claude-/, '')}/${plan.effort ?? '-'}` - refresh($, st) + safely(st, $, 'turn.step status', () => refresh($, st)) } function endMainTurn($: Api, st: State): void { @@ -3178,7 +3240,8 @@ function registerRouteTool(on: On, st: State): void { return out }).catch(($, e, next) => { warnOnce(st, $, 'route tool', next.error.kind) - return { result: `route failed (${next.error.kind}); nothing routed` } + return { result: `route failed (${next.error.kind}); ` + + 'the route may be partly applied, check /route show' } }) } @@ -3399,7 +3462,10 @@ async function expirePause($: Api, st: State): Promise { refresh($, st) } -/** The small model is the haiku id: marked down, or main already runs it. */ +/** + * Best effort: `$.model.classify` takes no model and runs on the engine's + * small model, assumed to be the `haiku` alias (marked down, or main on it). + */ async function smallModelBusy($: Api, st: State): Promise { await prune($, st) const small = st.cfg.models.haiku diff --git a/update-all.sh b/update-all.sh index ba76c77..85cd913 100644 --- a/update-all.sh +++ b/update-all.sh @@ -599,6 +599,9 @@ if command -v claude &>/dev/null; then if [ -n "$_plugins" ]; then while IFS= read -r _p; do _name="${_p%%@*}" + # A mod loaded from skills/ (@skills-dir) lives in this repo: nothing + # to fetch, and `plugin update` warns on it every run. + case "$_p" in *@skills-dir) info "$_name: in-repo mod, skipped"; continue ;; esac info "Updating $_name..." # Pass the full "name@marketplace" spec — the CLI rejects # the bare name when several marketplaces are registered.