diff --git a/CHANGELOG.md b/CHANGELOG.md index 961c1f398..0146b6d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,56 @@ # Changelog +## [1.61.0.0] - 2026-07-09 + +## **Nine guard bugs fixed in one wave.** +## **Every fix ships with a tripwire that proves the guard actually guards.** + +This release closes out the silent-failure class across gstack: guards and tools that reported success while doing nothing. Question cards render again on current Claude Code builds. /careful catches chained, substituted, and capital-flag deletes it used to wave through. The design CLI fails loudly on bad flags instead of billing you for a guess. Shared team brains (thin clients) get brain-aware planning instead of silent suppression. Four of the fixes came from community PRs, absorbed with authorship intact and hardened on top. + +### The six numbers that matter + +Source: this branch's diff against v1.58.5.0. Every new test was first run against the unfixed code and confirmed failing, then confirmed passing after the fix. + +| What | Before | After | +|------|--------|-------| +| AskUserQuestion on Claude Code 2.1.89+ | "Tool result missing due to internal error" | card renders | +| `rm -R /`, `rm -rf $(cmd)/node_modules` via /careful | silent allow | ask | +| `design variants --count abc` | 0 variants, exit 0 | exit 1 with usage hint | +| Thin-client team brains | broken-config, brain blocks suppressed | usable, sync stages skip with reason | +| /office-hours SESSION_COUNT | ~2x inflated | exact | +| New tripwire test cases | n/a | 72 | + +The first row is the one to feel. The question-card primitive every interactive skill depends on was orphaned on current Claude Code builds: the preference hook emitted `permissionDecision:'defer'`, whose semantics became "pause for external resumption" in CC v2.1.89. The fix is a two-branch pass-through (exact-empty stdout, or additionalContext-only output for plan-tune memory nuggets), plus a corrected protocol reference doc so the mistake cannot be re-learned from our own docs. + +### What this means for you + +Interactive skills ask you questions again on current Claude Code. Safety guards fail closed: chained deletes, command substitution, capital `-R`, and destructive credential phrasings ("reset my secrets") all reach a human now. If your team runs a shared remote brain, `/sync-gbrain` and brain-aware planning work on thin clients out of the box. Run `/gstack-upgrade` to get all of it. The hook fix arrives with the file update, no settings change needed. + +### Itemized changes + +#### Fixed + +- **AskUserQuestion orphaned on Claude Code 2.1.89+ (#2035, #2006).** `question-preference-hook` pass-through is now exit 0 with exactly empty stdout (or additionalContext-only output for plan-tune memory nuggets), never `permissionDecision:'defer'`. `defer()` renamed `passThrough()`; the protocol contract in `docs/spikes/claude-code-hook-mutation.md` corrected in the same commit; 13 assertions rewritten across 3 test files; the tripwire asserts exact-empty stdout so a garbage write cannot slip past an optional-chained parse. Existing installs pick the fix up via `/gstack-upgrade` (the registered hook shim execs the TypeScript live). +- **/careful chained-rm bypass (#2039).** Contributed by @jbetala7 (PR #2040): the safe-exception shortcut no longer judges a chained command by its last (safe) target. Hardened on top of the anchored full-command whitelist: the flag cluster accepts capital `-R` (the BSD/macOS recursive flag — `rm -R /` warned nowhere before; `rm -Rf node_modules` alone still allows) and safe-target tokens exclude `(` and backtick, so command substitution ending in a whitelisted suffix (`rm -rf $(./wipe-all)/node_modules`) cannot ride the whitelist. +- **/context-restore loading a sibling worktree's checkpoint (#2052).** Contributed by @jbetala7: restore prefers the current branch's own checkpoint over newer sibling-worktree saves (scans 200 newest, partitions by branch frontmatter), and keeps the Conductor handoff fallback when the branch has no checkpoint. +- **/sync-gbrain drift re-register on gbrain 0.42+ (#1985).** Contributed by @jbetala7: the drift remove passes `--confirm-destructive`. Hardened on top: the remove routes through the #1734 data-loss guards (refuses loudly while an autopilot runs), propagates `--keep-storage`, realpath-normalizes drift detection (a symlink alias of the same directory is a match, not drift, the probable cause of the reporter's unmoved-repo drift), and logs old vs new path whenever drift fires. +- **Developer-profile double counting (#2067).** Contributed by @mvann: `mode:"resources"` bookkeeping rows no longer inflate SESSION_COUNT, TIER, or the builder-to-founder nudge; 8 regression tests pin the tier boundaries from both sides. +- **One-way-door credential net: plurals + runtime wiring (#2024).** The credential nouns now match plurals ("reset my secrets" / "rotate the credentials" classify one-way), and the keyword net is wired into the runtime for the first time: `gstack-question-preference --check --summary-stdin` pipes the question text (stdin, never argv, so quotes and newlines survive), and the enforcement hook falls back to the classifier for unregistered ids, so an ad-hoc destructive question with a stored never-ask preference can no longer auto-decide. +- **design CLI silent NaN flags (#2032).** `--count`, `--retry`, and `--timeout` share one loud contract via `design/src/flag-utils.ts`: non-integer input errors with exit 1 ("3.7" is rejected, not truncated), above-max clamps with a stderr warning, and the variants ceiling derives from the style list instead of a magic 7. Previously `--retry abc` made generate a silent no-op and `--timeout abc` killed the serve board at boot. +- **Thin-client brains misclassified as broken (#2051).** New `thin-client` engine state, read from gbrain's own `remote_mcp` config marker before any probe. Usable at every suppression gate (`--is-ok`, gen-skill-docs detection, `gstack-config gbrain-refresh`) while the local sync stages skip with an accurate reason (code indexing runs on the brain server; memory syncs via the remote brain's artifacts pull). The detect JSON reports `gbrain_thin_client: {probed: false}`: config verified, reachability checked at use time where gbrain calls degrade gracefully. detectMcpMode also recognizes gbrain servers registered under variant names or matched by the config's `mcp_url`. + +#### Closed as already fixed, with receipts + +- #1965 (GBRAIN_PREPARE pooler breakage): `lib/gbrain-exec.ts:86` never sets it; pinned by `test/build-gbrain-env.test.ts:121-142`. +- #1950 (Windows git-bash learnings silently dropped): `bin/gstack-learnings-log:10-15` cygpath fix + stderr surfacing; pinned by `test/bin-windows-bun-import-paths.test.ts`. +- #1964 (slow engines misclassified): `probeTimeoutMs()` honors `GSTACK_GBRAIN_PROBE_TIMEOUT_MS`; timeout classifies usable; pinned by `test/gbrain-local-status.test.ts`. + +#### For contributors + +- 11 bisect commits; 4 community PRs absorbed with authorship preserved. Contributed by @jbetala7 (#2040, #2054, #2031) and @mvann (#1991). Thank you both. +- 72 new test cases across 9 files, each verified failing against the unfixed code before the fix landed. +- Three follow-ups filed in TODOS.md: wire `design/test/` into CI (all 8 existing files are invisible to every runner today, plus a documented pre-existing timing flake), /context-save worktree-identity hardening (the #2052 residual), and conditional gbrain reindex-in-place gated on the new drift log. + ## [1.60.2.0] - 2026-08-07 ## **Three free-suite tests fail-proofed against machine drift.** diff --git a/TODOS.md b/TODOS.md index fc98f0a83..0b27e403a 100644 --- a/TODOS.md +++ b/TODOS.md @@ -45,6 +45,73 @@ a silent mistake breaks all 52 skills. High blast radius — needs its own focus ## Test infrastructure +### P2: Wire `design/test/` into CI (all 8 files are invisible to every runner) + +**What:** Add `design/test/` to the `bun test` glob (`package.json:21`) and +`TEST_ROOTS` (`scripts/test-free-shards.ts:32`) after auditing its 8 files for +server-spawning/flakiness (they were plausibly excluded on purpose). While in +there, fix the known timing flake: `variants-retry-after.test.ts` "HTTP-date: +honors a future date with no extra leading exponential" fails ~1-2 in 9 runs +under parallel suite load (verified pre-existing on v1.58.5.0 during the +June 2026 fix wave — wall-clock assertion with a ~2s window). + +**Why:** Every test in `design/test/` runs only when someone types the path by +hand — a silent coverage hole, the fix wave's theme at meta-level. The wave's +own design tests went into `test/design-flag-utils.test.ts` to dodge this. + +**Pros:** design binary gets CI coverage; kills a latent "we have tests" illusion. +**Cons:** unaudited files may spawn servers or flake; audit first, wire second. + +**Context:** Filed from the June 2026 fix-wave eng review (issue 11 + flake +receipts). Start with the audit: which of the 8 files are hermetic? Wire the +hermetic ones, quarantine or fix the rest. + +**Effort:** S-M (human ~1d, CC ~30min). **Depends on:** None. + +### P2: /context-save worktree-identity hardening (the #2052 residual) + +**What:** Persist a stable worktree identity (path hash or worktree name) into +checkpoint frontmatter at save time; `/context-restore` prefers identity match +over branch-name match. PR #2054 (@jbetala7, absorbed in the June 2026 wave) +fixed restore ORDERING (current-branch first), but branch frontmatter is not a +stable worktree identity: same-name branches across clones/remotes, renamed +branches, and detached HEAD can still restore the wrong checkpoint. + +**Why:** Closes the residual wrong-checkpoint class entirely instead of the +common case. Codex outside-voice concurred during the wave's eng review. + +**Pros:** Eliminates cross-clone checkpoint collisions. +**Cons:** Frontmatter schema change; needs a migration story for old +checkpoints (no-identity checkpoints rank as fallback, like #2054's +no-branch handling). + +**Context:** Filed from the June 2026 fix-wave eng review (NOT-in-scope item). +Start at `context-restore/SKILL.md.tmpl` Step 1 + `/context-save`'s frontmatter +writer; mirror #2054's partition logic with identity as the first key. + +**Effort:** S (human ~4h, CC ~20min). **Depends on:** #2054 (landed in the wave). + +### P3: gbrain reindex-in-place on perpetual drift (conditional — check the drift log first) + +**What:** IF the `[gbrain-sources] drift:` stderr line (added in the June 2026 +wave) shows drift firing on every sync for some environment, implement #1985's +reporter design: refresh an existing source in place with `gbrain reindex-code` +instead of remove+add (which drops and re-embeds the full index — 768 pages / +6,786 embeddings in the reporter's case). + +**Why:** Perpetual drift means paying full re-embed cost every sync. The wave's +`realpathSync` normalization (symlink aliases are a match, not drift) may have +eliminated the drift class entirely — that's why this is conditional. + +**Pros:** Avoids repeated embedding spend for affected environments. +**Cons:** Speculative until the drift log produces evidence; reindex-in-place +has its own consistency questions (stale chunks for deleted files). + +**Context:** Filed from the June 2026 fix-wave eng review (4A observability). +Trigger condition documented in `lib/gbrain-sources.ts` at the drift log line. + +**Effort:** M (human ~1d, CC ~45min). **Depends on:** drift-log evidence from +the wave's `ensureSourceRegistered` logging. ### P1: Free suite exit code is untrustworthy — in-process force-exits mask failures **Priority:** P1 diff --git a/VERSION b/VERSION index 762f17554..82b3fa4c6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.60.2.0 +1.61.0.0 diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 5346f1d43..a8e480914 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -711,7 +711,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/bin/gstack-config b/bin/gstack-config index 15c7300c5..01176c44c 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -414,9 +414,11 @@ case "${1:-}" in fi case "$STATUS" in - ok|timeout) - # "timeout" = slow-but-healthy engine (#1964) — same treatment as - # "ok", matching gstack-gbrain-detect --is-ok and gen-skill-docs. + ok|timeout|thin-client) + # "timeout" = slow-but-healthy engine (#1964); "thin-client" = + # remote-HTTP MCP brain, no local engine by design (#2051) — same + # treatment as "ok", matching gstack-gbrain-detect --is-ok and + # gen-skill-docs. echo "Detected gbrain v$VERSION (local-status: $STATUS)." # Render brain-aware blocks INTO the global install so EVERY project's # Claude sessions get them (other projects read SKILL.md + sections from diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index a5721a9c5..1b0594630 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -225,17 +225,19 @@ do_read() { cat "$PROFILE_FILE" | bun -e " const p = JSON.parse(await Bun.stdin.text()); const sessions = p.sessions || []; - const count = sessions.length; + + // SESSION_COUNT / TIER / CROSS_PROJECT / NUDGE must reflect real sessions, not + // resource-tracking events (the Phase 6 auto-append). Without this filter, a + // session's resources entry written immediately after the real session inflates + // the count (bumping TIER), clobbers LAST_PROJECT/LAST_ASSIGNMENT/LAST_DESIGN_TITLE, + // and pushes NUDGE_ELIGIBLE over its threshold from bookkeeping alone. + const realSessions = sessions.filter(e => e.mode !== 'resources'); + + const count = realSessions.length; let tier = 'introduction'; if (count >= 8) tier = 'inner_circle'; else if (count >= 4) tier = 'regular'; else if (count >= 1) tier = 'welcome_back'; - - // LAST_* / CROSS_PROJECT must reflect real sessions, not resource-tracking - // events (the Phase 6 auto-append). Without this filter, a session's - // resources entry written immediately after the real session would clobber - // LAST_PROJECT/LAST_ASSIGNMENT/LAST_DESIGN_TITLE. - const realSessions = sessions.filter(e => e.mode !== 'resources'); const last = realSessions[realSessions.length - 1] || {}; const prev = realSessions[realSessions.length - 2] || {}; const crossProject = prev.project_slug && last.project_slug @@ -252,7 +254,11 @@ do_read() { for (const v of Object.values(signalCounts)) totalSignals += v; const signalStr = Object.entries(signalCounts).map(([k,v]) => k + ':' + v).join(','); - const builderSessions = sessions.filter(e => e.mode !== 'startup').length; + // Builder-mode design sessions only — the builder->founder nudge is about + // someone who keeps returning to *build*, not startup-mode diagnostics and not + // resources bookkeeping. (Was \`e.mode !== 'startup'\`, which counted resources + // entries and excluded real startup sessions.) + const builderSessions = realSessions.filter(e => e.mode === 'builder').length; const nudgeEligible = builderSessions >= 3 && totalSignals >= 5; const resources = p.resources_shown || []; diff --git a/bin/gstack-gbrain-detect b/bin/gstack-gbrain-detect index 2100f913b..19797a495 100755 --- a/bin/gstack-gbrain-detect +++ b/bin/gstack-gbrain-detect @@ -18,7 +18,7 @@ * "gstack_brain_sync_mode": "off"|"artifacts-only"|"full", * "gstack_brain_git": true|false, * "gstack_artifacts_remote": "https://..." | "", - * "gbrain_local_status": "ok"|"no-cli"|"missing-config"|"broken-config"|"broken-db"|"engine-locked"|"timeout", + * "gbrain_local_status": "ok"|"no-cli"|"missing-config"|"broken-config"|"broken-db"|"engine-locked"|"timeout"|"thin-client", * "gbrain_pooler_mode": "transaction"|"session"|null * } * @@ -175,10 +175,13 @@ function detectMcpMode(): "local-stdio" | "remote-http" | "none" { // fall through } } - // Tier 2: `claude mcp list` text-grep + // Tier 2: `claude mcp list` text-grep. Name-pattern generalized (#2051): + // a gbrain server registered as e.g. "gbrain-remote" or "gbrain_work" + // still counts. Anchored to the gbrain token so unrelated servers can't + // false-positive. const list = tryExec("claude", ["mcp", "list"], 3_000); if (list) { - const line = list.split("\n").find((l) => /^gbrain:/.test(l)); + const line = list.split("\n").find((l) => /^gbrain([-_][\w-]*)?:/.test(l)); if (line) { if (/\b(http|HTTP)\b/.test(line)) return "remote-http"; return "local-stdio"; @@ -186,20 +189,56 @@ function detectMcpMode(): "local-stdio" | "remote-http" | "none" { } } // Tier 3: read ~/.claude.json directly + interface McpServerEntry { + type?: string; + transport?: string; + command?: string; + url?: string; + } const cj = tryReadJSON(CLAUDE_JSON) as - | { mcpServers?: { gbrain?: { type?: string; transport?: string; command?: string; url?: string } } } + | { mcpServers?: Record } | null; - const entry = cj?.mcpServers?.gbrain; - if (entry) { + const classify = (entry: McpServerEntry): "local-stdio" | "remote-http" | null => { const mtype = entry.type || entry.transport || ""; if (mtype === "url" || mtype === "http" || mtype === "sse") return "remote-http"; if (mtype === "stdio") return "local-stdio"; if (entry.url) return "remote-http"; if (entry.command) return "local-stdio"; + return null; + }; + const servers = cj?.mcpServers || {}; + const exact = servers["gbrain"]; + if (exact) { + const c = classify(exact); + if (c) return c; + } + // #2051 generalization, deterministic identifiers first: + // (a) a server whose url matches the config's remote_mcp.mcp_url is THE + // thin-client brain regardless of its registered name (URL-path + // heuristics are impossible — gbrain mounts at the generic /mcp); + // (b) name pattern gbrain[-_]* ; + // (c) a stdio server whose command mentions gbrain. + const remoteMcpUrl = readRemoteMcpUrl(); + for (const [name, entry] of Object.entries(servers)) { + if (remoteMcpUrl && entry.url && entry.url === remoteMcpUrl) return "remote-http"; + if (/^gbrain([-_][\w-]*)?$/.test(name)) { + const c = classify(entry); + if (c) return c; + } + if (entry.command && /\bgbrain\b/.test(entry.command)) return "local-stdio"; } return "none"; } +/** remote_mcp.mcp_url from gbrain's own config (thin-client marker, #2051). */ +function readRemoteMcpUrl(): string { + const gbrainHome = process.env.GBRAIN_HOME || join(userHome(), ".gbrain"); + const cfg = tryReadJSON(join(gbrainHome, "config.json")) as + | { remote_mcp?: { mcp_url?: string } } + | null; + return cfg?.remote_mcp?.mcp_url || ""; +} + // --- artifacts remote URL with brain-* fallback during the rename migration window --- function detectArtifactsRemote(): string { const newPath = join(userHome(), ".gstack-artifacts-remote.txt"); @@ -237,20 +276,29 @@ function main(): void { gbrain_pooler_mode: detectPoolerMode(), }; - process.stdout.write(JSON.stringify(out, null, 2) + "\n"); + // #2051 honesty marker: on a thin client the classifier verified the CONFIG + // (remote_mcp present), not the remote's reachability — that is checked at + // use time, where gbrain calls degrade gracefully. + const withThinClient = + out.gbrain_local_status === "thin-client" + ? { ...out, gbrain_thin_client: { probed: false } } + : out; + + process.stdout.write(JSON.stringify(withThinClient, null, 2) + "\n"); } -// --is-ok: live engine-status gate. Exits 0 iff gbrain is usable ("ok", or -// "timeout" — a slow-but-healthy engine, #1964 — slow must not silently -// suppress brain features), 1 otherwise. Runs detection live (never reads -// the possibly-stale gbrain-detection.json), so callers — setup, +// --is-ok: live engine-status gate. Exits 0 iff gbrain is usable ("ok"; +// "timeout" — a slow-but-healthy engine, #1964; or "thin-client" — remote-HTTP +// MCP brain with no local engine by design, #2051 — neither slow nor remote +// must silently suppress brain features), 1 otherwise. Runs detection live +// (never reads the possibly-stale gbrain-detection.json), so callers — setup, // bin/dev-setup, and `gstack-config gbrain-refresh` — can decide whether to // render the gbrain :user variant without duplicating the JSON grep. // Prints nothing on stdout. if (process.argv.includes("--is-ok")) { const noCache = process.env.GSTACK_DETECT_NO_CACHE === "1"; const status = localEngineStatus({ noCache }); - process.exit(status === "ok" || status === "timeout" ? 0 : 1); + process.exit(status === "ok" || status === "timeout" || status === "thin-client" ? 0 : 1); } main(); diff --git a/bin/gstack-gbrain-sync.ts b/bin/gstack-gbrain-sync.ts index 07a94af3f..786c50255 100644 --- a/bin/gstack-gbrain-sync.ts +++ b/bin/gstack-gbrain-sync.ts @@ -720,6 +720,9 @@ function dreamMarkerPid(): number | null { * engine-locked → PGLite is busy; stop its holder or sync outside the live session * timeout → kept for Record totality; stages PROCEED on timeout (#1964) * via the gate's warnProbeTimeout path, never this skip. + * thin-client → remote-HTTP MCP brain, no local engine by design (#2051); + * local sync stages skip (gbrain refuses sources/sync there), + * but suppression gates treat the brain as USABLE. */ function skipStageForLocalStatus( stage: "code" | "memory" | "dream", @@ -738,6 +741,10 @@ function skipStageForLocalStatus( "PGLite is busy (often held by gbrain serve); stop the holding process or run /sync-gbrain outside the live Claude session, then retry", "timeout": "engine probe timed out; raise GSTACK_GBRAIN_PROBE_TIMEOUT_MS if your pooler is slow", + "thin-client": + "thin client (remote-HTTP MCP brain, no local engine by design, #2051); " + + "code indexing runs on the brain server, memory syncs via the remote " + + "brain's artifacts pull — nothing to do locally", }; const reason = reasons[status as Exclude]; return { diff --git a/bin/gstack-question-preference b/bin/gstack-question-preference index eb951ebd3..34271aeef 100755 --- a/bin/gstack-question-preference +++ b/bin/gstack-question-preference @@ -5,7 +5,9 @@ # Schema: { "": "always-ask" | "never-ask" | "ask-only-for-one-way" } # # Subcommands: -# --check → emit ASK_NORMALLY | AUTO_DECIDE | ASK_ONLY_ONE_WAY +# --check [--summary-stdin] → emit ASK_NORMALLY | AUTO_DECIDE | ASK_ONLY_ONE_WAY +# (--summary-stdin pipes the question text so the +# keyword net can catch ad-hoc destructive ids, #2024) # --write '{...}' → set a preference (user-origin gate enforced) # --read → dump preferences JSON # --clear [] → clear one or all preferences @@ -44,22 +46,39 @@ ensure_file() { # --check # ----------------------------------------------------------------------- do_check() { - local QID="${1:-}" + local QID="" SUMMARY_STDIN=false + while [ $# -gt 0 ]; do + case "$1" in + --summary-stdin) SUMMARY_STDIN=true; shift ;; + *) [ -z "$QID" ] && QID="$1"; shift ;; + esac + done if [ -z "$QID" ]; then echo "ASK_NORMALLY" return 0 fi + # #2024: the question text feeds the keyword classifier for unregistered + # ids. Transport is stdin (not argv) — summaries carry quotes, newlines, + # and shell metacharacters an argv tail would mangle. Handed to bun via + # env so no shell re-quoting happens. Empty summary = id-only behavior. + local QSUMMARY="" + if [ "$SUMMARY_STDIN" = true ]; then + QSUMMARY=$(cat 2>/dev/null || true) + fi ensure_file cd "$ROOT_DIR" - PREF_FILE_PATH="$PREF_FILE" QID="$QID" bun -e " + PREF_FILE_PATH="$PREF_FILE" QID="$QID" QSUMMARY="$QSUMMARY" bun -e " import('./scripts/one-way-doors.ts').then((oneway) => { const fs = require('fs'); const qid = process.env.QID; + const summary = process.env.QSUMMARY || undefined; const prefs = JSON.parse(fs.readFileSync(process.env.PREF_FILE_PATH, 'utf-8')); const pref = prefs[qid]; // Always check one-way status first — safety overrides preferences. - const oneWay = oneway.isOneWayDoor({ question_id: qid }); + // summary (when piped) lets the keyword net catch ad-hoc destructive + // questions whose id has no registry entry (#2024). + const oneWay = oneway.isOneWayDoor({ question_id: qid, summary }); if (oneWay) { console.log('ASK_NORMALLY'); diff --git a/canary/SKILL.md b/canary/SKILL.md index 08d4d7369..a7eded4fe 100644 --- a/canary/SKILL.md +++ b/canary/SKILL.md @@ -703,7 +703,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 22bf8b922..1e488bae7 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -30,7 +30,14 @@ CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]') # syntax or comments can hide an earlier destructive command, for example: # rm -rf / # rm -rf node_modules # Unknown syntax fails closed and falls through to the destructive checks. -if printf '%s' "$CMD" | grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*r[a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' 2>/dev/null; then +# Two hardenings on top of the anchored shape (#2039 wave): +# - flag cluster accepts capital -R (BSD/macOS recursive), so a single +# `rm -Rf node_modules` stays allowed instead of prompting; +# - target tokens exclude `(` and backtick, so command substitution that +# ENDS in a whitelisted suffix (`rm -rf $(./wipe-all)/node_modules`) +# cannot ride the whitelist. Plain $VAR expansion (no parenthesis) is +# still allowed. +if printf '%s' "$CMD" | grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*[rR][a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#(`]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' 2>/dev/null; then echo '{}' exit 0 fi @@ -39,8 +46,8 @@ fi WARN="" PATTERN="" -# rm -rf / rm -r / rm --recursive -if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*r|--recursive)' 2>/dev/null; then +# rm -rf / rm -r / rm -R / rm --recursive (capital -R is BSD/macOS recursive) +if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' 2>/dev/null; then WARN="Destructive: recursive delete (rm -r). This permanently removes files." PATTERN="rm_recursive" fi diff --git a/codex/SKILL.md b/codex/SKILL.md index 33228ff9b..af7fa4de9 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -706,7 +706,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/context-restore/SKILL.md b/context-restore/SKILL.md index 59b40e82c..500731936 100644 --- a/context-restore/SKILL.md +++ b/context-restore/SKILL.md @@ -23,8 +23,8 @@ triggers: ## When to invoke this skill Loads the most recent -saved state (across all branches by default) so you can pick up where you -left off — even across Conductor workspace handoffs. +saved state (preferring the current branch, falling back across branches) so +you can pick up where you left off — even across Conductor workspace handoffs. Use when asked to "resume", "restore context", "where was I", or "pick up where I left off". Pair with /context-save. Formerly /checkpoint resume — renamed because Claude Code treats /checkpoint @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. @@ -791,13 +791,19 @@ context and present it clearly so the user can resume work without losing a beat **HARD GATE:** Do NOT implement code changes. This skill only reads saved context files and presents the summary. -**Default: load the most recent saved context across ALL branches.** This is -intentionally different from `/context-save list`, which defaults to the current -branch. `/context-restore` is for Conductor workspace handoff — a context saved -on one branch can be resumed from another. +**Default: prefer the most recent checkpoint saved on the CURRENT branch; if +this branch has none, fall back to the most recent across ALL branches.** The +fallback is for Conductor workspace handoff — a context saved on one branch can +be resumed from another. The current-branch preference exists because every +worktree of a repo shares one checkpoints directory (same origin-derived slug), +so without it `/context-restore` in one worktree could silently load a sibling +worktree's newer checkpoint. -**Do NOT filter the candidate set by current branch.** The `list` flow does -that; `/context-restore` does not. +**Do NOT hard-filter the candidate set to the current branch** — other-branch +checkpoints stay in the set as a fallback. They are just ordered *after* the +current branch's own, so a current-branch save is never shadowed by a newer +sibling-worktree save. (`/context-save list` is the flow that hard-scopes to one +branch.) --- @@ -828,27 +834,53 @@ else # copies/rsync). Filesystem mtime drifts and is not authoritative. # 2. On macOS, `find ... | xargs ls -1t` with zero results falls back to # listing cwd. `sort -r` on empty input cleanly returns nothing. - # Cap at 20 most recent: a user with 10k saved files shouldn't blow the - # context window just listing them. /context-save list handles pagination. - FILES=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -20) - if [ -z "$FILES" ]; then + # Scan the 200 newest so a current-branch checkpoint sitting below a burst of + # sibling-worktree saves can still be found; the result is capped at 20 below. + ALL=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -200) + if [ -z "$ALL" ]; then echo "NO_CHECKPOINTS" else + # Order current-branch checkpoints first, other branches after. A git branch + # is checked out in at most one worktree, and all worktrees of a repo share + # one checkpoints dir (same origin-derived slug), so without this preference + # `/context-restore` in worktree A could load worktree B's newer checkpoint. + # Cross-branch resume (Conductor handoff) is preserved as the fallback: when + # the current branch has no checkpoint, the full newest-first set is used. + # CURRENT_BRANCH may be pre-set (tests); otherwise resolve it from git. + : "${CURRENT_BRANCH:=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}" + SAME=""; OTHER="" + while IFS= read -r f; do + [ -n "$f" ] || continue + b=$(grep -m1 '^branch:' "$f" 2>/dev/null | sed 's/^branch:[[:space:]]*//') + if [ -n "$CURRENT_BRANCH" ] && [ "$b" = "$CURRENT_BRANCH" ]; then + SAME="${SAME}${f} +" + else + OTHER="${OTHER}${f} +" + fi + done </dev/null | sort -r | head -20) - if [ -z "$FILES" ]; then + # Scan the 200 newest so a current-branch checkpoint sitting below a burst of + # sibling-worktree saves can still be found; the result is capped at 20 below. + ALL=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -200) + if [ -z "$ALL" ]; then echo "NO_CHECKPOINTS" else + # Order current-branch checkpoints first, other branches after. A git branch + # is checked out in at most one worktree, and all worktrees of a repo share + # one checkpoints dir (same origin-derived slug), so without this preference + # `/context-restore` in worktree A could load worktree B's newer checkpoint. + # Cross-branch resume (Conductor handoff) is preserved as the fallback: when + # the current branch has no checkpoint, the full newest-first set is used. + # CURRENT_BRANCH may be pre-set (tests); otherwise resolve it from git. + : "${CURRENT_BRANCH:=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}" + SAME=""; OTHER="" + while IFS= read -r f; do + [ -n "$f" ] || continue + b=$(grep -m1 '^branch:' "$f" 2>/dev/null | sed 's/^branch:[[:space:]]*//') + if [ -n "$CURRENT_BRANCH" ] && [ "$b" = "$CURRENT_BRANCH" ]; then + SAME="${SAME}${f} +" + else + OTHER="${OTHER}${f} +" + fi + done <"`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/cso/SKILL.md b/cso/SKILL.md index a08d7e9fe..cd1e346fc 100644 --- a/cso/SKILL.md +++ b/cso/SKILL.md @@ -709,7 +709,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/design-consultation/SKILL.md b/design-consultation/SKILL.md index 83eed0a2d..9b52914ea 100644 --- a/design-consultation/SKILL.md +++ b/design-consultation/SKILL.md @@ -729,7 +729,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/design-html/SKILL.md b/design-html/SKILL.md index a480bd62c..8222a3f6c 100644 --- a/design-html/SKILL.md +++ b/design-html/SKILL.md @@ -710,7 +710,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/design-review/SKILL.md b/design-review/SKILL.md index 645453162..56bd0ff41 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/design-shotgun/SKILL.md b/design-shotgun/SKILL.md index 3386d18fa..d18254b8b 100644 --- a/design-shotgun/SKILL.md +++ b/design-shotgun/SKILL.md @@ -724,7 +724,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/design/src/cli.ts b/design/src/cli.ts index a55d92555..b266c6f38 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -25,6 +25,7 @@ import { evolve } from "./evolve"; import { generateDesignToCodePrompt } from "./design-to-code"; import { serve } from "./serve"; import { gallery } from "./gallery"; +import { normalizeIntFlag } from "./flag-utils"; import { daemonStatus as daemonStatusClient, ensureDaemon, @@ -137,7 +138,7 @@ async function main(): Promise { briefFile: flags["brief-file"] as string, output: (flags.output as string) || "/tmp/gstack-mockup.png", check: !!flags.check, - retry: flags.retry ? parseInt(flags.retry as string) : 0, + retry: normalizeIntFlag(flags.retry, { name: "retry", def: 0, min: 0 }), size: flags.size as string, quality: flags.quality as string, }); @@ -163,7 +164,7 @@ async function main(): Promise { if (flags["no-daemon"]) { await serve({ html: outputPath, - timeout: flags.timeout ? parseInt(flags.timeout as string) : 600, + timeout: normalizeIntFlag(flags.timeout, { name: "timeout", def: 600, min: 1 }), }); } else { await publishToDaemon({ @@ -197,7 +198,9 @@ async function main(): Promise { await variants({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, - count: flags.count ? parseInt(flags.count as string) : 3, + // #2032: pass the RAW flag through — variants() normalizes at its + // consumption site (a pre-parseInt here would silently truncate "3.7"). + count: flags.count, outputDir: (flags["output-dir"] as string) || "/tmp/gstack-variants/", size: flags.size as string, quality: flags.quality as string, diff --git a/design/src/flag-utils.ts b/design/src/flag-utils.ts new file mode 100644 index 000000000..9959a653b --- /dev/null +++ b/design/src/flag-utils.ts @@ -0,0 +1,74 @@ +/** + * Integer flag normalization for the design CLI (#2032). + * + * The CLI's flag parser yields a string ("3"), boolean true (bare flag with + * no value), or undefined (flag absent). parseInt on those produced NaN that + * flowed silently into loop bounds and setTimeout: + * --count abc → for (i < NaN) never runs → ZERO variants, exit 0 + * --retry abc → attempt <= NaN is false → generate() silent no-op + * --timeout abc → setTimeout(NaN) fires ~immediately → serve dies at boot + * + * Contract (matches the --viewports precedent, variants.ts: error LOUDLY on + * nonsense; these commands spend real image-API money, so a silent fixup + * hides typos from calling agents): + * - undefined → default (flag absent) + * - true / "" (bare flag) → error: requires a value + * - non-numeric / non-integer → error ("3.7" is rejected, not truncated) + * - below min → error + * - above max (when given) → clamp to max, stderr warning + * - repeated flag → parser is last-wins before we ever see it + */ + +export interface IntFlagSpec { + name: string; + def: number; + min: number; + max?: number; +} + +export type IntFlagResult = + | { ok: true; value: number; warning?: string } + | { ok: false; error: string }; + +/** Pure decision function — unit-testable without process.exit. */ +export function parseIntFlag(raw: unknown, spec: IntFlagSpec): IntFlagResult { + const { name, def, min, max } = spec; + const bounds = `an integer >= ${min}${max !== undefined ? ` (max ${max})` : ""}`; + + if (raw === undefined || raw === false) return { ok: true, value: def }; + if (raw === true) { + return { ok: false, error: `--${name} requires a value. Expected ${bounds}.` }; + } + + const s = String(raw).trim(); + if (s === "") { + return { ok: false, error: `--${name} requires a value. Expected ${bounds}.` }; + } + if (!/^-?\d+$/.test(s)) { + return { ok: false, error: `Invalid --${name}: "${s}" is not an integer. Expected ${bounds}.` }; + } + + const n = parseInt(s, 10); + if (n < min) { + return { ok: false, error: `Invalid --${name}: ${n} is below the minimum of ${min}.` }; + } + if (max !== undefined && n > max) { + return { + ok: true, + value: max, + warning: `--${name} ${n} exceeds the maximum of ${max}; using ${max}.`, + }; + } + return { ok: true, value: n }; +} + +/** CLI wrapper: loud exit(1) on invalid input, stderr warning on clamp. */ +export function normalizeIntFlag(raw: unknown, spec: IntFlagSpec): number { + const r = parseIntFlag(raw, spec); + if (!r.ok) { + console.error(r.error); + process.exit(1); + } + if (r.warning) console.error(r.warning); + return r.value; +} diff --git a/design/src/variants.ts b/design/src/variants.ts index 257079dea..15be75e5c 100644 --- a/design/src/variants.ts +++ b/design/src/variants.ts @@ -8,11 +8,17 @@ import fs from "fs"; import path from "path"; import { requireApiKey } from "./auth"; import { parseBrief } from "./brief"; +import { normalizeIntFlag } from "./flag-utils"; export interface VariantsOptions { brief?: string; briefFile?: string; - count: number; + /** + * Raw CLI flag value or a number. Normalized inside variants() (#2032): + * nonsense errors loudly; above STYLE_VARIATIONS.length clamps with a + * warning — past that index variants degrade to duplicate base-brief runs. + */ + count?: number | string | boolean; outputDir: string; size?: string; quality?: string; @@ -153,7 +159,15 @@ export async function variants(options: VariantsOptions): Promise { return; } - const count = Math.min(options.count, 7); // Cap at 7 style variations + // #2032: normalize at the consumption site so every caller (CLI or + // programmatic) gets the loud-on-nonsense contract; the ceiling derives + // from STYLE_VARIATIONS so it self-adjusts when styles are added. + const count = normalizeIntFlag(options.count, { + name: "count", + def: 3, + min: 1, + max: STYLE_VARIATIONS.length, + }); const size = options.size || "1536x1024"; console.error(`Generating ${count} variants...`); diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 7ef324b3e..4903ac4f0 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -709,7 +709,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/diagram/SKILL.md b/diagram/SKILL.md index 9e5a41066..215f90efd 100644 --- a/diagram/SKILL.md +++ b/diagram/SKILL.md @@ -704,7 +704,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/docs/spikes/claude-code-hook-mutation.md b/docs/spikes/claude-code-hook-mutation.md index 9d91e16cd..6dde72d87 100644 --- a/docs/spikes/claude-code-hook-mutation.md +++ b/docs/spikes/claude-code-hook-mutation.md @@ -51,7 +51,13 @@ Optional in subagent context: `agent_id`, `agent_type`. - `"deny"` — block (feedback to Claude, NOT a synthetic answer per Codex correction in D-prefixed decisions) - `"ask"` — escalate to user -- `"defer"` — let permission flow continue +- `"defer"` — pause the tool call for EXTERNAL resumption (Claude Code + v2.1.89+, a headless feature: resume with `-p --resume` to re-evaluate). + NEVER emit this to mean "no opinion" — in an interactive session nothing + resumes the paused call and the tool dies with "Tool result missing due to + internal error" (#2035, #2006). To abstain, exit 0 with EMPTY stdout + (optionally `hookSpecificOutput` with `additionalContext` only, no + `permissionDecision`). **`updatedInput` semantics:** shallow merge of fields present in the returned object onto the original `tool_input`. Only valid with @@ -106,15 +112,20 @@ required for our hook to fire there. } ``` -**Pass-through (no preference, or one-way safety override):** +**Pass-through (no preference, or one-way safety override):** exit 0 with +EMPTY stdout. When there is context to inject (plan-tune memory nuggets), +emit `additionalContext` WITHOUT a `permissionDecision`: ```json { "hookSpecificOutput": { "hookEventName": "PreToolUse", - "permissionDecision": "defer" + "additionalContext": "[plan-tune memory] Past answers suggest: ..." } } ``` +(Historical note: this example originally emitted `permissionDecision: +"defer"`, which broke every AskUserQuestion once CC v2.1.89 gave 'defer' +pause-for-resume semantics — #2035.) **PostToolUse capture (always):** ```json diff --git a/document-generate/SKILL.md b/document-generate/SKILL.md index 30846fc4d..f681a612e 100644 --- a/document-generate/SKILL.md +++ b/document-generate/SKILL.md @@ -709,7 +709,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/document-release/SKILL.md b/document-release/SKILL.md index b95873625..b7fe35f0e 100644 --- a/document-release/SKILL.md +++ b/document-release/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/health/SKILL.md b/health/SKILL.md index e68199dec..659ba78d2 100644 --- a/health/SKILL.md +++ b/health/SKILL.md @@ -705,7 +705,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/hosts/claude/hooks/question-preference-hook.ts b/hosts/claude/hooks/question-preference-hook.ts index 12cbd5ea2..505eea382 100644 --- a/hosts/claude/hooks/question-preference-hook.ts +++ b/hosts/claude/hooks/question-preference-hook.ts @@ -12,11 +12,15 @@ * 2. Look up door_type from scripts/question-registry.ts (default two-way). * 3. Read preferences with precedence: project-local > global (D8). * 4. Apply: - * never-ask + one-way → defer (safety override; one-way always asks). + * never-ask + one-way → pass through (safety override; one-way always asks). * never-ask + two-way + marker → deny with auto-decided recommendation * in reason. Mark tool_use_id so PostToolUse logs as 'auto-decided'. * ask-only-for-one-way + two-way + marker → same as never-ask. - * always-ask, or no preference → defer. + * always-ask, or no preference → pass through. + * + * Pass-through = exit 0 with empty stdout (or additionalContext-only output + * when memory nuggets exist) — NEVER permissionDecision:'defer', whose + * CC v2.1.89+ semantics are pause-for-external-resumption (#2035, #2006). * * Why deny+reason instead of allow+updatedInput: * AskUserQuestion's `updatedInput` shape for "pre-resolve this question" @@ -31,7 +35,7 @@ * - First: (recommended) label suffix on an option. * - Fall back: "Recommendation: X" prose match against option labels. * - Refuse to auto-decide if ambiguous (multiple labels OR no parseable - * recommendation): defer instead of silent-wrong. + * recommendation): pass through instead of silent-wrong. * * Always exits 0. Hook errors land in ~/.gstack/hook-errors.log. * See docs/spikes/claude-code-hook-mutation.md for the protocol contract. @@ -41,6 +45,7 @@ import * as path from 'path'; import * as os from 'os'; import { spawnSync } from 'child_process'; import { isConductor } from '../../../lib/is-conductor'; +import { classifyQuestion } from '../../../scripts/one-way-doors'; interface HookStdin { session_id?: string; @@ -92,13 +97,25 @@ function readStdin(): Promise { }); } -function defer(additionalContext?: string): void { - const out: Record = { - hookEventName: 'PreToolUse', - permissionDecision: 'defer', - }; - if (additionalContext) out.additionalContext = additionalContext; - process.stdout.write(JSON.stringify({ hookSpecificOutput: out })); +function passThrough(additionalContext?: string): void { + // Abstain = exit 0 with EMPTY stdout (#2035, #2006). Never emit a + // permissionDecision here: 'defer' is a real PreToolUse value, but since + // Claude Code v2.1.89 its semantics are "pause this tool call for external + // resumption" (a headless-resume feature) — NOT "no opinion". In an + // interactive session nothing resumes the paused call, so every + // AskUserQuestion died with "Tool result missing due to internal error". + // additionalContext-only hookSpecificOutput is the documented shape for + // injecting context (plan-tune memory nuggets) without a decision. + if (additionalContext) { + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + additionalContext, + }, + }), + ); + } process.exit(0); } @@ -347,7 +364,7 @@ function logAutoDecided( async function main(): Promise { const raw = await readStdin(); if (!raw.trim()) { - defer(); + passThrough(); return; } let stdin: HookStdin; @@ -355,7 +372,7 @@ async function main(): Promise { stdin = JSON.parse(raw); } catch (e) { logHookError(`stdin parse failed: ${(e as Error).message}`); - defer(); + passThrough(); return; } @@ -364,26 +381,26 @@ async function main(): Promise { toolName !== 'AskUserQuestion' && !toolName.match(/^mcp__.+__AskUserQuestion$/) ) { - defer(); + passThrough(); return; } const questions = stdin.tool_input?.questions || []; if (questions.length === 0) { - defer(); + passThrough(); return; } // For multi-question AUQ, enforcement is all-or-nothing per call: // we deny only if ALL questions have marker + never-ask + safe door type. - // Mixed cases pass through (defer) so the user still gets to answer. + // Mixed cases pass through so the user still gets to answer. const registry = loadRegistry(); const slug = slugFromCwd(stdin.cwd); const memoryNuggets = loadMemoryNuggets(stdin.session_id); // Compute Layer 8 memory context inline: any nuggets matching the // signal_keys of the questions in this AUQ get surfaced as additionalContext. - // This applies whether we defer OR deny — gives the agent + user the + // This applies whether we pass through OR deny — gives the agent + user the // relevant prior context either way. const contextNuggets: string[] = []; for (const q of questions) { @@ -402,7 +419,7 @@ async function main(): Promise { : undefined; // Determine whether EVERY question is eligible for never-ask auto-decide. - // We deliberately do NOT early-return defer on the first ineligible question: + // We deliberately do NOT early-return pass-through on the first ineligible question: // a Conductor session still needs the [conductor] prose deny as a fallback, // so we compute eligibility, then branch. memoryContext is preserved on every // non-enforcing exit. (All-or-nothing per-call semantics are unchanged: any @@ -418,7 +435,21 @@ async function main(): Promise { if (!pref.preference || pref.preference === 'always-ask') { fullyAutoDecidable = false; break; } const entry = registry[questionId]; - const doorType = entry?.door_type || 'two-way'; + let doorType: string = entry?.door_type || 'two-way'; + if (!entry) { + // #2024: an unregistered id used to default straight to two-way without + // consulting the keyword net, so an ad-hoc DESTRUCTIVE question with a + // stored never-ask preference auto-decided. classifyQuestion is a pure + // regex pass over the question text; on any failure keep the default + // (enforcement still requires an explicit stored preference). + try { + if (classifyQuestion({ summary: qText.replace(MARKER_RE, '').trim() }).oneWay) { + doorType = 'one-way'; + } + } catch (e) { + logHookError(`one-way classifier failed: ${(e as Error).message}`); + } + } // Safety override — even never-ask doesn't bypass one-way doors. if (doorType === 'one-way') { fullyAutoDecidable = false; break; } @@ -471,10 +502,10 @@ async function main(): Promise { return; } - defer(memoryContext); + passThrough(memoryContext); } main().catch((e) => { logHookError(`main crash: ${(e as Error).message}`); - defer(); + passThrough(); }); diff --git a/investigate/SKILL.md b/investigate/SKILL.md index 5d54b4256..e7d81a45b 100644 --- a/investigate/SKILL.md +++ b/investigate/SKILL.md @@ -744,7 +744,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/ios-clean/SKILL.md b/ios-clean/SKILL.md index 6c466896b..a1280e395 100644 --- a/ios-clean/SKILL.md +++ b/ios-clean/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/ios-design-review/SKILL.md b/ios-design-review/SKILL.md index 904da7589..633dd1820 100644 --- a/ios-design-review/SKILL.md +++ b/ios-design-review/SKILL.md @@ -709,7 +709,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/ios-fix/SKILL.md b/ios-fix/SKILL.md index 3ddae1ac0..81176c9ea 100644 --- a/ios-fix/SKILL.md +++ b/ios-fix/SKILL.md @@ -710,7 +710,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/ios-qa/SKILL.md b/ios-qa/SKILL.md index 24624be5f..4151d269b 100644 --- a/ios-qa/SKILL.md +++ b/ios-qa/SKILL.md @@ -713,7 +713,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/ios-sync/SKILL.md b/ios-sync/SKILL.md index 95e0bca0a..ceef05de6 100644 --- a/ios-sync/SKILL.md +++ b/ios-sync/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/land-and-deploy/SKILL.md b/land-and-deploy/SKILL.md index 54ebf52c0..0df40c312 100644 --- a/land-and-deploy/SKILL.md +++ b/land-and-deploy/SKILL.md @@ -702,7 +702,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/landing-report/SKILL.md b/landing-report/SKILL.md index 8f7e6e210..927244c58 100644 --- a/landing-report/SKILL.md +++ b/landing-report/SKILL.md @@ -703,7 +703,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/learn/SKILL.md b/learn/SKILL.md index a0c6ae053..cf08d6b80 100644 --- a/learn/SKILL.md +++ b/learn/SKILL.md @@ -705,7 +705,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index 2b6caa780..174409ca1 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -24,6 +24,12 @@ * Timeout → probe exceeded GSTACK_GBRAIN_PROBE_TIMEOUT_MS (default 15s) with no * recognized error — engine is likely healthy but slow (e.g. a cold * pooler connection, #1964). Consumers treat this as usable. + * Thin-client → config carries gbrain's remote_mcp marker (#2051): NO local + * engine by design; queries go to a remote-HTTP MCP brain. Usable + * for brain-aware prose gates; sync stages that need a LOCAL engine + * (code/memory/dream) skip. Remote reachability is verified at USE + * time (gbrain calls degrade gracefully), never by a classifier + * network probe — that's the #1964 pathology. * Ok → DB reachable, sources list returned valid JSON. */ @@ -50,7 +56,8 @@ export type LocalEngineStatus = | "broken-config" | "broken-db" | "engine-locked" - | "timeout"; + | "timeout" + | "thin-client"; export interface ClassifyOptions { /** Bypass the 60s cache. Used after any state-mutating operation. */ @@ -270,6 +277,26 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus { // 2. Config file present? if (!existsSync(gbrainConfigPath(env))) return "missing-config"; + // 2.5 Thin client? gbrain's own marker (mirrors gbrain isThinClient(): + // truthy remote_mcp in config). A thin client has NO local engine — gbrain + // REFUSES `sources` commands on it (THIN_CLIENT_REFUSED_COMMANDS, exit 1 + // with no recognized error string), so the probe below would fall to the + // defensive broken-config default and silently suppress brain-aware blocks + // (#2051). Detected PRE-probe from the config file: zero network cost, + // immune to gbrain error-string drift. Remote reachability is deliberately + // NOT probed here — a classifier network probe is the #1964 pathology. + try { + const cfg = JSON.parse(readFileSync(gbrainConfigPath(env), "utf-8")) as { + remote_mcp?: unknown; + }; + if (cfg && typeof cfg === "object" && cfg.remote_mcp) { + return "thin-client"; + } + } catch { + // Unparseable config: fall through to the probe, whose stderr + // classification surfaces broken-config with the raw error upstream. + } + // 3. Probe gbrain sources list. // // Seed DATABASE_URL from ~/.gbrain/config.json (via buildGbrainEnv, the @@ -301,7 +328,11 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus { if (e.code === "ENOENT") return "no-cli"; // Pattern match against gbrain's known error strings. Order matters: - // "Cannot connect to database" is the more specific DB-unreachable signal. + // thin-client refusal first (backstop for a config the pre-probe check + // couldn't read — gbrain's dispatch guard says e.g. "`gbrain sources` is + // not routable ... (thin-client of )"), then the more specific + // DB-unreachable signal. + if (/thin[- ]client/i.test(stderr)) return "thin-client"; if (stderr.includes("Cannot connect to database")) return "broken-db"; if (stderr.includes("config.json")) return "broken-config"; diff --git a/lib/gbrain-sources.ts b/lib/gbrain-sources.ts index cdaa25eaa..826e905ac 100644 --- a/lib/gbrain-sources.ts +++ b/lib/gbrain-sources.ts @@ -10,8 +10,15 @@ */ import { execFileSync, spawnSync } from "child_process"; +import { realpathSync } from "fs"; import { withErrorContext } from "./gstack-memory-helpers"; import { execGbrainJson, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; +import { + detectAutopilot, + decideSourceRemove, + type AutopilotProbe, + type DecideRemoveOpts, +} from "./gbrain-guards"; export interface SourceState { /** "absent" — id not registered. "match" — id at expected path. "drift" — id at different path. */ @@ -70,6 +77,33 @@ export interface EnsureOptions { * mutations of process.env.PATH unless env is passed explicitly). */ env?: NodeJS.ProcessEnv; + /** + * #1734 test hooks for the drift-remove guards. Production callers leave + * these unset (real autopilot detection + real remove decision). Tests pin + * them so a live autopilot on the dev machine can't flip test outcomes. + */ + autopilotProbe?: AutopilotProbe; + removeDecision?: DecideRemoveOpts; +} + +/** + * Path equality with realpath normalization (macOS /tmp -> /private/tmp, + * symlinked worktrees). A registered path that resolves to the same real + * directory is NOT drift — declaring it drift triggers a destructive + * remove+add and a full re-index for a no-op (#1985 reporter hit the remove + * on an unmoved repo). + */ +function samePath(registered: string | undefined, requested: string): boolean { + if (!registered) return false; + if (registered === requested) return true; + const real = (p: string): string => { + try { + return realpathSync(p); + } catch { + return p; + } + }; + return real(registered) === real(requested); } /** @@ -124,7 +158,7 @@ export function probeSource(id: string, env?: NodeJS.ProcessEnv): SourceState { * Behavior: * - status=absent → `gbrain sources add --path [--federated]`, returns changed=true. * - status=match + same path → no-op, returns changed=false. - * - status=match + different path → `sources remove` + `sources add`, returns changed=true. + * - status=match + different path → `sources remove --confirm-destructive` + `sources add`, returns changed=true. * (Skip when reregister_on_drift=false; returns changed=false.) * * Caller is responsible for catching errors. The function uses withErrorContext for @@ -142,9 +176,10 @@ export async function ensureSourceRegistered( return withErrorContext(`ensureSourceRegistered:${id}`, () => { const probed = probeSource(id, env); - // Disambiguate match-but-different-path + // Disambiguate match-but-different-path (realpath-normalized: a symlink + // alias of the same directory is a match, not drift). let state: SourceState = probed; - if (probed.status === "match" && probed.registered_path !== path) { + if (probed.status === "match" && !samePath(probed.registered_path, path)) { state = { status: "drift", registered_path: probed.registered_path }; } @@ -157,13 +192,48 @@ export async function ensureSourceRegistered( } // For drift, remove first. + // + // #1985: gbrain >= 0.42 gates `sources remove` behind --confirm-destructive + // (`--yes` alone no longer suppresses the data-loss prompt). Without it the + // remove fails with "To proceed, pass --confirm-destructive", which surfaces + // as "source registration failed" and aborts the whole /sync-gbrain code + // stage for any source that has drifted to a new path. This matches the + // flag the orchestrator's own safeSourcesRemove() already passes. if (state.status === "drift") { - const rm = spawnSync("gbrain", ["sources", "remove", id, "--yes"], { - encoding: "utf-8", - timeout: 30_000, - env, - shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows - }); + // Loud drift observability: if this line shows up on every sync for some + // environment, drift is perpetual there and the reindex-in-place design + // from #1985 should be promoted (drop+rebuild re-embeds the full index). + console.error( + `[gbrain-sources] drift: ${id} registered at ${state.registered_path} -> re-registering at ${path}`, + ); + + // #1734: this remove deletes the source's pages/chunks/embeddings, so it + // runs only behind the same data-loss guards as the orchestrator's + // safeSourcesRemove(). A refusal is FATAL here (not best-effort): without + // the remove the add cannot proceed, and returning changed=false would + // silently hide the drifted registration. + const ap = detectAutopilot(env ?? process.env, options.autopilotProbe ?? {}); + if (ap.active) { + throw new Error( + `refusing drift re-register of ${id}: autopilot active (${ap.signal}). ` + + `Stop autopilot, then re-run /sync-gbrain.`, + ); + } + const decision = decideSourceRemove(id, env ?? process.env, options.removeDecision ?? {}); + if (!decision.allow) { + throw new Error(`refusing drift re-register of ${id}: ${decision.reason}`); + } + + const rm = spawnSync( + "gbrain", + ["sources", "remove", id, "--yes", "--confirm-destructive", ...decision.extraArgs], + { + encoding: "utf-8", + timeout: 30_000, + env, + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows + }, + ); if (rm.status !== 0) { throw new Error(`gbrain sources remove ${id} failed: ${rm.stderr || rm.stdout || `exit ${rm.status}`}`); } diff --git a/office-hours/SKILL.md b/office-hours/SKILL.md index 83161b8ca..b304a23b4 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -740,7 +740,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/open-gstack-browser/SKILL.md b/open-gstack-browser/SKILL.md index 28fb1ddb2..507ecceb2 100644 --- a/open-gstack-browser/SKILL.md +++ b/open-gstack-browser/SKILL.md @@ -702,7 +702,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/package.json b/package.json index 78b98a86f..9ffb1caf7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.60.2.0", + "version": "1.61.0.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/pair-agent/SKILL.md b/pair-agent/SKILL.md index eed9d171a..367f56d2a 100644 --- a/pair-agent/SKILL.md +++ b/pair-agent/SKILL.md @@ -704,7 +704,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index 3d3208bee..4ad55529a 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -734,7 +734,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index e81f7f12a..dfd4f4253 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -706,7 +706,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/plan-devex-review/SKILL.md b/plan-devex-review/SKILL.md index 20a32da8b..698900edb 100644 --- a/plan-devex-review/SKILL.md +++ b/plan-devex-review/SKILL.md @@ -712,7 +712,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 5557a33fa..448c36e24 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -710,7 +710,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/plan-tune/SKILL.md b/plan-tune/SKILL.md index f49b66fac..cbb6bb618 100644 --- a/plan-tune/SKILL.md +++ b/plan-tune/SKILL.md @@ -715,7 +715,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/qa-only/SKILL.md b/qa-only/SKILL.md index 801a935c0..9953eb228 100644 --- a/qa-only/SKILL.md +++ b/qa-only/SKILL.md @@ -705,7 +705,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/qa/SKILL.md b/qa/SKILL.md index c1ac10253..38b1131cd 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -711,7 +711,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/retro/SKILL.md b/retro/SKILL.md index 3fbc44726..3016295a6 100644 --- a/retro/SKILL.md +++ b/retro/SKILL.md @@ -722,7 +722,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/review/SKILL.md b/review/SKILL.md index 5f26e2e42..dbc28f704 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/scrape/SKILL.md b/scrape/SKILL.md index dc965ec5f..86ae0fc7b 100644 --- a/scrape/SKILL.md +++ b/scrape/SKILL.md @@ -703,7 +703,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 71aa1a34c..5e11a2efe 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -47,9 +47,15 @@ function loadGbrainOverride(): { detected: boolean } { const detectionPath = path.join(stateDir, 'gbrain-detection.json'); try { const json = JSON.parse(fs.readFileSync(detectionPath, 'utf-8')) as { gbrain_local_status?: string }; - // "timeout" = slow-but-healthy engine (#1964) — same treatment as "ok", - // matching gstack-gbrain-detect --is-ok. - return { detected: json.gbrain_local_status === 'ok' || json.gbrain_local_status === 'timeout' }; + // "timeout" = slow-but-healthy engine (#1964); "thin-client" = remote-HTTP + // MCP brain with no local engine by design (#2051). Both usable — same + // treatment as "ok", matching gstack-gbrain-detect --is-ok. + return { + detected: + json.gbrain_local_status === 'ok' || + json.gbrain_local_status === 'timeout' || + json.gbrain_local_status === 'thin-client', + }; } catch { return { detected: false }; } diff --git a/scripts/one-way-doors.ts b/scripts/one-way-doors.ts index 35f0f69cd..61c0b24ea 100644 --- a/scripts/one-way-doors.ts +++ b/scripts/one-way-doors.ts @@ -63,12 +63,14 @@ const DESTRUCTIVE_PATTERNS: RegExp[] = [ /\brollback\b/i, // Credentials / auth — allow filler words ("the", "my") between verb and noun. - // Keep the noun alternation identical across revoke/reset/rotate so the three - // verbs stay parallel; a noun in one but not the others is a false-negative - // safety hole (e.g. "reset my secret" must be one-way just like "rotate my secret"). - /\brevoke\s+[\w\s]*\b(api key|token|secret|credential|access key|password)\b/i, - /\breset\s+[\w\s]*\b(api key|token|secret|credential|access key|password)\b/i, - /\brotate\s+[\w\s]*\b(api key|token|secret|credential|access key|password)\b/i, + // Keep the noun alternation IDENTICAL across revoke/reset/rotate — a noun in + // one but not the others is a false-negative safety hole (#2024: "reset my + // secret" / "reset my access key" / "revoke my secret" leaked as two-way). + // Optional plural `s?` on the noun: \b(...)\b alone cannot match + // "credentials" / "tokens" / "passwords". + /\brevoke\s+[\w\s]*\b(api key|token|secret|credential|access key|password)s?\b/i, + /\breset\s+[\w\s]*\b(api key|token|secret|credential|access key|password)s?\b/i, + /\brotate\s+[\w\s]*\b(api key|token|secret|credential|access key|password)s?\b/i, // Scope / architecture forks (reversible with effort — still deserve confirmation) /\barchitectur(e|al)\s+(change|fork|shift|decision)\b/i, diff --git a/scripts/proactive-suggestions.json b/scripts/proactive-suggestions.json index d08c60853..daf7b3742 100644 --- a/scripts/proactive-suggestions.json +++ b/scripts/proactive-suggestions.json @@ -40,7 +40,7 @@ }, "context-restore": { "lead": "Restore working context saved earlier by /context-save.", - "routing": "Loads the most recent\nsaved state (across all branches by default) so you can pick up where you\nleft off — even across Conductor workspace handoffs.\nUse when asked to \"resume\", \"restore context\", \"where was I\", or\n\"pick up where I left off\". Pair with /context-save.\nFormerly /checkpoint resume — renamed because Claude Code treats /checkpoint\nas a native rewind alias in current environments.", + "routing": "Loads the most recent\nsaved state (preferring the current branch, falling back across branches) so\nyou can pick up where you left off — even across Conductor workspace handoffs.\nUse when asked to \"resume\", \"restore context\", \"where was I\", or\n\"pick up where I left off\". Pair with /context-save.\nFormerly /checkpoint resume — renamed because Claude Code treats /checkpoint\nas a native rewind alias in current environments.", "voice_line": null }, "context-save": { diff --git a/scripts/resolvers/question-tuning.ts b/scripts/resolvers/question-tuning.ts index d9c843a3e..fe6f78c94 100644 --- a/scripts/resolvers/question-tuning.ts +++ b/scripts/resolvers/question-tuning.ts @@ -23,7 +23,7 @@ export function generateQuestionTuning(ctx: TemplateContext): string { const bin = binDir(ctx); return `## Question Tuning (skip entirely if \`QUESTION_TUNING: false\`) -Before each AskUserQuestion, choose \`question_id\` from \`scripts/question-registry.ts\` or \`{skill}-{slug}\`, then run \`${bin}/gstack-question-preference --check ""\`. \`AUTO_DECIDE\` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." \`ASK_NORMALLY\` means ask. +Before each AskUserQuestion, choose \`question_id\` from \`scripts/question-registry.ts\` or \`{skill}-{slug}\`, then run \`printf '%s' "" | ${bin}/gstack-question-preference --check "" --summary-stdin\` (piped summary feeds the one-way keyword net, #2024). \`AUTO_DECIDE\` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." \`ASK_NORMALLY\` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append \`\` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered \`question_id\`. @@ -51,7 +51,7 @@ export function generateQuestionPreferenceCheck(ctx: TemplateContext): string { const bin = binDir(ctx); return `## Question Preference Check (skip if \`QUESTION_TUNING: false\`) -Before each AskUserQuestion, run: \`${bin}/gstack-question-preference --check ""\`. +Before each AskUserQuestion, run: \`printf '%s' "" | ${bin}/gstack-question-preference --check "" --summary-stdin\`. \`AUTO_DECIDE\` → auto-choose recommended with inline annotation. \`ASK_NORMALLY\` → ask.`; } diff --git a/setup-deploy/SKILL.md b/setup-deploy/SKILL.md index 3465dc564..eba0e3bf8 100644 --- a/setup-deploy/SKILL.md +++ b/setup-deploy/SKILL.md @@ -706,7 +706,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/setup-gbrain/SKILL.md b/setup-gbrain/SKILL.md index a3892f1c9..ad1382c01 100644 --- a/setup-gbrain/SKILL.md +++ b/setup-gbrain/SKILL.md @@ -705,7 +705,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. @@ -820,9 +820,13 @@ Capture the JSON output. It contains: `gbrain_on_path`, `gbrain_version`, `gbrain_config_exists`, `gbrain_engine`, `gbrain_doctor_ok`, `gbrain_mcp_mode`, `gstack_brain_sync_mode`, `gstack_brain_git`, `gstack_artifacts_remote`, and the v1.34.0.0+ `gbrain_local_status` field (one of: `ok`, `no-cli`, -`missing-config`, `broken-config`, `broken-db`, `engine-locked`, `timeout`). Treat `timeout` -like `ok` (slow-but-healthy engine, #1964) — it never triggers Step 1.5 -remediation. +`missing-config`, `broken-config`, `broken-db`, `engine-locked`, `timeout`, +`thin-client`). Treat `timeout` like `ok` (slow-but-healthy engine, #1964) — it +never triggers Step 1.5 remediation. Treat `thin-client` like `ok` too (#2051): +the machine is a thin client of a remote-HTTP MCP brain, no local engine by +design — brain-aware blocks render, and the detect JSON carries +`gbrain_thin_client: {probed: false}` (config verified; remote reachability +is checked at use time, where gbrain calls degrade gracefully). Skip downstream steps that are already done. Report the detected state in one line so the user knows what you found: diff --git a/setup-gbrain/SKILL.md.tmpl b/setup-gbrain/SKILL.md.tmpl index f475987c1..37953d5fe 100644 --- a/setup-gbrain/SKILL.md.tmpl +++ b/setup-gbrain/SKILL.md.tmpl @@ -66,9 +66,13 @@ Capture the JSON output. It contains: `gbrain_on_path`, `gbrain_version`, `gbrain_config_exists`, `gbrain_engine`, `gbrain_doctor_ok`, `gbrain_mcp_mode`, `gstack_brain_sync_mode`, `gstack_brain_git`, `gstack_artifacts_remote`, and the v1.34.0.0+ `gbrain_local_status` field (one of: `ok`, `no-cli`, -`missing-config`, `broken-config`, `broken-db`, `engine-locked`, `timeout`). Treat `timeout` -like `ok` (slow-but-healthy engine, #1964) — it never triggers Step 1.5 -remediation. +`missing-config`, `broken-config`, `broken-db`, `engine-locked`, `timeout`, +`thin-client`). Treat `timeout` like `ok` (slow-but-healthy engine, #1964) — it +never triggers Step 1.5 remediation. Treat `thin-client` like `ok` too (#2051): +the machine is a thin client of a remote-HTTP MCP brain, no local engine by +design — brain-aware blocks render, and the detect JSON carries +`gbrain_thin_client: {probed: false}` (config verified; remote reachability +is checked at use time, where gbrain calls degrade gracefully). Skip downstream steps that are already done. Report the detected state in one line so the user knows what you found: diff --git a/ship/SKILL.md b/ship/SKILL.md index eadffaa8f..3062cda60 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/skillify/SKILL.md b/skillify/SKILL.md index 7cb434d0c..af2f7ec38 100644 --- a/skillify/SKILL.md +++ b/skillify/SKILL.md @@ -703,7 +703,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/spec/SKILL.md b/spec/SKILL.md index 0894e98d1..4301ec3e7 100644 --- a/spec/SKILL.md +++ b/spec/SKILL.md @@ -704,7 +704,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. @@ -1774,7 +1774,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/sync-gbrain/SKILL.md b/sync-gbrain/SKILL.md index 02055bb6f..eac766352 100644 --- a/sync-gbrain/SKILL.md +++ b/sync-gbrain/SKILL.md @@ -705,7 +705,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. @@ -893,6 +893,14 @@ BEFORE invoking the orchestrator: slow (cold pooler connection, #1964). Tell the user in one line: "Engine probe timed out (>15s) — proceeding; raise `GSTACK_GBRAIN_PROBE_TIMEOUT_MS` if your pooler is slow." Do NOT treat this as a broken config. +- **`thin-client`**: proceed to Step 2 — this machine is a thin client of a + remote-HTTP MCP brain (#2051): no local engine BY DESIGN, so the code, + memory, and dream stages will SKIP with a thin-client reason (code indexing + runs on the brain server; memory syncs via the remote brain's artifacts + pull). Only the brain-sync push runs locally. Tell the user in one line: + "Thin client of a remote brain — local stages skip by design; brain queries + work via remote MCP (reachability is verified at use time, not probed + here)." Do NOT route this into the broken-config remediation. - **`engine-locked`**: STOP. "The local PGLite database is busy, usually because `gbrain serve` from a live Claude session owns it. Stop that process or run `/sync-gbrain` outside the live session, then retry. This identifies diff --git a/sync-gbrain/SKILL.md.tmpl b/sync-gbrain/SKILL.md.tmpl index aa97f7eb7..fa8becfd5 100644 --- a/sync-gbrain/SKILL.md.tmpl +++ b/sync-gbrain/SKILL.md.tmpl @@ -139,6 +139,14 @@ BEFORE invoking the orchestrator: slow (cold pooler connection, #1964). Tell the user in one line: "Engine probe timed out (>15s) — proceeding; raise `GSTACK_GBRAIN_PROBE_TIMEOUT_MS` if your pooler is slow." Do NOT treat this as a broken config. +- **`thin-client`**: proceed to Step 2 — this machine is a thin client of a + remote-HTTP MCP brain (#2051): no local engine BY DESIGN, so the code, + memory, and dream stages will SKIP with a thin-client reason (code indexing + runs on the brain server; memory syncs via the remote brain's artifacts + pull). Only the brain-sync push runs locally. Tell the user in one line: + "Thin client of a remote brain — local stages skip by design; brain queries + work via remote MCP (reachability is verified at use time, not probed + here)." Do NOT route this into the broken-config remediation. - **`engine-locked`**: STOP. "The local PGLite database is busy, usually because `gbrain serve` from a live Claude session owns it. Stop that process or run `/sync-gbrain` outside the live session, then retry. This identifies diff --git a/test/context-save-hardening.test.ts b/test/context-save-hardening.test.ts index 95f49aba1..11f9c725c 100644 --- a/test/context-save-hardening.test.ts +++ b/test/context-save-hardening.test.ts @@ -36,15 +36,33 @@ echo "TITLE_SLUG=$TITLE_SLUG" echo "FILE=$FILE" `; -// The exact find + sort + head used by context-restore/SKILL.md Step 1. +// The exact selection used by context-restore/SKILL.md Step 1: scan newest 200, +// order current-branch checkpoints first (fallback: all branches), cap at 20. +// CURRENT_BRANCH is injected via env in tests; the skill resolves it from git. const RESTORE_FIND_BASH = ` if [ ! -d "$CHECKPOINT_DIR" ]; then echo "NO_CHECKPOINTS" else - FILES=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -20) - if [ -z "$FILES" ]; then + ALL=$(find "$CHECKPOINT_DIR" -maxdepth 1 -name "*.md" -type f 2>/dev/null | sort -r | head -200) + if [ -z "$ALL" ]; then echo "NO_CHECKPOINTS" else + : "\${CURRENT_BRANCH:=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)}" + SAME=""; OTHER="" + while IFS= read -r f; do + [ -n "$f" ] || continue + b=$(grep -m1 '^branch:' "$f" 2>/dev/null | sed 's/^branch:[[:space:]]*//') + if [ -n "$CURRENT_BRANCH" ] && [ "$b" = "$CURRENT_BRANCH" ]; then + SAME="\${SAME}\${f} +" + else + OTHER="\${OTHER}\${f} +" + fi + done < { }); }); +// ─── Current-branch preference (#2052) ────────────────────────────────────── +// +// All worktrees of a repo share one origin-derived slug → one checkpoints dir. +// Restore must prefer the CURRENT branch's own checkpoint so a sibling +// worktree's newer save can't shadow it, while still falling back across +// branches (Conductor handoff) when the current branch has none. + +describe('context-restore: current-branch preference (#2052)', () => { + let tmp: string; + beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ctx-branch-')); }); + afterEach(() => { try { fs.rmSync(tmp, { recursive: true, force: true }); } catch {} }); + + function writeCheckpoint(ts: string, branch: string | null): string { + const file = `${tmp}/${ts}-work.md`; + const fm = branch === null + ? `---\nstatus: in-progress\n---\n` + : `---\nstatus: in-progress\nbranch: ${branch}\n---\n`; + fs.writeFileSync(file, fm); + return file; + } + + function firstCandidate(currentBranch?: string): string { + const env: Record = { CHECKPOINT_DIR: tmp }; + if (currentBranch !== undefined) env.CURRENT_BRANCH = currentBranch; + const out = runBash(RESTORE_FIND_BASH, env).stdout; + return out.trim().split('\n').filter(Boolean)[0] ?? ''; + } + + test('the bug: current-branch save is NOT shadowed by a newer sibling-worktree save', () => { + const mine = writeCheckpoint('20260101-120000', 'feature-a'); // older, my branch + writeCheckpoint('20260619-120000', 'feature-b'); // newer, sibling worktree + // On feature-a, restore must load feature-a's own (older) checkpoint. + expect(firstCandidate('feature-a')).toBe(mine); + }); + + test('fallback: current branch has no checkpoint → newest across all branches (Conductor handoff)', () => { + writeCheckpoint('20260101-120000', 'feature-a'); + const newest = writeCheckpoint('20260619-120000', 'feature-b'); + // On feature-c (no own checkpoint), cross-branch resume still works. + expect(firstCandidate('feature-c')).toBe(newest); + }); + + test('back-compat: empty current branch (non-git) → newest across all', () => { + writeCheckpoint('20260101-120000', 'feature-a'); + const newest = writeCheckpoint('20260619-120000', 'feature-b'); + expect(firstCandidate('')).toBe(newest); + }); + + test('checkpoints without a branch frontmatter still rank as fallback, never lost', () => { + const mine = writeCheckpoint('20260101-120000', 'feature-a'); + writeCheckpoint('20260301-120000', null); // legacy save, no branch field + const out = runBash(RESTORE_FIND_BASH, { CHECKPOINT_DIR: tmp, CURRENT_BRANCH: 'feature-a' }).stdout; + const lines = out.trim().split('\n').filter(Boolean); + expect(lines[0]).toBe(mine); // current branch first + expect(lines.length).toBe(2); // legacy file is still present + }); + + test('within the current branch, ordering stays newest-first', () => { + const older = writeCheckpoint('20260101-120000', 'feature-a'); + const newer = writeCheckpoint('20260619-120000', 'feature-a'); + const out = runBash(RESTORE_FIND_BASH, { CHECKPOINT_DIR: tmp, CURRENT_BRANCH: 'feature-a' }).stdout; + const lines = out.trim().split('\n').filter(Boolean); + expect(lines[0]).toBe(newer); + expect(lines[1]).toBe(older); + }); +}); + // ─── Migration HOME guard ────────────────────────────────────────────────── describe('migration v1.1.3.0: HOME guard', () => { diff --git a/test/design-flag-utils.test.ts b/test/design-flag-utils.test.ts new file mode 100644 index 000000000..9e3734081 --- /dev/null +++ b/test/design-flag-utils.test.ts @@ -0,0 +1,113 @@ +/** + * design/src/flag-utils.ts — integer-flag contract (#2032, eng-review 7A/8A). + * + * Lives under test/ (NOT design/test/) deliberately: design/test/ is invisible + * to the bun test glob, scripts/test-free-shards TEST_ROOTS, and every CI + * workflow (eng-review 11A), so a tripwire there guards nothing. flag-utils is + * a pure module, so importing it from here is clean. + * + * The bug class: the design CLI parser yields string | true | undefined; + * parseInt produced NaN that flowed silently into loop bounds and setTimeout — + * `variants --count abc` generated ZERO variants and exited 0, `generate + * --retry abc` was a silent no-op, `serve --timeout abc` died at boot. + */ + +import { describe, test, expect } from "bun:test"; +import { spawnSync } from "child_process"; +import * as path from "path"; +import { parseIntFlag } from "../design/src/flag-utils"; + +const ROOT = path.resolve(import.meta.dir, ".."); + +const COUNT_SPEC = { name: "count", def: 3, min: 1, max: 7 } as const; +const RETRY_SPEC = { name: "retry", def: 0, min: 0 } as const; +const TIMEOUT_SPEC = { name: "timeout", def: 600, min: 1 } as const; + +describe("parseIntFlag contract (#2032, codex 17a-c)", () => { + test("undefined → default (flag absent)", () => { + expect(parseIntFlag(undefined, COUNT_SPEC)).toEqual({ ok: true, value: 3 }); + expect(parseIntFlag(undefined, RETRY_SPEC)).toEqual({ ok: true, value: 0 }); + }); + + test("non-numeric string → error, never a silent default ('--count abc')", () => { + const r = parseIntFlag("abc", COUNT_SPEC); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain('"abc" is not an integer'); + }); + + test("bare flag (parser yields true) → error 'requires a value'", () => { + const r = parseIntFlag(true, COUNT_SPEC); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain("requires a value"); + }); + + test("empty string → error 'requires a value'", () => { + const r = parseIntFlag("", COUNT_SPEC); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain("requires a value"); + }); + + test("non-integer '3.7' → error (rejected, not silently truncated to 3)", () => { + const r = parseIntFlag("3.7", COUNT_SPEC); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain("not an integer"); + }); + + test("below min → error ('--count 0' billed the user for 1 they asked 0 of; now loud)", () => { + expect(parseIntFlag("0", COUNT_SPEC).ok).toBe(false); + expect(parseIntFlag("-2", COUNT_SPEC).ok).toBe(false); + // retry allows 0 (min: 0) + expect(parseIntFlag("0", RETRY_SPEC)).toEqual({ ok: true, value: 0 }); + }); + + test("above max → clamp WITH warning (capability limit, not a user mistake)", () => { + const r = parseIntFlag("99", COUNT_SPEC); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.value).toBe(7); + expect(r.warning).toContain("exceeds the maximum"); + } + }); + + test("in-range integers pass through untouched", () => { + expect(parseIntFlag("3", COUNT_SPEC)).toEqual({ ok: true, value: 3 }); + expect(parseIntFlag(5, COUNT_SPEC)).toEqual({ ok: true, value: 5 }); + expect(parseIntFlag("120", TIMEOUT_SPEC)).toEqual({ ok: true, value: 120 }); + }); + + test("retry-NaN and timeout-NaN are errors, not silent no-ops (#2032 siblings)", () => { + // Pre-fix: --retry abc → generate() loop never ran (attempt <= NaN), + // printed null, exited 0. --timeout abc → setTimeout(NaN) ≈ immediate + // SERVE_TIMEOUT. Both members of the same NaN class, same file. + expect(parseIntFlag("abc", RETRY_SPEC).ok).toBe(false); + expect(parseIntFlag("abc", TIMEOUT_SPEC).ok).toBe(false); + }); + + test("NaN number input (legacy pre-parsed callers) → error", () => { + expect(parseIntFlag(Number.NaN, COUNT_SPEC).ok).toBe(false); + }); +}); + +describe("normalizeIntFlag CLI wrapper (exit-1 semantics)", () => { + function runWrapper(rawExpr: string, specExpr: string): { status: number; stderr: string } { + const script = ` + import { normalizeIntFlag } from "${ROOT}/design/src/flag-utils"; + const v = normalizeIntFlag(${rawExpr}, ${specExpr}); + console.log("VALUE:" + v); + `; + const res = spawnSync("bun", ["-e", script], { encoding: "utf-8", cwd: ROOT }); + return { status: res.status ?? -1, stderr: res.stderr ?? "" }; + } + + test("invalid input exits 1 with the error on stderr", () => { + const r = runWrapper('"abc"', '{ name: "count", def: 3, min: 1, max: 7 }'); + expect(r.status).toBe(1); + expect(r.stderr).toContain("not an integer"); + }); + + test("clamp warns on stderr but exits 0", () => { + const r = runWrapper('"99"', '{ name: "count", def: 3, min: 1, max: 7 }'); + expect(r.status).toBe(0); + expect(r.stderr).toContain("exceeds the maximum"); + }); +}); diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index eadffaa8f..3062cda60 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -707,7 +707,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | ~/.claude/skills/gstack/bin/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index d99630c4b..1d9b707bf 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -693,7 +693,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `$GSTACK_BIN/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | $GSTACK_BIN/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index a2acad24f..40ad2e3f3 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -695,7 +695,7 @@ If you are looping on the same diagnostic, same file, or failed fix variants, ST ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) -Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `$GSTACK_BIN/gstack-question-preference --check ""`. `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. +Before each AskUserQuestion, choose `question_id` from `scripts/question-registry.ts` or `{skill}-{slug}`, then run `printf '%s' "" | $GSTACK_BIN/gstack-question-preference --check "" --summary-stdin` (piped summary feeds the one-way keyword net, #2024). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. **Embed the question_id as a marker in the question text** so hooks can identify it deterministically (plan-tune cathedral T14 / D18 progressive markers). Append `` somewhere in the rendered question (the leading line or trailing line is fine; the marker doesn't render visibly to the user when wrapped in HTML-style angle brackets, but the hook strips it). Without the marker the PreToolUse enforcement hook treats the AUQ as observed-only and never auto-decides — so always include it when the question matches a registered `question_id`. diff --git a/test/gbrain-local-status.test.ts b/test/gbrain-local-status.test.ts index 0d5a44539..495d28371 100644 --- a/test/gbrain-local-status.test.ts +++ b/test/gbrain-local-status.test.ts @@ -32,7 +32,7 @@ import { utimesSync, } from "fs"; import { tmpdir } from "os"; -import { join } from "path"; +import { join, dirname } from "path"; import { spawnSync } from "child_process"; @@ -62,8 +62,10 @@ interface FakeEnv { */ function makeEnv(opts: { withGbrain?: boolean; - gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow"; + gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow" | "thin-refusal"; withConfig?: boolean; + /** #2051: config carries gbrain's remote_mcp thin-client marker. */ + thinClientConfig?: boolean; }): FakeEnv { const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-")); const bindir = join(tmp, "bin"); @@ -77,7 +79,12 @@ function makeEnv(opts: { mkdirSync(gstackHome, { recursive: true }); mkdirSync(configDir, { recursive: true }); - if (opts.withConfig) { + if (opts.thinClientConfig) { + writeFileSync( + configPath, + JSON.stringify({ remote_mcp: { mcp_url: "https://brain.example.com/mcp" } }), + ); + } else if (opts.withConfig) { writeFileSync( configPath, JSON.stringify({ engine: "pglite", database_url: "pglite:///fake" }), @@ -103,7 +110,7 @@ function makeEnv(opts: { } function makeFakeGbrainScript( - behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow", + behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "throws" | "slow" | "thin-refusal", ): string { // "slow": healthy engine on a cold pooler connection (#1964) — sleeps past // the (test-lowered) probe timeout, then would answer fine. @@ -130,7 +137,9 @@ exit 0 ? 'echo "gbrain sources: connect timed out (default 10000ms; pass --timeout=Ns to override)." >&2' : behavior === "throws" ? 'echo "unexpected gbrain failure" >&2' - : ""; + : behavior === "thin-refusal" + ? 'echo "Error: gbrain sources is not routable to the remote brain (thin-client of https://brain.example.com/mcp)" >&2' + : ""; const exitCode = behavior === "ok" ? 0 : behavior === "engine-locked" ? 124 : 1; return `#!/bin/sh if [ "$1" = "--version" ]; then @@ -448,3 +457,72 @@ describe("lib/gbrain-local-status — cache behavior", () => { } }); }); + +// --------------------------------------------------------------------------- +// #2051: thin-client classification + the end-to-end --is-ok gate +// --------------------------------------------------------------------------- + +describe("lib/gbrain-local-status — thin-client (#2051)", () => { + let env: FakeEnv | null = null; + let restoreEnv: (() => void) | null = null; + + afterEach(() => { + if (restoreEnv) restoreEnv(); + if (env) env.cleanup(); + env = null; + restoreEnv = null; + }); + + it("returns 'thin-client' when config carries gbrain's remote_mcp marker (pre-probe, no engine call)", () => { + // The fake gbrain would answer "ok" if probed — proving the marker is + // read from config BEFORE any probe (zero network, no error-string + // dependence). + env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", thinClientConfig: true }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + it("returns 'thin-client' via the stderr refusal fallback when the config marker is unreadable", () => { + // Regular (non-thin) config on disk, but gbrain itself refuses with the + // dispatch-guard message — the catch-path backstop. + env = makeEnv({ withGbrain: true, gbrainBehavior: "thin-refusal", withConfig: true }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + // The eng-review 3A tripwire: the END-TO-END gate, not just the classifier + // return. --is-ok drives setup:1299 and gstack-config gbrain-refresh — this + // exit code is what decides whether brain-aware blocks render for a + // thin-client user (the #2051 report). + it("--is-ok exits 0 on a thin-client fixture (end-to-end gate)", () => { + env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", thinClientConfig: true }); + const detectBin = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect"); + const bunDir = dirname(process.execPath); + const r = spawnSync(detectBin, ["--is-ok"], { + encoding: "utf-8", + env: { + HOME: env.home, + PATH: `${env.bindir}:${bunDir}:/usr/bin:/bin`, + GSTACK_HOME: env.gstackHome, + GSTACK_DETECT_NO_CACHE: "1", + }, + }); + expect(r.status).toBe(0); + }); + + it("--is-ok still exits 1 on broken-config (thin-client did not widen the gate)", () => { + env = makeEnv({ withGbrain: true, gbrainBehavior: "broken-config", withConfig: true }); + const detectBin = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect"); + const bunDir = dirname(process.execPath); + const r = spawnSync(detectBin, ["--is-ok"], { + encoding: "utf-8", + env: { + HOME: env.home, + PATH: `${env.bindir}:${bunDir}:/usr/bin:/bin`, + GSTACK_HOME: env.gstackHome, + GSTACK_DETECT_NO_CACHE: "1", + }, + }); + expect(r.status).toBe(1); + }); +}); diff --git a/test/gbrain-sources.test.ts b/test/gbrain-sources.test.ts index 9486eb06c..810624755 100644 --- a/test/gbrain-sources.test.ts +++ b/test/gbrain-sources.test.ts @@ -8,7 +8,7 @@ */ import { describe, it, expect } from "bun:test"; -import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync } from "fs"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync, mkdirSync, rmSync, chmodSync, symlinkSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; @@ -32,8 +32,11 @@ interface FakeGbrainSetup { * Build a temp dir with a fake `gbrain` shell script on PATH. The fake honors: * gbrain sources list --json → cat $STATE_PATH * gbrain sources add --path

