diff --git a/CHANGELOG.md b/CHANGELOG.md index 45af59585..a87ad2fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,119 @@ - No threshold, budget, timeout, trial count, case kind or skip changed. The user file can relax lanes only where the base has no policy at all, and only from outside the repository; the two fixes make that containment stricter. - No new dependency, no network call, no workflow change. +## [1.91.205.0] - 2026-10-25 + +**/ship's Step 9 can re-review just the fixes after a fixing pass, and a narrowed pass can never be the review that ships (#3339).** Before, every fix cycle reran every specialist and the Red Team over the whole branch. On a large branch each full pass tends to find something new in code earlier passes already read, so the loop often ran to the three-cycle stop without converging. Now the pass after a fixing pass can review only what changed since that pass. One full pass still has to come last: a fix-range record is never CURRENT, and Step 11.5 can't bind it. Garry approved this on 2026-10-24 ("only with a mandatory final full pass"). + +### Added +- **`bin/gstack-review-log --start review --fix-range`** prints the start token, then `FIX_BASE=`. The base comes from the review log, never from the caller or the working tree, so `gstack-wtree`'s fingerprint is untouched. It is the commit of the latest pass of the same skill, and that commit's tree must be exactly the content the pass started from, so `git diff ` holds every fix, committed or not. The start refuses (exit 1, "run a full pass") in each of these cases: there is no earlier pass, that pass didn't complete (restoring coverage always takes a full pass), it made no fixes, it has no captured start, it ran on another branch, a Step 11 review followed it, it started from uncommitted or untracked content, or its commit no longer resolves or is no longer an ancestor of HEAD (after a rebase). On the start-work-then-ship flow, where Step 9 first reviews uncommitted work, the first repeat therefore stays full. +- **`lib/review-evidence.ts`**: the start token carries `scope: "fix-range"` and `fix_base`. The logged row takes `scope` and `fix_base` only from the token (a caller's values are dropped either way), and a token that names any fix base stays narrowed even if its scope is rewritten. On unchanged content its `review_binding` is `{state: "fix-range", range_start_wtree, range_end_wtree, …}`, with no `start_wtree`/`end_wtree` and no root `wtree`. `reviewFreshness` returns UNVERIFIED for any fix-range row ("fix-range pass; only a later full pass can clear review"), even one with a verified-looking binding. +- **`bin/gstack-specialist-stats`** skips fix-range rows, so narrowed dispatches don't push specialists toward `[GATE_CANDIDATE]`. + +### Changed +- **`ship/sections/review-army.md.tmpl`**: the core checklist explains the fix-range start, and that `FIX_BASE` stands in for `origin/` and `` for that pass. Decision 3 allows the repeat to be a fix-range pass. Decision 4 continues to Step 10 only from a full pass. Nearby prose was tightened to pay for the growth, so ship's skeleton plus sections is 313,246 bytes against its 313,281 cap (35 bytes below main). + +### Tests +- `test/review-fix-range.test.ts` (20 cases) follows Step 9.4's real order (fixes applied before the row is logged). It covers the base with committed and uncommitted fixes, chained fix-range passes, the eight refusals, a rewritten start token, a caller claiming `scope: "full"` on a fix-range token (it is still recorded as fix-range) or `"fix-range"` on a full one (it is dropped), Step 11.5's missing binding fields, UNVERIFIED freshness followed by CURRENT after a full pass, shared-libs reuse refusing a fix-range row, the stats exclusion, and the rendered Step 9 rules. + +### Gate integrity +- One gate change to review: Step 9 may narrow a repeat pass. Verdict: not a weakening of what ships, because the narrowed pass can't be the final review. Three things enforce that: its row is never CURRENT, Step 11.5 has no `start_wtree`/`end_wtree` to bind, and decision 4 requires a full pass before Step 10. Mutation checks, each caught by the suite: dropping the freshness check, giving a narrowed row the normal binding, allowing a fix-range start after a clean, an incomplete or a Step 11-followed pass, accepting a non-ancestor base, accepting a start from uncommitted content, trusting the token's scope alone, and counting fix-range rows in stats. No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.204.0] - 2026-10-25 + +**An unattended run is told to end with `GSTACK_RESULT` only when its skill can write that line.** The unattended block `gstack-skill-start` prints told every unattended skill to log choices in `decisions.jsonl`, write a gate list and end with `GSTACK_RESULT: skill= status=gate_pending run=`. Only `/autoplan` has helpers that write a run directory, so `/ship`, `/review`, `/qa` and the rest were asked to produce files and a result line nothing writes, and a parent that trusted the block would wait for them. `docs/unattended.md` already names which skills write the line and what a parent reads for the others (#3281); the runtime text now says the same thing. + +### Fixed +- **`bin/gstack-skill-start`**: the decision rules in the unattended block are the same for every skill: take the recommended option, leave unrecommended questions, consents, destructive options and the final gate pending, and never approve, publish or push. Only the sentence about the run's record changes by skill. `/autoplan` keeps its run directory, its `decisions.jsonl` rows, the gate list and `GSTACK_RESULT: skill=autoplan status=gate_pending run=`. `/eval-plan` ends at its spend approval with `GSTACK_RESULT: skill=eval-plan status=gate_pending run=`, and the block says no helper writes a `gate.json` for it yet. Every other skill is told it writes no run directory and prints no result line, to stop at its first pending item and list it in its final report, and which record the parent reads: the ship receipt (`gstack-ship-receipt read --pr --json`) for `/ship`, the review-log rows (`gstack-review-read`) for `/review`, the `qa-report-*.md` file for `/qa`, and the skill's own record in `docs/unattended.md` otherwise. + +### Tests +- `test/gstack-skill-start.test.ts` renders the unattended block for `autoplan`, `eval-plan`, `ship`, `review`, `qa` and `investigate`. It checks that each keeps the shared decision rules, that only `autoplan` and `eval-plan` get a result line (with their own skill name, never ``), and that the other four get no result line, gate list, `decisions.jsonl` or `run.json` and name their record. Against the old block it fails, and so does the existing unattended test, which now expects `skill=autoplan`. + +### Gate integrity +- One test expectation changed: the existing unattended test expects `GSTACK_RESULT: skill=autoplan …` instead of the `skill=` placeholder, which is stricter. No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.203.0] - 2026-10-25 + +**Asking gstack to fix a failure no longer always opens /investigate (#3157, #3158).** The router sent every bug report straight to the full /investigate workflow, even when the user had named the cause or one look at the failing code showed it. Now the router checks first. When the user names a cause and a narrow fix, it makes one read-only check of the evidence. When that confirms the cause, it makes the fix and runs the focused test. When the cause is unknown, it makes one bounded, read-only triage pass over the supplied evidence and the directly implicated test and code, with no reproduction, hypothesis testing or wider search, and fixes the failure directly only when that pass establishes the root cause. Anything still unclear, including a cause the check contradicts, goes to /investigate before any edit, so the root-cause bar holds. A failing test or stack trace no longer counts as proof of its cause. + +### Changed +- **`SKILL.md.tmpl`** (root router): the bug-report rule points to a new **Failures** block that carries the two paths. Its text comes from `generateFailureRouting()` in `scripts/resolvers/preamble/generate-test-failure-triage.ts`. +- **`bin/gstack-skill-start`** and `CLAUDE.md`: the routing section gstack offers for a project's CLAUDE.md now reads "Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test)", so it agrees with the router. +- **`/ship` Test Failure Ownership Triage**, "Investigate and fix now": "Switch to /investigate mindset" becomes "One read-only root-cause pass, then `/investigate` if unclear." + +### Tests +- New paid eval `test/skill-e2e-failure-routing.test.ts` (gate, `rule`): four fixture repos, two per issue. `router-failure-verified-cause` and `router-failure-triage-resolves` must fix the bug without /investigate and leave the suite green. `router-failure-contradicted-cause` (a user-supplied cause the code contradicts) and `router-failure-triage-unclear` (an intermittent CI-only failure) must invoke /investigate, and an edit before it is a contract violation. Every case needs one to four read-only calls before it routes. `GSTACK_ROUTER_ARM=before` runs the same cases against the base branch's router. +- `test/skill-validation.test.ts` checks that the rendered router carries the rule and no longer sends bug reports straight to /investigate, that both paths keep the /investigate fallback, that the shipped CLAUDE.md routing line matches this repo's own, and that /ship's fix-now step uses the bounded pass. + +### Gate integrity +- No threshold, budget, timeout, trial count or existing case kind changed. The ship skill's parity size stays under its cap (313,260 of 313,281 bytes; 313,276 with #3372 merged). The new eval's negative controls are the two /investigate cases, and on the base router the triage cases go red. +- Registries and pins the new gate-tier file needs, each with a stated reason: `test/eng-finding-retry-budget.test.ts` counts 110 paid test files (was 109). `test/helpers/e2e-hosts.ts` lists the four cases as `skill-tool` Claude-only, because they assert on the Claude Skill tool call. `test/codex-host.test.ts` counts 33 Claude-specific gate cases (was 29). +- Each train gate now runs the four new cases, one trial each, for about $0.37 (measured at this head: $0.06, $0.06, $0.11 and $0.14; about 67 s wall when they run in parallel). + +## [1.91.202.0] - 2026-10-25 + +**Team-mode session auto-update waits for live gstack sessions instead of moving the checkout under them (#3363).** `bin/gstack-upgrade` already refuses with `UPGRADE_BUSY` while another live session holds a workflow lease on the installation (`bin/gstack-lease`), but the SessionStart hook `bin/gstack-session-update` never read the leases. A shared `./setup --team` install could fast-forward and re-run `./setup` while another session was mid-workflow. Reproduced on main: with a live lease held, the hook logged `UPDATING` and moved the install's HEAD. + +### Fixed +- **`bin/gstack-session-update`**: after taking its setup lock and before fetching, the background updater asks `gstack-lease live --root --exclude-ancestry-of `. Any live lease from another session logs `SKIP leased pid=/` and exits without fetching, fast-forwarding, retrying a pending setup stage or writing the hourly throttle stamp, so the next session start tries again. `GSTACK_UPGRADE_FORCE=1` overrides, as it does for `gstack-upgrade`. A dead or stale lease never blocks. + +### Tests +- `test/session-update-stages.test.ts` holds a lease from a live process: the install's HEAD, setup runs and the throttle stamp are unchanged, and the first session start after the holder exits updates. A second case shows a lease also holds a pending setup retry. Both fail on main. Controls: `GSTACK_UPGRADE_FORCE=1` updates under a live lease, and a lease whose process is dead does not block. + +## [1.91.201.0] - 2026-10-25 + +**/ship's review army and /plan-eng-review now ask for one severity scale, CRITICAL or INFORMATIONAL, the same scale /review, the specialist JSON and the review checklist already use (#3349).** The shared confidence rubric those two skills render told the model to label findings `[P1]` and `[P2]` and to report a 1-2 confidence finding "only if severity would be P0", while the rest of /ship's Step 9 (Pass 1 CRITICAL, Pass 2 INFORMATIONAL, `"severity":"CRITICAL|INFORMATIONAL"`) used the other scale. A finding from one skill couldn't be compared, deduplicated or gated against a finding from another. Garry picked CRITICAL/INFORMATIONAL for plan-eng-review on 2026-10-24. + +### Fixed +- **`scripts/resolvers/confidence.ts`** (default branch, rendered into `ship/sections/review-army.md` and `plan-eng-review/sections/review-sections.md`): every finding carries a severity, CRITICAL (blocks shipping) or INFORMATIONAL (lower severity, still actioned). The format is `[CRITICAL|INFORMATIONAL] (confidence: N/10) file:line — description` with CRITICAL and INFORMATIONAL examples. A 1-2 score is reported only for a suspected release-blocking catastrophe, at CRITICAL severity and marked explicitly speculative, the rule /review already uses. Repeated sentences in the verification gate and the calibration-learning note were shortened, so ship's skeleton-plus-sections size falls by 77 bytes (313,204 against its 313,281 cap). + +### Tests +- `test/eng-first-review.test.ts`: the two captured plan-eng-review first-review questions (`eng-annotated-cache-au`, `eng-declarative-as`) still open review when their `[P1]` label becomes `[CRITICAL]` or `[INFORMATIONAL]`. They stay closed for `[HIGH]`, `[critical]`, `[Critical]`, `[P4]`, `[CRITICAL|INFORMATIONAL]` and no label. + +### Gate integrity +- One accepted-input change to review: the paid-test detectors `test/helpers/eng-cache-writer-decision.ts` and `test/helpers/pty/boundaries.ts` accept `[CRITICAL]` and `[INFORMATIONAL]` alongside `[P0]`-`[P3]` in a plan finding's annotation, because the skill now asks for those labels. Without it, both captures relabelled `[CRITICAL]` are rejected. Verdict: not a weakening. The label is metadata, and every substantive check (decision, assessment, actors, options) is unchanged. Mutation checks: restoring `P[0-3]` alone fails the positive cases; `[^\]]+`, `P[0-9]` or a case-insensitive CRITICAL each fail the negative controls. `scripts/harness-version.json` records `test/helpers/pty/boundaries.ts` as non-behavioral at HARNESS_VERSION 6: only the changed plan-eng-review prompt asks for the new labels, and the cases that run it already start their own series. The 11 plan-eng-review size pins in `test/fixtures/skill-md-size-allowlist.json` come down by 75 characters. No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.200.0] - 2026-10-25 + +**/cso doctor no longer calls a Go repository "stack: python".** A repository with a root `go.mod` and no Node, Bun, Rails or JVM build matched no supported stack, so preparation fell back to Python and doctor reported `stack: python` with `MISSING_METADATA requirements.txt`, which sent Go users looking for a Python file they never needed (#3263). Go runtime support itself is not planned for now (Garry, 2026-10-24), so /cso keeps running static assessment on Go repositories; the runtime catalog and the /cso release contract are unchanged. + +### Fixed +- **`lib/cso/preparation.ts`**: a root `go.mod` with no other supported stack is reported as `stack: go` with `UNSUPPORTED_STACK` ("Go (go.mod) is not a supported runtime stack yet; supported runtime stacks are Node, Bun, Python, and Rails."), the way a Maven or Gradle build is already reported as Java. Python helper metadata beside `go.mod` (a docs `requirements.txt`, say) no longer turns the repository into Python. A `go.mod` beside a Node, Bun or Rails lockfile leaves that stack's detection as it was. + +### Tests +- `test/cso-preparation.test.ts` checks that a Go module is `go` with exactly that prerequisite and never mentions `requirements.txt`, and that a hashed `requirements.txt` beside `go.mod` stays Go. Both fail on main. Controls: `go.mod` beside a Node lockfile stays a ready Node plan, a Python project without `go.mod` stays Python, and a repository with no stack metadata keeps the Python fallback. +- `test/helpers/cso-runtime-catalog.ts` leaves `go` out of its qualified-runtime fixture list, as it already did `java`. + +### Gate integrity +- No gate edit. No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.199.0] - 2026-10-25 + +**When /sync-gbrain refuses because a gbrain autopilot is running, it now says which process it found and what to do.** The guard against syncing while `gbrain autopilot` runs is machine-wide on purpose: two syncs sharing one brain or working tree can race and delete files (#1734). But the refusal only said `(process:gbrain autopilot)`, so someone whose autopilot served a different brain could not tell what was blocking them (#3360, part 1). Garry decided on 2026-10-24 to keep the check machine-wide and improve the message. + +### Changed +- **`lib/gbrain-guards.ts`**: the live-process probe still runs `pgrep -f "gbrain autopilot"` across the whole machine, then reads each match's command line with `ps`. The refusal names every match as `pid \`\``, says the check counts any process on the machine whose command line contains "gbrain autopilot" (an autopilot serving another brain or repo included), and says to stop that autopilot or its `gbrain autopilot --install` service, or end a process that only mentions it. Commands are cut at 200 characters and the user and password part of a URL is masked. A pid that `ps` cannot describe still refuses, as `command unavailable`. The header comment now gives the machine-wide reason in place of the stale gbrain #1226 note. + +### Tests +- `test/gbrain-guards.test.ts` pins the full refusal for one matched process, lists two matches, keeps an empty match inactive, and runs the real probe against a live process whose command line carries "gbrain autopilot" and a database URL with a password: the refusal names its pid and masks the password. All four fail on main. The existing probe, lock and stale-lock tests are unchanged. + +### Gate integrity +- No gate edit: the guard refuses in exactly the cases it did before, and only its message changed. No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.198.0] - 2026-10-25 + +**A second-opinion reviewer that says "not ready" is no longer reported as clean.** When the outside reviewer (Codex or Claude Code) ended its reply with `Verdict: not ready` but listed only advisory P2 or P3 findings, `lib/outside-review-result.ts` printed `VERDICT: clean` and exited 0, so /ship and /codex carried on as if the reviewer had approved. The not-ready check only ran when no severity parsed at all, so a parsed P2 or P3 switched it off (#3329). Ported from #3364 by @mvanhorn. + +### Fixed +- **`lib/outside-review-result.ts`**: when the reply's last recognized `Verdict:` line says `not ready`, the result is `findings` (exit 3) whatever severities parsed, and `FINDINGS:` still names the highest one. Advisory P2/P3 findings with a ready verdict, or with no verdict, stay `clean`. A `Verdict:` line quoted in a code fence or inline code still does not count, and the proposal and spec gates ignore it as before. +- **`docs/troubleshooting.md`**: the "Every review gate" rule under *How the verdict is read from a review* now says a not-ready reply blocks even when P2 or P3 parsed. + +### Tests +- `test/outside-review-unparsed-severity.test.ts` covers the issue's reply (numbered bold P2 plus P3, then `Verdict: not ready`) on the execution, structured and review gates, P3-only and unlabeled not-ready replies, the last verdict winning in both directions, bold, bulleted and heading verdict lines, and an unparsed label beside a parsed P2. Five of its tests fail on main's classifier and pass with the fix. Controls that stay `clean`: P2/P3 with `Verdict: ready` or no verdict, a not-ready verdict inside a code fence or inline code, and `ready-one-p3.txt`. Missing review markers, a failed execution, and the proposal and spec gates keep their old results. + +### Gate integrity +- One gate edit, and it strengthens the gate: the pinned `bare-dash-p2.txt` fixture (two P2 lines, then `Verdict: not ready (2 P2)`) moves from `VERDICT: clean`, exit 0 to `VERDICT: findings`, exit 3. Garry approved this reversal on 2026-10-24. No threshold, budget, timeout, trial count or case kind changed. + ## [1.91.197.0] - 2026-10-25 **Pregate gives touched Rust files a test lane, and runs a nested pnpm or vitest package with its own runner (#3344).** In a mixed Cargo and JavaScript repo, every touched `.rs` file failed `lanes` with "has no test lane", so /ship could not get a receipt, and a nested package's vitest tests ran under `bun test` from the repo root (about 98 false failures in the reporter's repo). Ported from #3346 by vilano93, whose two commits carry the feature; our commits on top close four findings from a security-focused review of the port and make the new fixtures hermetic. diff --git a/CLAUDE.md b/CLAUDE.md index a3ed96bc2..095a8d7b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -544,7 +544,7 @@ Key routing rules: - Architecture → invoke /plan-eng-review - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan -- Bugs/errors → invoke /investigate +- Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) - QA/testing site behavior → invoke /qa or /qa-only - Code review/diff check → invoke /review - Visual polish → invoke /design-review diff --git a/SKILL.md b/SKILL.md index 526670028..7ab37f435 100644 --- a/SKILL.md +++ b/SKILL.md @@ -189,7 +189,7 @@ quality gates that produce better results than answering inline. - User asks to review design of a plan → invoke `/plan-design-review` - User asks about developer experience of a plan, API/CLI/SDK design → invoke `/plan-devex-review` - User wants all reviews done automatically, "review everything" → invoke `/autoplan` -- User reports a bug, error, broken behavior, "why is this broken", "this doesn't work", "wtf", "something's wrong" → invoke `/investigate` +- User reports a bug, error, failing test or broken behavior, "why is this broken", "this doesn't work", "wtf", "something's wrong" → check the cause by **Failures** below, then fix it or invoke `/investigate` - User asks to test the site, find bugs, QA, "does this work", "check the deploy" → invoke `/qa` - User asks to just report bugs without fixing → invoke `/qa-only` - User asks to review code, check the diff, pre-landing review, "look at my changes" → invoke `/review` @@ -244,6 +244,11 @@ Generic “second opinion”, “outside review”, or “cross-model review” - Voice trigger "quality check", "test the app", "run QA" → invoke `/qa` - Voice trigger "bug report", "just check for bugs" → invoke `/qa-only` +**Failures: check the cause, then route.** A failing test, error or stack trace shows where a failure surfaces, not its root cause. + +- **Cause supplied** (the user names a specific cause and a narrow fix): make one read-only check of the evidence it rests on, such as the named file, test or log. If that confirms the cause, make the fix and run the focused test that covers it. If it does not, invoke `/investigate` before any further tracing or edit. +- **Cause unknown:** make one read-only triage pass over the supplied evidence and the directly implicated test and code. Do not reproduce, test hypotheses or search wider. If that pass establishes the root cause and a narrow fix, make the fix and run the focused test that covers it. If the cause, scope or intended behavior is still unclear, invoke `/investigate` before any broader diagnosis or edit. + **Route only to skills in your available-skills list.** A rule above whose skill is not listed there is turned off on this install; skip it, never try to invoke it. diff --git a/SKILL.md.tmpl b/SKILL.md.tmpl index 3326854f2..c230671ce 100644 --- a/SKILL.md.tmpl +++ b/SKILL.md.tmpl @@ -65,7 +65,7 @@ quality gates that produce better results than answering inline. - User asks to review design of a plan → invoke `/plan-design-review` - User asks about developer experience of a plan, API/CLI/SDK design → invoke `/plan-devex-review` - User wants all reviews done automatically, "review everything" → invoke `/autoplan` -- User reports a bug, error, broken behavior, "why is this broken", "this doesn't work", "wtf", "something's wrong" → invoke `/investigate` +- User reports a bug, error, failing test or broken behavior, "why is this broken", "this doesn't work", "wtf", "something's wrong" → check the cause by **Failures** below, then fix it or invoke `/investigate` - User asks to test the site, find bugs, QA, "does this work", "check the deploy" → invoke `/qa` - User asks to just report bugs without fixing → invoke `/qa-only` - User asks to review code, check the diff, pre-landing review, "look at my changes" → invoke `/review` @@ -99,6 +99,11 @@ quality gates that produce better results than answering inline. - User asks to tune question sensitivity, "stop asking me that" → invoke `/plan-tune` - User asks for code quality dashboard, "health check" → invoke `/health`{{VOICE_TRIGGER_ROUTING}} +**Failures: check the cause, then route.** A failing test, error or stack trace shows where a failure surfaces, not its root cause. + +- **Cause supplied** (the user names a specific cause and a narrow fix): make one read-only check of the evidence it rests on, such as the named file, test or log. If that confirms the cause, make the fix and run the focused test that covers it. If it does not, invoke `/investigate` before any further tracing or edit. +- **Cause unknown:** make one read-only triage pass over the supplied evidence and the directly implicated test and code. Do not reproduce, test hypotheses or search wider. If that pass establishes the root cause and a narrow fix, make the fix and run the focused test that covers it. If the cause, scope or intended behavior is still unclear, invoke `/investigate` before any broader diagnosis or edit. + **Route only to skills in your available-skills list.** A rule above whose skill is not listed there is turned off on this install; skip it, never try to invoke it. diff --git a/VERSION b/VERSION index a82223dcb..89e8820dd 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.197.0 +1.91.205.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 7d73ae158..05999fcf2 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.197.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.205.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-review-log b/bin/gstack-review-log index 8a4dd2288..102fb1dc1 100755 --- a/bin/gstack-review-log +++ b/bin/gstack-review-log @@ -3,6 +3,10 @@ # Usage: gstack-review-log '{"skill":"...","timestamp":"...","status":"..."}' # # Before reading a diff: gstack-review-log --start review +# Fix-range repeat (#3339): gstack-review-log --start review --fix-range +# prints the token, then FIX_BASE=; refuses (exit 1) unless the latest +# pass completed with fixes on unchanged content and its commit is an +# ancestor of HEAD. Its row is never CURRENT; a later full pass must follow. # After that pass: gstack-review-log '{...,"completed":true,"converged":true}' --finish TOKEN # With a findings file: gstack-review-log '{...}' --findings findings.jsonl # status/unresolved/issues_found/critical_gaps are DERIVED from the file (B7); @@ -72,9 +76,11 @@ fi SESSION_KIND=$("$SCRIPT_DIR/gstack-session-kind" 2>/dev/null || echo interactive) export GSTACK_STAMP_COMMIT_FULL="$COMMIT_FULL" GSTACK_STAMP_TREE="$TREE" GSTACK_STAMP_WTREE="$WTREE" GSTACK_STAMP_DIRTY="$DIRTY" GSTACK_STAMP_SESSION_KIND="$SESSION_KIND" if [ "$INPUT" = --start ]; then - GSTACK_REVIEW_SKILL="${2:-}" bun --no-env-file --config=/dev/null -e ' + case "${3:-}" in ''|--fix-range) ;; *) echo 'Usage: gstack-review-log --start SKILL [--fix-range]' >&2; exit 2 ;; esac + GSTACK_REVIEW_SKILL="${2:-}" GSTACK_REVIEW_FIX_RANGE="${3:-}" bun --no-env-file --config=/dev/null -e ' const { captureReviewStart } = await import(process.env.GSTACK_REVIEW_LIB); - console.log(captureReviewStart(process.env.GSTACK_REVIEW_SKILL)); + try { console.log(captureReviewStart(process.env.GSTACK_REVIEW_SKILL, process.env, process.env.GSTACK_REVIEW_FIX_RANGE === "--fix-range")); } + catch (e) { console.error(e.message); process.exit(1); } ' exit $? fi @@ -86,7 +92,7 @@ if [ "$INPUT" = --check-shared-libs ] && [ "$#" -eq 2 ]; then exit $? fi usage() { - echo 'Usage: gstack-review-log JSON [--finish TOKEN] [--findings FILE] | --start SKILL | --check-shared-libs TOKEN < finding.json' >&2 + echo 'Usage: gstack-review-log JSON [--finish TOKEN] [--findings FILE] | --start SKILL [--fix-range] | --check-shared-libs TOKEN < finding.json' >&2 echo 'Exit codes: 0 written · 1 rejected · 2 usage' >&2 exit 2 } diff --git a/bin/gstack-session-update b/bin/gstack-session-update index 7166abc6d..2ae01297f 100755 --- a/bin/gstack-session-update +++ b/bin/gstack-session-update @@ -53,6 +53,7 @@ LOG_FILE="$STATE_DIR/analytics/session-update.log" PENDING_FILE="$STATE_DIR/session-update-pending" HOOK_CHECK="$(cd "$(dirname "$0")" && pwd)/gstack-hook-check" FF_UPDATE="$(cd "$(dirname "$0")" && pwd)/gstack-ff-update" +LEASE="$(cd "$(dirname "$0")" && pwd)/gstack-lease" RESCUE_FILE="$STATE_DIR/session-update-rescues" # Path spelling (Git Bash /c/x vs native C:/x) for the held-reason prefix strip. . "$(cd "$(dirname "$0")" && pwd)/gstack-path-spelling.sh" 2>/dev/null || { gstack_path_shell() { printf '%s\n' "$1"; }; gstack_path_native() { printf '%s\n' "$1"; }; } @@ -362,6 +363,16 @@ fi ) & HB_PID=$! + # ── Workflow leases (#3363): a live gstack session on this installation + # (bin/gstack-lease) means neither the fast-forward nor a pending setup + # retry runs under it, the rule bin/gstack-upgrade enforces as UPGRADE_BUSY. + # The throttle file is not written, so the next session start tries again. + if [ "${GSTACK_UPGRADE_FORCE:-}" != 1 ] && [ -x "$LEASE" ] \ + && BUSY=$("$LEASE" live --root "$GSTACK_DIR" --exclude-ancestry-of $$ 2>/dev/null); then + log_entry "SKIP leased $(printf '%s\n' "$BUSY" | awk 'NF { printf "%spid=%s/%s", n++ ? " " : "", $1, $2 }')" + exit 0 + fi + # ── Fetch, check the incoming release's Bun floor, then fast-forward (E1) ── OLD_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) UPDATE_URL=$(git -C "$GSTACK_DIR" remote get-url origin 2>/dev/null || echo "") diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index 6592d5746..680a645b9 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -604,7 +604,7 @@ Key routing rules: - Architecture → invoke /plan-eng-review - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan -- Bugs/errors → invoke /investigate +- Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) - QA/testing site behavior → invoke /qa or /qa-only - Code review/diff check → invoke /review - Visual polish → invoke /design-review @@ -639,10 +639,21 @@ fi # end interactive-onboarding guard (kind != spawned/unattended) # final gate is written and never approved, nothing is published or synced. # It also carries the host adaptation a harness without a Skill tool, slash # commands or AskUserQuestion needs, printed here so every host's render has it -# without growing any skill's prose. +# without growing any skill's prose. The record sentence is per skill: only +# skills with steps that write GSTACK_RESULT are told to end with it, and the +# rest name the record the parent reads instead (docs/unattended.md, #3281). if [ "$_SESSION_KIND" = "unattended" ]; then + _UA_NONE="This skill writes no run directory and prints no GSTACK_RESULT line, so do not invent either; end at its first pending item and list that item, with every recommended option you took, in your final report." + case "$SKILL_NAME" in + autoplan) _UA_RECORD="This skill writes a run directory through gstack-autoplan (plan.md, review-record.md, tasks.jsonl, decisions.jsonl, findings.jsonl, timing.json, run.json, gate.json): append each recommended choice to decisions.jsonl with kind \"auto\" and each pending one with kind \"approval\" and status \"pending\". The final approval gate is written as a gate list (gstack-gate render) and this run ENDS there with \`GSTACK_RESULT: skill=autoplan status=gate_pending run=\`." ;; + eval-plan) _UA_RECORD="This run ENDS at the Step 4 spend approval, left pending, with \`GSTACK_RESULT: skill=eval-plan status=gate_pending run=\`, where is the pilot's --out directory. No helper writes a gate.json for this skill yet, so the parent reads the pilot report and spend ledger in ." ;; + ship) _UA_RECORD="$_UA_NONE The parent reads the ship receipt in the PR body (\`gstack-ship-receipt read --pr --json\`)." ;; + review) _UA_RECORD="$_UA_NONE The parent reads the review-log rows (\`gstack-review-read\`)." ;; + qa) _UA_RECORD="$_UA_NONE The parent reads the \`qa-report-*.md\` file." ;; + *) _UA_RECORD="$_UA_NONE The parent reads this skill's own record (docs/unattended.md)." ;; + esac _emit_block unattended-session < status=gate_pending run=\`; you never approve it, never mark a plan APPROVED, never publish, push, sync or send data off this machine, and never record a consent. Skip every onboarding, upgrade, telemetry, routing and lake instruction. Host adaptation: there may be no Skill tool, no slash commands and no hooks here; read the skill file with your file-reading tool, run its bash blocks as written, treat each bash block's printed STATUS lines as the only source of session facts, and write results to files the parent names rather than relying on chat text being read. +You are running unattended: a parent agent started this run and reads its record, not your prose (docs/unattended.md names each skill's record). Rules: do NOT call AskUserQuestion and do NOT render prose decision briefs. At every question take the option marked (recommended). A question with no recommendation, any consent, any destructive or irreversible option, and the final approval gate stay pending: never choose them. You never approve a gate, never mark a plan APPROVED, never publish, push, sync or send data off this machine, and never record a consent. $_UA_RECORD Skip every onboarding, upgrade, telemetry, routing and lake instruction. Host adaptation: there may be no Skill tool, no slash commands and no hooks here; read the skill file with your file-reading tool, run its bash blocks as written, treat each bash block's printed STATUS lines as the only source of session facts, and write results to files the parent names rather than relying on chat text being read. EOI fi diff --git a/bin/gstack-specialist-stats b/bin/gstack-specialist-stats index 3162e3033..af0b3d15e 100755 --- a/bin/gstack-specialist-stats +++ b/bin/gstack-specialist-stats @@ -5,6 +5,8 @@ # Reads all *-reviews.jsonl files across branches, parses specialist fields, # and outputs hit rates. Tags specialists as GATE_CANDIDATE (0 findings in 10+ # dispatches) or NEVER_GATE (security, data-migration, state-machines — insurance policy). +# Fix-range passes (scope "fix-range", #3339) are skipped: a narrowed dispatch +# says nothing about a specialist's hit rate on a whole branch. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" @@ -40,7 +42,7 @@ let reviewed = 0; for (const line of lines) { try { const e = JSON.parse(line); - if (!e.specialists) continue; + if (!e.specialists || e.scope === 'fix-range') continue; reviewed++; for (const [name, info] of Object.entries(e.specialists)) { if (!stats[name]) stats[name] = { dispatched: 0, findings: 0 }; diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 8e8707cd9..f9ca4cef2 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -170,8 +170,10 @@ the readiness dashboard and the PR body, and never count it as a pass. findings. - **Every review gate.** These rules apply to every gate that summarises a review, including the `execution` gate behind `/codex` consult and challenge. - A reply whose last `Verdict:` line says `not ready` but carries no readable - severity prints `FINDINGS: unspecified` and exits 3 like a blocking finding. + A reply whose last `Verdict:` line says `not ready` is `findings` and exits + 3 even when readable advisory severities (P2 or P3) were parsed; `FINDINGS:` + names the highest one. A not-ready reply with no readable severity still + prints `FINDINGS: unspecified` and exits 3. - **No findings.** A review with no tag and no label is `clean` only when its conclusion says so: the last sentence (or its last ", and" clause), or the `because` clause of its `Recommendation:` line, opens with `NO_FINDINGS`, diff --git a/lib/cso/preparation.ts b/lib/cso/preparation.ts index 26a68bc73..35612e9ee 100644 --- a/lib/cso/preparation.ts +++ b/lib/cso/preparation.ts @@ -5,8 +5,8 @@ import { createHash } from 'node:crypto'; import { lstatSync, readFileSync, realpathSync } from 'node:fs'; import { isAbsolute, relative, resolve, sep } from 'node:path'; -/** 'java' (Maven or Gradle) is detected so it is reported as itself; no runtime prepares it. */ -export type CsoStack = 'node' | 'bun' | 'python' | 'rails' | 'java'; +/** 'java' (Maven or Gradle) and 'go' (go.mod) are detected so they are reported as themselves; no runtime prepares them. */ +export type CsoStack = 'node' | 'bun' | 'python' | 'rails' | 'java' | 'go'; export interface PreparationPrerequisite { code: string; message: string; @@ -1205,12 +1205,15 @@ export function inspectPreparation(snapshotPath: string, stack?: CsoStack): Prep detectedStacks.push('node'); if (read(root, 'Gemfile.lock', true) !== undefined || read(root, 'Gemfile', true) !== undefined) detectedStacks.push('rails'); - // A root Maven or Gradle build names the repository's language; Python - // metadata beside it is helper tooling, not a second application stack. + // A root Maven or Gradle build, or a root go.mod with no other supported + // stack, names the repository's language; Python metadata beside it is + // helper tooling, not a second application stack. const jvmBuild = ['pom.xml', 'build.gradle', 'build.gradle.kts'].find( (file) => read(root, file, true) !== undefined, ); if (jvmBuild) detectedStacks.push('java'); + else if (detectedStacks.length === 0 && read(root, 'go.mod', true) !== undefined) + detectedStacks.push('go'); else if ( read(root, 'uv.lock', true) !== undefined || read(root, 'requirements.txt', true) !== undefined || @@ -1231,10 +1234,16 @@ export function inspectPreparation(snapshotPath: string, stack?: CsoStack): Prep `Java (${jvmBuild ?? 'Maven or Gradle'}) is not a supported runtime stack yet; supported runtime stacks are Node, Bun, Python, and Rails.`, jvmBuild, ); + if (detected === 'go') + fail( + 'UNSUPPORTED_STACK', + 'Go (go.mod) is not a supported runtime stack yet; supported runtime stacks are Node, Bun, Python, and Rails.', + 'go.mod', + ); if (!['node', 'bun', 'python', 'rails'].includes(detected)) fail('UNSUPPORTED_STACK', 'Supported runtime stacks are Node, Bun, Python, and Rails.'); ({ node: inspectNode, bun: inspectBun, python: inspectPython, rails: inspectRails })[ - detected as Exclude + detected as Exclude ](root, plan); } catch (error) { plan.status = 'prerequisites'; diff --git a/lib/gbrain-guards.ts b/lib/gbrain-guards.ts index 1c46b4ad4..0df4576e7 100644 --- a/lib/gbrain-guards.ts +++ b/lib/gbrain-guards.ts @@ -17,9 +17,12 @@ * subcommand `--help` is generic — so capability detection is best-effort and * defaults to "unsupported". When we can't protect a user-managed source's * files, we FAIL CLOSED (refuse the remove) rather than delete unprotected. - * - The autopilot lock filename isn't documented and (gbrain #1226) ignores - * GBRAIN_HOME, so the live `gbrain autopilot` process is the PRIMARY signal; - * known lock paths under both the configured home and ~/.gbrain are secondary. + * - The live `gbrain autopilot` process is the PRIMARY signal and it is + * deliberately machine-wide (#3360, Garry 2026-10-24): any autopilot on the + * machine refuses, whichever brain or repo it serves, because two syncs that + * share one brain or working tree can race. The refusal names the matched + * pid and command so the user can tell whose it is. Known lock paths under + * both the configured home and ~/.gbrain are secondary. * - We refuse only on an AFFIRMATIVE autopilot signal — inability to introspect * never blocks a normal sync (that would brick the tool). * - Path containment uses realpath so a symlink inside ~/.gbrain/clones can't @@ -69,11 +72,17 @@ export interface AutopilotStatus { signal: string | null; } +/** A running process whose command line matches `gbrain autopilot`. */ +export interface AutopilotProcess { + pid: number; + command: string; +} + export interface AutopilotProbe { /** Override the lock-path list (tests). */ lockPaths?: string[]; - /** Override the live-process check (tests). */ - processRunning?: () => boolean; + /** Override the live-process check (tests): a bare boolean, or the matched processes. */ + processRunning?: () => boolean | AutopilotProcess[]; } /** @@ -109,10 +118,17 @@ export function detectAutopilot( // Stale lock (holder pid is dead): ignore this signal, keep checking. Pure // decision function — we do NOT delete the file here; the caller may clean it. } - // Primary signal: a live `gbrain autopilot` process. - const running = (probe.processRunning ?? defaultProcessRunning)(); - if (running) return { active: true, signal: "process:gbrain autopilot" }; - return { active: false, signal: null }; + // Primary signal: a live `gbrain autopilot` process anywhere on the machine. + const running = (probe.processRunning ?? autopilotProcesses)(); + if (running === true) return { active: true, signal: "process:gbrain autopilot" }; + if (!Array.isArray(running) || running.length === 0) return { active: false, signal: null }; + const listed = running.map(p => `pid ${p.pid} \`${p.command}\``).join(", "); + return { + active: true, + signal: `process:gbrain autopilot ${listed}; this check counts any process on this machine whose command line contains ` + + `"gbrain autopilot", even an autopilot serving another brain or repo. Stop that autopilot (or its service, if ` + + `\`gbrain autopilot --install\` set one up), or end the process that only mentions it`, + }; } /** Read the holder pid from a lock/pid file. Returns null if no integer pid is present. */ @@ -143,11 +159,22 @@ function isPidAlive(pid: number): boolean { } } -function defaultProcessRunning(): boolean { +function autopilotProcesses(): AutopilotProcess[] { // No reliable pgrep on Windows; rely on the lock-file signal there. - if (process.platform === "win32") return false; - const r = spawnSync("pgrep", ["-f", "gbrain autopilot"], { encoding: "utf-8", timeout: 3_000 }); - return r.status === 0 && (r.stdout || "").trim().length > 0; + if (process.platform === "win32") return []; + const found = spawnSync("pgrep", ["-f", "gbrain autopilot"], { encoding: "utf-8", timeout: 3_000 }); + const pids = found.status === 0 ? (found.stdout || "").split(/\s+/).filter(p => /^\d+$/.test(p)) : []; + if (pids.length === 0) return []; + const ps = spawnSync("ps", ["-o", "pid=,args=", "-p", pids.join(",")], { encoding: "utf-8", timeout: 3_000 }); + const commands = new Map(); + for (const line of (ps.stdout || "").split("\n")) { + const m = line.match(/^\s*(\d+)\s+(.*\S)/); + if (m) commands.set(m[1], m[2]); + } + return pids.map(pid => ({ + pid: Number(pid), + command: (commands.get(pid) ?? "command unavailable").replace(/\/\/[^\s/@]+@/g, "//***@").slice(0, 200), + })); } // ── Capability detection (E4 + Codex: per-process memo, no persistent cache) ─ diff --git a/lib/outside-review-result.ts b/lib/outside-review-result.ts index 8409c73b2..ad76233ab 100644 --- a/lib/outside-review-result.ts +++ b/lib/outside-review-result.ts @@ -263,7 +263,8 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC } if (!reviewed) return result(blocking ? 'findings' : 'clean'); const unparsed = firstUnparsed(code); - const notReady = !findings.highest && verdictNotReady(plainCode); + // A last explicit not-ready blocks even when the only parsed severities are advisory. + const notReady = verdictNotReady(plainCode); if (blocking || notReady) return { ...result('findings'), ...(unparsed ? { unparsed } : {}) }; if (unparsed) return { ...result('unverified', 'unparsed_severity', unparsed), unparsed }; if ((input.gate === 'structured' || input.gate === 'review') && !findings.highest && !concludesNoFindings(code)) { diff --git a/lib/review-evidence.ts b/lib/review-evidence.ts index 52586833d..418d08ab5 100644 --- a/lib/review-evidence.ts +++ b/lib/review-evidence.ts @@ -7,6 +7,7 @@ import { resultLine, type ResultCodeName } from './result-codes'; const DIFF_REVIEWS = new Set(['review', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']); const SHARED_LIBS_COVERAGE_VERSION = 1; +const HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; function record(value: unknown): value is Record { return value !== null && typeof value === 'object' && !Array.isArray(value); @@ -163,18 +164,54 @@ export function checkSharedLibsReuse(finding: unknown, token: string, env = proc return result; } -export function captureReviewStart(skill: string, env = process.env): string { +/** + * #3339: a fix-range repeat reviews only what changed since the latest pass of + * the same skill, and only when that pass completed and made fixes on this + * branch, no Step 11 review followed it, and the content it started from is + * exactly its commit's tree. That commit, held in the review log rather than + * the working tree, is the base; it must still be an ancestor of HEAD, so + * `git diff ` holds every fix. Anything else needs a full pass. + */ +function fixRangeBase(skill: string, env: NodeJS.ProcessEnv): string { + const refuse = (reason: string): never => { + throw new Error(`gstack-review-log: fix-range refused: ${reason}; run a full pass (gstack-review-log --start ${skill})`); + }; + let rows: Record[] = []; + try { + rows = readFileSync(env.GSTACK_REVIEW_LOG!, 'utf8').split('\n').filter(Boolean).flatMap(line => { + try { const row = JSON.parse(line); return record(row) ? [row] : []; } catch { return []; } + }); + } catch {} + const at = rows.findLastIndex(row => row.skill === skill); + if (at < 0) refuse('no earlier pass on this branch'); + if (rows.slice(at + 1).some(row => row.skill === 'adversarial-review' || row.skill === 'codex-review')) refuse('a Step 11 review followed the latest pass'); + const prior = rows[at]!, binding = prior.review_binding; + if (prior.completed !== true) refuse('the latest pass did not complete'); + if (prior.converged !== false) refuse('the latest pass made no fixes'); + if (!record(binding) || (binding.state !== 'changed' && binding.state !== 'incomplete') || typeof binding.start_wtree !== 'string') refuse('the latest pass has no captured start'); + if (binding.branch_id !== sha256(env.GSTACK_REVIEW_BRANCH ?? '')) refuse('the latest pass is from another branch'); + const base = prior.commit_full; + const git = (...args: string[]) => spawnSync('git', args, { cwd: env.GSTACK_REVIEW_REPO, encoding: 'utf8', timeout: 10_000 }); + if (typeof base !== 'string' || !HASH.test(base) || git('cat-file', '-e', `${base}^{commit}`).status !== 0) refuse('the latest pass has no resolvable commit'); + if (git('rev-parse', `${base}^{tree}`).stdout.trim() !== binding.start_wtree) refuse('the latest pass started from uncommitted content, so no commit marks what it read'); + if (git('merge-base', '--is-ancestor', base, 'HEAD').status !== 0) refuse(`${base} is not an ancestor of HEAD`); + return base; +} + +export function captureReviewStart(skill: string, env = process.env, fixRange = false): string { if (!DIFF_REVIEWS.has(skill) || !env.GSTACK_STAMP_WTREE || !env.GSTACK_REVIEW_REPO) { throw new Error('cannot capture a diff review without a working-tree fingerprint'); } + const fixBase = fixRange ? fixRangeBase(skill, env) : undefined; const dir = join(env.GSTACK_REVIEW_DIR!, '.review-starts'); mkdirSync(dir, { recursive: true, mode: 0o700 }); const token = crypto.randomUUID(); writeFileSync(join(dir, `${token}.json`), JSON.stringify({ skill, repo: env.GSTACK_REVIEW_REPO, branch: env.GSTACK_REVIEW_BRANCH, wtree: env.GSTACK_STAMP_WTREE, started_at: new Date().toISOString(), + ...(fixBase ? { scope: 'fix-range', fix_base: fixBase } : {}), }), { mode: 0o600, flag: 'wx' }); - return token; + return fixBase ? `${token}\nFIX_BASE=${fixBase}` : token; } const GATE_ACTIONS = new Set(['kept', 'restored', 'open']); @@ -226,7 +263,7 @@ export function deriveFromFindings(rec: Record, findingsPath: strin } export function bindReview(rec: Record, token: string, env = process.env, findingsPath?: string): Record { - for (const key of ['commit_full', 'tree', 'wtree', 'dirty', 'review_binding', 'review_freshness', 'shared_libs_coverage_version', 'status_source', 'findings_total', 'findings_open', 'findings_resolved', 'findings_file']) delete rec[key]; + for (const key of ['commit_full', 'tree', 'wtree', 'dirty', 'review_binding', 'review_freshness', 'shared_libs_coverage_version', 'status_source', 'findings_total', 'findings_open', 'findings_resolved', 'findings_file', 'scope', 'fix_base']) delete rec[key]; // The findings file is validated BEFORE the start token is consumed below: // a rejected row must leave the token for the retry. rec = findingsPath ? deriveFromFindings(rec, findingsPath) : { ...rec, status_source: 'claimed' }; @@ -256,11 +293,19 @@ export function bindReview(rec: Record, token: string, env = proces const state = !start || !end ? 'uncaptured' : start.wtree !== end ? 'changed' : rec.completed !== true || rec.converged !== true ? 'incomplete' : 'verified'; - rec.review_binding = { - state, start_wtree: start?.wtree, end_wtree: end, started_at: start?.started_at, - ...(typeof start?.branch === 'string' && start.branch.length > 0 ? { branch_id: sha256(start.branch) } : {}), - }; - if (state === 'verified') rec.wtree = end; + const branchId = typeof start?.branch === 'string' && start.branch.length > 0 ? { branch_id: sha256(start.branch) } : {}; + const narrowed = start?.scope === 'fix-range' || start?.fix_base !== undefined; + if (narrowed) { + rec.scope = 'fix-range'; + if (typeof start.fix_base === 'string' && HASH.test(start.fix_base)) rec.fix_base = start.fix_base; + } + // A narrowed pass on unchanged content never carries start_wtree/end_wtree, + // so Step 11.5 cannot bind it and it never becomes CURRENT; only a later full pass can. + rec.review_binding = narrowed && (state === 'incomplete' || state === 'verified') + ? { state: 'fix-range', fix_base: rec.fix_base, range_start_wtree: start.wtree, range_end_wtree: end, started_at: start.started_at, ...branchId } + : { state, start_wtree: start?.wtree, end_wtree: end, started_at: start?.started_at, ...branchId }; + const verified = rec.review_binding.state === 'verified'; + if (verified) rec.wtree = end; if (rec.skill === 'review' && Array.isArray(rec.findings)) { rec.shared_libs_coverage_version = SHARED_LIBS_COVERAGE_VERSION; for (const finding of rec.findings) { @@ -270,7 +315,7 @@ export function bindReview(rec: Record, token: string, env = proces const fingerprint = sharedLibsFingerprint(finding); if (!fingerprint) continue; finding.fingerprint = fingerprint; - finding.snapshot_covered_paths = state === 'verified' && finding.action === 'skipped' + finding.snapshot_covered_paths = verified && finding.action === 'skipped' ? sharedLibsSnapshotCoverage(env.GSTACK_REVIEW_REPO!, end!, finding.evidence_paths, env) : []; } } @@ -282,6 +327,7 @@ export function reviewFreshness(rec: Record, currentWtree: string): if (rec.skill === 'ship') return { status: 'UNVERIFIED', reason: 'ship telemetry is not a review pass' }; const binding = rec.review_binding; if (binding?.state === 'changed') return { status: 'STALE', reason: 'content changed during review' }; + if (rec.scope === 'fix-range') return { status: 'UNVERIFIED', reason: 'fix-range pass; only a later full pass can clear review' }; if (binding?.state !== 'verified' || rec.completed !== true || rec.converged !== true || !rec.wtree || binding.start_wtree !== rec.wtree || binding.end_wtree !== rec.wtree) { return { status: 'UNVERIFIED', reason: 'missing start capture or incomplete/nonconverged pass' }; diff --git a/package.json b/package.json index 4333f42c5..d41cbae09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.197", + "version": "1.91.205", "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/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index 0f43c741c..399ee8401 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -125,7 +125,8 @@ building proposed code. Keep suppressed findings for the output appendix. ## Confidence Calibration -Every finding MUST include a confidence score (1-10): +Every finding MUST include a severity, CRITICAL (blocks shipping) or INFORMATIONAL +(lower severity, still actioned), and a confidence score (1-10): | Score | Meaning | Display rule | |-------|---------|-------------| @@ -133,20 +134,18 @@ Every finding MUST include a confidence score (1-10): | 7-8 | High confidence pattern match. Very likely correct. | Show normally | | 5-6 | Moderate. Could be a false positive. | Show with caveat: "Medium confidence, verify this is actually an issue" | | 3-4 | Low confidence. Pattern is suspicious but may be fine. | Suppress from main report. Include in appendix only. | -| 1-2 | Speculation. | Only report if severity would be P0. | +| 1-2 | Speculation. | Only report a suspected release-blocking catastrophe (widespread data loss, total outage or system-wide compromise), at CRITICAL severity and marked explicitly speculative. | **Finding format:** -`[SEVERITY] (confidence: N/10) file:line — description` +`[CRITICAL|INFORMATIONAL] (confidence: N/10) file:line — description` Example: -`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause` -`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs` +`[CRITICAL] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause` +`[INFORMATIONAL] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs` ### Pre-emit verification gate -Before any finding is promoted to the report, the gate requires: - 1. **Quote the specific code line that motivates the finding** — file:line plus the verbatim text of the line(s) that triggered it. If the finding is "field X doesn't exist on model Y", quote the lines of class Y where the field @@ -156,9 +155,7 @@ Before any finding is promoted to the report, the gate requires: 2. **If you cannot quote the motivating line(s), the finding is unverified.** Force its confidence to 4-5. Use 4 when it should be suppressed from the main report; use 5 only when it belongs in the report with the medium-confidence - caveat. Keep suppressed items in the appendix so reviewers can audit - calibration. Do not work around this by inventing - speculative confidence 7+ — that defeats the gate. + caveat. Never invent speculative confidence 7+. **Framework-meta nudge:** When the symbol is generated by a framework metaclass, descriptor, ORM Meta inner-class, or migration history (Django @@ -178,10 +175,8 @@ False-positive classes the gate catches: | "save() might lose fields" | Requires quoting the ORM signature or model definition | | "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident | -**Calibration learning:** If you report a finding with confidence < 7 and the user -confirms it IS a real issue, that is a calibration event. Your initial confidence was -too low. Log the corrected pattern as a learning so future reviews catch it with -higher confidence. +**Calibration learning:** If the user confirms a reported finding scored < 7 is +real, log the corrected pattern as a learning. ## Decision procedure diff --git a/scripts/harness-version.json b/scripts/harness-version.json index d95e24d5a..bcf395dd3 100644 --- a/scripts/harness-version.json +++ b/scripts/harness-version.json @@ -34,7 +34,7 @@ "test/helpers/pty-screen.ts": {"blob":"4f77569f9ce11f9cee9f347216e468cd7c956820","since":6}, "test/helpers/pty/auq.ts": {"blob":"16e5ea2bb5b9f28f44808a1a09024dbd3e6f28c4","since":6}, "test/helpers/pty/binary.ts": {"blob":"404d7515076d5c00804f89e23ea69296312c00e1","since":6}, - "test/helpers/pty/boundaries.ts": {"blob":"8c87100c9dd0cfc28c02598f250726fd93bfbdea","since":6,"nonBehavioral":"Removes the plan-design 0D focus picker from the shared harness; floor.ts gains an optional pickSetupChoice hook that no other caller passes, so no other case's input or verdict can change. plan-design-finding-floor, the only case whose seed raises that menu, passes the picker from its owned fixture test/fixtures/design-floor-focus-picker.ts, so its owned bytes start its own new series."}, + "test/helpers/pty/boundaries.ts": {"blob":"c5520118da36f96f60a3c9720751bb57a669865c","since":6,"nonBehavioral":"Widens the plan-eng-review first-review severity annotation from [P0-3] to [P0-3|CRITICAL|INFORMATIONAL] (#3349). Only a question carrying the new labels can change verdict, and only the changed plan-eng-review prompt asks for them; every case that runs it owns that generated SKILL.md, which this change also edits, so those cases already start their own series. No other case's input or verdict can change."}, "test/helpers/pty/classify.ts": {"blob":"3f2daa12ff6484f96b8d28d910a50231c4bb3ae8","since":6}, "test/helpers/pty/fake-session.ts": {"blob":"554e064ef259b8444280eddfe9b0db93c66a8202","since":6}, "test/helpers/pty/judge.ts": {"blob":"81c5bb4c48ab928aca2f186a1ade9df7169d7fbe","since":6}, diff --git a/scripts/resolvers/confidence.ts b/scripts/resolvers/confidence.ts index f10d15a79..747bdc225 100644 --- a/scripts/resolvers/confidence.ts +++ b/scripts/resolvers/confidence.ts @@ -53,7 +53,8 @@ real, log the corrected pattern as a learning.`; const result = `## Confidence Calibration -Every finding MUST include a confidence score (1-10): +Every finding MUST include a severity, CRITICAL (blocks shipping) or INFORMATIONAL +(lower severity, still actioned), and a confidence score (1-10): | Score | Meaning | Display rule | |-------|---------|-------------| @@ -61,20 +62,18 @@ Every finding MUST include a confidence score (1-10): | 7-8 | High confidence pattern match. Very likely correct. | Show normally | | 5-6 | Moderate. Could be a false positive. | Show with caveat: "Medium confidence, verify this is actually an issue" | | 3-4 | Low confidence. Pattern is suspicious but may be fine. | Suppress from main report. Include in appendix only. | -| 1-2 | Speculation. | Only report if severity would be P0. | +| 1-2 | Speculation. | Only report a suspected release-blocking catastrophe (widespread data loss, total outage or system-wide compromise), at CRITICAL severity and marked explicitly speculative. | **Finding format:** -\\\`[SEVERITY] (confidence: N/10) file:line — description\\\` +\\\`[CRITICAL|INFORMATIONAL] (confidence: N/10) file:line — description\\\` Example: -\\\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\\\` -\\\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\\\` +\\\`[CRITICAL] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\\\` +\\\`[INFORMATIONAL] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\\\` ### Pre-emit verification gate -Before any finding is promoted to the report, the gate requires: - 1. **Quote the specific code line that motivates the finding** — file:line plus the verbatim text of the line(s) that triggered it. If the finding is "field X doesn't exist on model Y", quote the lines of class Y where the field @@ -84,9 +83,7 @@ Before any finding is promoted to the report, the gate requires: 2. **If you cannot quote the motivating line(s), the finding is unverified.** Force its confidence to 4-5. Use 4 when it should be suppressed from the main report; use 5 only when it belongs in the report with the medium-confidence - caveat. Keep suppressed items in the appendix so reviewers can audit - calibration. Do not work around this by inventing - speculative confidence 7+ — that defeats the gate. + caveat. Never invent speculative confidence 7+. **Framework-meta nudge:** When the symbol is generated by a framework metaclass, descriptor, ORM Meta inner-class, or migration history (Django @@ -106,9 +103,7 @@ False-positive classes the gate catches: | "save() might lose fields" | Requires quoting the ORM signature or model definition | | "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident | -**Calibration learning:** If you report a finding with confidence < 7 and the user -confirms it IS a real issue, that is a calibration event. Your initial confidence was -too low. Log the corrected pattern as a learning so future reviews catch it with -higher confidence.`; +**Calibration learning:** If the user confirms a reported finding scored < 7 is +real, log the corrected pattern as a learning.`; return _ctx.skillName === 'plan-eng-review' ? result.replaceAll('\\`', '`') : result; } diff --git a/scripts/resolvers/preamble/generate-test-failure-triage.ts b/scripts/resolvers/preamble/generate-test-failure-triage.ts index 9b1b55f19..d54f975e5 100644 --- a/scripts/resolvers/preamble/generate-test-failure-triage.ts +++ b/scripts/resolvers/preamble/generate-test-failure-triage.ts @@ -1,6 +1,13 @@ import { FREE_TEXT_WRITE_RULE, freeTextFileBash, freeTextFileUse } from '../free-text-file'; +export function generateFailureRouting(): string { + return `**Failures: check the cause, then route.** A failing test, error or stack trace shows where a failure surfaces, not its root cause. + +- **Cause supplied** (the user names a specific cause and a narrow fix): make one read-only check of the evidence it rests on, such as the named file, test or log. If that confirms the cause, make the fix and run the focused test that covers it. If it does not, invoke \`/investigate\` before any further tracing or edit. +- **Cause unknown:** make one read-only triage pass over the supplied evidence and the directly implicated test and code. Do not reproduce, test hypotheses or search wider. If that pass establishes the root cause and a narrow fix, make the fix and run the focused test that covers it. If the cause, scope or intended behavior is still unclear, invoke \`/investigate\` before any broader diagnosis or edit.`; +} + export function generateTestFailureTriage(): string { return `## Test Failure Ownership Triage @@ -48,7 +55,7 @@ Ask with AskUserQuestion in the AskUserQuestion Format. List each failure the sa ### Step T4: Execute the chosen action **If "Investigate and fix now":** -- Switch to /investigate mindset: root cause first, then minimal fix. +- One read-only root-cause pass, then \`/investigate\` if unclear. - Fix the pre-existing failure. - Commit the fix separately from the branch's changes: \`git commit -m "fix: pre-existing test failure in "\` - Continue with the workflow. diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 348458c08..53c6f44c8 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -3,8 +3,8 @@ ## Step 9: Pre-Landing Review Set CYCLES to 0 on first entry only. Keep existing approvals; changed finding scope -needs a new decision. Run checklist/design, specialists (9.1), merge/Red Team (9.2), -exploratory QA (9.2.1), dedup (9.3), then fixes and logging (9.4). +needs a new decision. Order: checklist/design, specialists (9.1), Red Team (9.2), +QA (9.2.1), dedup (9.3), fixes and logging (9.4). Gated/unsupported specialists skip only their dispatch, never QA or Step 11. Steps 10–11 queue findings without editing; include those findings in this pass. Every repeat starts before the checklist read and captures a fresh REVIEW_START. @@ -12,7 +12,8 @@ Finish the complete review and QA before applying any fix in Step 9.4. ## Confidence Calibration -Every finding MUST include a confidence score (1-10): +Every finding MUST include a severity, CRITICAL (blocks shipping) or INFORMATIONAL +(lower severity, still actioned), and a confidence score (1-10): | Score | Meaning | Display rule | |-------|---------|-------------| @@ -20,20 +21,18 @@ Every finding MUST include a confidence score (1-10): | 7-8 | High confidence pattern match. Very likely correct. | Show normally | | 5-6 | Moderate. Could be a false positive. | Show with caveat: "Medium confidence, verify this is actually an issue" | | 3-4 | Low confidence. Pattern is suspicious but may be fine. | Suppress from main report. Include in appendix only. | -| 1-2 | Speculation. | Only report if severity would be P0. | +| 1-2 | Speculation. | Only report a suspected release-blocking catastrophe (widespread data loss, total outage or system-wide compromise), at CRITICAL severity and marked explicitly speculative. | **Finding format:** -\`[SEVERITY] (confidence: N/10) file:line — description\` +\`[CRITICAL|INFORMATIONAL] (confidence: N/10) file:line — description\` Example: -\`[P1] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\` -\`[P2] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\` +\`[CRITICAL] (confidence: 9/10) app/models/user.rb:42 — SQL injection via string interpolation in where clause\` +\`[INFORMATIONAL] (confidence: 5/10) app/controllers/api/v1/users_controller.rb:18 — Possible N+1 query, verify with production logs\` ### Pre-emit verification gate -Before any finding is promoted to the report, the gate requires: - 1. **Quote the specific code line that motivates the finding** — file:line plus the verbatim text of the line(s) that triggered it. If the finding is "field X doesn't exist on model Y", quote the lines of class Y where the field @@ -43,9 +42,7 @@ Before any finding is promoted to the report, the gate requires: 2. **If you cannot quote the motivating line(s), the finding is unverified.** Force its confidence to 4-5. Use 4 when it should be suppressed from the main report; use 5 only when it belongs in the report with the medium-confidence - caveat. Keep suppressed items in the appendix so reviewers can audit - calibration. Do not work around this by inventing - speculative confidence 7+ — that defeats the gate. + caveat. Never invent speculative confidence 7+. **Framework-meta nudge:** When the symbol is generated by a framework metaclass, descriptor, ORM Meta inner-class, or migration history (Django @@ -65,10 +62,8 @@ False-positive classes the gate catches: | "save() might lose fields" | Requires quoting the ORM signature or model definition | | "update_fields might miss X" | Requires quoting the field set; if X doesn't exist, the FP is self-evident | -**Calibration learning:** If you report a finding with confidence < 7 and the user -confirms it IS a real issue, that is a calibration event. Your initial confidence was -too low. Log the corrected pattern as a learning so future reviews catch it with -higher confidence. +**Calibration learning:** If the user confirms a reported finding scored < 7 is +real, log the corrected pattern as a learning. ### Core checklist @@ -78,6 +73,10 @@ This pass is static; defer product probes to Step 9.2.1. 2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START. Then run `git diff origin/`. Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. + **Fix-range repeat** (decision 3): start with `--start review --fix-range`. It prints + REVIEW_START, then `FIX_BASE=`, which stands in for `origin/` and `` in this pass. + If it refuses, start a full pass. + 2.5. **Gate scan.** Before applying the checklist, run the gate scan on the same candidate: List every gate edit in the candidate (test, CI, runner/lint-config, snapshot and @@ -121,9 +120,9 @@ fi ### Design-lite checklist -Its numbering is local to this checklist. When frontend review applies, `/ship` -automatically attempts this optional design check; `enabled` expresses that choice, -not a new user question. Step 11 has its own outside-review switch and required native pass. +Its numbering is local. `/ship` attempts this design check automatically when frontend +review applies; `enabled` is that choice, not a user question. Step 11 has its own +outside-review switch and required native pass. ## Design Review (conditional, diff-scoped) @@ -257,10 +256,9 @@ Use the original DESIGN_START token. COMPLETED is true only when the native chec Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count, D = counted detector findings from step 0 (0 when the detector did not run), COMMIT = output of `git rev-parse --short HEAD`. -The parent owns design-lite; the Design specialist is an independent read. -Before final counting/Fix-First, merge the same evidenced design defect at the same path/line -into one item with both sources and stricter ASK. Retain actual specialist stats; -distinct defects stay separate and neither pass substitutes for the other. +Design-lite (parent) and the Design specialist are independent reads. Before counting +and Fix-First, merge one evidenced design defect at the same path/line into one item +with both sources and the stricter ASK. Keep actual specialist stats and distinct defects. ## Step 9.1: Review Army — Specialist Dispatch @@ -623,11 +621,9 @@ but missing dispatched output still blocks continuation, even with a QA exceptio 2. **Auto-fix all AUTO-FIX items.** Apply each fix. Output one line per fix: `[AUTO-FIXED] [file:line] Problem → what you did` -3. **If ASK items remain,** present them in ONE AskUserQuestion: - - List each with number, severity, problem, recommended fix - - Per-item options: A) Fix B) Skip - - Overall RECOMMENDATION - - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead +3. **If ASK items remain,** present them in ONE AskUserQuestion (or one per item for 3 or + fewer): each item's number, severity, problem and recommended fix with A) Fix B) Skip, + plus an overall RECOMMENDATION. Save each explicit Skip immediately in the invocation action list with its identity, scope and supporting source evidence; keep it across repeats. @@ -674,10 +670,10 @@ but missing dispatched output still blocks continuation, even with a QA exceptio ```bash ~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"review","timestamp":"TIMESTAMP","status":"STATUS","issues_found":N,"critical":N,"informational":N,"quality_score":SCORE,"specialists":SPECIALISTS_JSON,"findings":FINDINGS_JSON,"commit":"'"$(git rev-parse --short HEAD)"'","via":"ship","completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES}' --finish REVIEW_START ``` -- `TIMESTAMP`: ISO 8601. `STATUS`: `unavailable` for missing dispatched reviewer output; - otherwise `clean` only for completed coverage with no - unresolved non-advisory defects; otherwise `issues_found`. N counts current - unresolved defects, not original totals. Missing coverage is not a defect. +- `TIMESTAMP`: ISO 8601. `STATUS`: `unavailable` for missing dispatched reviewer output, + else `clean` only for completed coverage with no unresolved non-advisory defects, + else `issues_found`. N counts unresolved defects now, not original totals; missing + coverage is not a defect. - `REVIEW_START`: this pass's Step 9 token captured before reading the diff; never recapture at persistence to certify unreviewed fixes. - `COMPLETED`: checklist and dispatched specialists/Red Team finish, and all required probes pass. @@ -699,13 +695,13 @@ but missing dispatched output still blocks continuation, even with a QA exceptio `open` counts in `issues_found` and by severity. The logger stamps `actor` from `gstack-session-kind` (a supplied `actor` is discarded) and forces `"open"` on gate findings in a non-interactive session. -Save the review output and the `Gate edits:` line — they go into the PR body in Step 19. -Any `open` gate finding also decides Step 19's draft state. +Save the review output and `Gate edits:` line for Step 19's PR body; an `open` gate +finding makes that PR a draft. ### Decide whether to repeat Step 9 -After persistence, record missing dispatched output, CYCLES and applied fixes in -the invocation record. Apply these decisions in order: +After persistence, record missing output, CYCLES and fixes in the invocation record. +Apply these decisions in order: 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, @@ -715,10 +711,10 @@ the invocation record. Apply these decisions in order: `converged:false`; do not run a fourth fixing cycle. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of Step 9 before the pending Step 10 in the work list. Tests must pass or retain approval for the same verified pre-existing - failures and scope. Keep CYCLES and scoped approvals across this repeat. + failures and scope. Keep CYCLES and scoped approvals across this repeat; it may be a fix-range repeat. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it - clears may you continue to Step 10. Undispatched gated/unsupported specialists - do not block independently, but never replace QA or required native review. + clears may you continue to Step 10, and only from a full pass: after a fix-range + pass, run one full Step 9 first. **Required-probe parent gate:** With completed checklist and dispatched reviewers, failed/unavailable required probes block continuation. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 9d6413919..f6d3a7b2a 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -1,8 +1,8 @@ ## Step 9: Pre-Landing Review Set CYCLES to 0 on first entry only. Keep existing approvals; changed finding scope -needs a new decision. Run checklist/design, specialists (9.1), merge/Red Team (9.2), -exploratory QA (9.2.1), dedup (9.3), then fixes and logging (9.4). +needs a new decision. Order: checklist/design, specialists (9.1), Red Team (9.2), +QA (9.2.1), dedup (9.3), fixes and logging (9.4). Gated/unsupported specialists skip only their dispatch, never QA or Step 11. Steps 10–11 queue findings without editing; include those findings in this pass. Every repeat starts before the checklist read and captures a fresh REVIEW_START. @@ -18,6 +18,10 @@ This pass is static; defer product probes to Step 9.2.1. 2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START. Then run `git diff origin/`. Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. + **Fix-range repeat** (decision 3): start with `--start review --fix-range`. It prints + REVIEW_START, then `FIX_BASE=`, which stands in for `origin/` and `` in this pass. + If it refuses, start a full pass. + 2.5. **Gate scan.** Before applying the checklist, run the gate scan on the same candidate: {{GATE_SCAN_BLOCK}} @@ -33,16 +37,15 @@ This pass is static; defer product probes to Step 9.2.1. ### Design-lite checklist -Its numbering is local to this checklist. When frontend review applies, `/ship` -automatically attempts this optional design check; `enabled` expresses that choice, -not a new user question. Step 11 has its own outside-review switch and required native pass. +Its numbering is local. `/ship` attempts this design check automatically when frontend +review applies; `enabled` is that choice, not a user question. Step 11 has its own +outside-review switch and required native pass. {{DESIGN_REVIEW_LITE}} -The parent owns design-lite; the Design specialist is an independent read. -Before final counting/Fix-First, merge the same evidenced design defect at the same path/line -into one item with both sources and stricter ASK. Retain actual specialist stats; -distinct defects stay separate and neither pass substitutes for the other. +Design-lite (parent) and the Design specialist are independent reads. Before counting +and Fix-First, merge one evidenced design defect at the same path/line into one item +with both sources and the stricter ASK. Keep actual specialist stats and distinct defects. {{REVIEW_ARMY}} @@ -63,11 +66,9 @@ but missing dispatched output still blocks continuation, even with a QA exceptio 2. **Auto-fix all AUTO-FIX items.** Apply each fix. Output one line per fix: `[AUTO-FIXED] [file:line] Problem → what you did` -3. **If ASK items remain,** present them in ONE AskUserQuestion: - - List each with number, severity, problem, recommended fix - - Per-item options: A) Fix B) Skip - - Overall RECOMMENDATION - - If 3 or fewer ASK items, you may use individual AskUserQuestion calls instead +3. **If ASK items remain,** present them in ONE AskUserQuestion (or one per item for 3 or + fewer): each item's number, severity, problem and recommended fix with A) Fix B) Skip, + plus an overall RECOMMENDATION. Save each explicit Skip immediately in the invocation action list with its identity, scope and supporting source evidence; keep it across repeats. @@ -114,10 +115,10 @@ but missing dispatched output still blocks continuation, even with a QA exceptio ```bash ~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"review","timestamp":"TIMESTAMP","status":"STATUS","issues_found":N,"critical":N,"informational":N,"quality_score":SCORE,"specialists":SPECIALISTS_JSON,"findings":FINDINGS_JSON,"commit":"'"$(git rev-parse --short HEAD)"'","via":"ship","completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES}' --finish REVIEW_START ``` -- `TIMESTAMP`: ISO 8601. `STATUS`: `unavailable` for missing dispatched reviewer output; - otherwise `clean` only for completed coverage with no - unresolved non-advisory defects; otherwise `issues_found`. N counts current - unresolved defects, not original totals. Missing coverage is not a defect. +- `TIMESTAMP`: ISO 8601. `STATUS`: `unavailable` for missing dispatched reviewer output, + else `clean` only for completed coverage with no unresolved non-advisory defects, + else `issues_found`. N counts unresolved defects now, not original totals; missing + coverage is not a defect. - `REVIEW_START`: this pass's Step 9 token captured before reading the diff; never recapture at persistence to certify unreviewed fixes. - `COMPLETED`: checklist and dispatched specialists/Red Team finish, and all required probes pass. @@ -139,13 +140,13 @@ but missing dispatched output still blocks continuation, even with a QA exceptio `open` counts in `issues_found` and by severity. The logger stamps `actor` from `gstack-session-kind` (a supplied `actor` is discarded) and forces `"open"` on gate findings in a non-interactive session. -Save the review output and the `Gate edits:` line — they go into the PR body in Step 19. -Any `open` gate finding also decides Step 19's draft state. +Save the review output and `Gate edits:` line for Step 19's PR body; an `open` gate +finding makes that PR a draft. ### Decide whether to repeat Step 9 -After persistence, record missing dispatched output, CYCLES and applied fixes in -the invocation record. Apply these decisions in order: +After persistence, record missing output, CYCLES and fixes in the invocation record. +Apply these decisions in order: 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, @@ -155,10 +156,10 @@ the invocation record. Apply these decisions in order: `converged:false`; do not run a fourth fixing cycle. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of Step 9 before the pending Step 10 in the work list. Tests must pass or retain approval for the same verified pre-existing - failures and scope. Keep CYCLES and scoped approvals across this repeat. + failures and scope. Keep CYCLES and scoped approvals across this repeat; it may be a fix-range repeat. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it - clears may you continue to Step 10. Undispatched gated/unsupported specialists - do not block independently, but never replace QA or required native review. + clears may you continue to Step 10, and only from a full pass: after a fix-range + pass, run one full Step 9 first. **Required-probe parent gate:** With completed checklist and dispatched reviewers, failed/unavailable required probes block continuation. diff --git a/ship/sections/tests.md b/ship/sections/tests.md index 280834f6d..d083d3f19 100644 --- a/ship/sections/tests.md +++ b/ship/sections/tests.md @@ -319,7 +319,7 @@ Ask with AskUserQuestion in the AskUserQuestion Format. List each failure the sa ### Step T4: Execute the chosen action **If "Investigate and fix now":** -- Switch to /investigate mindset: root cause first, then minimal fix. +- One read-only root-cause pass, then `/investigate` if unclear. - Fix the pre-existing failure. - Commit the fix separately from the branch's changes: `git commit -m "fix: pre-existing test failure in "` - Continue with the workflow. diff --git a/test/codex-host.test.ts b/test/codex-host.test.ts index 0976a72e6..d6dda8b79 100644 --- a/test/codex-host.test.ts +++ b/test/codex-host.test.ts @@ -128,13 +128,13 @@ describe('per-case host metadata', () => { for (const id of Object.keys(E2E_HOSTS)) expect(Object.hasOwn(E2E_TOUCHFILES, id), id).toBe(true); }); - test('classifies 52 adapter-only, 29 Claude-specific, 26 rework and 1 host-neutral case', () => { + test('classifies 52 adapter-only, 33 Claude-specific, 26 rework and 1 host-neutral case', () => { const counts: Record = {}; for (const support of Object.values(E2E_HOSTS)) counts[support] = (counts[support] ?? 0) + 1; expect(counts.both).toBe(52); expect(counts.rework).toBe(26); expect(counts['host-neutral']).toBe(1); - expect(['pty', 'sdk', 'subagents', 'skill-tool', 'hermetic', 'self-host'].reduce((sum, reason) => sum + (counts[reason] ?? 0), 0)).toBe(29); + expect(['pty', 'sdk', 'subagents', 'skill-tool', 'hermetic', 'self-host'].reduce((sum, reason) => sum + (counts[reason] ?? 0), 0)).toBe(33); }); }); diff --git a/test/cso-preparation.test.ts b/test/cso-preparation.test.ts index 6addab47f..174ebd8b3 100644 --- a/test/cso-preparation.test.ts +++ b/test/cso-preparation.test.ts @@ -170,6 +170,32 @@ describe('CSO inert Python preparation', () => { }); }); +describe('CSO preparation names a Go module as Go (#3263)', () => { + const goFiles = { + 'go.mod': 'module example.com/app\n\ngo 1.23\n\nrequire github.com/google/uuid v1.6.0\n', + 'go.sum': 'github.com/google/uuid v1.6.0 h1:abc=\n', + 'main.go': 'package main\n\nfunc main() {}\n', + }; + const GO_UNSUPPORTED = { code: 'UNSUPPORTED_STACK', message: 'Go (go.mod) is not a supported runtime stack yet; supported runtime stacks are Node, Bun, Python, and Rails.', path: 'go.mod' }; + test('a go.mod repository is Go, unsupported, and never asks for requirements.txt', () => { + const plan = inspectPreparation(fixture(goFiles)); + expect([plan.stack, plan.runtimeProfile, plan.status]).toEqual(['go', 'go', 'prerequisites']); + expect(plan.prerequisites).toEqual([GO_UNSUPPORTED]); + expect(JSON.stringify(plan)).not.toContain('requirements.txt'); + }); + test('Python helper metadata beside go.mod stays Go', () => { + const plan = inspectPreparation(fixture({ ...goFiles, 'requirements.txt': `mkdocs==1.6.1 --hash=sha256:${hash}\n` })); + expect(plan.stack).toBe('go'); + expect(plan.prerequisites).toEqual([GO_UNSUPPORTED]); + }); + test('controls: go.mod beside a Node lockfile stays Node; Python without go.mod stays Python', () => { + const node = inspectPreparation(fixture({ ...goFiles, ...nodeFiles() })); + expect([node.stack, node.status]).toEqual(['node', 'ready']); + expect(inspectPreparation(fixture({ 'requirements.txt': `Flask==3.1.0 --hash=sha256:${hash}\n` })).stack).toBe('python'); + expect(inspectPreparation(fixture({ 'go.sum': '', 'main.go': '' })).stack).toBe('python'); + }); +}); + describe('CSO inert Rails preparation', () => { test('does not silently choose Node in a multi-stack repository',()=>{const plan=inspectPreparation(fixture({...nodeFiles(),'Gemfile.lock':gemLock,'Gemfile':''}));expect(plan.status).toBe('prerequisites');expect(plan.prerequisites[0].code).toBe('MULTIPLE_STACKS');expect(inspectPreparation(fixture({...nodeFiles(),'Gemfile.lock':gemLock,'Gemfile':''}),'rails').stack).toBe('rails');}); test('fetches exact gems without evaluating Gemfiles or building native extensions', () => { diff --git a/test/eng-finding-retry-budget.test.ts b/test/eng-finding-retry-budget.test.ts index 9d35a6f0a..0320c653d 100644 --- a/test/eng-finding-retry-budget.test.ts +++ b/test/eng-finding-retry-budget.test.ts @@ -141,7 +141,7 @@ test('live periodic census fits the declared CI wall including setup', () => { test('registered allocation is deterministic and preserves every discovered file', () => { const files = collectPaidTestFiles(); - expect(files).toHaveLength(109); // + skill-e2e-dx-error-contract (#3226); + skill-e2e-gate-integrity (HONEST_WORK_GATE_INTEGRITY W8); + skill-e2e-autoplan-schema-canary, -guard-pty and -long-session (Oct 7 wave, CEO-13, ENG-18, CEO-15); + skill-e2e-ship-measure-seeded-flake (v1.91.32.0); W5a: 15 carve wrappers became one case-sharded file; + codex-e2e-multiblock-live and skill-e2e-design-model-smoke (v1.91.30.0); + expect(files).toHaveLength(110); // + skill-e2e-failure-routing (#3157/#3158); + skill-e2e-dx-error-contract (#3226); + skill-e2e-gate-integrity (HONEST_WORK_GATE_INTEGRITY W8); + skill-e2e-autoplan-schema-canary, -guard-pty and -long-session (Oct 7 wave, CEO-13, ENG-18, CEO-15); + skill-e2e-ship-measure-seeded-flake (v1.91.32.0); W5a: 15 carve wrappers became one case-sharded file; + codex-e2e-multiblock-live and skill-e2e-design-model-smoke (v1.91.30.0); expect(files).toContain('test/skill-e2e-ship-skip.test.ts'); const m = livePlan(files); expect(livePlan([...files].reverse())).toEqual(m); diff --git a/test/eng-first-review.test.ts b/test/eng-first-review.test.ts index 26d19c8bc..26e74de94 100644 --- a/test/eng-first-review.test.ts +++ b/test/eng-first-review.test.ts @@ -1467,3 +1467,19 @@ describe('Y whole-plan complexity setup decision', () => { }); }); }); + +describe('plan finding severity labels (#3349)', () => { +const relabel = (captured: { call: unknown }, label: string) => { + const c = structuredClone(captured.call) as NativePlanQuestionCall, q = c.questions[0]!, picked = c.answers![q.question]!; + q.question = label ? q.question.replace('[P1] (confidence', `${label} (confidence`) : q.question.replace('[P1] ', ''); + c.answers = { [q.question]: picked }; + return engFirstReviewAUQ(nativePlanCallFingerprint(c, Date.parse(c.answeredAt!), true)); +}; +const captures = [['eng-annotated-cache-au', captured_eng_annotated_cache_au], ['eng-declarative-as', captured_eng_declarative_as]] as const; +test.each(captures)('%s opens review under either severity scale', (_, captured) => { + for (const label of ['[P1]', '[CRITICAL]', '[INFORMATIONAL]']) expect(relabel(captured, label)).toBe(true); +}); +test.each(captures)('%s rejects labels outside both scales', (_, captured) => { + for (const label of ['[HIGH]', '[critical]', '[Critical]', '[P4]', '[CRITICAL|INFORMATIONAL]', '']) expect(relabel(captured, label)).toBe(false); +}); +}); diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index cd196ee0d..948752968 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -8,7 +8,7 @@ "agy/gstack-land-and-deploy": 117722, "agy/gstack-plan-design-review": 140013, "agy/gstack-plan-devex-review": 135760, - "agy/gstack-plan-eng-review": 152701, + "agy/gstack-plan-eng-review": 152626, "agy/gstack-review": 124218, "claude/design-review": 136531, "codex/gstack-autoplan": 130628, @@ -18,7 +18,7 @@ "codex/gstack-office-hours": 102339, "codex/gstack-plan-design-review": 142466, "codex/gstack-plan-devex-review": 138649, - "codex/gstack-plan-eng-review": 155247, + "codex/gstack-plan-eng-review": 155172, "codex/gstack-review": 125122, "copilot/gstack-autoplan": 131637, "copilot/gstack-codex": 100235, @@ -28,7 +28,7 @@ "copilot/gstack-office-hours": 100495, "copilot/gstack-plan-design-review": 140884, "copilot/gstack-plan-devex-review": 136912, - "copilot/gstack-plan-eng-review": 153426, + "copilot/gstack-plan-eng-review": 153351, "copilot/gstack-review": 125489, "cursor/gstack-autoplan": 129278, "cursor/gstack-design-consultation": 107222, @@ -36,7 +36,7 @@ "cursor/gstack-land-and-deploy": 116645, "cursor/gstack-plan-design-review": 138968, "cursor/gstack-plan-devex-review": 134591, - "cursor/gstack-plan-eng-review": 151555, + "cursor/gstack-plan-eng-review": 151480, "cursor/gstack-review": 140750, "factory/gstack-autoplan": 131562, "factory/gstack-codex": 100655, @@ -46,34 +46,34 @@ "factory/gstack-office-hours": 101374, "factory/gstack-plan-design-review": 141749, "factory/gstack-plan-devex-review": 137960, - "factory/gstack-plan-eng-review": 154669, + "factory/gstack-plan-eng-review": 154594, "factory/gstack-review": 144529, "gbrain/gstack-autoplan": 129815, "gbrain/gstack-design-review": 132842, "gbrain/gstack-land-and-deploy": 117167, "gbrain/gstack-plan-design-review": 127855, "gbrain/gstack-plan-devex-review": 115129, - "gbrain/gstack-plan-eng-review": 134287, + "gbrain/gstack-plan-eng-review": 134212, "hermes/gstack-autoplan": 129905, "hermes/gstack-design-review": 132949, "hermes/gstack-land-and-deploy": 117269, "hermes/gstack-plan-design-review": 127964, "hermes/gstack-plan-devex-review": 115215, - "hermes/gstack-plan-eng-review": 134372, + "hermes/gstack-plan-eng-review": 134297, "kiro/gstack-autoplan": 129040, "kiro/gstack-design-consultation": 107064, "kiro/gstack-design-review": 142321, "kiro/gstack-land-and-deploy": 116533, "kiro/gstack-plan-design-review": 138808, "kiro/gstack-plan-devex-review": 134431, - "kiro/gstack-plan-eng-review": 151405, + "kiro/gstack-plan-eng-review": 151330, "kiro/gstack-review": 140562, "openclaw/gstack-autoplan": 129520, "openclaw/gstack-design-review": 131647, "openclaw/gstack-land-and-deploy": 116757, "openclaw/gstack-plan-design-review": 126706, "openclaw/gstack-plan-devex-review": 113968, - "openclaw/gstack-plan-eng-review": 132684, + "openclaw/gstack-plan-eng-review": 132609, "opencode/gstack-autoplan": 131640, "opencode/gstack-design-consultation": 109357, "opencode/gstack-design-review": 144705, @@ -81,7 +81,7 @@ "opencode/gstack-office-hours": 100329, "opencode/gstack-plan-design-review": 140738, "opencode/gstack-plan-devex-review": 136812, - "opencode/gstack-plan-eng-review": 153307, + "opencode/gstack-plan-eng-review": 153232, "opencode/gstack-review": 142985, "slate/gstack-autoplan": 129159, "slate/gstack-design-consultation": 107143, @@ -89,7 +89,7 @@ "slate/gstack-land-and-deploy": 116589, "slate/gstack-plan-design-review": 138888, "slate/gstack-plan-devex-review": 134511, - "slate/gstack-plan-eng-review": 151480, + "slate/gstack-plan-eng-review": 151405, "slate/gstack-review": 140656 } } diff --git a/test/gbrain-guards.test.ts b/test/gbrain-guards.test.ts index 4ba388f6d..6ebe940a3 100644 --- a/test/gbrain-guards.test.ts +++ b/test/gbrain-guards.test.ts @@ -1,4 +1,5 @@ import { describe, test, expect, afterEach } from "bun:test"; +import { spawn } from "child_process"; import * as fs from "fs"; import * as os from "os"; import { join } from "path"; @@ -78,6 +79,52 @@ describe("detectAutopilot", () => { expect(r.signal).toBe("process:gbrain autopilot"); }); + // #3360: the probe stays machine-wide; the refusal names whose autopilot it is. + test("a matched autopilot process is named by pid and command, with how to proceed", () => { + const r = detectAutopilot(process.env, { + lockPaths: [], + processRunning: () => [{ pid: 4242, command: "gbrain autopilot --repo /src/other-brain" }], + }); + expect(r.active).toBe(true); + expect(r.signal).toBe( + "process:gbrain autopilot pid 4242 `gbrain autopilot --repo /src/other-brain`; this check counts any process on this machine " + + "whose command line contains \"gbrain autopilot\", even an autopilot serving another brain or repo. Stop that autopilot " + + "(or its service, if `gbrain autopilot --install` set one up), or end the process that only mentions it", + ); + }); + + test("every matched autopilot process is listed", () => { + const r = detectAutopilot(process.env, { + lockPaths: [], + processRunning: () => [ + { pid: 4242, command: "gbrain autopilot --repo /a" }, + { pid: 4343, command: "gbrain autopilot --repo /b" }, + ], + }); + expect(r.signal).toContain("pid 4242 `gbrain autopilot --repo /a`, pid 4343 `gbrain autopilot --repo /b`;"); + }); + + test("no matched process is inactive", () => { + const r = detectAutopilot(process.env, { lockPaths: [], processRunning: () => [] }); + expect(r).toEqual({ active: false, signal: null }); + }); + + test.skipIf(process.platform === "win32")("the real probe names a live process anywhere on the machine, credentials masked", async () => { + const url = ["postgres://u", "hunter2@h/db"].join(":"); + const child = spawn(process.execPath, ["-e", "console.log('ready'); setTimeout(() => {}, 30000)", `gbrain autopilot --repo /tmp/gstack-3360 --db ${url}`], { stdio: ["ignore", "pipe", "ignore"] }); + try { + await new Promise(resolve => child.stdout!.once("data", resolve)); + const r = detectAutopilot(process.env, { lockPaths: [] }); + expect(r.active).toBe(true); + expect(r.signal).toContain(`pid ${child.pid} \``); + expect(r.signal).toContain("gbrain autopilot --repo /tmp/gstack-3360 --db postgres://***@h/db"); + expect(r.signal).not.toContain("hunter2"); + expect(r.signal).toContain("Stop that autopilot"); + } finally { + child.kill("SIGKILL"); + } + }); + test("a lock with no parseable pid stays conservative (active, no pid in signal)", () => { const tmp = fs.mkdtempSync(join(os.tmpdir(), "ap-")); const lock = join(tmp, "autopilot.lock"); diff --git a/test/gstack-skill-start.test.ts b/test/gstack-skill-start.test.ts index df6e4ea5d..17d0528b8 100644 --- a/test/gstack-skill-start.test.ts +++ b/test/gstack-skill-start.test.ts @@ -348,7 +348,7 @@ describe('gstack-skill-start behavior', () => { expect(out.match(/GUARD_NOT_INSTALLED/g)).toHaveLength(1); const ids = (out.match(/^GSTACK_INSTRUCTION_BEGIN: (\S+)/gm) ?? []).map((h) => h.replace(/^GSTACK_INSTRUCTION_BEGIN: /, '')); expect(ids).toEqual(['unattended-session']); - expect(out).toContain('GSTACK_RESULT: skill= status=gate_pending run='); + expect(out).toContain('GSTACK_RESULT: skill=autoplan status=gate_pending run='); expect(out).toMatch(/never approve/i); expect(out).toMatch(/^FIRST_TASK: $/m); expect(out).not.toContain('.telemetry-prompted'); @@ -378,6 +378,46 @@ describe('gstack-skill-start behavior', () => { } }); + test('unattended block promises GSTACK_RESULT only to skills that write it; the rest name their record (#3281)', () => { + const block = (skill: string) => { + const out = runStart(['--skill', skill], { GSTACK_SESSION_KIND: 'unattended' }); + const m = out.match(/^GSTACK_INSTRUCTION_BEGIN: unattended-session .*\n([\s\S]*?)\nGSTACK_INSTRUCTION_END$/m); + expect(m).not.toBeNull(); + return m![1]; + }; + const blocks = Object.fromEntries(['autoplan', 'eval-plan', 'ship', 'review', 'qa', 'investigate'].map((s) => [s, block(s)])); + for (const text of Object.values(blocks)) { + expect(text).not.toContain('skill='); + expect(text).toMatch(/never choose them/); + expect(text).toMatch(/never approve a gate/); + expect(text).toMatch(/never publish, push, sync or send data off this machine/); + } + const autoplan = blocks.autoplan; + expect(autoplan).toContain('`GSTACK_RESULT: skill=autoplan status=gate_pending run=`'); + expect(autoplan).toContain('gate list (gstack-gate render)'); + expect(autoplan).toMatch(/decisions\.jsonl with kind "auto"/); + expect(autoplan).toContain('gate.json'); + const evalPlan = blocks['eval-plan']; + expect(evalPlan).toContain('`GSTACK_RESULT: skill=eval-plan status=gate_pending run=`'); + expect(evalPlan).toContain('No helper writes a gate.json'); + expect(evalPlan).not.toContain('gate list'); + const records: Record = { + ship: 'gstack-ship-receipt read --pr --json', + review: 'gstack-review-read', + qa: 'qa-report-*.md', + investigate: "this skill's own record (docs/unattended.md)", + }; + for (const [skill, record] of Object.entries(records)) { + const text = blocks[skill]; + expect(text).toContain('writes no run directory and prints no GSTACK_RESULT line'); + expect(text).toContain(record); + expect(text).not.toContain('GSTACK_RESULT: skill='); + expect(text).not.toContain('gate list'); + expect(text).not.toContain('decisions.jsonl'); + expect(text).not.toContain('run.json'); + } + }, 60_000); + test('unattended session makes no egress call even with telemetry and artifacts sync enabled and a queue pending (plan B1)', () => { // Shadow bin: every helper is the real one except the egress sinks, which // record their invocation. Bins resolve siblings through dirname "$0", so a diff --git a/test/helpers/autoplan-preconfigured-fixture.ts b/test/helpers/autoplan-preconfigured-fixture.ts index fcb79d784..a57b10059 100644 --- a/test/helpers/autoplan-preconfigured-fixture.ts +++ b/test/helpers/autoplan-preconfigured-fixture.ts @@ -12,7 +12,7 @@ Key routing rules: - Architecture → invoke /plan-eng-review - Design system/plan review → invoke /design-consultation or /plan-design-review - Full review pipeline → invoke /autoplan -- Bugs/errors → invoke /investigate +- Bugs/errors → invoke /investigate, unless one read-only check confirms the cause and a narrow fix (then fix it and run the focused test) - QA/testing site behavior → invoke /qa or /qa-only - Code review/diff check → invoke /review - Visual polish → invoke /design-review diff --git a/test/helpers/cso-runtime-catalog.ts b/test/helpers/cso-runtime-catalog.ts index 1e55877d1..10c5a414d 100644 --- a/test/helpers/cso-runtime-catalog.ts +++ b/test/helpers/cso-runtime-catalog.ts @@ -1,6 +1,6 @@ import type { CsoStack } from '../../lib/cso/preparation'; -type RuntimeStack = Exclude; +type RuntimeStack = Exclude; import { canonical, sha256 } from '../../lib/cso/contracts'; import { ISOLATION_POLICY_HASH } from '../../lib/cso/docker'; import { diff --git a/test/helpers/e2e-hosts.ts b/test/helpers/e2e-hosts.ts index 622c3756d..90ca0344f 100644 --- a/test/helpers/e2e-hosts.ts +++ b/test/helpers/e2e-hosts.ts @@ -98,7 +98,8 @@ const CLAUDE_ONLY: Readonly> = { 'shared-libs-review-prior-coverage', 'shared-libs-review-revalidation', 'ship-local-hook-preservation', 'ship-managed-hook-refresh', 'ship-skipped-queued-finding', 'ship-unmanaged-hook-consent'], subagents: ['docsync-spawned', 'review-army-perf-n-plus-one'], - 'skill-tool': ['skillify-approval-reject', 'skillify-happy-path', 'skillify-provenance-refusal'], + 'skill-tool': ['router-failure-contradicted-cause', 'router-failure-triage-resolves', 'router-failure-triage-unclear', + 'router-failure-verified-cause', 'skillify-approval-reject', 'skillify-happy-path', 'skillify-provenance-refusal'], hermetic: ['hermetic-canary', 'hermetic-sentinel'], 'self-host': ['safety-codex-consult-embed'], rework: ['qa-functional-cli-fix', 'qa-functional-cli-report', 'qa-functional-webhook-fix', 'qa-functional-webhook-report', diff --git a/test/helpers/eng-cache-writer-decision.ts b/test/helpers/eng-cache-writer-decision.ts index a39ecbc9d..50686b8d5 100644 --- a/test/helpers/eng-cache-writer-decision.ts +++ b/test/helpers/eng-cache-writer-decision.ts @@ -6,7 +6,7 @@ export function engCacheWriterDecision(q: NativePlanQuestion): boolean { const lines = q.question.split('\n'); // Severity, confidence and source citations annotate an owned issue; they // never replace its current defect, assessment or opposed choices. - const annotated = /^D([1-9]\d*) [—–:-] Issue ([1-9]\d*) \[P[0-3]\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?(?: \+ :[1-9]\d*(?:-[1-9]\d*)?)? [—–:-] ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both mutate one module-level ([A-Za-z_$][\w$]*) that does not serialize mutations\. How should the shared cache be wired\?$/.exec(lines[0] ?? ''); + const annotated = /^D([1-9]\d*) [—–:-] Issue ([1-9]\d*) \[(?:P[0-3]|CRITICAL|INFORMATIONAL)\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?(?: \+ :[1-9]\d*(?:-[1-9]\d*)?)? [—–:-] ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both mutate one module-level ([A-Za-z_$][\w$]*) that does not serialize mutations\. How should the shared cache be wired\?$/.exec(lines[0] ?? ''); const architecture = /^D([1-9]\d*) [—–:-] Architecture issue ([1-9]\d*): global mutable ([A-Za-z_$][\w$]*) shared by two services with unserialized mutations\.?$/.exec(lines[0] ?? ''); const declared = annotated || architecture; const ordinal = declared?.[1] ?? /^D([1-9]\d*) [—–:-] Who is allowed to write to the auth cache\?$/.exec(lines[0] ?? '')?.[1]; diff --git a/test/helpers/pty/boundaries.ts b/test/helpers/pty/boundaries.ts index 8c87100c9..c5520118d 100644 --- a/test/helpers/pty/boundaries.ts +++ b/test/helpers/pty/boundaries.ts @@ -234,7 +234,7 @@ function engNumberedFindingAUQ(fp: AskUserQuestionFingerprint): boolean { // A declarative severity-labelled issue can own the same cache repair as // an interrogative title. Require its native decision, current assessment, // and opposed implementation options; metadata alone never opens review. - const declaredCache = /^(?:Issue|Finding) ([1-9]\d*) \[P[0-3]\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?: ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) share a global mutable ([A-Za-z_$][\w$]*) via module-level export, and both mutate it\.$/.exec(title); + const declaredCache = /^(?:Issue|Finding) ([1-9]\d*) \[(?:P[0-3]|CRITICAL|INFORMATIONAL)\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?: ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) share a global mutable ([A-Za-z_$][\w$]*) via module-level export, and both mutate it\.$/.exec(title); if (declaredCache) return engDeclaredCacheFinding(q, declaredCache); if (engCacheWriterDecision(q)) return true; const libraryHooks = engLibraryHooksFinding(q, title); diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index e48a40bdd..1e9b5f62d 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -1125,6 +1125,10 @@ export const E2E_TOUCHFILES: Record = { "scripts/resolvers/preamble/generate-preamble-bash.ts", "scripts/resolvers/preamble/generate-completion-status.ts" ], + 'router-failure-verified-cause': ['SKILL.md.tmpl', 'SKILL.md', 'scripts/resolvers/preamble/generate-test-failure-triage.ts', 'investigate/SKILL.md.tmpl', 'lib/bun-isolation.ts', 'test/skill-e2e-failure-routing.test.ts'], + 'router-failure-contradicted-cause': ['SKILL.md.tmpl', 'SKILL.md', 'scripts/resolvers/preamble/generate-test-failure-triage.ts', 'investigate/SKILL.md.tmpl', 'lib/bun-isolation.ts', 'test/skill-e2e-failure-routing.test.ts'], + 'router-failure-triage-resolves': ['SKILL.md.tmpl', 'SKILL.md', 'scripts/resolvers/preamble/generate-test-failure-triage.ts', 'investigate/SKILL.md.tmpl', 'lib/bun-isolation.ts', 'test/skill-e2e-failure-routing.test.ts'], + 'router-failure-triage-unclear': ['SKILL.md.tmpl', 'SKILL.md', 'scripts/resolvers/preamble/generate-test-failure-triage.ts', 'investigate/SKILL.md.tmpl', 'lib/bun-isolation.ts', 'test/skill-e2e-failure-routing.test.ts'], }; /** @@ -1525,6 +1529,10 @@ export const E2E_TIERS: Record = { 'overlay-harness-claude-dedicated-tools-vs-bash-sonnet': 'periodic', 'overlay-harness-claude-dedicated-tools-vs-bash-opus-5-5': 'periodic', 'journey-negatives': 'periodic', + 'router-failure-verified-cause': 'gate', + 'router-failure-contradicted-cause': 'gate', + 'router-failure-triage-resolves': 'gate', + 'router-failure-triage-unclear': 'gate', }; /** @@ -1890,6 +1898,10 @@ export const E2E_KINDS: Record = { 'overlay-harness-claude-dedicated-tools-vs-bash-sonnet': 'rule', 'overlay-harness-claude-dedicated-tools-vs-bash-opus-5-5': 'rule', 'journey-negatives': 'rule', + 'router-failure-verified-cause': 'rule', + 'router-failure-contradicted-cause': 'rule', + 'router-failure-triage-resolves': 'rule', + 'router-failure-triage-unclear': 'rule', 'review/SKILL.md workflow': 'judge', 'setup-browser-cookies/SKILL.md workflow': 'judge', 'browse/SKILL.md reference': 'judge', diff --git a/test/outside-review-unparsed-severity.test.ts b/test/outside-review-unparsed-severity.test.ts index bca28a2cc..82e7d0f1a 100644 --- a/test/outside-review-unparsed-severity.test.ts +++ b/test/outside-review-unparsed-severity.test.ts @@ -5,12 +5,14 @@ * FINDINGS: none` for all of them. The fixtures in * test/fixtures/outside-review-severity/codex-3105/ are synthesized from the * forms the issue documents (it describes 13 replies but does not include them). + * #3329: that same gate then printed `clean` for a parsed advisory finding + * whose last `Verdict:` line says `not ready`. `bare-dash-p2.txt` is unchanged. */ import { describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import { classifyOutsideReview, type OutsideGate } from '../lib/outside-review-result'; +import { classifyOutsideReview, validateOutsideReview, type OutsideGate } from '../lib/outside-review-result'; import { GATE_OUTCOMES } from '../lib/gate-outcomes'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -26,7 +28,7 @@ describe('#3105 Codex reply shapes under the execution gate', () => { for (const [name, status, stdout] of [ ['plan-bold-dash.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'], ['diff-numbered-dash.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'], - ['bare-dash-p2.txt', 0, 'VERDICT: clean\nFINDINGS: P2\n'], + ['bare-dash-p2.txt', 3, 'VERDICT: findings\nFINDINGS: P2\n'], ['ready-one-p3.txt', 0, 'VERDICT: clean\nFINDINGS: P3\n'], ['table-p1.txt', 3, 'VERDICT: findings\nFINDINGS: P1\n'], ['not-ready-unlabeled.txt', 3, 'VERDICT: findings\nFINDINGS: unspecified\n'], @@ -112,3 +114,73 @@ describe('#3105 severity forms are read by every gate that summarises a review', expect(classifyOutsideReview({ text: proposal, gate: 'proposal' }).verdict).toBe('clean'); }); }); + +describe('#3329 a last not-ready Verdict blocks even with advisory findings', () => { + const ISSUE = '1. **P2 — The docs lag the new flag.**\n2. **P3 — Rename the helper.**\nVerdict: not ready (1 P1/P2)'; + const P2 = '1. **P2 — The docs lag the new flag.**'; + const textFor = (gate: OutsideGate, text: string) => gate === 'review' ? `${text}\n${REC}` : text; + const see = (text: string, gate: OutsideGate) => { + const r = classifyOutsideReview({ text: textFor(gate, text), gate }); + return { verdict: r.verdict as string, highest: r.findings.highest as string | null, reason: (r.reason ?? null) as string | null, unparsed: r.unparsed ?? null }; + }; + + test('numbered bold P2 plus P3 with not ready is findings/P2 on execution, structured and review', () => { + for (const gate of ['execution', 'structured', 'review'] as const) + expect({ gate, ...see(ISSUE, gate) }).toEqual({ gate, verdict: 'findings', highest: 'P2', reason: null, unparsed: null }); + expect(validateOutsideReview(ISSUE, 'structured')).toEqual({ completed: true, gate: 'fail' }); + }); + + test('P3-only and unlabeled not-ready replies block; P0/P1 block even when the last Verdict says ready', () => { + for (const gate of ['execution', 'structured', 'review'] as const) { + expect({ gate, ...see('1. **P3 — Rename the helper.**\nVerdict: not ready', gate) }).toEqual({ gate, verdict: 'findings', highest: 'P3', reason: null, unparsed: null }); + expect({ gate, ...see('The retry path can double-send.\nVerdict: not ready', gate) }).toEqual({ gate, verdict: 'findings', highest: null, reason: null, unparsed: null }); + } + expect(see('1. **P1 — The cap is ignored.**\nVerdict: ready', 'review')).toMatchObject({ verdict: 'findings', highest: 'P1' }); + expect(see('**P0 — Secrets reach the log.**\nVerdict: ready', 'execution')).toMatchObject({ verdict: 'findings', highest: 'P0' }); + }); + + test('advisory P2/P3 with ready or no Verdict stay clean', () => { + for (const text of [ + `${P2}\n2. **P3 — Rename the helper.**\nVerdict: ready`, + `${P2}\n2. **P3 — Rename the helper.**`, + '1. **P3 — Rename the helper.**\nVerdict: ready (1 P3)', + ]) { + const r = see(text, 'execution'); + expect({ text, verdict: r.verdict, highest: r.highest }).toEqual({ text, verdict: 'clean', highest: text.includes('P2') ? 'P2' : 'P3' }); + } + expect(see(`${P2}\nVerdict: ready`, 'review').verdict).toBe('clean'); + expect(validateOutsideReview(`${P2}\nVerdict: ready`, 'structured')).toEqual({ completed: true, gate: 'pass' }); + }); + + test('the last recognized Verdict wins, including supported formatting; quotes do not', () => { + expect(see(`${P2}\nVerdict: ready\nVerdict: not ready (1 P1/P2)`, 'execution')).toMatchObject({ verdict: 'findings', highest: 'P2' }); + expect(see(`${P2}\nVerdict: not ready (1 P1/P2)\nVerdict: ready`, 'execution').verdict).toBe('clean'); + for (const verdictLine of ['**Verdict:** not ready', '- Verdict: not ready', '### Verdict: not ready']) + expect({ verdictLine, ...see(`${P2}\n${verdictLine}`, 'execution') }).toMatchObject({ verdict: 'findings', highest: 'P2' }); + expect(see(`${P2}\nVerdict: not ready\n### Verdict: ready`, 'execution').verdict).toBe('clean'); + const fence = ['```', 'Verdict: not ready (2 P2)', '```'].join('\n'); + expect(see(`${P2}\nVerdict: ready\n${fence}`, 'execution').verdict).toBe('clean'); + expect(see(`${fence}\n${P2}\nVerdict: not ready`, 'execution')).toMatchObject({ verdict: 'findings', highest: 'P2' }); + expect(see(`${P2}\nVerdict: ready\nThe rubric says \`Verdict: not ready\`.`, 'execution').verdict).toBe('clean'); + expect(see(`${P2}\nThe rubric says \`Verdict: not ready\`.`, 'execution').verdict).toBe('clean'); + expect(see(`No issues found.\n${fence}`, 'review').verdict).toBe('clean'); + }); + + test('parsed P2 plus an unparsed label and not-ready still blocks and keeps the diagnostic', () => { + const text = `${P2}\n## P1/P2 findings\nVerdict: not ready`; + expect(see(text, 'execution')).toEqual({ verdict: 'findings', highest: 'P2', reason: null, unparsed: 'line 2: "## P1/P2 findings"' }); + expect(see(`${P2}\n## P1/P2 findings`, 'structured')).toMatchObject({ verdict: 'unverified', reason: 'unparsed_severity', unparsed: 'line 2: "## P1/P2 findings"' }); + }); + + test('missing markers and failed execution stay unavailable; proposal and spec ignore the verdict', () => { + expect(classifyOutsideReview({ text: `${P2}\nVerdict: not ready`, gate: 'review' }).reason).toBe('missing_markers'); + const failed = classifyOutsideReview({ text: textFor('review', `${P2}\nVerdict: not ready`), gate: 'review', exit: 1, stderr: 'fatal: provider exploded' }); + expect(failed.verdict).toBe('unavailable'); + expect(failed.reason).toBe('execution_failed'); + expect(classifyOutsideReview({ text: `${P2}\nVerdict: not ready\n${REC}`, gate: 'proposal' }).verdict).toBe('clean'); + const passing = classifyOutsideReview({ text: `SCORE: 8\nAMBIGUITIES: NONE\n${P2}\nVerdict: not ready`, gate: 'spec' }); + expect([passing.verdict, passing.score]).toEqual(['clean', 8]); + const failing = classifyOutsideReview({ text: 'SCORE: 6\nAMBIGUITIES: NONE\nVerdict: ready', gate: 'spec' }); + expect([failing.verdict, failing.score]).toEqual(['findings', 6]); + }); +}); diff --git a/test/review-fix-range.test.ts b/test/review-fix-range.test.ts new file mode 100644 index 000000000..defb04c03 --- /dev/null +++ b/test/review-fix-range.test.ts @@ -0,0 +1,216 @@ +/** + * #3339: /ship Step 9 may re-review only the fix range, but a fix-range pass + * can never be the last review. The fix base comes from the review log (never + * the working tree or the caller) and is the commit whose tree the fixing pass + * started from, a narrowed row is never CURRENT and carries no + * start_wtree/end_wtree for Step 11.5 to bind, and narrowed dispatches stay out + * of specialist hit rates. + */ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { canReuseSharedLibsAdvisory, reviewFreshness, sharedLibsFingerprint } from '../lib/review-evidence'; +import { gitIn } from './helpers/scratch-repo'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BIN = path.join(ROOT, 'bin'); +const W = 'a'.repeat(40); + +let root: string, repo: string, env: NodeJS.ProcessEnv; +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-fix-range-')); + repo = path.join(root, 'repo'); + fs.mkdirSync(repo); + env = { ...process.env, GSTACK_STATE_ROOT: path.join(root, 'state') }; + delete env.GSTACK_HOME; + gitIn(repo, 'init -q -b feature'); + fs.writeFileSync(path.join(repo, 'a.ts'), 'export const a = 1;\n'); + gitIn(repo, 'add a.ts'); + gitIn(repo, 'commit -q -m init'); +}); +afterEach(() => { fs.rmSync(root, { recursive: true, force: true }); }); + +function bin(name: string, args: string[]) { + const r = spawnSync(path.join(BIN, name), args, { cwd: repo, encoding: 'utf8', timeout: 60_000, env }); + return { code: r.status, out: r.stdout.trim(), err: r.stderr }; +} +const head = () => gitIn(repo, 'rev-parse HEAD').trim(); +const edit = (text: string) => fs.writeFileSync(path.join(repo, 'a.ts'), text); +const commit = () => gitIn(repo, 'commit -q -am fix'); +const row = (fields: Record) => JSON.stringify({ skill: 'review', via: 'ship', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...fields }); +const fixing = { status: 'issues_found', issues_found: 1, completed: true, converged: false }; +const clean = { completed: true, converged: true }; + +// Step 9.4's order: start, review, apply fixes, log with the original token, then commit the fixes. +function pass(fields: Record, start: string[] = ['--start', 'review'], fixes?: string, commitFixes = true) { + const started = bin('gstack-review-log', start); + expect(started.code).toBe(0); + const [token, base] = started.out.split('\n'); + if (fixes) edit(fixes); + expect(bin('gstack-review-log', [row(fields), '--finish', token!]).code).toBe(0); + if (fixes && commitFixes) commit(); + return base; +} +function rows(): any[] { + return bin('gstack-review-read', []).out.split('\n---CONFIG---')[0]!.split('\n').filter(line => line.startsWith('{')).map(line => JSON.parse(line)); +} +const narrowStart = () => bin('gstack-review-log', ['--start', 'review', '--fix-range']); + +describe('fix-range start', () => { + test('after a completed fixing pass it prints the token and the reviewed commit as FIX_BASE', () => { + const reviewed = head(); + pass(fixing, undefined, 'export const a = 2;\n'); + expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${reviewed}`); + expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts'); + }); + + test('fixes still uncommitted at the repeat are inside the range', () => { + const reviewed = head(); + pass(fixing, undefined, 'export const a = 2;\n', false); + expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${reviewed}`); + expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts'); + }); + + test('chained fix-range passes take the latest fixing pass as base', () => { + pass(fixing, undefined, 'export const a = 2;\n'); + const second = head(); + pass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n'); + expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${second}`); + }); + + test.each([ + ['no earlier pass', () => {}, 'no earlier pass on this branch'], + ['a clean latest pass', () => pass(clean), 'the latest pass made no fixes'], + ['an incomplete latest pass (coverage restoration stays full)', () => pass({ ...fixing, status: 'unavailable', completed: false }, undefined, 'export const a = 2;\n'), 'the latest pass did not complete'], + ['a Step 11 review after the latest pass', () => { + pass(fixing, undefined, 'export const a = 2;\n'); + pass({ skill: 'adversarial-review', ...clean }, ['--start', 'adversarial-review']); + }, 'a Step 11 review followed the latest pass'], + ['a latest pass logged without a start capture', () => { + edit('export const a = 2;\n'); + expect(bin('gstack-review-log', [row(fixing)]).code).toBe(0); + }, 'the latest pass has no captured start'], + ['a pass that started from uncommitted content (a reverted dirty hunk would fall outside the range)', () => { + edit('export const a = 1; // dirty\n'); + pass(fixing, undefined, 'export const a = 1;\n', false); + }, 'started from uncommitted content'], + ['a pass that started with an untracked file', () => { + fs.writeFileSync(path.join(repo, 'b.ts'), 'export const b = 1;\n'); + pass(fixing, undefined, 'export const a = 2;\n'); + }, 'started from uncommitted content'], + ['a base that is no longer an ancestor of HEAD', () => { + edit('export const a = 2;\n'); + commit(); + pass(fixing, undefined, 'export const a = 3;\n'); + gitIn(repo, 'reset -q --hard HEAD~2'); + edit('export const a = 4;\n'); + commit(); + }, 'is not an ancestor of HEAD'], + ])('refuses %s', (_, setup, reason) => { + setup(); + const r = narrowStart(); + expect(r.code).toBe(1); + expect(r.out).toBe(''); + expect(r.err).toContain('fix-range refused: '); + expect(r.err).toContain(reason); + expect(r.err).toContain('run a full pass (gstack-review-log --start review)'); + }); + + test('rejects an unknown start flag with usage', () => { + expect(bin('gstack-review-log', ['--start', 'review', '--narrow']).code).toBe(2); + }); +}); + +describe('a fix-range pass is never the last review', () => { + test('its row is fix-range scoped whatever the caller claims, unbindable at Step 11.5 and never CURRENT', () => { + const reviewed = head(); + pass(fixing, undefined, 'export const a = 2;\n'); + pass({ ...clean, scope: 'full', fix_base: 'f'.repeat(40), review_binding: { state: 'verified', start_wtree: W, end_wtree: W }, wtree: W }, + ['--start', 'review', '--fix-range']); + const last = rows().at(-1); + expect(last.scope).toBe('fix-range'); + expect(last.fix_base).toBe(reviewed); + expect(last.review_binding.state).toBe('fix-range'); + expect(last.review_binding.range_start_wtree).toBe(last.review_binding.range_end_wtree); + expect(last.review_binding).not.toHaveProperty('start_wtree'); + expect(last.review_binding).not.toHaveProperty('end_wtree'); + expect(last).not.toHaveProperty('wtree'); + expect(last.review_freshness).toEqual({ status: 'UNVERIFIED', reason: 'fix-range pass; only a later full pass can clear review' }); + expect(narrowStart().err).toContain('the latest pass made no fixes'); + pass(clean); + expect(rows().at(-1).review_freshness.status).toBe('CURRENT'); + }); + + test('a fixing fix-range pass is not CURRENT either', () => { + pass(fixing, undefined, 'export const a = 2;\n'); + pass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n'); + const last = rows().at(-1); + expect(last.scope).toBe('fix-range'); + expect(last.review_freshness.status).not.toBe('CURRENT'); + }); + + test('a caller cannot narrow a full pass by claiming fix-range', () => { + pass({ ...clean, scope: 'fix-range', fix_base: W }); + const last = rows().at(-1); + expect(last).not.toHaveProperty('scope'); + expect(last).not.toHaveProperty('fix_base'); + expect(last.review_freshness.status).toBe('CURRENT'); + }); + + test('a start token that still names a fix base stays narrowed even if its scope is rewritten', () => { + pass(fixing, undefined, 'export const a = 2;\n'); + const [token] = narrowStart().out.split('\n'); + const slug = bin('gstack-slug', []).out.match(/^SLUG=(.+)$/m)![1]!; + const file = path.join(root, 'state', 'projects', slug, '.review-starts', `${token}.json`); + fs.writeFileSync(file, JSON.stringify({ ...JSON.parse(fs.readFileSync(file, 'utf8')), scope: 'full', fix_base: 'not-a-commit' })); + expect(bin('gstack-review-log', [row(clean), '--finish', token!]).code).toBe(0); + const last = rows().at(-1); + expect(last.scope).toBe('fix-range'); + expect(last).not.toHaveProperty('fix_base'); + expect(last.review_freshness.status).toBe('UNVERIFIED'); + }); + + test('freshness rejects a fix-range row even with a verified-looking binding', () => { + const rec = { skill: 'review', scope: 'fix-range', status: 'clean', issues_found: 0, critical: 0, completed: true, converged: true, wtree: W, + review_binding: { state: 'verified', start_wtree: W, end_wtree: W } }; + expect(reviewFreshness(rec, W)?.status).toBe('UNVERIFIED'); + expect(reviewFreshness({ ...rec, scope: undefined }, W)?.status).toBe('CURRENT'); + }); + + test('shared-libs advisory reuse never trusts a fix-range row', () => { + const finding = { severity: 'INFORMATIONAL', advisory: true, action: 'skipped', evidence_paths: ['a.ts'], helper_target: { path: 'b.ts', symbol: 'b' } }; + const prior = { ...finding, fingerprint: sharedLibsFingerprint(finding), snapshot_covered_paths: ['a.ts'] }; + const snapshot = { wtree: W, branch_id: 'd'.repeat(64), covered_paths: ['a.ts'] }; + const full = { skill: 'review', completed: true, converged: true, wtree: W, review_binding: { state: 'verified', start_wtree: W, end_wtree: W, branch_id: 'd'.repeat(64) } }; + expect(canReuseSharedLibsAdvisory(prior, finding, full, snapshot)).toBe(true); + const narrowed = { ...full, scope: 'fix-range', review_binding: { state: 'fix-range', range_start_wtree: W, range_end_wtree: W, branch_id: 'd'.repeat(64) } }; + expect(canReuseSharedLibsAdvisory(prior, finding, narrowed, snapshot)).toBe(false); + }); +}); + +describe('gstack-specialist-stats', () => { + test('fix-range passes do not count toward hit rates or gate candidacy', () => { + const entry = (scope?: string) => JSON.stringify({ ...(scope ? { scope } : {}), specialists: { testing: { dispatched: true, findings: 0 } } }); + const slug = bin('gstack-slug', []).out.match(/^SLUG=(.+)$/m)![1]!; + const dir = path.join(root, 'state', 'projects', slug); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'feature-reviews.jsonl'), [...Array(9).fill(entry()), ...Array(5).fill(entry('fix-range'))].join('\n') + '\n'); + const out = bin('gstack-specialist-stats', []).out; + expect(out).toContain('SPECIALIST_STATS: 9 reviews analyzed'); + expect(out).toContain('testing: 9/9 dispatched, 0 findings (0%)'); + expect(out).not.toContain('GATE_CANDIDATE'); + }); +}); + +describe('ship Step 9 instructions', () => { + const army = fs.readFileSync(path.join(ROOT, 'ship/sections/review-army.md'), 'utf8').replace(/\s+/g, ' '); + test('the fix-range start, its base substitution and the mandatory full pass are stated', () => { + expect(army).toContain('start with `--start review --fix-range`'); + expect(army).toContain('`FIX_BASE=`, which stands in for `origin/` and `` in this pass'); + expect(army).toContain('If it refuses, start a full pass.'); + expect(army).toContain('it may be a fix-range repeat'); + expect(army).toContain('continue to Step 10, and only from a full pass: after a fix-range pass, run one full Step 9 first.'); + }); +}); diff --git a/test/session-update-stages.test.ts b/test/session-update-stages.test.ts index ef0c1d581..7bace11a9 100644 --- a/test/session-update-stages.test.ts +++ b/test/session-update-stages.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, test, expect } from 'bun:test'; -import { execFileSync, spawnSync } from 'child_process'; +import { execFileSync, spawn, spawnSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -97,13 +97,13 @@ function bunStub(fx: Fx, version: string): string { return dir; } -function runHook(fx: Fx, opts: { noBun?: boolean; bun?: string } = {}) { +function runHook(fx: Fx, opts: { noBun?: boolean; bun?: string; env?: Record } = {}) { const PATH = opts.noBun ? pathWithoutBun(fx.base) : opts.bun ? `${bunStub(fx, opts.bun)}:${pathWithoutBun(fx.base)}` : `${path.dirname(process.execPath)}:${process.env.PATH}`; return spawnSync('bash', [SCRIPT], { encoding: 'utf8', - env: { PATH, HOME: fx.home, GSTACK_DIR: fx.install, GSTACK_STATE_ROOT: fx.state, SETUP_CONTROL: fx.control, SETUP_CALLS: fx.calls, TMPDIR: fx.base }, + env: { PATH, HOME: fx.home, GSTACK_DIR: fx.install, GSTACK_STATE_ROOT: fx.state, SETUP_CONTROL: fx.control, SETUP_CALLS: fx.calls, TMPDIR: fx.base, ...opts.env }, timeout: 20_000, }); } @@ -287,3 +287,82 @@ describe.skipIf(process.platform === 'win32')('gstack-session-update Bun floor ( expect(banner).not.toContain('git pull did not finish'); }); }); + +describe.skipIf(process.platform === 'win32')('gstack-session-update honors workflow leases (#3363)', () => { + const head = (fx: Fx) => git(fx.install, 'rev-parse', 'HEAD'); + const sleepers: ReturnType[] = []; + afterEach(() => { for (const s of sleepers.splice(0)) s.kill('SIGKILL'); }); + // The test preload points GSTACK_LEASE_DIR at a per-run directory; each + // fixture gets its own, shared by the lease writer and the hook. + const leaseEnv = (fx: Fx) => ({ GSTACK_LEASE_DIR: path.join(fx.base, 'leases') }); + function touchLease(fx: Fx, pid: number) { + const touch = spawnSync('bash', [path.join(ROOT, 'bin', 'gstack-lease'), 'touch', '--root', fx.install, '--pid', String(pid)], + { encoding: 'utf8', env: { ...process.env, ...leaseEnv(fx) }, timeout: 10_000 }); + expect(touch.status, touch.stderr).toBe(0); + } + /** A live process holding a workflow lease on the fixture install, as a running skill would. */ + function holdLease(fx: Fx): number { + const sleeper = spawn('sleep', ['120'], { stdio: 'ignore' }); + sleepers.push(sleeper); + touchLease(fx, sleeper.pid!); + return sleeper.pid!; + } + const throttle = (fx: Fx) => path.join(fx.state, '.last-session-update'); + + test('a live lease: no fast-forward, no setup, no throttle stamp; the next start after it ends updates', async () => { + const fx = makeFixture(); + const before = head(fx); + upstreamRelease(fx, '1.1.0'); + const pid = holdLease(fx); + + expect(runHook(fx, { env: leaseEnv(fx) }).stdout).toBe(''); + const log = await waitFor(fx, /SKIP leased /); + expect(log).toMatch(new RegExp(`SKIP leased pid=${pid}/\\d+m`)); + expect(log).not.toContain('UPDATING'); + expect(head(fx)).toBe(before); + expect(setupRuns(fx)).toBe(0); + expect(fs.existsSync(throttle(fx))).toBe(false); + expect(fs.existsSync(marker(fx))).toBe(false); + + await Promise.all(sleepers.splice(0).map(s => new Promise(r => { s.once('exit', r); s.kill('SIGKILL'); }))); + runHook(fx, { env: leaseEnv(fx) }); + await waitFor(fx, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(head(fx)).not.toBe(before); + expect(setupRuns(fx)).toBe(1); + }, 30_000); + + test('a live lease also holds a pending setup retry', async () => { + const fx = makeFixture(); + fs.writeFileSync(fx.control, 'fail'); + upstreamRelease(fx, '1.1.0'); + runHook(fx); + await waitFor(fx, /Z PENDING stages=setup attempt=1/); + expireThrottle(fx); + expireBackoff(fx); + fs.writeFileSync(fx.control, 'ok'); + holdLease(fx); + runHook(fx, { env: leaseEnv(fx) }); + const log = await waitFor(fx, /SKIP leased /); + expect(log).not.toContain('RETRY_PENDING'); + expect(setupRuns(fx)).toBe(1); + }, 30_000); + + test('controls: GSTACK_UPGRADE_FORCE=1 updates under a live lease, and a dead holder never blocks', async () => { + const forced = makeFixture(); + upstreamRelease(forced, '1.1.0'); + holdLease(forced); + runHook(forced, { env: { ...leaseEnv(forced), GSTACK_UPGRADE_FORCE: '1' } }); + await waitFor(forced, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(setupRuns(forced)).toBe(1); + + const dead = makeFixture(); + upstreamRelease(dead, '1.1.0'); + const sleeper = spawn('sleep', ['120'], { stdio: 'ignore' }); + touchLease(dead, sleeper.pid!); + await new Promise(r => { sleeper.once('exit', r); sleeper.kill('SIGKILL'); }); + runHook(dead, { env: leaseEnv(dead) }); + const log = await waitFor(dead, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(log).not.toContain('SKIP leased'); + }, 30_000); +}); + diff --git a/test/skill-e2e-failure-routing.test.ts b/test/skill-e2e-failure-routing.test.ts new file mode 100644 index 000000000..c08c5b1f6 --- /dev/null +++ b/test/skill-e2e-failure-routing.test.ts @@ -0,0 +1,175 @@ +/** + * Router failure routing (#3157, #3158). The gstack router checks a reported + * failure's cause before it routes: a user-supplied cause that one read-only + * check confirms is fixed directly; an unknown cause gets one bounded triage + * pass; anything still unverified goes to /investigate before any edit. + * + * Four fixtures, two per issue. The direct cases (verified cause, triage that + * resolves it) must fix the bug without /investigate and leave the suite + * green. The fallback cases (contradicted cause, unclear intermittent failure) + * are the negative controls: an edit before /investigate is a contract + * violation. Every case needs one to four read-only calls before it routes. The agent sees the rendered router section, sliced from + * SKILL.md; GSTACK_ROUTER_ARM=before slices the base branch's router instead. + */ +import { afterAll, expect } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { runSkillTest } from './helpers/session-runner'; +import type { SkillTestResult } from './helpers/session-runner'; +import { expectContract } from './helpers/eval-store'; +import { + ROOT, runId, describeIfSelected, testConcurrentIfSelected, logCost, recordE2E, + createEvalCollector, finalizeEvalCollector, +} from './helpers/e2e-helpers'; +import { CAPTURE_MS } from './helpers/eval-budgets'; +import { extractSkillHead, sliceBetween } from './helpers/skill-fixture'; +import { bunIsolationArgs } from '../lib/bun-isolation'; + +const collector = createEvalCollector('e2e-failure-routing'); +const ARM = process.env.GSTACK_ROUTER_ARM === 'before' ? 'before' : 'current'; +const EDIT_TOOLS = new Set(['Edit', 'Write', 'MultiEdit', 'NotebookEdit']); +const READ_TOOLS = new Set(['Read', 'Grep', 'Glob', 'Bash']); +const SHELL_WRITE = /\b(?:sed|perl)\b[^|;&]*\s-[a-z]*i|(?])>>?\s*(?!&|\/dev\/null)\S|\btee\b|\b(?:mv|cp|rm)\s|\bgit\s+(?:apply|checkout|restore)\b/; + +function isEdit(call: { tool: string; input: any }): boolean { + return EDIT_TOOLS.has(call.tool) || (call.tool === 'Bash' && SHELL_WRITE.test(String(call.input?.command ?? ''))); +} + +interface Fixture { + name: string; + route: 'direct' | 'investigate'; + files: Record; + request: string; +} + +const FIXTURES: Fixture[] = [ + { + name: 'router-failure-verified-cause', + route: 'direct', + files: { + 'src/slug.js': "export function slugify(title) {\n return title.trim().toLowerCase().replace(/ /, '-');\n}\n", + 'test/slug.test.js': "import { expect, test } from 'bun:test';\nimport { slugify } from '../src/slug.js';\n\ntest('slugify joins every word with a hyphen', () => {\n expect(slugify('Hello Big World')).toBe('hello-big-world');\n});\n", + }, + request: "test/slug.test.js fails: slugify('Hello Big World') returns 'hello-big world'. The cause is the regex in src/slug.js: / / has no g flag, so only the first space is replaced. Fix it by making it / /g.", + }, + { + name: 'router-failure-contradicted-cause', + route: 'investigate', + files: { + 'src/price.js': "import { discountRate } from './rates.js';\n\nexport function applyDiscount(price, code) {\n return Math.round(price * (1 - discountRate(code)));\n}\n", + 'src/rates.js': "import rates from '../config/rates.json' with { type: 'json' };\n\nexport function discountRate(code) {\n return rates[code] ?? 0;\n}\n", + 'config/rates.json': '{\n "SAVE10": 10,\n "SAVE25": 0.25\n}\n', + 'test/price.test.js': "import { expect, test } from 'bun:test';\nimport { applyDiscount } from '../src/price.js';\n\ntest('SAVE10 takes ten percent off', () => {\n expect(applyDiscount(200, 'SAVE10')).toBe(180);\n});\n", + }, + request: "test/price.test.js fails: applyDiscount(200, 'SAVE10') returns -1800 instead of 180. It's a floating-point rounding problem in src/price.js; wrap the result in Math.round() and it will pass.", + }, + { + name: 'router-failure-triage-resolves', + route: 'direct', + files: { + 'src/initials.js': "export function initials(fullName) {\n const [first, last] = fullName.split(',');\n return (first[0] + last[0]).toUpperCase();\n}\n", + 'test/initials.test.js': "import { expect, test } from 'bun:test';\nimport { initials } from '../src/initials.js';\n\ntest('initials of a first and last name', () => {\n expect(initials('Ada Lovelace')).toBe('AL');\n});\n", + }, + request: "bun test fails on test/initials.test.js with: TypeError: undefined is not an object (evaluating 'last[0]') at initials (src/initials.js:3). Can you fix it?", + }, + { + name: 'router-failure-triage-unclear', + route: 'investigate', + files: { + 'src/queue.js': "const pending = [];\nlet timer = null;\n\nexport function enqueue(job, sink) {\n pending.push(job);\n if (!timer) timer = setTimeout(() => flush(sink), Number(process.env.FLUSH_MS ?? 5));\n}\n\nexport function flush(sink) {\n timer = null;\n sink.push(...pending.splice(0));\n}\n", + 'src/metrics.js': "import { enqueue } from './queue.js';\n\nexport function track(event, sink) {\n if (event.sampled !== false) enqueue({ type: 'metric', event }, sink);\n}\n", + 'test/queue.test.js': "import { expect, test } from 'bun:test';\nimport { enqueue } from '../src/queue.js';\n\ntest('three jobs reach the sink', async () => {\n const sink = [];\n enqueue(1, sink); enqueue(2, sink); enqueue(3, sink);\n await new Promise(r => setTimeout(r, 10));\n expect(sink.length).toBe(3);\n});\n", + 'test/metrics.test.js': "import { expect, test } from 'bun:test';\nimport { track } from '../src/metrics.js';\n\ntest('sampled events are tracked', async () => {\n const sink = [];\n track({ name: 'signup' }, sink);\n await new Promise(r => setTimeout(r, 10));\n expect(sink.length).toBe(1);\n});\n", + }, + request: "test/queue.test.js fails about one CI run in five with 'expected 3, received 2'. It has never failed on my machine. Can you fix it?", + }, +]; + +function routerSection(): string { + const skill = ARM === 'before' + ? spawnSync('git', ['show', `${process.env.EVALS_BASE || 'origin/main'}:SKILL.md`], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }).stdout + : fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf8'); + return sliceBetween(skill, '## Route first', 'Best-effort, record') + sliceBetween(skill, '**Routing rules', 'If the user opts out'); +} + +function makeRepo(fixture: Fixture): string { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'failure-routing-')); + fs.writeFileSync(path.join(repo, 'package.json'), JSON.stringify({ name: 'fixture', type: 'module', scripts: { test: 'bun test' } }, null, 2)); + for (const [file, body] of Object.entries(fixture.files)) { + fs.mkdirSync(path.dirname(path.join(repo, file)), { recursive: true }); + fs.writeFileSync(path.join(repo, file), body); + } + for (const skill of ['investigate', 'qa', 'review', 'ship']) { + fs.mkdirSync(path.join(repo, '.claude', 'skills', skill), { recursive: true }); + fs.writeFileSync(path.join(repo, '.claude', 'skills', skill, 'SKILL.md'), extractSkillHead(path.join(ROOT, skill, 'SKILL.md'))); + } + const git = (...args: string[]) => spawnSync('git', ['-c', 'user.email=test@example.invalid', '-c', 'user.name=Test', ...args], { cwd: repo, timeout: 10_000 }); + git('init', '-q', '-b', 'main'); + git('add', '.'); + git('commit', '-qm', 'initial'); + return repo; +} + +function sourceSnapshot(repo: string, fixture: Fixture): string { + return Object.keys(fixture.files).filter(f => !f.startsWith('test/')).map(f => fs.readFileSync(path.join(repo, f), 'utf8')).join('\0'); +} + +function routing(result: SkillTestResult) { + const calls = result.toolCalls; + const investigateAt = calls.findIndex(c => c.tool === 'Skill' && String(c.input?.skill ?? '').includes('investigate')); + const firstEditAt = calls.findIndex(isEdit); + const before = investigateAt >= 0 ? calls.slice(0, investigateAt) : firstEditAt >= 0 ? calls.slice(0, firstEditAt) : calls; + return { + investigated: investigateAt >= 0, + editedFirst: firstEditAt >= 0 && (investigateAt < 0 || firstEditAt < investigateAt), + readsBeforeRoute: before.filter(c => READ_TOOLS.has(c.tool) && !isEdit(c)).length, + sequence: calls.map(c => c.tool === 'Skill' ? `Skill(${c.input?.skill})` : c.tool).join(' → '), + }; +} + +describeIfSelected('Router failure routing (#3157, #3158)', FIXTURES.map(f => f.name), () => { + const dirs: string[] = []; + afterAll(() => { + for (const dir of dirs) fs.rmSync(dir, { recursive: true, force: true }); + finalizeEvalCollector(collector); + }); + + for (const fixture of FIXTURES) { + testConcurrentIfSelected(fixture.name, async () => { + const repo = makeRepo(fixture); + dirs.push(repo); + const sourceBefore = sourceSnapshot(repo, fixture); + const result = await runSkillTest({ + prompt: `The user invoked the gstack router skill (/gstack) in this repository. The router's instructions:\n\n${routerSection()}\n\nThe user's request: "${fixture.request}"`, + workingDirectory: repo, + maxTurns: 6, + tools: ['Skill', 'Read', 'Grep', 'Glob', 'Edit', 'Bash'], + allowedTools: ['Skill', 'Read', 'Grep', 'Glob', 'Edit', 'Bash'], + timeout: CAPTURE_MS, + testName: fixture.name, + runId, + }); + logCost(`router ${fixture.name} (${ARM})`, result); + const r = routing(result); + const suite = spawnSync(process.execPath, [...bunIsolationArgs(), 'test'], { cwd: repo, encoding: 'utf8', timeout: 60_000 }); + const green = suite.status === 0; + const sourceChanged = sourceSnapshot(repo, fixture) !== sourceBefore; + const summary = `arm=${ARM} route=${r.investigated ? 'investigate' : 'direct'} reads=${r.readsBeforeRoute} green=${green} tools: ${r.sequence}`; + const bounded = r.readsBeforeRoute >= 1 && r.readsBeforeRoute <= 4; + const passed = bounded && (fixture.route === 'direct' ? !r.investigated && green : r.investigated && !r.editedFirst); + recordE2E(collector, fixture.name, 'Router failure routing', result, { passed, output: summary }); + + if (fixture.route === 'investigate') { + expectContract(!r.editedFirst && (r.investigated || !sourceChanged), `edited before /investigate on an unverified cause: ${summary}`, { collector, name: fixture.name }); + expect(r.investigated, summary).toBe(true); + } else { + expect(r.investigated, summary).toBe(false); + expect(green, `suite still red after the direct fix: ${summary}\n${suite.stdout}${suite.stderr}`).toBe(true); + } + expect(r.readsBeforeRoute, `no read-only check before routing: ${summary}`).toBeGreaterThanOrEqual(1); + expect(r.readsBeforeRoute, `triage was not bounded: ${summary}`).toBeLessThanOrEqual(4); + }, CAPTURE_MS); + } +}); diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 3a003970f..454694de0 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -7,6 +7,8 @@ import * as os from 'os'; import * as path from 'path'; import { usePrivateStateRoot } from './helpers/private-state-root'; import { expectMentions } from './helpers/prompt-structure'; +import { generateFailureRouting } from '../scripts/resolvers/preamble/generate-test-failure-triage'; +import { readShippedSkillRouting } from './helpers/shipped-skill-routing'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -2043,6 +2045,37 @@ describe('Test failure triage in ship skill', () => { }); }); +describe('Failure routing: verified cause → fix, unknown cause → bounded triage (#3157, #3158)', () => { + const rule = generateFailureRouting(); + const unknownCause = rule.slice(rule.indexOf('**Cause unknown:** ') + '**Cause unknown:** '.length); + + test('the router sends bug reports through the failure rule, not straight to /investigate', () => { + const router = fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8'); + expect(router).toContain(rule); + const bugRule = router.split('\n').find(line => line.startsWith('- User reports a bug')); + expect(bugRule).toContain('**Failures**'); + expect(bugRule).not.toMatch(/→ invoke `\/investigate`$/); + }); + + test('the rule keeps the root-cause bar on both paths', () => { + expect(rule).toMatch(/\*\*Cause supplied\*\*[^\n]*one read-only check[^\n]*If it does not, invoke `\/investigate` before any further tracing or edit/); + expect(unknownCause).toMatch(/^make one read-only triage pass[^\n]*Do not reproduce, test hypotheses or search wider[^\n]*still unclear, invoke `\/investigate` before any broader diagnosis or edit\.$/); + }); + + test('the routing gstack offers for CLAUDE.md, and this repo\'s own, check the cause before /investigate', () => { + const bugRule = readShippedSkillRouting().rules.find(line => line.startsWith('- Bugs/errors')); + expect(bugRule).toMatch(/→ invoke \/investigate, unless one read-only check confirms the cause/); + expect(fs.readFileSync(path.join(ROOT, 'CLAUDE.md'), 'utf-8').split('\n')).toContain(bugRule!); + }); + + test('/ship fixes a pre-existing failure through the same bounded triage', () => { + const tests = fs.readFileSync(path.join(ROOT, 'ship', 'sections', 'tests.md'), 'utf-8'); + const fixNow = tests.slice(tests.indexOf('**If "Investigate and fix now":**'), tests.indexOf('**If "Add as P0 TODO":**')); + expect(fixNow).toContain('- One read-only root-cause pass, then `/investigate` if unclear.'); + expect(fixNow).not.toContain('/investigate mindset'); + }); +}); + describe('no compiled binaries in git', () => { // Tracked files enumerated once and reused by both assertions. git ls-files -z // + split is ~ms; the previous xargs-per-file shell loops blew past 5s on CI.