[--federated] → append to state, log - * gbrain sources remove --yes → drop from state, log - * gbrain --version → echo "gbrain 0.25.1" + * gbrain sources remove --confirm-destructive → drop from state, log + * (#1985: remove WITHOUT + * --confirm-destructive + * fails like gbrain >= 0.42) + * gbrain --version → echo "gbrain 0.42.40.0" * Anything else exits 1. */ function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path: string; federated?: boolean; page_count?: number }> }): FakeGbrainSetup { @@ -49,7 +52,7 @@ function makeFakeGbrain(initialState: { sources: Array<{ id: string; local_path: echo "$@" >> "${logPath}" case "$1 $2" in "--version ") - echo "gbrain 0.25.1" + echo "gbrain 0.42.40.0" exit 0 ;; "sources list") @@ -75,6 +78,16 @@ case "$1 $2" in ;; "sources remove") ID="$3" + # #1985: gbrain >= 0.42 gates remove behind --confirm-destructive; --yes + # alone no longer suppresses the data-loss prompt. Refuse without it so the + # drift re-register path is exercised against real gbrain 0.42 behavior. + case " $* " in + *" --confirm-destructive "*) : ;; + *) + echo "This will permanently delete pages. To proceed, pass --confirm-destructive" >&2 + exit 1 + ;; + esac NEW=$(jq --arg id "$ID" '.sources = (.sources | map(select(.id != $id)))' "${statePath}") echo "$NEW" > "${statePath}" exit 0 @@ -155,6 +168,13 @@ describe("ensureSourceRegistered", () => { fake.cleanup(); }); + // Drift-path tests pin the #1734 guard inputs (inactive autopilot, allowed + // remove) so a REAL autopilot running on the dev machine can't flip them. + const guardsPinnedInactive = { + autopilotProbe: { lockPaths: [], processRunning: () => false }, + removeDecision: { keepStorage: false }, + } as const; + it("recreates source when path differs (gbrain has no `sources update`), returns changed=true", async () => { const fake = makeFakeGbrain({ sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], @@ -162,17 +182,138 @@ describe("ensureSourceRegistered", () => { const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", { federated: true, env: fake.env, + ...guardsPinnedInactive, }); expect(result.changed).toBe(true); expect(result.state.status).toBe("match"); expect(result.state.registered_path).toBe("/new/path"); const log = readFileSync(fake.logPath, "utf-8"); - expect(log).toContain("sources remove gstack-code-foo --yes"); + // #1985: the remove must carry --confirm-destructive (gbrain >= 0.42 gate). + expect(log).toContain("sources remove gstack-code-foo --yes --confirm-destructive"); expect(log).toContain("sources add gstack-code-foo --path /new/path --federated"); fake.cleanup(); }); + // #1985: regression. gbrain >= 0.42 refuses `sources remove` without + // --confirm-destructive. On the drift path that surfaces as "source + // registration failed" and aborts the /sync-gbrain code stage for every + // already-registered source whose path drifted. Before the fix the remove + // was issued with `--yes` only, so the guard-simulating fake rejects it and + // ensureSourceRegistered throws. The fix passes --confirm-destructive, so + // the re-register succeeds. + it("re-registers across the gbrain >= 0.42 destructive-remove guard (does not throw)", async () => { + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], + }); + const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", { + federated: true, + env: fake.env, + ...guardsPinnedInactive, + }); + expect(result.changed).toBe(true); + expect(result.state.status).toBe("match"); + expect(result.state.registered_path).toBe("/new/path"); + + // The old source was actually removed (the guarded remove succeeded), then + // the new path was added — not left behind as a stale duplicate. + const finalState = JSON.parse(readFileSync(fake.statePath, "utf-8")); + expect(finalState.sources).toHaveLength(1); + expect(finalState.sources[0].local_path).toBe("/new/path"); + fake.cleanup(); + }); + + // #1734 tripwire: the drift remove deletes pages/chunks/embeddings, so it + // must refuse while a gbrain autopilot is active — and refuse LOUDLY (throw), + // not silently return changed=false. Before the guard routing, this path + // issued the remove unconditionally. + it("REFUSES the drift remove while autopilot is active (throws, no remove issued)", async () => { + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], + }); + await expect( + ensureSourceRegistered("gstack-code-foo", "/new/path", { + env: fake.env, + autopilotProbe: { lockPaths: [], processRunning: () => true }, + removeDecision: { keepStorage: false }, + }), + ).rejects.toThrow(/autopilot active/); + + const log = readFileSync(fake.logPath, "utf-8"); + expect(log).not.toContain("sources remove"); + expect(log).not.toContain("sources add"); + fake.cleanup(); + }); + + it("REFUSES the drift remove when decideSourceRemove disallows (fail closed, throws)", async () => { + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], + }); + await expect( + ensureSourceRegistered("gstack-code-foo", "/new/path", { + env: fake.env, + autopilotProbe: { lockPaths: [], processRunning: () => false }, + // A sources-list read failure makes decideSourceRemove fail closed. + removeDecision: { + keepStorage: false, + fetchRows: () => { + throw new Error("sources list unavailable"); + }, + }, + }), + ).rejects.toThrow(/fail closed/); + + const log = readFileSync(fake.logPath, "utf-8"); + expect(log).not.toContain("sources remove"); + fake.cleanup(); + }); + + it("propagates decideSourceRemove extraArgs (--keep-storage) to the drift remove", async () => { + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], + }); + const result = await ensureSourceRegistered("gstack-code-foo", "/new/path", { + env: fake.env, + autopilotProbe: { lockPaths: [], processRunning: () => false }, + removeDecision: { keepStorage: true }, + }); + expect(result.changed).toBe(true); + + const log = readFileSync(fake.logPath, "utf-8"); + expect(log).toContain( + "sources remove gstack-code-foo --yes --confirm-destructive --keep-storage", + ); + fake.cleanup(); + }); + + // Realpath normalization: a registered path that is a symlink alias of the + // requested path is a MATCH, not drift. Declaring it drift triggers a + // destructive remove + full re-index for a no-op (#1985 reporter hit the + // remove on an unmoved repo; macOS /tmp -> /private/tmp is the usual cause). + it("does NOT declare drift when registered path is a symlink alias of the requested path", async () => { + const base = mkdtempSync(join(tmpdir(), "gbrain-sources-realpath-")); + const realDir = join(base, "real-repo"); + const linkDir = join(base, "link-repo"); + mkdirSync(realDir, { recursive: true }); + symlinkSync(realDir, linkDir); + + const fake = makeFakeGbrain({ + sources: [{ id: "gstack-code-foo", local_path: realDir }], + }); + const result = await ensureSourceRegistered("gstack-code-foo", linkDir, { + env: fake.env, + ...guardsPinnedInactive, + }); + expect(result.changed).toBe(false); + expect(result.state.status).toBe("match"); + + const log = readFileSync(fake.logPath, "utf-8"); + expect(log).not.toContain("sources remove"); + expect(log).not.toContain("sources add"); + fake.cleanup(); + rmSync(base, { recursive: true, force: true }); + }); + it("when reregister_on_drift=false and source is at different path, returns changed=false", async () => { const fake = makeFakeGbrain({ sources: [{ id: "gstack-code-foo", local_path: "/old/path" }], diff --git a/test/gstack-developer-profile.test.ts b/test/gstack-developer-profile.test.ts index ed683bf34..507706509 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -556,3 +556,143 @@ describe('gstack-developer-profile --log-session (#1671 fix)', () => { }); }); +// ----------------------------------------------------------------------- +// SESSION_COUNT / TIER / NUDGE_ELIGIBLE must ignore mode:resources entries. +// +// Phase 6 of /office-hours auto-appends one (or more) mode:resources bookkeeping +// entries every run, to dedupe which founder-resource links the user has seen. +// Those are not sessions. Counting them inflated SESSION_COUNT (and therefore +// TIER) and pushed NUDGE_ELIGIBLE over its threshold from bookkeeping alone — +// e.g. a single real session plus three closings reported as tier `regular` +// with the builder->founder nudge armed. +// ----------------------------------------------------------------------- + +describe('gstack-developer-profile resources entries do not inflate count/tier/nudge', () => { + function logStartup(extra: Record = {}) { + return runDev('--log-session', JSON.stringify({ + date: '2026-05-20T00:00:00Z', mode: 'startup', project_slug: 'p', + signal_count: 5, signals: ['a', 'b', 'c', 'd', 'e'], ...extra, + })); + } + function logResources(i: number) { + return runDev('--log-session', JSON.stringify({ + date: '2026-05-20T01:00:00Z', mode: 'resources', project_slug: 'p', + resources_shown: [`url${i}`], + })); + } + + test('SESSION_COUNT counts only real sessions, not resources entries', () => { + logStartup(); + logResources(1); + logResources(2); + logResources(3); + const r = runDev('--read'); + expect(r.stdout).toContain('SESSION_COUNT: 1'); + expect(r.stdout).toContain('TIER: welcome_back'); + }); + + test('TIER is not bumped to regular by resources bookkeeping', () => { + // 3 real sessions = welcome_back; adding resources entries must not reach the + // 4-session `regular` threshold. + logStartup(); + logStartup(); + logStartup(); + for (let i = 0; i < 4; i++) logResources(i); + const r = runDev('--read'); + expect(r.stdout).toContain('SESSION_COUNT: 3'); + expect(r.stdout).toContain('TIER: welcome_back'); + }); + + test('NUDGE_ELIGIBLE stays false when builder-session bar is unmet despite resources noise', () => { + // One startup session carrying 5 signals, plus resources entries. builderSessions + // (mode === "builder") is 0, so the nudge must not arm regardless of signal count. + logStartup(); + logResources(1); + logResources(2); + logResources(3); + const r = runDev('--read'); + expect(r.stdout).toContain('NUDGE_ELIGIBLE: false'); + }); + + test('NUDGE_ELIGIBLE arms on 3 real builder sessions with enough signals', () => { + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['a', 'b'], + })); + runDev('--log-session', JSON.stringify({ + date: '2026-05-21T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['c', 'd'], + })); + runDev('--log-session', JSON.stringify({ + date: '2026-05-22T00:00:00Z', mode: 'builder', project_slug: 'p', signals: ['e'], + })); + logResources(1); // bookkeeping must not change the verdict either way + const r = runDev('--read'); + expect(r.stdout).toContain('NUDGE_ELIGIBLE: true'); + }); + + // Boundary cases around the two `>=` gates, so a future >= → > regression + // (or a re-loosening of the builder filter) is caught, not just the happy path. + function logBuilder(signals: string[], day = 20) { + return runDev('--log-session', JSON.stringify({ + date: `2026-05-${day}T00:00:00Z`, mode: 'builder', project_slug: 'p', signals, + })); + } + + test('NUDGE_ELIGIBLE stays false at 2 builder sessions (below the 3-session gate)', () => { + logBuilder(['a', 'b', 'c'], 20); + logBuilder(['d', 'e', 'f'], 21); // 6 signals total — signal gate met, session gate is not + logResources(1); + const r = runDev('--read'); + expect(r.stdout).toContain('NUDGE_ELIGIBLE: false'); + }); + + test('NUDGE_ELIGIBLE stays false at 3 builder sessions with too few signals', () => { + logBuilder(['a'], 20); + logBuilder(['b'], 21); + logBuilder(['c', 'd'], 22); // 4 signals total — session gate met, signal gate (>=5) is not + const r = runDev('--read'); + expect(r.stdout).toContain('NUDGE_ELIGIBLE: false'); + }); + + test('TIER reaches regular at 4 real sessions even when resources entries are present', () => { + logStartup(); + logStartup(); + logStartup(); + logStartup(); + for (let i = 0; i < 5; i++) logResources(i); + const r = runDev('--read'); + expect(r.stdout).toContain('SESSION_COUNT: 4'); + expect(r.stdout).toContain('TIER: regular'); + }); + + test('TIER stays regular at 7 real sessions and crosses to inner_circle at 8 (resources ignored)', () => { + // Upper-tier boundary: the >=8 inner_circle gate must key off real sessions + // only, so a pile of resources bookkeeping can never tip a regular into the + // inner circle, and 8 genuine sessions still reach it. + for (let i = 0; i < 7; i++) logStartup(); + for (let i = 0; i < 6; i++) logResources(i); // 13 raw rows; pre-fix would read inner_circle + let r = runDev('--read'); + expect(r.stdout).toContain('SESSION_COUNT: 7'); + expect(r.stdout).toContain('TIER: regular'); + + logStartup(); // 8th real session + r = runDev('--read'); + expect(r.stdout).toContain('SESSION_COUNT: 8'); + expect(r.stdout).toContain('TIER: inner_circle'); + }); + + test('CROSS_PROJECT ignores a trailing resources entry on a different project', () => { + // The last two REAL sessions are the same project, so CROSS_PROJECT is false. + // A trailing resources row carrying a different project_slug must not become + // the `last` entry and flip CROSS_PROJECT true off bookkeeping. + logStartup({ project_slug: 'samep' }); + logStartup({ project_slug: 'samep' }); + runDev('--log-session', JSON.stringify({ + date: '2026-05-20T02:00:00Z', mode: 'resources', project_slug: 'otherp', + resources_shown: ['url1'], + })); + const r = runDev('--read'); + expect(r.stdout).toContain('CROSS_PROJECT: false'); + expect(r.stdout).toContain('LAST_PROJECT: samep'); + }); +}); + diff --git a/test/gstack-gbrain-detect-mcp-mode.test.ts b/test/gstack-gbrain-detect-mcp-mode.test.ts index ebf58c409..c4793e6fc 100644 --- a/test/gstack-gbrain-detect-mcp-mode.test.ts +++ b/test/gstack-gbrain-detect-mcp-mode.test.ts @@ -208,6 +208,61 @@ describe('gbrain_mcp_mode — Tier 3: ~/.claude.json jq read', () => { ); expect(runDetect().json.gbrain_mcp_mode).toBe('none'); }); + + // #2051 name generalization: a gbrain server registered under a variant + // name still counts. Identification order: url-match against the config's + // remote_mcp.mcp_url (deterministic — gbrain mounts at generic /mcp so + // URL-path heuristics are impossible) → name pattern gbrain[-_]* → stdio + // command token. + test('server named gbrain-remote (name pattern) → remote-http', () => { + fs.writeFileSync( + path.join(tmpHome, '.claude.json'), + JSON.stringify({ + mcpServers: { 'gbrain-remote': { type: 'url', url: 'https://brain.corp.example/mcp' } }, + }) + ); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('arbitrarily-named server whose url matches config remote_mcp.mcp_url → remote-http', () => { + fs.mkdirSync(path.join(tmpHome, '.gbrain'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ remote_mcp: { mcp_url: 'https://team-brain.example.com/mcp' } }) + ); + fs.writeFileSync( + path.join(tmpHome, '.claude.json'), + JSON.stringify({ + mcpServers: { 'our-team-brain': { type: 'url', url: 'https://team-brain.example.com/mcp' } }, + }) + ); + expect(runDetect().json.gbrain_mcp_mode).toBe('remote-http'); + }); + + test('unrelated server with a non-matching url does NOT false-positive → none', () => { + fs.mkdirSync(path.join(tmpHome, '.gbrain'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ remote_mcp: { mcp_url: 'https://team-brain.example.com/mcp' } }) + ); + fs.writeFileSync( + path.join(tmpHome, '.claude.json'), + JSON.stringify({ + mcpServers: { linear: { type: 'url', url: 'https://mcp.linear.app/mcp' } }, + }) + ); + expect(runDetect().json.gbrain_mcp_mode).toBe('none'); + }); + + test('stdio server with gbrain in the command token → local-stdio', () => { + fs.writeFileSync( + path.join(tmpHome, '.claude.json'), + JSON.stringify({ + mcpServers: { 'my-brain': { type: 'stdio', command: '/usr/local/bin/gbrain' } }, + }) + ); + expect(runDetect().json.gbrain_mcp_mode).toBe('local-stdio'); + }); }); describe('gbrain_mcp_mode — no info anywhere', () => { diff --git a/test/gstack-question-preference.test.ts b/test/gstack-question-preference.test.ts index 863cd9e74..8b6fcbe04 100644 --- a/test/gstack-question-preference.test.ts +++ b/test/gstack-question-preference.test.ts @@ -39,6 +39,20 @@ function run(...args: string[]): { stdout: string; stderr: string; status: numbe }; } +function runWithStdin(input: string, ...args: string[]): { stdout: string; stderr: string; status: number } { + const res = spawnSync(BIN, args, { + env: { ...process.env, GSTACK_HOME: tmpHome }, + encoding: 'utf-8', + cwd: ROOT, + input, + }); + return { + stdout: res.stdout ?? '', + stderr: res.stderr ?? '', + status: res.status ?? -1, + }; +} + // ----------------------------------------------------------------------- // --check // ----------------------------------------------------------------------- @@ -103,6 +117,49 @@ describe('--check with preferences set', () => { }); }); +// #2024: the keyword net only fires when the question TEXT reaches the +// classifier. --summary-stdin pipes it (stdin, not argv — summaries carry +// quotes/newlines/shell metacharacters). Without the summary, an unregistered +// id with never-ask auto-decides even for destructive phrasings. +describe('--check --summary-stdin (#2024 keyword net plumb-through)', () => { + function setPref(id: string, pref: string) { + return run('--write', JSON.stringify({ question_id: id, preference: pref, source: 'plan-tune' })); + } + + test('destructive summary on unregistered never-ask id → ASK_NORMALLY (keyword net fires)', () => { + setPref('adhoc-cleanup-question', 'never-ask'); + const r = runWithStdin('Should I reset my secrets now?', '--check', 'adhoc-cleanup-question', '--summary-stdin'); + expect(r.status).toBe(0); + expect(r.stdout).toContain('ASK_NORMALLY'); + expect(r.stdout).toContain('one-way door overrides'); + }); + + test('same id WITHOUT summary still AUTO_DECIDEs (id-only fallback, current semantics)', () => { + setPref('adhoc-cleanup-question', 'never-ask'); + const r = run('--check', 'adhoc-cleanup-question'); + expect(r.stdout.trim()).toContain('AUTO_DECIDE'); + }); + + test('benign summary on unregistered never-ask id → AUTO_DECIDE (no over-match)', () => { + setPref('adhoc-cleanup-question', 'never-ask'); + const r = runWithStdin('Reorganize the TODOs file?', '--check', 'adhoc-cleanup-question', '--summary-stdin'); + expect(r.stdout.trim()).toContain('AUTO_DECIDE'); + }); + + test('summary with quotes/newlines/dashes survives the stdin transport', () => { + setPref('adhoc-cleanup-question', 'never-ask'); + const summary = 'Run "cleanup" --now\nthen rotate the access keys?'; + const r = runWithStdin(summary, '--check', 'adhoc-cleanup-question', '--summary-stdin'); + expect(r.stdout).toContain('ASK_NORMALLY'); + }); + + test('empty stdin with --summary-stdin → id-only behavior (fail-safe)', () => { + setPref('adhoc-cleanup-question', 'never-ask'); + const r = runWithStdin('', '--check', 'adhoc-cleanup-question', '--summary-stdin'); + expect(r.stdout.trim()).toContain('AUTO_DECIDE'); + }); +}); + // Split-chain carve-out: question_ids matching -split- // must always ASK_NORMALLY regardless of stored preferences. // See scripts/resolvers/preamble/generate-ask-user-format.ts diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index db2e7629f..cab08198e 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -97,6 +97,90 @@ describe('check-careful.sh', () => { expect(output.message).toContain('recursive delete'); }); + // The safe exception matches the COMPLETE command against an anchored + // whitelist shape — anything else (chains, comments, substitution) falls + // through to the destructive-pattern warning. + test('rm -rf /; rm -rf node_modules warns (semicolon chain, dangerous first)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /; rm -rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -rf /etc/data && rm -rf dist warns (&& chain, dangerous first)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/data && rm -rf dist')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -rf node_modules; rm -rf /home/user/data warns (safe first, dangerous last)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules; rm -rf /home/user/data')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + // Command substitution can end in a whitelisted suffix while running + // anything inside $(...) or backticks — the whitelist's target tokens + // exclude `(` and backtick so these cannot ride the safe exception. + test('rm -rf $(./wipe-all)/node_modules warns (command substitution)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf $(./wipe-all)/node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -rf `./wipe-all`/node_modules warns (backtick substitution)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf `./wipe-all`/node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + // Capital -R is the documented recursive flag on BSD rm (macOS) and accepted + // by GNU rm. Both greps previously required a lowercase r, so `rm -R /` + // silently allowed. + test('rm -R / warns (capital -R recursive)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -R /')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -fR /home/user warns (capital R in flag cluster)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -fR /home/user')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + test('rm -Rf node_modules allows (capital R, single safe target)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -Rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBeUndefined(); + }); + + // JSON-escaped newline (literal two-char \n surviving the grep extraction + // path) breaks the anchored whitelist shape → falls through to the warn. + test('newline-chained rm warns (escaped-newline separator branch)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/x\nrm -rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + + // Deliberate false positive, pinned: a safe-prefix chain ending in a safe rm + // is indistinguishable from the dangerous-first exploit shape without real + // shell parsing, so warn-on-all-chains is the designed fail-closed direction. + // A future per-segment parser must consciously change this test. + test('cd app && rm -rf node_modules asks (fail-closed on chains, by design)', () => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('cd app && rm -rf node_modules')); + expect(exitCode).toBe(0); + expect(output.permissionDecision).toBe('ask'); + expect(output.message).toContain('recursive delete'); + }); + test.each([ 'rm -rf /; rm -rf node_modules', 'rm -rf / && rm -rf node_modules', diff --git a/test/memory-cache-injection.test.ts b/test/memory-cache-injection.test.ts index 3ab6a2144..991a4c019 100644 --- a/test/memory-cache-injection.test.ts +++ b/test/memory-cache-injection.test.ts @@ -2,7 +2,7 @@ * Layer 8 memory cache + injection (plan-tune cathedral T12). * * Verifies the PreToolUse hook reads ~/.gstack/free-text-memory.json and - * surfaces matching nuggets via additionalContext on the hook response. + * surfaces matching nuggets via additionalContext-only output (#2035: never a permissionDecision). * Cache: per-session memory-cache.json populated on first read, sub-1ms * thereafter (D13 perf). */ @@ -43,9 +43,9 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe env.GSTACK_STATE_ROOT = stateRoot; env.GSTACK_QUESTION_LOG_NO_DERIVE = '1'; delete env.GSTACK_HOME; - // These cases assert the defer-path memoryContext injection. Strip ambient + // These cases assert the pass-through memoryContext injection. Strip ambient // Conductor markers so running inside Conductor (CONDUCTOR_WORKSPACE_PATH/PORT - // set) doesn't flip the hook into the [conductor] prose deny instead of defer. + // set) doesn't flip the hook into the [conductor] prose deny instead of pass-through. delete env.CONDUCTOR_WORKSPACE_PATH; delete env.CONDUCTOR_PORT; const res = spawnSync(HOOK, [], { @@ -69,7 +69,7 @@ function runHook(stdin: object): { stdout: string; stderr: string; status: numbe // ---------------------------------------------------------------------- describe('memory injection', () => { - test('injects matching nugget into additionalContext on defer', () => { + test('injects matching nugget into additionalContext on pass-through', () => { writeMemory([ { nugget: 'User prefers verbose explanations with tradeoffs', @@ -91,7 +91,10 @@ describe('memory injection', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + // #2035: nugget delivery is additionalContext-ONLY — a permissionDecision + // here (any value) would orphan the tool call on CC >= 2.1.89. + expect('permissionDecision' in (r.parsed?.hookSpecificOutput ?? {})).toBe(false); + expect(r.parsed?.hookSpecificOutput?.hookEventName).toBe('PreToolUse'); expect(r.parsed?.hookSpecificOutput?.additionalContext).toContain('verbose explanations'); }); @@ -115,8 +118,9 @@ describe('memory injection', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); - expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined(); + // No nugget → pure pass-through: exit 0 with EXACTLY empty stdout. + expect(r.status).toBe(0); + expect(r.stdout).toBe(''); }); test('caps to 3 most-recent nuggets when many match', () => { @@ -219,7 +223,8 @@ describe('per-session memory cache', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); - expect(r.parsed?.hookSpecificOutput?.additionalContext).toBeUndefined(); + // No nugget → pure pass-through: exit 0 with EXACTLY empty stdout. + expect(r.status).toBe(0); + expect(r.stdout).toBe(''); }); }); diff --git a/test/one-way-doors.test.ts b/test/one-way-doors.test.ts index dcd1a5b38..a3adcc4b7 100644 --- a/test/one-way-doors.test.ts +++ b/test/one-way-doors.test.ts @@ -43,3 +43,50 @@ describe("one-way-door credential keyword net (#1839)", () => { } }); }); + +describe("one-way-door credential keyword net (#2024)", () => { + const VERBS = ["revoke", "reset", "rotate"]; + const NOUNS = ["api key", "token", "secret", "credential", "access key", "password"]; + + // #2024 repro rows: these leaked as two-way pre-fix because the noun + // alternations were mismatched across verbs (revoke lacked secret; reset + // lacked secret AND access key). The password-parallel test above passes on + // buggy code, so THESE rows are the fails-first proof. + test('"reset my secret" / "reset my access key" / "revoke my secret" classify one-way', () => { + for (const summary of ["reset my secret", "reset my access key", "revoke my secret"]) { + const r = classifyQuestion({ summary }); + expect(r.oneWay).toBe(true); + expect(r.reason).toBe("keyword"); + } + }); + + test("full verbs x nouns matrix classifies one-way (singular and plural)", () => { + for (const verb of VERBS) { + for (const noun of NOUNS) { + for (const form of [noun, `${noun}s`]) { + const r = classifyQuestion({ summary: `${verb} the production ${form}` }); + expect(r.oneWay).toBe(true); + expect(r.reason).toBe("keyword"); + } + } + } + }); + + // Plural forms leaked before AND after the original #2024 report: \b(...)\b + // cannot match "credentials" (no word boundary between the noun and its s). + test('plurals: "rotate the credentials" / "revoke all tokens" / "reset the passwords" classify one-way', () => { + for (const summary of ["rotate the credentials", "revoke all tokens", "reset the passwords"]) { + expect(classifyQuestion({ summary }).oneWay).toBe(true); + } + }); + + test("benign summaries stay two-way (no over-match)", () => { + for (const summary of [ + "reset the flaky test runner", + "rotate the log files nightly", + "revoke the meeting invite", + ]) { + expect(classifyQuestion({ summary }).oneWay).toBe(false); + } + }); +}); diff --git a/test/question-preference-hook.test.ts b/test/question-preference-hook.test.ts index 39de02f4e..d96843f4e 100644 --- a/test/question-preference-hook.test.ts +++ b/test/question-preference-hook.test.ts @@ -3,15 +3,18 @@ * * Covers: * - never-ask + marker + two-way + clean recommendation → deny+reason - * - never-ask + no marker → defer (D18 marker gate) - * - never-ask + one-way → defer (safety override) - * - never-ask + ambiguous recommendation → defer (D2 refuse-on-ambiguous) - * - always-ask → defer - * - no preference → defer + * - never-ask + no marker → pass-through (D18 marker gate) + * - never-ask + one-way → pass-through (safety override) + * - never-ask + ambiguous recommendation → pass-through (D2 refuse-on-ambiguous) + * - always-ask → pass-through + * - no preference → pass-through * - project preference wins over global (D8 precedence) * - global preference applies when no project preference set * - mcp__*__AskUserQuestion matcher accepted - * - empty stdin → defer (crash safety) + * - empty stdin → pass-through (crash safety) + * + * Pass-through contract (#2035/#2006): exit 0 + EXACTLY empty stdout, or + * additionalContext-only hookSpecificOutput — never a permissionDecision. * - auto-decided event logged via gstack-question-log (PostToolUse won't fire) * - auto-decided marker written to ~/.gstack/sessions//.auto-decided- */ @@ -97,6 +100,20 @@ function runHook(stdin: object, cwd?: string, extraEnv?: Record) }; } +/** + * #2035/#2006 contract: pass-through (abstain) is exit 0 with EXACTLY empty + * stdout — never a permissionDecision. 'defer' is a real PreToolUse value, + * but its semantics are pause-for-external-resumption (CC v2.1.89+), so + * emitting it orphans the tool call in interactive sessions. Exact-empty + * (not trim) is deliberate: whitespace on stdout is still hook output, and a + * garbage/partial write must fail this assertion rather than slip past an + * optional-chained parse. + */ +function expectPassThrough(r: { status: number; stdout: string }): void { + expect(r.status).toBe(0); + expect(r.stdout).toBe(''); +} + function autoDecidedEvents(): Array> { const f = path.join(stateRoot, 'projects', cwdSlug, 'question-log.jsonl'); if (!fs.existsSync(f)) return []; @@ -113,8 +130,8 @@ function autoDecidedEvents(): Array> { // Defer paths // ---------------------------------------------------------------------- -describe('defers (no enforcement)', () => { - test('no preference set → defer', () => { +describe('passes through (no enforcement)', () => { + test('no preference set → pass-through (empty stdout, no permissionDecision)', () => { const r = runHook({ session_id: 's1', tool_name: 'AskUserQuestion', @@ -125,11 +142,10 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.status).toBe(0); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); - test('marker missing → defer (D18)', () => { + test('marker missing → pass-through (D18)', () => { writeProjectPref('test-q', 'never-ask'); const r = runHook({ session_id: 's2', @@ -141,10 +157,10 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); - test('always-ask preference → defer', () => { + test('always-ask preference → pass-through', () => { writeProjectPref('test-q', 'always-ask'); const r = runHook({ session_id: 's3', @@ -156,10 +172,10 @@ describe('defers (no enforcement)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); - test('empty stdin → defer (crash safety)', () => { + test('empty stdin → pass-through (crash safety)', () => { const env: Record = {}; for (const [k, v] of Object.entries(process.env)) { if (v !== undefined) env[k] = v; @@ -167,14 +183,39 @@ describe('defers (no enforcement)', () => { env.GSTACK_STATE_ROOT = stateRoot; const res = spawnSync(HOOK, [], { env, input: '', encoding: 'utf-8' }); expect(res.status).toBe(0); - const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer'); + expect(res.stdout).toBe(''); }); - test('non-AUQ tool_name → defer (defensive)', () => { + test('non-AUQ tool_name → pass-through (defensive)', () => { writeProjectPref('test-q', 'never-ask'); const r = runHook({ session_id: 's4', tool_name: 'Bash', tool_use_id: 'tu-4', tool_input: {} }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); + }); + + // #2035 tripwire: no non-deny/non-allow path may EVER put the string + // "permissionDecision" on stdout. Emitting one on a pass-through path (any + // value — 'defer' included) hands the platform a decision where the hook + // has none, and 'defer' specifically pauses the call for a resumption that + // never comes in interactive sessions. + test('pass-through stdout never contains "permissionDecision" (#2035)', () => { + const paths = [ + runHook({ + session_id: 's-trip-1', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-trip-1', + tool_input: { + questions: [ + { question: ' Approve?', options: ['A) Yes (recommended)', 'B) No'] }, + ], + }, + }), + runHook({ session_id: 's-trip-2', tool_name: 'Bash', tool_use_id: 'tu-trip-2', tool_input: {} }), + runHook({ session_id: 's-trip-3', tool_name: 'AskUserQuestion', tool_use_id: 'tu-trip-3', tool_input: { questions: [] } }), + ]; + for (const r of paths) { + expect(r.status).toBe(0); + expect(r.stdout).not.toContain('"permissionDecision"'); + } }); }); @@ -204,7 +245,7 @@ describe('enforces never-ask preferences', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('Fix now'); }); - test('one-way door → defer even with never-ask (safety override)', () => { + test('one-way door → pass-through even with never-ask (safety override)', () => { writeProjectPref('ship-test-failure-triage', 'never-ask'); const r = runHook({ session_id: 's6', @@ -219,10 +260,10 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); - test('ambiguous recommendation (two labels) → defer (D2 refuse-on-ambiguous)', () => { + test('ambiguous recommendation (two labels) → pass-through (D2 refuse-on-ambiguous)', () => { writeProjectPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ session_id: 's7', @@ -237,10 +278,10 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); - test('no recommendation marker AND no prose match → defer', () => { + test('no recommendation marker AND no prose match → pass-through', () => { writeProjectPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ session_id: 's8', @@ -255,7 +296,48 @@ describe('enforces never-ask preferences', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); + }); + + // #2024: unregistered ids used to default straight to two-way without ever + // consulting the keyword classifier — an ad-hoc DESTRUCTIVE question with a + // stored never-ask preference auto-decided. The hook now falls back to + // classifyQuestion on the question text when the registry lookup misses. + test('unregistered id + never-ask + destructive text → pass-through (keyword net fires, #2024)', () => { + writeProjectPref('adhoc-credential-cleanup', 'never-ask'); + const r = runHook({ + session_id: 's-kw-1', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-kw-1', + tool_input: { + questions: [ + { + question: ' Reset my secret and proceed?', + options: ['A) Yes (recommended)', 'B) No'], + }, + ], + }, + }); + expectPassThrough(r); + }); + + test('unregistered id + never-ask + benign text → still deny (auto-decide unchanged)', () => { + writeProjectPref('adhoc-credential-cleanup', 'never-ask'); + const r = runHook({ + session_id: 's-kw-2', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-kw-2', + tool_input: { + questions: [ + { + question: ' Reorganize the TODOs file?', + options: ['A) Yes (recommended)', 'B) No'], + }, + ], + }, + }); + expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); + expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).toContain('plan-tune auto-decide'); }); }); @@ -301,7 +383,7 @@ describe('precedence: project wins over global (D8)', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('deny'); }); - test('project always-ask + global never-ask → defer (project wins)', () => { + test('project always-ask + global never-ask → pass-through (project wins)', () => { writeProjectPref('ship-pre-landing-review-fix', 'always-ask'); writeGlobalPref('ship-pre-landing-review-fix', 'never-ask'); const r = runHook({ @@ -317,7 +399,7 @@ describe('precedence: project wins over global (D8)', () => { ], }, }); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); }); @@ -437,13 +519,13 @@ describe('Conductor prose redirect', () => { expect(r.parsed?.hookSpecificOutput?.permissionDecisionReason).not.toContain('[conductor]'); }); - test('non-AUQ tool in Conductor → still defer (no redirect on unrelated tools)', () => { + test('non-AUQ tool in Conductor → still pass-through (no redirect on unrelated tools)', () => { const r = runHook( { session_id: 'c6', tool_name: 'Bash', tool_use_id: 'tu-c6', tool_input: {} }, undefined, CONDUCTOR, ); - expect(r.parsed?.hookSpecificOutput?.permissionDecision).toBe('defer'); + expectPassThrough(r); }); }); diff --git a/test/skill-e2e-plan-tune-cathedral.test.ts b/test/skill-e2e-plan-tune-cathedral.test.ts index f9c006914..85a455036 100644 --- a/test/skill-e2e-plan-tune-cathedral.test.ts +++ b/test/skill-e2e-plan-tune-cathedral.test.ts @@ -296,7 +296,10 @@ describeIfSelected('PlanTune cathedral E2E: annotation', ['plan-tune-annotation' }); expect(res.status).toBe(0); const parsed = JSON.parse(res.stdout || '{}'); - expect(parsed.hookSpecificOutput?.permissionDecision).toBe('defer'); + // #2035: memory-nugget delivery is additionalContext-ONLY. Emitting a + // permissionDecision here (the old 'defer') pauses the tool call for a + // resumption that never comes in interactive sessions. + expect('permissionDecision' in (parsed.hookSpecificOutput ?? {})).toBe(false); expect(parsed.hookSpecificOutput?.additionalContext).toContain('verbose explanations'); }); }); diff --git a/test/skill-e2e-plan.test.ts b/test/skill-e2e-plan.test.ts index 27e4d74d8..a1aac4f45 100644 --- a/test/skill-e2e-plan.test.ts +++ b/test/skill-e2e-plan.test.ts @@ -549,7 +549,13 @@ Summarize what the "Spec Review Loop" section does — specifically: Write your summary to ${ohDir}/spec-review-summary.md`, workingDirectory: ohDir, - maxTurns: 8, + // 12, not 8 (#2473): the Spec Review Loop content is CARVED out of + // SKILL.md into office-hours/sections/, so the agent legitimately needs + // discovery hops (grep SKILL.md -> ls sections/ -> read the section) + // before it can write. The 8-turn budget predates the carve — observed + // failures wrote a correct summary on tool-turn 8 and hit the cap on + // the closing text turn (error_max_turns at 9 turns, deterministic). + maxTurns: 12, timeout: 120_000, testName: 'office-hours-spec-review', runId, diff --git a/test/skill-e2e-review.test.ts b/test/skill-e2e-review.test.ts index 1adbe25c7..5eaf16ca3 100644 --- a/test/skill-e2e-review.test.ts +++ b/test/skill-e2e-review.test.ts @@ -427,7 +427,11 @@ This is a local-only repo so use the local branch (main) instead of origin/main Write your retrospective to ${dir}/retro-output.md`, workingDirectory: dir, maxTurns: 25, - timeout: 240_000, + // 360s, not 240s: same runner-contention class as review-dashboard-via. + // /retro is a long multi-step flow — a clean pass measured 225s and the + // next CI run timed out at the 240s line (exitReason "timeout", 3/3 + // attempts). Outer bun timeout below rises to 480s for headroom. + timeout: 360_000, testName: 'retro-base-branch', runId, }); @@ -444,7 +448,7 @@ Write your retrospective to ${dir}/retro-output.md`, const content = fs.readFileSync(retroPath, 'utf-8'); expect(content.length).toBeGreaterThan(100); } - }, 300_000); + }, 480_000); }); // --- Retro E2E --- @@ -614,7 +618,13 @@ Skip the preamble, lake intro, telemetry, and all other ship steps. Write the dashboard output to ${dashDir}/dashboard-output.md`, workingDirectory: dashDir, maxTurns: 12, - timeout: 180_000, + // 300s, not 180s: on a saturated CI runner this file's concurrent + // sessions queue behind each other and session STARTUP can eat the + // whole budget — observed as deterministic timeout at 0 turns/$0.00 + // for exactly 180s across 3 attempts (PR #2472 CI + its baseline), + // while the 240s-budget tests in the same job passed. Outer bun + // timeout below rises to 360s to keep headroom over the inner budget. + timeout: 300_000, testName: 'review-dashboard-via', runId, }); @@ -648,7 +658,7 @@ Write the dashboard output to ${dashDir}/dashboard-output.md`, ); // Ship dashboard should not gate when eng review is clear expect(gateQuestions).toHaveLength(0); - }, 240_000); + }, 360_000); }); // Module-level afterAll — finalize eval collector after all tests complete