From 6b57ededeb5dbd4469f4a4fb336e6dc42ae6d72c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 19 Oct 2026 13:50:36 -0700 Subject: [PATCH 1/5] v1.91.153.0 fix(outside-review): no-findings counts only as the review's conclusion; add regressions (#3294) (#3304) * fix(outside-review): no-findings counts only as the review's conclusion; add regressions/defects (#3294) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(outside-review): a trailing ', and' clause can conclude; elsewhere/otherwise are exceptions; real codex captures Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(outside-review): whitelist the conclusion's qualifier; join wrapped Recommendation clauses; outside-review findings as controls Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(outside-review): join a wrapped line only when it continues in lowercase Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 11 ++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- docs/troubleshooting.md | 14 +++++-- lib/outside-review-result.ts | 56 +++++++++++++++++++++++++-- package.json | 2 +- test/outside-review-result.test.ts | 61 ++++++++++++++++++++++++++++++ 7 files changed, 138 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d6f54f8..cfe3ed984 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [1.91.153.0] - 2026-10-19 + +**An outside review whose findings are untagged prose no longer reads `clean` because a "no issues" phrase appears somewhere in it, and a clean Codex review that says "no actionable regressions" no longer reads `unverified`.** Reported in #3294 by @SchizasNik, with the false-pass half pointed out by @xg-gh-25. `lib/outside-review-result.ts` matched its no-findings vocabulary anywhere in the reply, so "The parser change has no new issues. However, … drops users_email_idx" passed /ship's structured gate. It also lacked `regressions`, so `codex review --uncommitted`'s "I found no actionable regressions in the staged, unstaged, or untracked changes." was missing coverage. + +### Fixed +- **A no-findings statement counts only as the review's conclusion.** It must open the reply's last sentence or last ", and" clause (or the `because` clause of a review's `Recommendation:` line) and carry no contrast or exception ("but", "however", "except", "other than", "elsewhere"). When other prose comes first, anything after the statement other than a predicate ("were found", "is evident") or a colon explanation must scope it to the change as a whole, so "No issues in the parser." and "No regressions were found by the existing tests." do not conclude the review. Anything else stays `unverified` / `untagged_review`, the same reason code and troubleshooting anchor as before. +- **`regressions` and `defects` join the vocabulary**, and `NO_FINDINGS` written as inline code still counts. + +### Gate integrity +- The structured and review gates' no-findings check (`classifyOutsideReview`) is stricter about position and wider in vocabulary. New free regressions in `test/outside-review-result.test.ts` cover the reporter's exact reply through their CLI invocation, two clean `codex review --uncommitted` replies captured from codex-cli 0.160.0 with gpt-6-sol (both read `unverified` before), fifteen mixed replies that must stay `unverified` under both gates (the two strings from the issue, the phrase before, after and inside untagged findings, exception conclusions, conclusions scoped to one place or to the tests, a quoted `NO_FINDINGS`, a no-issues line inside a code fence), and Recommendation-clause cases including a wrapped clause with "but". A GPT-6.1 Sol outside review probed the classifier; its false passes are among those controls. A known residual, unchanged from before: untagged findings followed by "No issues found in the changed parser." still read `clean`, because "changed" names the change and the parser cannot tell a finding from a summary without tags. All four new tests fail on the previous classifier. Every existing negative control passes unchanged, including the stored Codex clean review that still reads `unverified` ("no actionable correctness issues"). No threshold, trial count or case kind changed. + ## [1.91.152.0] - 2026-10-19 **`/cso` now starts and finishes on ordinary repositories that hold a nested Git repository, a long patch history, or a Docker socket on macOS 27.** Three user reports (#3255, #3248) each stopped a run on a normal repo. diff --git a/VERSION b/VERSION index 8219c59aa..eaffa97d8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.152.0 +1.91.153.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 942806459..fc7e7065c 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.152.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.153.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/docs/troubleshooting.md b/docs/troubleshooting.md index 15094f27e..1553aa67a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -172,9 +172,17 @@ the readiness dashboard and the PR body, and never count it as a pass. 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. -- **No findings.** A review with no tag and no label is `clean` only when it - says so explicitly (`NO_FINDINGS`, "no issues", "did not find any bugs"). - Otherwise it is `unverified` (see below), never `clean`. A `/codex` consult +- **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`, + "No issues found", "I found no actionable regressions" or "I did not find + any bugs", with no "but", "however", "except", "other than" or "elsewhere". + When other prose comes first, anything after that statement except a + predicate ("were found", "is evident") or a colon explanation must scope it + to the change as a whole ("in this change", "in the staged changes"); "No + issues in the parser." does not conclude the review. A "no issues" phrase + anywhere else in the reply does not count. Otherwise the review is + `unverified` (see below), never `clean`. A `/codex` consult answer with no severity labels at all is not a review summary and stays `clean`. - **Design proposals** (the design-direction voices in /design-consultation diff --git a/lib/outside-review-result.ts b/lib/outside-review-result.ts index e9871427d..8409c73b2 100644 --- a/lib/outside-review-result.ts +++ b/lib/outside-review-result.ts @@ -135,8 +135,56 @@ const SEVERITY_LABELS = [ function severityWords(text: string): Severity[] { return SEVERITY_LABELS.flatMap(re => [...text.matchAll(re)].map(m => SEVERITY_WORDS[m[1]!.toLowerCase()]!)); } -/** An explicit conclusion that the review found nothing to report. */ -const NO_FINDINGS = /\bNO_FINDINGS\b|\bno (?:actionable |significant |new |concrete )?(?:bugs?|issues?|findings?|problems?)\b|\b(?:did not|didn't|cannot|can't|could not|couldn't) (?:find|identify) any (?:actionable |new |concrete )?(?:bugs|issues|findings|problems)\b/i; +/** + * A no-findings statement opening a sentence: NO_FINDINGS (its underscore is + * gone after plainReview), "No issues found", "I found no actionable + * regressions", "I did not find any bugs". Never a phrase inside another + * clause ("renaming the flag is no problem for callers, but ..."). + */ +const FINDING_NOUN = '(?:(?:actionable|significant|new|concrete)\\s+)?(?:bugs?|issues?|findings?|problems?|regressions?|defects?)\\b'; +const NO_FINDINGS_LEAD = '(?:(?:overall|in summary|in short|summary|conclusion|result)[,:]?\\s+|(?:after|having|on|upon)\\s+(?:reviewing|reading|checking|inspecting|reviewed|read|checked|inspected)\\b[^,.;]{0,80},\\s+)?'; +const NO_FINDINGS = new RegExp(`^${NO_FINDINGS_LEAD}(?:NO_?FINDINGS\\b|(?:(?:i|we)\\s+(?:found|see|saw|identified)\\s+|there\\s+(?:are|were|is)\\s+)?no\\s+${FINDING_NOUN}|(?:i|we)\\s+(?:did not|didn't|could not|couldn't|cannot|can't)\\s+(?:find|identify|see)\\s+any\\s+${FINDING_NOUN})`, 'i'); +/** A contrast or exception qualifies the claim, so the sentence does not conclude that nothing was found. */ +const NO_FINDINGS_EXCEPTION = /(? sentence.trim()).filter(Boolean); + const total = sentences.length + (because ? 1 : 0); + return [sentences.at(-1), because].some(conclusion => { + const statement = conclusion?.match(NO_FINDINGS)?.[0]; + if (!conclusion || !statement || NO_FINDINGS_EXCEPTION.test(conclusion)) return false; + const qualifier = conclusion.slice(statement.length).replace(NO_FINDINGS_PREDICATE, '').replace(/[\s.!]+$/, '').trim(); + return total === 1 || !qualifier || qualifier.startsWith(':') || WHOLE_CHANGE_SCOPE.test(qualifier); + }); +} /** Formatting the requested marker in bold, inline code, or a list does not invalidate a completed review. */ function plainReview(text: string): string { @@ -159,7 +207,7 @@ function withoutCode(text: string): string { } if (marker) { fence = marker; return ''; } return line.replace(/`([^`]+)`/g, (span, inner: string, at: number) => - /^\[?(?:P[0-3]|critical|high|medium|low)\]?$/i.test(inner.trim()) && CODE_LABEL_BEFORE.test(line.slice(0, at)) ? inner : ' '); + inner.trim() === 'NO_FINDINGS' || /^\[?(?:P[0-3]|critical|high|medium|low)\]?$/i.test(inner.trim()) && CODE_LABEL_BEFORE.test(line.slice(0, at)) ? inner : ' '); }).join('\n'); } @@ -218,7 +266,7 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC const notReady = !findings.highest && 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 && !NO_FINDINGS.test(input.text)) { + if ((input.gate === 'structured' || input.gate === 'review') && !findings.highest && !concludesNoFindings(code)) { return result('unverified', 'untagged_review', 'missing severity or explicit no-findings conclusion'); } return result('clean'); diff --git a/package.json b/package.json index f4e22657b..99f8e4a4b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.152", + "version": "1.91.153", "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/test/outside-review-result.test.ts b/test/outside-review-result.test.ts index 5d3a2ab7e..572b90f30 100644 --- a/test/outside-review-result.test.ts +++ b/test/outside-review-result.test.ts @@ -65,6 +65,67 @@ describe('classifyOutsideReview: separate execution, findings and verdict', () = }); }); +// #3294: "clean" comes only from a no-findings statement that concludes the reply, +// never from a "no X" phrase among untagged findings anywhere in it. +describe('a no-findings statement counts only as the reply\'s conclusion (#3294)', () => { + const REPORTER = 'I found no actionable regressions in the staged, unstaged, or untracked changes.'; + const FINDING = 'The fallback path now swallows ENOENT and returns an empty config.'; + + test("the reporter's exact codex review reply reads clean through the reporter's CLI invocation", () => { + expect(classifyOutsideReview({ text: REPORTER, gate: 'structured' }).verdict).toBe('clean'); + const r = cli(['--label', 'probe', '--exit', '0', '--stderr', file(''), 'structured', file(REPORTER)]); + expect([r.status, r.stdout]).toEqual([0, 'VERDICT: clean\nFINDINGS: none\n']); + }); + + test('clean conclusions, with the added regressions/defects vocabulary and the requested marker formatted', () => { + for (const text of ['No regressions found.', 'I did not find any defects.', 'There are no actionable regressions.', + 'NO_FINDINGS', '**NO_FINDINGS**', '`NO_FINDINGS`', '- NO_FINDINGS', + `The change adds a retry cap and its tests.\n${REPORTER}`, 'Reviewed the retry helper.\n\nNo issues found in this change.', + 'After reviewing the patch, I found no issues.', 'I found no issues in this read-only change.', + 'Summary of the retry helper\nNo issues found.', 'I found no actionable regressions in the staged\nchanges.', + // Clean `codex review --uncommitted` replies captured from codex-cli 0.160.0 with gpt-6-sol. + 'The change validates the retry limit before execution. No actionable regression was found.', + 'The change validates `max` before retrying. No affected callers or tests are present in the repository, and no actionable regression is evident.']) { + expect({ text, verdict: classifyOutsideReview({ text, gate: 'structured' }).verdict }).toEqual({ text, verdict: 'clean' }); + } + }); + + test('mixed replies: untagged findings beside a "no X" phrase are unverified, never clean', () => { + for (const text of [ + 'The parser change has no new issues. However, db/002_drop_idx.sql drops users_email_idx, which will regress login latency under load.', + 'Renaming the flag is no problem for callers, but the fallback path now swallows ENOENT and returns an empty config.', + `No issues in the parser.\n\n${FINDING}`, + `${FINDING}\n\nNo issues in the parser.`, + `${FINDING} No regressions in the existing tests.`, + 'No regressions in the existing tests, but the fallback path now swallows ENOENT.', + `${FINDING}\nI found no issues other than the fallback path.`, + `${FINDING}\nNo other issues.`, + 'No issues found, except that the fallback path now swallows ENOENT.', + 'No issues in the parser, and the fallback path now swallows ENOENT.', + 'Missing configuration now silently disables authentication. No regressions were found by the existing tests.', + 'Missing configuration now silently disables authentication. No issues are apparent in the parser.', + 'The fallback path now swallows ENOENT, and no issues elsewhere.', + 'The diff adds a NO_FINDINGS check, but the regex misses lowercase input.', + `${FINDING}\n\`\`\`\nNo issues found.\n\`\`\``, + ]) { + for (const gate of ['structured', 'review'] as const) { + const r = classifyOutsideReview({ text: gate === 'review' ? `${text}\nRecommendation: ship because it looks fine.` : text, gate }); + expect({ text, gate, verdict: r.verdict, reason: r.reason }).toEqual({ text, gate, verdict: 'unverified', reason: 'untagged_review' }); + } + } + }); + + test("a review's Recommendation because-clause concludes under the same rules", () => { + const verdict = (text: string) => classifyOutsideReview({ text, gate: 'review' }).verdict; + expect(verdict('Recommendation: ship because I found no regressions in this change.')).toBe('clean'); + expect(verdict(`The change adds a retry cap.\nRecommendation: ship because no issues remain.`)).toBe('clean'); + expect(verdict('No issues in the parser.\nRecommendation: fix the fallback because it swallows ENOENT.')).toBe('unverified'); + expect(verdict(`${FINDING}\nRecommendation: ship because no regressions in the existing tests.`)).toBe('unverified'); + expect(verdict(`${FINDING}\nRecommendation: ship because no issues remain, but the fallback needs a follow-up.`)).toBe('unverified'); + expect(verdict('Recommendation: address the migration because no regressions were found in the parser change,\nbut the migration drops users_email_idx.')).toBe('unverified'); + }); +}); + describe('validateOutsideReview keeps the text-only shape for direct importers', () => { test('existing shapes are unchanged', () => { expect(validateOutsideReview('', 'review')).toEqual({ completed: false, reason: 'empty response' }); From 133cb360842d0a7cf88fc1d10dcb624f2b7b841f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 19 Oct 2026 14:11:16 -0700 Subject: [PATCH 2/5] v1.91.154.0 fix(cso): safe-status sets, withheld credential history, ASSERTION_FAILED, Rails read-only verify (#3302) * fix(cso): withhold excluded credential files from retained history; name failed witness assertions ASSERTION_FAILED * fix(cso): Rails verify replaces read-only copied config instead of writing into it * fix(cso): security expectations may list several safe statuses while excluding the vulnerable marker (cherry picked from commit aa9f45e0ece0e12152e7d02bf88e1559ee42b1eb) * docs(changelog): safe status sets * v1.91.154.0 stamp * style(cso): format merged snapshot history capture --- CHANGELOG.md | 12 ++++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- lib/cso/contracts.ts | 40 +++++++++++++++++++------- lib/cso/history.ts | 23 +++++++++++++++ lib/cso/preparation-docker.ts | 27 ++++++++++++----- lib/cso/snapshot.ts | 5 +++- lib/cso/verification.ts | 13 +++++++-- lib/cso/verifier.ts | 2 +- package.json | 2 +- test/cso-contracts.test.ts | 23 +++++++++++++-- test/cso-git-hardening.test.ts | 14 +++++++++ test/cso-history.test.ts | 34 +++++++++++++++++++++- test/cso-preparation-container.test.ts | 28 +++++++++++++++++- test/cso-verifier.test.ts | 13 +++++++++ 15 files changed, 211 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfe3ed984..74a29736d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## [1.91.154.0] - 2026-10-19 + +**/cso can verify Rails repairs in a read-only checkout, its retained Git history no longer brings back credential files the snapshot leaves out, a repair whose external assertions fail is now reported as `ASSERTION_FAILED` instead of a misleading `INCOMPATIBLE_INPUT`, and a security assertion can accept several safe rejection statuses while still requiring the secret to be absent.** The helper keeps `.env`, `master.key`, keys and agent configuration out of the readable tree, but `history.txt` captured `git log -p` in full, so a committed `.env` came back through its diff whenever the value wasn't credential-shaped enough for the redactor. Paid qualification run 39186328569 caught this when the private corpus's planted `.env` value showed up in the judge's helper state. + +### Fixed +- **Rails `verify` on read-only sources:** the helper writes its own test `config/database.yml` (plus an initializer and `storage.yml`) into the execution copy. The copy keeps the source's file modes, so in a read-only checkout (the eval producer seals every source file at 0444, and Nix or Bazel trees are the same) the write failed with `EACCES`. That surfaced as the generic `INVALID_SCHEMA` "unexpected or unsafe input", and no Rails repair could reach `runtime_tested`. Each helper file now replaces the copied file instead of opening it for writing, so it also never writes through a copied symlink. +- **`history.txt`:** every diff section that touches a path the snapshot excludes (`exclusion()`, the same rule the readable tree uses) is replaced with `diff withheld: ()`. This covers either side of a rename and any diff header that can't be parsed. Commit preambles and other files' diffs stay. +- **`verify` with an authenticated witness:** a receipt or observation mismatch is still `INCOMPATIBLE_INPUT`. Receipts that bind correctly but record failed external assertions now raise `ASSERTION_FAILED`, with the phase and the booted, legitimate and security outcome. Nothing new is certified either way. In run 39186328569, three repairs that rejected the attack with a different status than the oracle expected surfaced as `INCOMPATIBLE_INPUT`. + +### Changed +- **A security assertion's `expected.status` may list up to eight safe statuses** (for example `[403, 404]`), so a repair that refuses an attack with a different reasonable rejection still passes. A set is accepted only on a security assertion that also carries a `vulnerable` expectation, and only with `excludes` set, so the response must still withhold the vulnerable marker. The fixed and vulnerable oracles must stay provably mutually exclusive: a set that contains the vulnerable status is accepted only when the vulnerable response's `includes` contains the excluded marker. Legitimate controls and `vulnerable` expectations still take exactly one status, so a patch that answers every route with 403 still fails its controls. A no-op patch, a listed rejection that still leaks the marker, and an unlisted status all stay `inconclusive`. + ## [1.91.153.0] - 2026-10-19 **An outside review whose findings are untagged prose no longer reads `clean` because a "no issues" phrase appears somewhere in it, and a clean Codex review that says "no actionable regressions" no longer reads `unverified`.** Reported in #3294 by @SchizasNik, with the false-pass half pointed out by @xg-gh-25. `lib/outside-review-result.ts` matched its no-findings vocabulary anywhere in the reply, so "The parser change has no new issues. However, … drops users_email_idx" passed /ship's structured gate. It also lacked `regressions`, so `codex review --uncommitted`'s "I found no actionable regressions in the staged, unstaged, or untracked changes." was missing coverage. diff --git a/VERSION b/VERSION index eaffa97d8..7836617d0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.153.0 +1.91.154.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index fc7e7065c..4222e853e 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.153.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.154.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/lib/cso/contracts.ts b/lib/cso/contracts.ts index 1f06b4565..c1bc250af 100644 --- a/lib/cso/contracts.ts +++ b/lib/cso/contracts.ts @@ -180,7 +180,7 @@ export interface HttpAssertion { method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'; headers?: Record; body?: string; - expected: { status: number; includes?: string; excludes?: string }; + expected: { status: number | number[]; includes?: string; excludes?: string }; vulnerable?: { status: number; includes?: string; excludes?: string }; } export interface Command { @@ -648,16 +648,34 @@ export function validateVerificationObservation(value: unknown): VerificationObs inputHash: v.inputHash, }; } -function assertion(value: unknown, name: string): HttpAssertion { +const MAX_SAFE_STATUSES = 8; +function assertion(value: unknown, name: string, safeStatusSet = false): HttpAssertion { const v = object(value, name), expected = object(v.expected, `${name}.expected`); exact(v, ['name', 'path', 'method', 'headers', 'body', 'expected', 'vulnerable'], name); - const oracle = (x: Record, n: string) => { + const httpStatus = (s: unknown) => Number.isInteger(s) && (s as number) >= 100 && (s as number) <= 599; + const oracle = (x: Record, n: string, statusSet = false) => { exact(x, ['status', 'includes', 'excludes'], n); - if (!Number.isInteger(x.status) || x.status < 100 || x.status > 599) - throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + if (!Array.isArray(x.status)) { + if (!httpStatus(x.status)) throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + } else if (!statusSet) throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + else if ( + !x.status.length || + x.status.length > MAX_SAFE_STATUSES || + !x.status.every(httpStatus) || + new Set(x.status).size !== x.status.length + ) + throw new CsoError( + 'INVALID_SCHEMA', + `${n}.status must be 1..${MAX_SAFE_STATUSES} distinct HTTP statuses`, + ); + else if (typeof x.excludes !== 'string' || !x.excludes) + throw new CsoError( + 'INVALID_SCHEMA', + `${n} with a set of safe statuses must exclude the vulnerable marker`, + ); return { - status: x.status, + status: Array.isArray(x.status) ? [...x.status] : x.status, ...(x.includes === undefined ? {} : { includes: string(x.includes, `${n}.includes`) }), ...(x.excludes === undefined ? {} : { excludes: string(x.excludes, `${n}.excludes`) }), }; @@ -683,7 +701,7 @@ function assertion(value: unknown, name: string): HttpAssertion { method: oneOf(v.method, ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'], `${name}.method`), ...(Object.keys(headers).length ? { headers } : {}), ...(v.body === undefined ? {} : { body: string(v.body, `${name}.body`, 65536) }), - expected: oracle(expected, `${name}.expected`), + expected: oracle(expected, `${name}.expected`, safeStatusSet && v.vulnerable !== undefined), ...(v.vulnerable === undefined ? {} : { vulnerable: oracle(object(v.vulnerable), `${name}.vulnerable`) }), }; } @@ -692,13 +710,13 @@ export function securityAssertions(security: VerificationRequest['security']): H return Array.isArray(security) ? security : [security]; } function securityAssertion(value: unknown): VerificationRequest['security'] { - if (!Array.isArray(value)) return assertion(value, 'security'); + if (!Array.isArray(value)) return assertion(value, 'security', true); if (!value.length || value.length > MAX_SECURITY_ASSERTIONS) throw new CsoError( 'INVALID_SCHEMA', `security must be one assertion or 1..${MAX_SECURITY_ASSERTIONS} assertions`, ); - const assertions = value.map((x, i) => assertion(x, `security[${i}]`)); + const assertions = value.map((x, i) => assertion(x, `security[${i}]`, true)); if (new Set(assertions.map((x) => x.name)).size !== assertions.length) throw new CsoError('INVALID_SCHEMA', 'security assertion names must be distinct'); return assertions; @@ -805,7 +823,9 @@ export function validateVerificationRequest(input: unknown): VerificationRequest 'Verification needs a legitimate control, distinct before/fixed security oracles, existing tests, immutable test files, and boundary files', ); const mutuallyExclusive = ({ expected: secure, vulnerable }: HttpAssertion) => - secure.status !== vulnerable!.status || + (Array.isArray(secure.status) + ? !secure.status.includes(vulnerable!.status) + : secure.status !== vulnerable!.status) || (secure.includes !== undefined && vulnerable!.excludes !== undefined && secure.includes.includes(vulnerable!.excludes)) || diff --git a/lib/cso/history.ts b/lib/cso/history.ts index 86c6159cd..92c8b8082 100644 --- a/lib/cso/history.ts +++ b/lib/cso/history.ts @@ -57,6 +57,29 @@ export function gitDiffHeaderPaths(line: string): [string, string] | undefined { } } +/** + * Replace every diff section touching a path `withheld` names with one marker line, so a file the snapshot keeps + * out of the readable tree cannot come back through retained history. An unparseable diff header is withheld too. + */ +export function withholdHistory(raw: string, withheld: (path: string) => string | undefined): string { + const output: string[] = []; + let skipping = false; + for (const line of raw.split('\n')) { + if (line.startsWith('commit ')) skipping = false; + else if (line.startsWith('diff --git ')) { + const paths = gitDiffHeaderPaths(line)?.map((value) => value.replace(/^[ab]\//, '')), + reason = paths ? paths.map(withheld).find(Boolean) : 'unparseable diff header'; + skipping = Boolean(reason); + if (reason) { + output.push(`diff withheld: ${paths?.[1] ?? 'unknown path'} (${reason})`); + continue; + } + } + if (!skipping) output.push(line); + } + return output.join('\n'); +} + /** Return only exact path hunks, keeping one commit preamble per matching commit. */ export function historyForPath(raw: string, path: string): string | undefined { const expected = new Set([`a/${path}`, `b/${path}`]), diff --git a/lib/cso/preparation-docker.ts b/lib/cso/preparation-docker.ts index e36e0a34d..7874d8762 100644 --- a/lib/cso/preparation-docker.ts +++ b/lib/cso/preparation-docker.ts @@ -1106,6 +1106,25 @@ export async function superviseRegistrySocket(options: { * Requires qualified runtime images to contain the fixed, compiled * /opt/cso/preparation@1.0.0 helper contract. */ +/** + * Write the helper's own files (Rails' test database.yml and friends) into an execution copy. The copy keeps the + * source's modes, and a sealed or read-only checkout makes those files 0444, so each copied file is replaced, never + * opened for writing (or written through). + */ +export function writeOfflineTransformations( + source: string, + transformations: OfflinePreparationRequest['transformations'], +): void { + for (const transformation of transformations) { + if (sha256(transformation.content) !== transformation.sha256) + fail('INCOMPATIBLE_INPUT', 'Offline transformation content hash changed'); + const file = contained(source, transformation.path); + secureDirectory(resolve(file, '..')); + fs.rmSync(file, { force: true }); + fs.writeFileSync(file, transformation.content, { mode: 0o600, flag: 'wx' }); + } +} + export class DockerPreparationSandboxRunner implements PreparationSandboxRunner { readonly qualification; private readonly runtime; @@ -1514,13 +1533,7 @@ export class DockerPreparationSandboxRunner implements PreparationSandboxRunner errorOnExist: false, preserveTimestamps: false, }); - for (const transformation of request.transformations) { - if (sha256(transformation.content) !== transformation.sha256) - fail('INCOMPATIBLE_INPUT', 'Offline transformation content hash changed'); - const file = contained(source, transformation.path); - secureDirectory(resolve(file, '..')); - fs.writeFileSync(file, transformation.content, { mode: 0o600 }); - } + writeOfflineTransformations(source, request.transformations); for (const item of request.metadata) { if (sha256(item.content) !== item.sha256) fail('INCOMPATIBLE_INPUT', `Offline metadata hash changed: ${item.path}`); diff --git a/lib/cso/snapshot.ts b/lib/cso/snapshot.ts index feb41b43e..7899dc0fd 100644 --- a/lib/cso/snapshot.ts +++ b/lib/cso/snapshot.ts @@ -14,6 +14,7 @@ import { snapshotPathId, MAX_OUTPUT, } from './contracts'; +import { withholdHistory } from './history'; import { childEnvironment, executable, git, redact, runProcess } from './process'; import { boundedList, readJson, secureDirectory, writeHelperJson, writeJson } from './state'; import { scan } from '../redact-engine'; @@ -952,7 +953,9 @@ export async function capture( if (!(error instanceof CsoError) || error.code !== 'MISSING_INPUT') throw error; } guard(); - const commits = (patched ?? '').split(/^(?=commit [0-9a-f]{40}(?:[0-9a-f]{24})?$)/m).filter(Boolean), + const commits = withholdHistory(patched ?? '', exclusion) + .split(/^(?=commit [0-9a-f]{40}(?:[0-9a-f]{24})?$)/m) + .filter(Boolean), sizes = commits.map((commit) => Buffer.byteLength(commit)), whole = sizes.reduce((sum, size) => sum + size, 0) <= MAX_OUTPUT; let kept = 0, diff --git a/lib/cso/verification.ts b/lib/cso/verification.ts index 6ccf9ef83..4b4807ff2 100644 --- a/lib/cso/verification.ts +++ b/lib/cso/verification.ts @@ -1219,14 +1219,21 @@ export function certify(params: { beforeReceipt.observationHash !== witnessObservationHash(beforeObservation) || afterReceipt.observationHash !== witnessObservationHash(afterObservation) || params.before.existingTests !== beforeReceipt.diagnosticTestsPassed || - params.after.existingTests !== afterReceipt.diagnosticTestsPassed || - !beforeReceipt.externalAssertionsPassed || - !afterReceipt.externalAssertionsPassed + params.after.existingTests !== afterReceipt.diagnosticTestsPassed ) throw new CsoError( 'INCOMPATIBLE_INPUT', 'Authenticated assertion witness receipts do not match the verifier observations', ); + for (const [phase, receipt] of [ + ['before', beforeReceipt], + ['after', afterReceipt], + ] as const) + if (!receipt.externalAssertionsPassed) + throw new CsoError( + 'ASSERTION_FAILED', + `External assertions failed in the ${phase} phase: booted ${params[phase].booted}, legitimate ${params[phase].legitimate}, security ${params[phase].security}`, + ); const stableExecutions = (receipt: AssertionWitnessReceipt) => receipt.executions.map(({ outputHash: _, ...execution }) => execution); if (canonical(stableExecutions(beforeReceipt)) !== canonical(stableExecutions(afterReceipt))) diff --git a/lib/cso/verifier.ts b/lib/cso/verifier.ts index f3364b764..88b61a3e0 100644 --- a/lib/cso/verifier.ts +++ b/lib/cso/verifier.ts @@ -18,7 +18,7 @@ interface Config { } function matches(status: number, body: string, oracle: HttpAssertion['expected']): boolean { return ( - status === oracle.status && + (Array.isArray(oracle.status) ? oracle.status.includes(status) : status === oracle.status) && (oracle.includes === undefined || body.includes(oracle.includes)) && (oracle.excludes === undefined || !body.includes(oracle.excludes)) ); diff --git a/package.json b/package.json index 99f8e4a4b..e11883f58 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.153", + "version": "1.91.154", "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/test/cso-contracts.test.ts b/test/cso-contracts.test.ts index a3434a54f..3f8f452be 100644 --- a/test/cso-contracts.test.ts +++ b/test/cso-contracts.test.ts @@ -18,8 +18,8 @@ const finding:any={title:'Cross-tenant read',rootCause:'Tenant query omits calle const request=(before:string,after:string)=>({findingId:'a'.repeat(32),runtimeProfile:'node',port:3456,start:{executable:'/usr/local/bin/node',args:['app.js']},legitimate:[{name:'health',path:'/health',method:'GET',expected:{status:200,includes:'ok'}}],security:{name:'tenant isolation',path:'/user?id=2',method:'GET',expected:{status:403},vulnerable:{status:200,includes:'tenant-b'}},existingTests:[{executable:'/usr/local/bin/node',args:['test.js']}],testFiles:['test.js'],fixtures:{'tenant.json':'{}'},boundaryFiles:['app.js'],changes:[{path:'app.js',beforeSha256:sha256(before),after,effect:'source'}],review:{reviewer:'independent-reviewer',independent:true,rootCauseRepaired:true,featurePreserved:true,boundaryMocks:false,rationale:'Predicate now binds the caller tenant and health behavior is preserved',reviewedPatchHash:''}}); // Structural signed fixture for bundle-reader mutation tests. Production // issuance uses AssertionWitnessSession in verifyRepair instead. -function authenticatedBundleFixture(certified:ReturnType,params:any,outcomes?:Record<'before'|'after',{executedTests:number;passingTests:number}>):any{ - const verification=structuredClone(certified.manifest),identityRequest=params.identityRequest??params.request,pair=crypto.generateKeyPairSync('ed25519'),publicKey=pair.publicKey.export({format:'der',type:'spki'}).toString('hex'),keyId=sha256(Buffer.from(publicKey,'hex')),createdAt=verification.createdAt,runner={testToolchain:params.testToolchain,startPlanHash:verification.startPlanHash,testPlanHash:verification.testPlanHash,commandsHash:sha256(canonical(params.request.existingTests)),minimumPassingTestsHash:sha256(canonical(params.request.existingTests.map(()=>1)))},make=(phase:'before'|'after')=>{const observation=verification[phase],counts=outcomes?.[phase]??{executedTests:1,passingTests:1},binding:any={schemaVersion:1,protocol:'gstack-cso-assertion-witness-v1',nonce:sha256(`${phase}:${createdAt}`),phase,issuedAt:createdAt,expiresAt:new Date(Date.parse(createdAt)+300_000).toISOString(),runId:params.runId,findingId:identityRequest.findingId,policyHash:verification.policyHash,auditPolicyHash:verification.auditPolicyHash,runtime:{image:params.runtime.image,verifierImage:params.verifier.image,platform:params.runtime.platform,profile:params.runtime.id},runner,sourceHash:phase==='before'?verification.beforeSourceHash:verification.afterSourceHash,dependencyHash:phase==='before'?verification.beforeDependencies:verification.afterDependencies,configurationHash:phase==='before'?verification.beforeConfiguration:verification.afterConfiguration,requestHash:verification.requestHash,patchHash:verification.patchHash,harnessHash:verification.harnessHash,assertionHash:sha256(canonical({legitimate:identityRequest.legitimate,security:identityRequest.security})),fixturesHash:verification.fixturesHash},executions=params.request.existingTests.map((command:any)=>({commandHash:sha256(canonical(command)),exitCode:0,outputHash:sha256(`structural signed fixture ${phase}`),minimumPassingTests:1,...counts,reportedPassed:counts.passingTests>=1})),unsigned:any={schemaVersion:1,binding,keyId,publicKey,observationHash:witnessObservationHash(observation),externalAssertionsPassed:true,diagnosticTestsPassed:executions.every((execution:any)=>execution.reportedPassed),executions};return{...unsigned,signature:crypto.sign(null,Buffer.from(canonical(unsigned)),pair.privateKey).toString('hex')}};const witness={before:make('before'),after:make('after')};verification.assertionAssurance='authenticated_out_of_process';verification.testCompletionAssurance='self_reported';verification.witnessHash=assertionWitnessPairHash(witness);verification.result='runtime_tested';verification.id='';const id=verificationIdentity(verification);verification.id=id; +function authenticatedBundleFixture(certified:ReturnType,params:any,outcomes?:Record<'before'|'after',{executedTests:number;passingTests:number}>,externalAssertionsPassed:Record<'before'|'after',boolean>={before:true,after:true}):any{ + const verification=structuredClone(certified.manifest),identityRequest=params.identityRequest??params.request,pair=crypto.generateKeyPairSync('ed25519'),publicKey=pair.publicKey.export({format:'der',type:'spki'}).toString('hex'),keyId=sha256(Buffer.from(publicKey,'hex')),createdAt=verification.createdAt,runner={testToolchain:params.testToolchain,startPlanHash:verification.startPlanHash,testPlanHash:verification.testPlanHash,commandsHash:sha256(canonical(params.request.existingTests)),minimumPassingTestsHash:sha256(canonical(params.request.existingTests.map(()=>1)))},make=(phase:'before'|'after')=>{const observation=verification[phase],counts=outcomes?.[phase]??{executedTests:1,passingTests:1},binding:any={schemaVersion:1,protocol:'gstack-cso-assertion-witness-v1',nonce:sha256(`${phase}:${createdAt}`),phase,issuedAt:createdAt,expiresAt:new Date(Date.parse(createdAt)+300_000).toISOString(),runId:params.runId,findingId:identityRequest.findingId,policyHash:verification.policyHash,auditPolicyHash:verification.auditPolicyHash,runtime:{image:params.runtime.image,verifierImage:params.verifier.image,platform:params.runtime.platform,profile:params.runtime.id},runner,sourceHash:phase==='before'?verification.beforeSourceHash:verification.afterSourceHash,dependencyHash:phase==='before'?verification.beforeDependencies:verification.afterDependencies,configurationHash:phase==='before'?verification.beforeConfiguration:verification.afterConfiguration,requestHash:verification.requestHash,patchHash:verification.patchHash,harnessHash:verification.harnessHash,assertionHash:sha256(canonical({legitimate:identityRequest.legitimate,security:identityRequest.security})),fixturesHash:verification.fixturesHash},executions=params.request.existingTests.map((command:any)=>({commandHash:sha256(canonical(command)),exitCode:0,outputHash:sha256(`structural signed fixture ${phase}`),minimumPassingTests:1,...counts,reportedPassed:counts.passingTests>=1})),unsigned:any={schemaVersion:1,binding,keyId,publicKey,observationHash:witnessObservationHash(observation),externalAssertionsPassed:externalAssertionsPassed[phase],diagnosticTestsPassed:executions.every((execution:any)=>execution.reportedPassed),executions};return{...unsigned,signature:crypto.sign(null,Buffer.from(canonical(unsigned)),pair.privateKey).toString('hex')}};const witness={before:make('before'),after:make('after')};verification.assertionAssurance='authenticated_out_of_process';verification.testCompletionAssurance='self_reported';verification.witnessHash=assertionWitnessPairHash(witness);verification.result='runtime_tested';verification.id='';const id=verificationIdentity(verification);verification.id=id; return{schemaVersion:3,runId:params.runId,id,createdAt,expiresAt:new Date(Date.parse(createdAt)+30*86400_000).toISOString(),requiredInputs:{sourceHash:params.manifest.executionHash,originalHash:params.manifest.originalHash,runtimeImage:params.runtime.image,platform:params.runtime.platform,archives:params.archives,...(params.dependencyClosures?{dependencyClosures:params.dependencyClosures}:{})},request:identityRequest,verification,transformations:params.manifest.entries.filter((entry:any)=>entry.transformation),...(params.preparation?{preparation:params.preparation}:{}),witness}; } @@ -170,6 +170,14 @@ describe('tested repair certificate gate',()=>{ const params:any={runId:'run',manifest:{originalHash:'d'.repeat(64),executionHash:treeHash(before),entries:[]},request:req,runtime:{id:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'},verifier:{image:'runtime@sha256:'+'a'.repeat(64)},before:{booted:true,legitimate:true,security:'intended_failure',existingTests:true,output:'before',inputHash:''},after:{booted:true,legitimate:true,security:'pass',existingTests:true,output:'after',inputHash:''},beforeRoot:before,afterRoot:after,policyHash:'c'.repeat(64),auditPolicyHash:'e'.repeat(64),archives:[],testToolchain:'runtime'},unsigned=certify(params),witness=authenticatedBundleFixture(unsigned,params,{before:{executedTests:2,passingTests:2},after:{executedTests:2,passingTests:1}}).witness; expect(()=>certify({...params,witness})).toThrow('existing-test execution count or outcome'); }); + test('a witness whose external assertions failed is an assertion failure, and a mismatched observation stays incompatible input',()=>{ + const root=tmp(),before=path.join(root,'before'),after=path.join(root,'after');fs.mkdirSync(before);fs.writeFileSync(path.join(before,'app.js'),'vulnerable\n');fs.writeFileSync(path.join(before,'test.js'),'one registered test\n');const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash=patchHash(raw);const req=validateVerificationRequest(raw);preparePatchedSource(before,after,req); + const params:any={runId:'run',manifest:{originalHash:'d'.repeat(64),executionHash:treeHash(before),entries:[]},request:req,runtime:{id:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'},verifier:{image:'runtime@sha256:'+'a'.repeat(64)},before:{booted:true,legitimate:true,security:'intended_failure',existingTests:true,output:'before',inputHash:''},after:{booted:true,legitimate:true,security:'inconclusive',existingTests:true,output:'after',inputHash:''},beforeRoot:before,afterRoot:after,policyHash:'c'.repeat(64),auditPolicyHash:'e'.repeat(64),archives:[],testToolchain:'runtime'},unsigned=certify(params); + const failed=authenticatedBundleFixture(unsigned,params,undefined,{before:true,after:false}).witness; + expect(()=>certify({...params,witness:failed})).toThrow(expect.objectContaining({code:'ASSERTION_FAILED',message:'External assertions failed in the after phase: booted true, legitimate true, security inconclusive'})); + const mismatched=authenticatedBundleFixture({...unsigned,manifest:{...unsigned.manifest,after:{...unsigned.manifest.after,output:'a different observation'}}},params,undefined,{before:true,after:false}).witness; + expect(()=>certify({...params,witness:mismatched})).toThrow(expect.objectContaining({code:'INCOMPATIBLE_INPUT'})); + }); test('canonical Python starts use direct prepared launchers and reject root framework shadows',()=>{ const pin=(name:string)=>`${name}==1.0.0 --hash=sha256:${'a'.repeat(64)}\n`; const flask=tmp();fs.writeFileSync(path.join(flask,'requirements.txt'),pin('Flask'));fs.writeFileSync(path.join(flask,'app.py'),'from flask import Flask\napp = Flask(__name__)\n');const flaskCommand=canonicalStartPlan(flask,'python',3456).command;expect(flaskCommand.executable).toBe('/work/.venv/bin/python');expect(flaskCommand.args.slice(0,3)).toEqual(['-I','-S','-c']);expect(flaskCommand.args[3]).toContain('from flask.cli import main');expect(flaskCommand.args.slice(4)).toEqual(['--app','app:app','run','--host','127.0.0.1','--port','3456']);fs.writeFileSync(path.join(flask,'flask.py'),'raise RuntimeError("shadow")\n');expect(()=>canonicalStartPlan(flask,'python',3456)).toThrow('root import shadow'); @@ -186,6 +194,17 @@ describe('tested repair certificate gate',()=>{ const runDir=tmp(),snapshot=path.join(runDir,'snapshot');fs.mkdirSync(snapshot);const files:Record={'package.json':JSON.stringify({scripts:{test:'jest'}}),'package-lock.json':'{}\n','app.js':'server.listen(3456)\n','app.test.js':'test("ok",()=>{})\n'};for(const [name,body] of Object.entries(files))fs.writeFileSync(path.join(snapshot,name),body);const plan=canonicalTestPlan(snapshot,'node'),raw:any=request(files['app.js'],'server.listen(3456)\n');raw.start={executable:'/usr/local/bin/node',args:['app.js']};raw.existingTests=plan.commands;raw.testFiles=plan.files;raw.changes=[{path:'package-lock.json',beforeSha256:sha256(files['package-lock.json']),after:'{"changed":true}\n',effect:'dependency'}];raw.review.reviewedPatchHash=patchHash(raw);const entries=Object.entries(files).map(([path,body])=>({path,originalHash:sha256(body),executionHash:sha256(body),bytes:Buffer.byteLength(body),mode:0o600})),manifest:any={version:3,createdAt:'2026-01-01T00:00:00Z',expiresAt:'2026-01-08T00:00:00Z',root:'/repo',headCommit:'a'.repeat(40),originalHash:'b'.repeat(64),executionHash:treeHash(snapshot),entries};let executed=false;await expect(verifyRepair({runId:'run',runDir,manifest,rawRequest:raw,runtime:{id:'node',stack:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'} as any,verifier:{id:'node',stack:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'} as any,policyHash:'c'.repeat(64),archives:[],executor:{observe:async()=>{executed=true;throw new Error('must not run');}},persist:false})).rejects.toMatchObject({code:'PREREQUISITE'});expect(executed).toBe(false); }); test('overlapping security oracles and no-op repairs cannot be certified',()=>{const overlapping:any=request('vulnerable\n','fixed\n');overlapping.review.reviewedPatchHash='pending';overlapping.security.vulnerable={...overlapping.security.expected};expect(()=>validateVerificationRequest(overlapping)).toThrow('mutually exclusive');const noop:any=request('vulnerable\n','vulnerable\n');noop.review.reviewedPatchHash='pending';expect(()=>validateVerificationRequest(noop)).toThrow('material patch');}); + test('a security expectation may list several safe statuses only while it excludes the vulnerable marker',()=>{ + const make=(edit:(raw:any)=>void)=>{const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash='pending';edit(raw);return raw;}; + expect(validateVerificationRequest(make(raw=>{raw.security.expected={status:[403,404],excludes:'tenant-b'};})).security).toMatchObject({expected:{status:[403,404],excludes:'tenant-b'}}); + expect(validateVerificationRequest(make(raw=>{raw.security.expected={status:[200,403],excludes:'tenant-b'};})).security).toMatchObject({expected:{status:[200,403]}}); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status:[200,403],excludes:'unrelated'};}))).toThrow('mutually exclusive'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status:[403,404]};}))).toThrow('must exclude the vulnerable marker'); + for(const status of [[],[403,403],[99,403],[400,401,403,404,405,409,410,422,429]])expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status,excludes:'tenant-b'};}))).toThrow('distinct HTTP statuses'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.vulnerable={status:[200,206],includes:'tenant-b'};}))).toThrow('security.vulnerable.status must be an HTTP status'); + expect(()=>validateVerificationRequest(make(raw=>{raw.legitimate[0].expected={status:[200,403],excludes:'denied'};}))).toThrow('legitimate[0].expected.status must be an HTTP status'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security=[{...raw.security,expected:{status:[403,404],excludes:'tenant-b'}},{...raw.security,name:'second',expected:{status:[403,404]}}];}))).toThrow('security[1].expected with a set of safe statuses'); + }); test('review is a separate content-bound but explicitly self-attested artifact',()=>{const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash=patchHash(raw);const req=validateVerificationRequest(raw),artifact=makeReviewArtifact('run',req,'producer-session');raw.review.artifactId=artifact.id;const bound=validateVerificationRequest(raw);expect(validateReviewArtifact(artifact,'run',bound)).toMatchObject({producer:'producer-session',assurance:'self_attested'});expect(()=>makeReviewArtifact('run',req,req.review.reviewer)).toThrow('distinct');expect(()=>validateReviewArtifact({...artifact,rationale:'changed'},'run',bound)).toThrow('does not bind');}); test('executor observations have a strict helper-owned shape',()=>{expect(()=>validateVerificationObservation({booted:true,legitimate:true,security:'pass',existingTests:true,output:'ok',inputHash:'',tested:true})).toThrow('Unexpected');expect(()=>validateVerificationObservation({booted:'yes',legitimate:true,security:'pass',existingTests:true,output:'ok',inputHash:''})).toThrow('booleans');}); test('secret-bearing fixtures cannot enter a replayable verification bundle',async()=>{const runDir=tmp(),snapshot=path.join(runDir,'snapshot');fs.mkdirSync(snapshot);fs.writeFileSync(path.join(snapshot,'app.js'),'vulnerable\n');const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash=patchHash(raw);raw.fixtures={'secret.txt':CREDENTIAL_CANARY};const manifest:any={entries:[{path:'app.js',transformation:undefined}]};const runtime:any={id:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'};expect(verifyRepair({runId:'run',runDir,manifest,rawRequest:raw,runtime,verifier:runtime,policyHash:'c'.repeat(64),archives:[],executor:{observe:async()=>{throw new Error('must not execute');}},persist:false})).rejects.toMatchObject({code:'REDACTION_FAILED'});}); diff --git a/test/cso-git-hardening.test.ts b/test/cso-git-hardening.test.ts index 567b1e1f7..cbea74b7a 100644 --- a/test/cso-git-hardening.test.ts +++ b/test/cso-git-hardening.test.ts @@ -92,6 +92,20 @@ describe('CSO Git metadata hardening',()=>{ expect(history).not.toContain('replacement-history'); }); + test.skipIf(process.platform==='win32')('retained history withholds diffs of credential files the snapshot excludes',async()=>{ + const {repo,runDir,git}=fixture(); + fs.writeFileSync(path.join(repo,'.env'),'UPSTREAM_TOKEN=planted-history-value\n');fs.writeFileSync(path.join(repo,'tracked.ts'),'export const tracked = false\n'); + git('add','.env','tracked.ts');git('commit','-qm','configure upstream'); + + await capture(repo,runDir); + const history=fs.readFileSync(path.join(runDir,'history.txt'),'utf8'); + + expect(history).not.toContain('planted-history-value'); + expect(history).toContain('diff withheld: .env (credential or execution configuration)'); + expect(history).toContain('+export const tracked = false'); + expect(history).toContain('Subject: configure upstream'); + }); + test.skipIf(process.platform==='win32')('rejects repository config includes before Git can consume them',async()=>{ const {root,repo,runDir}=fixture(),included=path.join(root,'included.conf'); fs.writeFileSync(included,'[core]\n\tworktree = /tmp/cso-decoy\n'); diff --git a/test/cso-history.test.ts b/test/cso-history.test.ts index 0e32b107c..71cde803b 100644 --- a/test/cso-history.test.ts +++ b/test/cso-history.test.ts @@ -1,5 +1,5 @@ import { describe,expect,test } from 'bun:test'; -import { gitDiffHeaderPaths,historyForPath } from '../lib/cso/history'; +import { gitDiffHeaderPaths,historyForPath,withholdHistory } from '../lib/cso/history'; describe('CSO retained Git history path filtering',()=>{ test('matches exact paths without leaking a prefix sibling hunk',()=>{ @@ -25,6 +25,38 @@ diff --git a/foo-extra b/foo-extra expect(historyForPath(raw,'café file.ts')).toContain('+unicode-only'); }); + test('withholds every diff section touching an excluded path, keeping commit preambles and other files',()=>{ + const raw=`commit ${'c'.repeat(40)} +Subject: add config +diff --git a/.env b/.env +--- /dev/null ++++ b/.env ++PLANTED_VALUE=excluded-file-body +diff --git a/app.ts b/app.ts +--- a/app.ts ++++ b/app.ts ++kept-line +commit ${'d'.repeat(40)} +Subject: rename +diff --git a/config/old.txt b/config/master.key +rename from config/old.txt +rename to config/master.key ++renamed-into-excluded +diff --git "a/foo b/foo ++unparseable-body +`; + const kept=withholdHistory(raw,(path)=>/(^|\/)(\.env|master\.key)$/.test(path)?'credential or execution configuration':undefined); + expect(kept).not.toContain('excluded-file-body'); + expect(kept).not.toContain('renamed-into-excluded'); + expect(kept).not.toContain('unparseable-body'); + expect(kept).toContain('diff withheld: .env (credential or execution configuration)'); + expect(kept).toContain('diff withheld: config/master.key (credential or execution configuration)'); + expect(kept).toContain('diff withheld: unknown path (unparseable diff header)'); + expect(kept).toContain('+kept-line'); + expect(kept).toContain('Subject: add config'); + expect(kept).toContain('Subject: rename'); + }); + test('rejects malformed quoted headers instead of broadening the match',()=>{ expect(gitDiffHeaderPaths('diff --git "a/foo b/foo')).toBeUndefined(); expect(historyForPath('diff --git "a/foo b/foo\n+secret\n','foo')).toBeUndefined(); diff --git a/test/cso-preparation-container.test.ts b/test/cso-preparation-container.test.ts index eb8b42581..6cde0dc9a 100644 --- a/test/cso-preparation-container.test.ts +++ b/test/cso-preparation-container.test.ts @@ -3,8 +3,9 @@ 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 { createHash } from 'node:crypto'; import { createPreparedExport, recordNpmArchiveEntry } from '../lib/cso/preparation-container'; -import { materializePreparedExport } from '../lib/cso/preparation-docker'; +import { materializePreparedExport, writeOfflineTransformations } from '../lib/cso/preparation-docker'; import { heldApplicationReady, validateSingleContainerProcessOutput } from '../lib/cso/docker'; const roots: string[] = []; @@ -68,3 +69,28 @@ describe('CSO preparation container archive limits', () => { expect(heldApplicationReady('PID COMMAND\n1 sh\n')).toBe(false); }); }); + +describe('offline transformations in an execution copy', () => { + test.skipIf(process.platform === 'win32')('replace a read-only copied file under a read-only directory instead of writing through it', () => { + const source = temporary(), config = path.join(source, 'config'), database = path.join(config, 'database.yml'), outside = path.join(temporary(), 'outside.yml'); + fs.mkdirSync(config); + fs.writeFileSync(database, 'production: sealed\n'); + fs.chmodSync(database, 0o444); + fs.chmodSync(config, 0o555); + fs.writeFileSync(outside, 'untouched\n'); + const content = 'test:\n adapter: sqlite3\n', sha256 = createHash('sha256').update(content).digest('hex'); + writeOfflineTransformations(source, [ + { path: 'config/database.yml', content, sha256, reason: 'helper test configuration' }, + { path: 'config/initializers/zzzz_cso_test.rb', content, sha256, reason: 'helper test configuration' }, + ]); + expect(fs.readFileSync(database, 'utf8')).toBe(content); + expect(fs.statSync(database).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(path.join(config, 'initializers', 'zzzz_cso_test.rb'), 'utf8')).toBe(content); + expect(() => writeOfflineTransformations(source, [{ path: 'config/database.yml', content, sha256: 'f'.repeat(64), reason: 'tampered' }])).toThrow('Offline transformation content hash changed'); + fs.rmSync(database); + fs.symlinkSync(outside, database); + writeOfflineTransformations(source, [{ path: 'config/database.yml', content, sha256, reason: 'helper test configuration' }]); + expect(fs.lstatSync(database).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(outside, 'utf8')).toBe('untouched\n'); + }); +}); diff --git a/test/cso-verifier.test.ts b/test/cso-verifier.test.ts index 48ab1e1b9..95cd725c0 100644 --- a/test/cso-verifier.test.ts +++ b/test/cso-verifier.test.ts @@ -65,6 +65,19 @@ describe('CSO verifier security judgement',()=>{ try{expect((await judgeSecurity('before',payloads,disproved.port)).security).toBe('pass');}finally{disproved.server.stop(true);} }); + test('a set of safe statuses passes any listed rejection that withholds the marker and nothing else',async()=>{ + const dotfile:HttpAssertion={name:'dotfile',path:'/.env',method:'GET',expected:{status:[403,404],excludes:'DOTFILE'},vulnerable:{status:200,includes:'DOTFILE'}}; + const answer=async(status:number,body:string)=>{ + const server=Bun.serve({port:0,hostname:'127.0.0.1',fetch:()=>new Response(body,{status})}); + try{return [(await judgeSecurity('before',dotfile,Number(server.port))).security,(await judgeSecurity('after',dotfile,Number(server.port))).security];}finally{server.stop(true);} + }; + expect(await answer(404,'not found')).toEqual(['pass','pass']); + expect(await answer(403,'denied')).toEqual(['pass','pass']); + expect(await answer(200,'DOTFILE=1')).toEqual(['intended_failure','inconclusive']); + expect(await answer(403,'denied DOTFILE=1')).toEqual(['inconclusive','inconclusive']); + expect(await answer(400,'bad request')).toEqual(['inconclusive','inconclusive']); + }); + test('a single assertion keeps its original before and after outcomes',async()=>{ const vulnerable=serve(new Set()),fixed=serve(ALL); try{ From 0c87a553c75c42c3c112eb66f203d6785ef2b8a6 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 19 Oct 2026 14:30:33 -0700 Subject: [PATCH 3/5] v1.91.155.0 fix(browse): a busy tab no longer blocks every command with "Daemon busy" (port of #3250) (#3301) * fix(browse): /health status is daemon liveness; active tab reported apart (#3247) /health answered `status` from a page.evaluate('1') race on the active tab, so one page stuck in a long script made the whole daemon read as unhealthy. The CLI gates every command on that answer, and its 2000 ms fetch timeout equalled the server's 2000 ms race, so it could not even read the reply: status, tabs, tab, newtab and stop all failed with "Daemon busy" until --force-restart threw the session away. - BrowserManager.isHealthy() is liveness only (Chromium connected). - probeActiveTab() reports { id, responsive }, bounded by TAB_PROBE_TIMEOUT_MS (1s); /health returns it as `activeTab`. - `status` adds a line naming a not-responding active tab and the commands that leave it. - The CLI reads /health through one helper whose timeout, HEALTH_REQUEST_TIMEOUT_MS (2s), stays above the probe. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit 322d2553545c15795da7271a6f70122bc36aa561) * fix(browse): bound per-tab reads in `tabs` and `state save` (#3247) A renderer stuck in a long script never answers page.title() or page.evaluate(). `tabs` awaited each title in turn and `state save` read each tab's storage unbounded, so both hung on one busy tab even though neither needs it. - getTabListWithTitles() reads titles in parallel, each bounded by TAB_PROBE_TIMEOUT_MS; a tab that misses it is listed as "(not responding)". - saveState() skips the storage of a tab that does not answer within 5s, the same outcome as a tab whose read fails. busy-active-tab.test.ts holds a real Chromium tab in a busy loop next to an idle one and checks /health, tabs, status, saveState and closetab against it. On main, /health takes 2004 ms and says unhealthy, `tabs` hangs past the test timeout and `status` never names the tab. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit d010d7b243a7c9039dfb06469d66a193e7e37556) * fix(browse): name the busy tab when a command times out (#3247) A command that needs a stuck tab still waits out the CLI's 30s reply timeout, and the report should say why. It never ran: Bun rejects an AbortSignal.timeout() fetch with TimeoutError, the generic branch only matched AbortError, so a timeout fell through to "[browse] The operation timed out." - isFetchTimeout() accepts both names; the cookie-import branch uses it too. - commandTimeoutReport() reads /health: when `activeTab.responsive` is false it names the tab and points at `browse tab ` and `browse closetab `; a live daemon otherwise keeps the busy-not-restarting line and a dead one the plain timeout line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit ef70b7d88094f64c6d57d37191045a83bb7d834c) * docs(browse): /health liveness vs active-tab responsiveness (#3247) BROWSER.md's busy-vs-dead lifecycle note, ARCHITECTURE.md's daemon model and the REMOTE_BROWSER_ACCESS /health reference now say that `status` is daemon liveness, that `activeTab` reports the active tab separately, and which commands keep working while a tab is busy. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit 3dbb256796cf3c99e687c4622c255f9dca7f8f96) * fix(browse): one pending probe per stuck tab, not one per poll (#3247) Review on #3250: the 1s race stops waiting but leaves page.evaluate() queued in the stuck renderer, so every /health poll (the extension polls every 10s), `status` and `tabs` call added another pending read, all drained at once when the page recovered. Playwright cannot cancel an in-flight evaluate, so the active-tab probe and the `tabs` title read now join the page's pending read of the same kind instead of starting a new one. busy-active-tab.test.ts counts page.evaluate calls across three probes of the stuck tab: 3 before this change, at most 1 after. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit 19fc9b97093dd954c6f69cbe0b700c25a11ef771) * fix(browse): keep a busy tab's last good storage in saved state (#3247) Review on #3250: with BROWSE_PERSIST_STATE=1 the periodic snapshot used to hang on a stuck tab, so session-state.json kept its last good copy. The 5s storage bound let the snapshot finish and write `storage: null` for that tab over it, so a restart lost its localStorage and sessionStorage even with persistence on. saveState() now records the storage each page answers with, together with its origin. A read that times out returns that last good storage when the origin still matches, and null otherwise. A failed read stays null, as before. The storage read also joins the page's pending read instead of queueing a new one on every persist tick. busy-active-tab.test.ts takes a good snapshot, hangs the tab, and checks that persistSessionState() still writes its localStorage. Live, with BROWSE_PERSIST_STATE=1 and a 2s interval, session-state.json kept {"probe":"kept"} for the busy tab, and its savedAt stayed current through 12s of busy page. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL (cherry picked from commit 60cbaebee5dd11befa8348212d6a6041b6acf354) * docs(changelog): busy-tab lockup fix from #3250 under [Unreleased] Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: Andrei Vershinin Co-authored-by: Claude Opus 5.5 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- ARCHITECTURE.md | 2 +- BROWSER.md | 8 ++ CHANGELOG.md | 11 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- browse/src/browser-manager.ts | 152 +++++++++++++++++---- browse/src/cli.ts | 98 ++++++++----- browse/src/meta-commands.ts | 7 +- browse/src/routes/core.ts | 8 +- browse/test/busy-active-tab.test.ts | 132 ++++++++++++++++++ browse/test/command-timeout-report.test.ts | 66 +++++++++ browse/test/terminal-agent.test.ts | 7 +- docs/REMOTE_BROWSER_ACCESS.md | 4 +- package.json | 2 +- 14 files changed, 429 insertions(+), 72 deletions(-) create mode 100644 browse/test/busy-active-tab.test.ts create mode 100644 browse/test/command-timeout-report.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4ff86a04f..d9e8082cd 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -91,7 +91,7 @@ The server writes `.gstack/browse.json` (atomic write via tmp + rename, mode 0o6 { "pid": 12345, "port": 34567, "token": "uuid-v4", "startedAt": "...", "binaryVersion": "abc123" } ``` -The CLI reads this file to find the server. If the file is missing or the daemon process is dead, the CLI spawns a new server. A process that is alive but not answering `/health` is busy, not dead: the CLI probes for a bounded ~8s, then reports busy with a nonzero exit — only an explicit `--force-restart` kills a live daemon. Process liveness uses signal-0 (`isProcessAlive`, EPERM counts as alive) on every platform, with the health check (GET /health) as the responsiveness signal. Daemon stdout/stderr persists to `/.gstack/browse-daemon.log`. +The CLI reads this file to find the server. If the file is missing or the daemon process is dead, the CLI spawns a new server. A process that is alive but not answering `/health` is busy, not dead: the CLI probes for a bounded ~8s, then reports busy with a nonzero exit — only an explicit `--force-restart` kills a live daemon. Process liveness uses signal-0 (`isProcessAlive`, EPERM counts as alive) on every platform, with the health check (GET /health) as the responsiveness signal. Its `status` covers the daemon and its Chromium connection only; the active tab's main thread is reported separately (`activeTab.responsive`, a probe bounded to 1s), so one page stuck in a long script never makes the CLI refuse commands. Daemon stdout/stderr persists to `/.gstack/browse-daemon.log`. ### Port selection diff --git a/BROWSER.md b/BROWSER.md index 396bb6e9b..3047da8f5 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -626,6 +626,14 @@ Storage stays intact by default. With explicit approval on a Chromium target, `- | `tab-each [args...]` | Fan out a command across every open tab; returns JSON | | `frame ` | Switch to iframe context (or back to main); clears refs | +**A busy tab is not a busy daemon.** `/health` `status` is daemon liveness +(Chromium connected); the active tab's main thread is reported separately as +`activeTab: { id, responsive }`, probed for at most 1s. While one page is stuck +in a long script, `status`, `tabs`, `tab `, `newtab`, `closetab`, +`state save` and `stop` keep working, and `status` and `tabs` mark the tab as +not responding. A command that needs the stuck tab times out after 30s and +names it, so you can switch away or close it without `--force-restart`. + ### Extraction | Command | Description | diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a29736d..b6fc8e044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [1.91.155.0] - 2026-10-19 + +**A busy page no longer locks up the browse daemon.** When one tab kept its main thread busy (a heavy dev bundle, a big grid render, a runaway script), every `browse` command, including `status`, `tabs`, `tab `, `closetab` and `stop`, failed with "Daemon busy" after about 9.6 s, and the only way out was `--force-restart`, which threw away cookies, tabs and logins. Fixes #3247. From #3250 by @AndreIntelas. + +### Fixed +- **`/health` `status` reports daemon liveness (Chromium connected)**, and the active tab's responsiveness is reported separately as `activeTab: {id, responsive}`. The CLI no longer gates every command on a 2 s `page.evaluate` race against the active tab. +- **`tabs` and `state save` bound their per-tab reads**, so a stuck tab shows as `(not responding)` instead of hanging the command, and one pending probe is shared per stuck tab rather than one per poll. A busy tab keeps its last good storage in saved state. +- **A command that really needs the stuck tab times out with an error naming that tab** and how to get out (switch tabs or close it). The CLI's timeout branch now matches Bun's `TimeoutError`; the old `AbortError` check never fired. + +### Changed +- **Gate integrity:** `browse/test/terminal-agent.test.ts` adds `activeTab` to the exact `/health` keys and updates its stub for the now-synchronous `isHealthy` and the new `probeActiveTab`; its no-token assertions are unchanged. `browse/test/busy-active-tab.test.ts` and `browse/test/command-timeout-report.test.ts` are new. No threshold, trial count or case kind changed. ## [1.91.154.0] - 2026-10-19 **/cso can verify Rails repairs in a read-only checkout, its retained Git history no longer brings back credential files the snapshot leaves out, a repair whose external assertions fail is now reported as `ASSERTION_FAILED` instead of a misleading `INCOMPATIBLE_INPUT`, and a security assertion can accept several safe rejection statuses while still requiring the secret to be absent.** The helper keeps `.env`, `master.key`, keys and agent configuration out of the readable tree, but `history.txt` captured `git log -p` in full, so a committed `.env` came back through its diff whenever the value wasn't credential-shaped enough for the redactor. Paid qualification run 39186328569 caught this when the private corpus's planted `.env` value showed up in the judge's helper state. diff --git a/VERSION b/VERSION index 7836617d0..29c1e5ddc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.154.0 +1.91.155.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 4222e853e..4e3b32732 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.154.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.155.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/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 338c6dd6b..7ad480813 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -290,6 +290,44 @@ export async function handleChromiumDisconnect(browser: Browser | null): Promise process.exit(1); } +/** + * How long a tab-independent read waits on one tab's renderer (#3247). A page + * stuck in a long script never answers page.evaluate() or page.title(); before + * this bound, one such tab made /health call the whole daemon unhealthy and + * hung `tabs`. Kept well under the CLI's /health request timeout + * (HEALTH_REQUEST_TIMEOUT_MS in cli.ts) so the CLI reads the answer instead of + * timing out on it. + */ +export const TAB_PROBE_TIMEOUT_MS = 1_000; + +/** + * Budget for saveState()'s per-tab storage read. Longer than the probe: a tab + * that misses it falls back to the last storage it answered with, which may + * be stale, so only a tab that is really stuck takes that path. + */ +const TAB_STORAGE_READ_TIMEOUT_MS = 5_000; + +type TabStorage = { localStorage: Record; sessionStorage: Record }; +type TabReadKind = 'probe' | 'title' | 'storage'; + +function urlOrigin(url: string): string { + return URL.canParse(url) ? new URL(url).origin : ''; +} + +/** + * Resolve with `work`'s value, or with undefined once `ms` passes. The losing + * read settles on its own when the renderer frees up, so callers pass a promise + * that cannot reject. + */ +function withinTabBudget(work: Promise, ms: number): Promise { + let timer: ReturnType | undefined; + const deadline = new Promise((resolve) => { + timer = setTimeout(() => resolve(undefined), ms); + (timer as { unref?: () => void }).unref?.(); + }); + return Promise.race([work, deadline]).finally(() => clearTimeout(timer)); +} + export type { RefEntry }; // Re-export TabSession for consumers @@ -300,7 +338,7 @@ export interface BrowserState { pages: Array<{ url: string; isActive: boolean; - storage: { localStorage: Record; sessionStorage: Record } | null; + storage: TabStorage | null; /** * HTML content loaded via load-html (setContent), replayed after context recreation. * In-memory only — never persisted to disk (HTML may contain secrets or customer data). @@ -360,6 +398,13 @@ export class BrowserManager { private extensionContext = false; /** Pending and recorded navigation-guard verdicts per page (D2). */ private navigationGuards = new WeakMap>; blocked: string | null }>(); + /** One pending read per page and kind (#3247). A read that outlives its + * budget stays queued in the renderer until the page frees up, so a stuck + * tab reuses it instead of collecting a new one on every /health poll, + * `tabs` call or persist tick. */ + private pendingTabReads = new WeakMap>>(); + /** The last storage each page answered with, and the origin it came from. */ + private lastTabStorage = new WeakMap(); // Our own 'disconnected' listeners per browser. Detach only these: removing // every 'disconnected' listener also strips Playwright's internal one that // resolves browser.close(), so each close waited the full race timeout (and @@ -1090,20 +1135,43 @@ export class BrowserManager { this.closeOwnedDisplay(); } - /** Health check — verifies Chromium is connected AND responsive */ - async isHealthy(): Promise { - if (!this.browser || !this.browser.isConnected()) return false; - try { - const page = this.pages.get(this.activeTabId); - if (!page) return true; // connected but no pages — still healthy - await Promise.race([ - page.evaluate('1'), - new Promise((_, reject) => setTimeout(() => reject(new Error('timeout')), 2000)), - ]); - return true; - } catch { - return false; + /** + * Daemon liveness for /health: Chromium is connected. The active tab is NOT + * part of it (#3247): a page stuck in a long script is one busy tab, not a + * dead daemon, and the CLI gates every command on this answer — including + * the tabs, tab, closetab and stop commands that get out of that state. + * probeActiveTab() reports the tab on its own. + */ + isHealthy(): boolean { + return Boolean(this.browser?.isConnected()); + } + + /** + * Does the active tab's main thread answer within `timeoutMs`? Null when + * there is no active page. A rejected evaluate (crashed or closing target) + * also counts as not responsive. + */ + async probeActiveTab(timeoutMs = TAB_PROBE_TIMEOUT_MS): Promise<{ id: number; responsive: boolean } | null> { + const id = this.activeTabId; + const page = this.pages.get(id); + if (!page) return null; + const probe = this.sharedTabRead(page, 'probe', () => page.evaluate('1').then(() => true, () => false)); + return { id, responsive: (await withinTabBudget(probe, timeoutMs)) === true }; + } + + /** Join the page's pending read of this kind, or start one. `read` must not + * reject. */ + private sharedTabRead(page: Page, kind: TabReadKind, read: () => Promise): Promise { + let reads = this.pendingTabReads.get(page); + if (!reads) { + reads = new Map(); + this.pendingTabReads.set(page, reads); } + const pending = reads.get(kind); + if (pending) return pending as Promise; + const started = read().finally(() => reads.delete(kind)); + reads.set(kind, started); + return started; } // ─── Tab Management ──────────────────────────────────────── @@ -1301,17 +1369,23 @@ export class BrowserManager { return released; } - async getTabListWithTitles(): Promise> { - const tabs: Array<{ id: number; url: string; title: string; active: boolean }> = []; - for (const [id, page] of this.pages) { - tabs.push({ + /** + * Every tab with its title. Titles are read in parallel, each bounded by + * TAB_PROBE_TIMEOUT_MS: a tab whose renderer is stuck comes back with + * `busy: true` and an empty title instead of hanging the list (#3247). + */ + async getTabListWithTitles(): Promise> { + return Promise.all([...this.pages].map(async ([id, page]) => { + const title = await withinTabBudget( + this.sharedTabRead(page, 'title', () => page.title().catch(() => '')), TAB_PROBE_TIMEOUT_MS); + return { id, url: page.url(), - title: await page.title().catch(() => ''), + title: title ?? '', active: id === this.activeTabId, - }); - } - return tabs; + busy: title === undefined, + }; + })); } // ─── Session Access ──────────────────────────────────────── @@ -1715,7 +1789,9 @@ export class BrowserManager { // ─── State Save/Restore (shared by recreateContext + handoff) ─ /** * Capture browser state: cookies, localStorage, sessionStorage, URLs, active tab. - * Skips pages that fail storage reads (e.g., already closed). + * Skips the storage of pages that fail the read (e.g., already closed). + * A page whose renderer is stuck past TAB_STORAGE_READ_TIMEOUT_MS does not + * hold up `state save` (#3247); see readTabStorage() for what it gets. */ async saveState(): Promise { if (!this.context) throw new Error('Browser not launched'); @@ -1725,13 +1801,7 @@ export class BrowserManager { for (const [id, page] of this.pages) { const url = page.url(); - let storage = null; - try { - storage = await page.evaluate(() => ({ - localStorage: { ...localStorage }, - sessionStorage: { ...sessionStorage }, - })); - } catch {} + const storage = await this.readTabStorage(page, url); // Capture load-html content so a later context recreation (viewport --scale) // can replay it via setTabContent. Never persisted to disk. @@ -1753,6 +1823,28 @@ export class BrowserManager { return { cookies, pages }; } + /** + * One tab's localStorage/sessionStorage for saveState(). A failed read + * (closed page, opaque origin) is null. A read that times out returns the + * last storage this page gave for the same origin: a periodic + * BROWSE_PERSIST_STATE snapshot must not replace good storage on disk with + * null just because the page is busy. + */ + private async readTabStorage(page: Page, url: string): Promise { + const origin = urlOrigin(url); + const read = await withinTabBudget(this.sharedTabRead(page, 'storage', () => page.evaluate(() => ({ + localStorage: { ...localStorage }, + sessionStorage: { ...sessionStorage }, + })).catch(() => null)), TAB_STORAGE_READ_TIMEOUT_MS); + if (read === undefined) { + const last = this.lastTabStorage.get(page); + return last?.origin === origin ? last.storage : null; + } + if (read) this.lastTabStorage.set(page, { origin, storage: read }); + else this.lastTabStorage.delete(page); + return read; + } + /** * Restore browser state into the current context: cookies, pages, storage. * Navigates to saved URLs, restores storage, wires page events. diff --git a/browse/src/cli.ts b/browse/src/cli.ts index d774d23d5..556947f0d 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -215,39 +215,50 @@ function readState(): ServerState | null { // isProcessAlive is imported from ./error-handling -/** - * HTTP health check — definitive proof the server is alive and responsive. - * Used in all polling loops instead of isProcessAlive() (which is slow on Windows). - */ -export async function isServerHealthy(port: number, timeoutMs = 2000): Promise { - try { - const resp = await fetch(`http://127.0.0.1:${port}/health`, { - signal: AbortSignal.timeout(timeoutMs), - }); - if (!resp.ok) return false; - const health = await resp.json() as any; - return health.status === 'healthy'; - } catch { - return false; - } -} +/** Client timeout for one GET /health. Must stay above the daemon's active-tab + * probe (TAB_PROBE_TIMEOUT_MS in browser-manager.ts): when the two were equal, + * the CLI aborted just before a busy tab's reply arrived and could not tell a + * busy tab from a hung daemon (#3247). */ +export const HEALTH_REQUEST_TIMEOUT_MS = 2000; -/** Best-effort tab count via GET /health (no auth, bounded). Returns null - * when the daemon doesn't answer in time or predates the `tabs` field — - * callers degrade to a countless phrasing, never block on this. */ -async function fetchDaemonTabCount(port: number, timeoutMs = 2000): Promise { +/** GET /health (no auth, bounded). Null when the daemon doesn't answer in + * time or the reply is not a 200. */ +async function fetchDaemonHealth(port: number, timeoutMs = HEALTH_REQUEST_TIMEOUT_MS): Promise | null> { try { const resp = await fetch(`http://127.0.0.1:${port}/health`, { signal: AbortSignal.timeout(timeoutMs), }); if (!resp.ok) return null; - const health = await resp.json() as any; - return typeof health.tabs === 'number' ? health.tabs : null; + return await resp.json() as Record; } catch { return null; } } +/** + * HTTP health check — definitive proof the server is alive and responsive. + * Used in all polling loops instead of isProcessAlive() (which is slow on Windows). + * `status` is daemon liveness only; a busy active tab does not fail it (#3247). + */ +export async function isServerHealthy(port: number, timeoutMs = HEALTH_REQUEST_TIMEOUT_MS): Promise { + return (await fetchDaemonHealth(port, timeoutMs))?.status === 'healthy'; +} + +/** Best-effort tab count via GET /health. Returns null when the daemon + * doesn't answer in time or predates the `tabs` field — callers degrade to a + * countless phrasing, never block on this. */ +async function fetchDaemonTabCount(port: number): Promise { + const tabs = (await fetchDaemonHealth(port))?.tabs; + return typeof tabs === 'number' ? tabs : null; +} + +/** The active tab's id when /health reports its main thread as not + * responding (#3247), else null — including daemons that predate the field. */ +async function fetchBusyActiveTab(port: number): Promise { + const activeTab = (await fetchDaemonHealth(port))?.activeTab as { id?: unknown; responsive?: unknown } | null | undefined; + return activeTab?.responsive === false && typeof activeTab.id === 'number' ? activeTab.id : null; +} + // ─── Process Management ───────────────────────────────────────── async function killServer(pid: number): Promise { if (!isProcessAlive(pid)) return; @@ -420,7 +431,7 @@ async function probeHealthWithBackoff( await Bun.sleep(intervalMs); const remainingMs = deadline - Date.now(); if (remainingMs <= 0) return false; - if (await isServerHealthy(port, Math.min(2000, remainingMs))) return true; + if (await isServerHealthy(port, Math.min(HEALTH_REQUEST_TIMEOUT_MS, remainingMs))) return true; } } @@ -1034,6 +1045,32 @@ export function extractTabId(args: string[]): { tabId: number | undefined; args: return { tabId, args: stripped }; } +/** Did this fetch fail because its AbortSignal fired? Bun rejects an + * AbortSignal.timeout() fetch with TimeoutError, not AbortError — matching + * only AbortError left the timeout reports below unreachable. */ +export function isFetchTimeout(err: unknown): boolean { + const name = (err as { name?: unknown } | null)?.name; + return name === 'AbortError' || name === 'TimeoutError'; +} + +/** + * What to print when a command's reply did not arrive within `timeoutMs`. + * #1781: a timeout on a heavy page usually means busy, not dead, so a live + * daemon is never restarted here. #3247: when /health names the active tab + * as not responding, say which tab and the commands that leave it without + * losing the session. + */ +export async function commandTimeoutReport(state: { pid: number; port: number } | null, timeoutMs: number): Promise { + const after = `[browse] Command timed out after ${timeoutMs / 1000}s`; + if (!state?.pid || !isProcessAlive(state.pid)) return [after]; + const busyTab = await fetchBusyActiveTab(state.port); + if (busyTab === null) return [`${after} (server still alive — busy, not restarting). Retry, or raise load.`]; + return [ + `${after}: tab ${busyTab} is not responding (its main thread is busy).`, + `[browse] The daemon, its other tabs and cookies still work: switch with 'browse tab ', close this one with 'browse closetab ${busyTab}', or retry once the page settles.`, + ]; +} + // ─── Command Dispatch ────────────────────────────────────────── export async function sendCommand(state: ServerState, command: string, args: string[], retries = 0): Promise { // Precedence: CLI --tab-id flag > BROWSE_TAB env var. @@ -1085,19 +1122,14 @@ export async function sendCommand(state: ServerState, command: string, args: str process.exit(1); } } catch (err: any) { - if (command === 'cookie-import-browser' && (['AbortError', 'TimeoutError'].includes(err.name) + if (command === 'cookie-import-browser' && (isFetchTimeout(err) || ['ECONNREFUSED', 'ECONNRESET'].includes(err.code) || err.message?.includes('fetch failed'))) { throw new Error('Cookie import response was lost or timed out. It may have partially completed; inspect the destination before retrying manually. The command was not replayed.'); } - if (err.name === 'AbortError') { - // #1781: a 30s timeout on a heavy page usually means busy, not dead. - // Don't kill a live server (that's what triggered the crash-loop) — report - // and exit so the user can retry rather than losing their (headed) window. - const ts = readState(); - const alive = ts?.pid ? isProcessAlive(ts.pid) : false; - console.error(alive - ? '[browse] Command timed out after 30s (server still alive — busy, not restarting). Retry, or raise load.' - : '[browse] Command timed out after 30s'); + if (isFetchTimeout(err)) { + // Report and exit so the user can retry rather than losing their + // (headed) window — killing a live server triggered the crash-loop. + for (const line of await commandTimeoutReport(readState(), timeoutMs)) console.error(line); process.exit(1); } // Connection error — server may have crashed, OR may just be busy. diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 8cc9e2e6c..af49910a9 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -265,7 +265,7 @@ export async function handleMetaCommand( case 'tabs': { const tabs = await bm.getTabListWithTitles(); return tabs.map(t => - `${t.active ? '→ ' : ' '}[${t.id}] ${t.title || '(untitled)'} — ${t.url}` + `${t.active ? '→ ' : ' '}[${t.id}] ${t.busy ? '(not responding)' : t.title || '(untitled)'} — ${t.url}` ).join('\n'); } @@ -404,12 +404,17 @@ export async function handleMetaCommand( case 'status': { const tabs = bm.getTabCount(); const mode = bm.getConnectionMode(); + const activeTab = await bm.probeActiveTab(); return [ `Status: healthy`, `Mode: ${mode}`, `URL: ${bm.getCurrentUrl()}`, `Tabs: ${tabs}`, `PID: ${process.pid}`, + // #3247: name the busy tab and the ways out that keep the session. + ...(activeTab && !activeTab.responsive + ? [`Active tab ${activeTab.id}: not responding. Switch with: browse tab , or close it: browse closetab ${activeTab.id}`] + : []), ].join('\n'); } diff --git a/browse/src/routes/core.ts b/browse/src/routes/core.ts index 491fc5f76..611448594 100644 --- a/browse/src/routes/core.ts +++ b/browse/src/routes/core.ts @@ -77,13 +77,19 @@ export const coreRoutes: RouteEntry[] = [ // Health check — no auth required, does NOT reset idle timer. NEVER carries // a token in any mode: token bootstrap is POST /extension-token and shell // auth is POST /pty-session. Liveness/status only. + // + // `status` is daemon liveness (Chromium connected); the CLI gates every + // command on it. `activeTab.responsive` is the active tab's main thread, + // probed for at most TAB_PROBE_TIMEOUT_MS and reported beside it: one busy + // page must not read as a dead daemon (#3247). { method: '*', path: '/health', auth: 'none', surfaces: ['local'], handler: async (_req, _r, ctx) => { const { browserManager } = ctx; - const healthy = await browserManager.isHealthy(); + const healthy = browserManager.isHealthy(); return json({ status: healthy ? 'healthy' : 'unhealthy', + activeTab: healthy ? await browserManager.probeActiveTab() : null, mode: browserManager.getConnectionMode(), uptime: Math.floor((Date.now() - ctx.startTime) / 1000), tabs: browserManager.getTabCount(), diff --git a/browse/test/busy-active-tab.test.ts b/browse/test/busy-active-tab.test.ts new file mode 100644 index 000000000..04a15861b --- /dev/null +++ b/browse/test/busy-active-tab.test.ts @@ -0,0 +1,132 @@ +/** + * #3247: one busy tab must not make the daemon look dead. A renderer stuck in + * a long script never answers page.evaluate() or page.title(). /health folded + * that into its `status`, so the CLI refused every command (status, tabs, + * tab, newtab, stop) with "Daemon busy" until --force-restart threw the + * session away, and `tabs` and `state save` hung on the tab as well. + * + * A real Chromium tab is held in a busy loop while a second tab stays idle; + * each surface must answer within its bound and name the busy tab, without + * piling up reads on the stuck renderer or dropping its persisted storage. + */ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Page } from 'playwright'; +import { BrowserManager, TAB_PROBE_TIMEOUT_MS } from '../src/browser-manager'; +import { HEALTH_REQUEST_TIMEOUT_MS } from '../src/cli'; +import { handleMetaCommand } from '../src/meta-commands'; +import { persistSessionState } from '../src/session-persist'; +import { callRoute, stubRouteContext } from './route-test-harness'; +import { startTestServer } from './test-server'; + +// Outlasts every check below; closing the busy tab at the end stops it sooner. +const BUSY_MS = 30_000; + +let testServer: ReturnType; +let bm: BrowserManager; +let busyId = 0; +let idleId = 0; +let busyPage: Page; +let profileDir = ''; +const originalProfile = process.env.CHROMIUM_PROFILE; + +beforeAll(async () => { + // Per-file profile, set in a hook (see commands.test.ts for why). + profileDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-busy-tab-profile-')); + process.env.CHROMIUM_PROFILE = profileDir; + testServer = startTestServer(0); + bm = new BrowserManager(); + await bm.launch(); + busyId = bm.getActiveTabId(); + await bm.getPage().goto(`${testServer.url}/basic.html`); + idleId = await bm.newTab(`${testServer.url}/basic.html`); + bm.switchTab(busyId, { bringToFront: false }); + busyPage = bm.getPage(); + // The good snapshot a BROWSE_PERSIST_STATE tick takes before the page hangs. + await busyPage.evaluate(() => localStorage.setItem('probe', 'kept')); + await bm.saveState(); + await busyPage.evaluate((ms) => { + setTimeout(() => { const end = Date.now() + ms; while (Date.now() < end) { /* busy */ } }, 50); + }, BUSY_MS); + await Bun.sleep(300); // let the loop start +}, 30_000); + +afterAll(async () => { + try { testServer.server.stop(true); } catch {} + try { await Promise.race([bm?.close(), Bun.sleep(3000)]); } catch {} + if (originalProfile === undefined) delete process.env.CHROMIUM_PROFILE; + else process.env.CHROMIUM_PROFILE = originalProfile; + if (profileDir) fs.rmSync(profileDir, { recursive: true, force: true }); +}); + +describe('busy active tab (#3247)', () => { + test('/health says the daemon is alive and names the busy tab before the CLI gives up', async () => { + // Equal budgets were the bug: the CLI aborted just before the reply landed. + expect(TAB_PROBE_TIMEOUT_MS).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + const ctx = stubRouteContext({ + browserManager: bm, + terminal: { readPort: () => null, grantToken: async () => true, restartSession: async () => true }, + }); + const started = Date.now(); + const body = await (await callRoute('GET', '/health', ctx)).json(); + expect(Date.now() - started).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + expect(body.status).toBe('healthy'); + expect(body.activeTab).toEqual({ id: busyId, responsive: false }); + }); + + test('tabs lists every tab without waiting on the busy one', async () => { + const started = Date.now(); + const out = await handleMetaCommand('tabs', [], bm, async () => {}); + expect(Date.now() - started).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + expect(out).toContain(`[${busyId}] (not responding)`); + expect(out).toContain(`[${idleId}] Test Page - Basic`); + }); + + test('status stays healthy and names the busy tab with a way out', async () => { + const out = await handleMetaCommand('status', [], bm, async () => {}); + expect(out).toContain('Status: healthy'); + expect(out).toContain(`Active tab ${busyId}`); + expect(out).toContain(`closetab ${busyId}`); + }); + + test('a stuck tab gets one pending probe, not one per poll', async () => { + let evaluates = 0; + const evaluate = busyPage.evaluate.bind(busyPage); + Object.assign(busyPage, { evaluate: (...args: Parameters) => { evaluates++; return evaluate(...args); } }); + try { + for (let i = 0; i < 3; i++) expect(await bm.probeActiveTab()).toEqual({ id: busyId, responsive: false }); + } finally { + delete (busyPage as { evaluate?: unknown }).evaluate; // back to the prototype method + } + // The /health check above may already hold the pending probe. + expect(evaluates).toBeLessThanOrEqual(1); + }, 10_000); + + test('a persisted snapshot taken while the tab is busy keeps its last good storage', async () => { + const file = path.join(profileDir, 'session-state.json'); + await persistSessionState(bm, file); + const saved = JSON.parse(fs.readFileSync(file, 'utf8')); + expect(saved.pages).toHaveLength(2); + expect(saved.pages.find((p: { isActive: boolean }) => p.isActive).storage.localStorage).toEqual({ probe: 'kept' }); + expect(saved.pages.find((p: { isActive: boolean }) => !p.isActive).storage).not.toBeNull(); + }, 15_000); + + test('the idle tab still answers', async () => { + bm.switchTab(idleId, { bringToFront: false }); + try { + expect(await bm.probeActiveTab()).toEqual({ id: idleId, responsive: true }); + expect(await bm.getPage().title()).toBe('Test Page - Basic'); + } finally { + bm.switchTab(busyId, { bringToFront: false }); + } + }); + + test('closetab closes the busy tab and the session carries on', async () => { + const out = await handleMetaCommand('closetab', [String(busyId)], bm, async () => {}); + expect(out).toContain(`Closed tab ${busyId}`); + expect(bm.getTabCount()).toBe(1); + expect(await bm.probeActiveTab()).toEqual({ id: idleId, responsive: true }); + }, 15_000); +}); diff --git a/browse/test/command-timeout-report.test.ts b/browse/test/command-timeout-report.test.ts new file mode 100644 index 000000000..0505c0737 --- /dev/null +++ b/browse/test/command-timeout-report.test.ts @@ -0,0 +1,66 @@ +/** + * What the CLI prints when a command's reply never arrives (#1781, #3247). + * + * Bun rejects an AbortSignal.timeout() fetch with TimeoutError, so the old + * AbortError-only check skipped the busy report and printed "The operation + * timed out." instead. When the daemon's /health names its active tab as not + * responding, the report names that tab and the commands that leave it + * without a restart; a dead daemon gets the plain line. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { commandTimeoutReport, isFetchTimeout } from '../src/cli'; + +let servers: Array> = []; +afterEach(() => { for (const s of servers) s.stop(true); servers = []; }); + +/** A fake daemon whose /health answers `body`. */ +function fakeHealth(body: unknown): number { + const srv = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => Response.json(body) }); + servers.push(srv); + if (!srv.port) throw new Error('fake /health: no port'); + return srv.port; +} + +/** A pid that no longer runs: a child that has already exited. */ +function deadPid(): number { + return Bun.spawnSync([process.execPath, '-e', '']).pid; +} + +describe('isFetchTimeout', () => { + test('a real fetch whose AbortSignal.timeout fires counts as a timeout', async () => { + const silent = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => new Promise(() => {}) }); + servers.push(silent); + const err = await fetch(`http://127.0.0.1:${silent.port}/`, { signal: AbortSignal.timeout(50) }) + .then(() => null, (e: unknown) => e); + expect(isFetchTimeout(err)).toBe(true); + expect(isFetchTimeout(new TypeError('fetch failed'))).toBe(false); + }); +}); + +describe('commandTimeoutReport', () => { + test('names the busy tab and the way out, never a restart', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = (await commandTimeoutReport({ pid: process.pid, port }, 30_000)).join('\n'); + expect(out).toContain('tab 7'); + expect(out).toContain('closetab 7'); + expect(out).not.toContain('--force-restart'); + }); + + test('a live daemon with a responsive or unreported tab gets the busy-not-restarting line', async () => { + for (const body of [ + { status: 'healthy', activeTab: { id: 1, responsive: true } }, + { status: 'healthy' }, // daemon from before activeTab existed + ]) { + const out = (await commandTimeoutReport({ pid: process.pid, port: fakeHealth(body) }, 30_000)).join('\n'); + expect(out).toContain('still alive'); + expect(out).not.toContain('closetab'); + } + }); + + test('a dead daemon gets only the timeout line', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = await commandTimeoutReport({ pid: deadPid(), port }, 30_000); + expect(out).toHaveLength(1); + expect(out[0]).toContain('timed out after 30s'); + }); +}); diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index f066d1a23..c5b247ac1 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -94,13 +94,16 @@ describe('/health does NOT surface ptyToken', () => { test('/health response body does not include ptyToken', async () => { // It's allowed to include `terminalPort` (a port number, not auth). const ctx = stubRouteContext({ - browserManager: { isHealthy: async () => true, getConnectionMode: () => 'launched', getTabCount: () => 1 } as any, + browserManager: { + isHealthy: () => true, probeActiveTab: async () => ({ id: 1, responsive: true }), + getConnectionMode: () => 'launched', getTabCount: () => 1, + } as any, terminal: { readPort: () => 4242, grantToken: async () => true, restartSession: async () => true }, }); const text = await (await callRoute('GET', '/health', ctx)).text(); const body = JSON.parse(text); expect(body.terminalPort).toBe(4242); - expect(Object.keys(body).sort()).toEqual(['mode', 'status', 'tabs', 'terminalPort', 'uptime']); + expect(Object.keys(body).sort()).toEqual(['activeTab', 'mode', 'status', 'tabs', 'terminalPort', 'uptime']); expect(text).not.toContain('ptyToken'); expect(text).not.toContain('gstack_pty'); }); diff --git a/docs/REMOTE_BROWSER_ACCESS.md b/docs/REMOTE_BROWSER_ACCESS.md index c5565ba0e..902df6efb 100644 --- a/docs/REMOTE_BROWSER_ACCESS.md +++ b/docs/REMOTE_BROWSER_ACCESS.md @@ -81,7 +81,9 @@ Response: (plain text result of the command) ``` #### GET /health -Server status. No auth required. Returns status, tabs, mode, uptime. +Server status. No auth required. Returns status, activeTab, tabs, mode, uptime. +`status` is daemon liveness; `activeTab` (`{ id, responsive }`, or null with no +page) says whether the active tab's main thread answered within 1s. Never carries a token — extension token bootstrap is `POST /extension-token` (local listener only, validates the pinned `chrome-extension://` Origin and a loopback Host; 403 otherwise). Not reachable over the tunnel (404). diff --git a/package.json b/package.json index e11883f58..de7b7c9d4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.154", + "version": "1.91.155", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", From 69a6d7fc3b98a67d0688b1c9e7561a96fd127a6a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 19 Oct 2026 14:55:24 -0700 Subject: [PATCH 4/5] v1.91.156.0 fix(evals): office-hours marathon fixture loads the formatter's imports; validator checks the observed cap fixes (#3246) (#3309) * fix(evals): office-hours marathon fixture copies the formatter import closure; validator gets the observed cap fixes (#3246) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * v1.91.156.0 chore: stamp #3309 (office-hours marathon fixture and cap-fix validation) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 9 +++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- package.json | 2 +- test/helpers/office-hours-completion.ts | 17 ++++++++- .../helpers/office-hours-formatter-fixture.ts | 31 +++++++++++++++ test/helpers/touchfiles-data.ts | 2 +- test/office-hours-completion.test.ts | 38 ++++++++++++++++++- test/office-hours-review.test.ts | 15 ++++++++ ...l-e2e-office-hours-section-loading.test.ts | 14 +++---- 10 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 test/helpers/office-hours-formatter-fixture.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6fc8e044..cafe27eea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [1.91.156.0] - 2026-10-19 + +**The weekly marathon's office-hours case can run its review formatter again, and its validator now checks cap fixes the way the formatter does.** Both defects came from #3164 and were in the test, not the skill. The marathon fixture copied `bin/gstack-office-hours-review` with a hand-kept list of two library files; #3164 added a third import (`lib/spec-review-cap.ts`), so every formatter command in the fixture failed at module load, the agent correctly declared the review UNREVIEWED, and the case went red in marathon run 39032308051 (#3246). With the module restored, a focused rerun showed the second defect: when the review stops at the round cap and the agent applies the remaining blocking fixes with `--fixes`, the formatter records them as verified, but the test never gave the validator that list, so it expected "fixes unconfirmed" and rejected a correct report. + +### Fixed +- **Office-hours cap fixes:** `observedSpecFixes()` in `test/helpers/office-hours-completion.ts` returns the last valid `--fixes` list the agent saved with Write, and the marathon test passes it to `validateOfficeHoursReviewArtifacts`. A free test in `test/office-hours-completion.test.ts` runs the real `finalize --fixes` at the cap and checks three cases: the observed list validates, no list fails with the marathon rerun's `Spec Review does not match` error, and a list naming a different finding also fails. +- **Office-hours marathon fixture:** `test/helpers/office-hours-formatter-fixture.ts` copies the formatter together with every local module it imports, so a new import can no longer be left out. A free test in `test/office-hours-review.test.ts` loads the copied formatter and fails on a missing module; the old three-file list reproduces the exact `Cannot find module '../lib/spec-review-cap.ts'` error. The case's touchfiles now include `lib/spec-review-cap.ts` and the new helper. +- **Gate integrity:** no threshold, trial count, case kind or judge changed. The marathon assertions are unchanged; only the fixture's file copy changed. + ## [1.91.155.0] - 2026-10-19 **A busy page no longer locks up the browse daemon.** When one tab kept its main thread busy (a heavy dev bundle, a big grid render, a runaway script), every `browse` command, including `status`, `tabs`, `tab `, `closetab` and `stop`, failed with "Daemon busy" after about 9.6 s, and the only way out was `--force-restart`, which threw away cookies, tabs and logins. Fixes #3247. From #3250 by @AndreIntelas. diff --git a/VERSION b/VERSION index 29c1e5ddc..59c1b7f37 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.155.0 +1.91.156.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 4e3b32732..b5dfb1d5f 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.155.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.156.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/package.json b/package.json index de7b7c9d4..603e2ffd8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.155", + "version": "1.91.156", "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/test/helpers/office-hours-completion.ts b/test/helpers/office-hours-completion.ts index 27b15b170..4ae26efe0 100644 --- a/test/helpers/office-hours-completion.ts +++ b/test/helpers/office-hours-completion.ts @@ -1,7 +1,7 @@ import { isDeepStrictEqual } from 'node:util'; import * as path from 'node:path'; import { renderOfficeHoursReviewerPrompt, validateOfficeHoursReview, renderOfficeHoursReview, extractOfficeHoursReviewBlock, parseOfficeHoursVerdictReceipt, verifyOfficeHoursVerdictReceipt, officeHoursDesignChanges, verifyOfficeHoursCitations, type OfficeHoursReview } from '../../lib/office-hours-review'; -import { verifySpecFixesAtCap, type SpecFix } from '../../lib/spec-review-cap'; +import { parseSpecFixes, verifySpecFixesAtCap, type SpecFix } from '../../lib/spec-review-cap'; export type OfficeHoursFile = { path: string; content: string | null }; /** The helper's captured diff before a later round; a missing snapshot fails closed. */ @@ -32,6 +32,21 @@ export interface OfficeHoursCompletionEvidence { fixes?: SpecFix[] | null; } +/** + * The cap fixes the agent saved for the finalizer: the last observed Write whose + * content is a valid `--fixes` list. Null when it saved none. The validator + * recomputes the cap record from this list, so a list that was not the one + * passed to finalize still produces a mismatch. + */ +export function observedSpecFixes(toolCalls: OfficeHoursCompletionEvidence['toolCalls']): SpecFix[] | null { + for (const call of toolCalls.toReversed()) { + if (call.tool !== 'Write') continue; + try { return parseSpecFixes(JSON.parse(String(call.input?.content ?? ''))); } + catch { /* not a fixes list */ } + } + return null; +} + export interface OfficeHoursReviewEvidence { verdict: string; concerns: string; diff --git a/test/helpers/office-hours-formatter-fixture.ts b/test/helpers/office-hours-formatter-fixture.ts new file mode 100644 index 000000000..0d11b3ea2 --- /dev/null +++ b/test/helpers/office-hours-formatter-fixture.ts @@ -0,0 +1,31 @@ +/** + * Copy bin/gstack-office-hours-review and every local module it imports into a + * fixture, so the copied formatter loads the same modules as the checkout. + * Marathon 39032308051 copied a hand-kept file list that missed + * lib/spec-review-cap.ts, and every formatter command failed at module load. + */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '../..'); + +export function copyOfficeHoursFormatter(fixtureRoot: string): string { + const pending = ['bin/gstack-office-hours-review']; + const copied = new Set(); + while (pending.length) { + const relative = pending.pop()!; + if (copied.has(relative)) continue; + copied.add(relative); + const source = path.join(ROOT, relative); + const target = path.join(fixtureRoot, relative); + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.copyFileSync(source, target); + for (const [, specifier] of fs.readFileSync(source, 'utf8').matchAll(/\b(?:from|import)\s+'(\.\.?\/[^']+)'/g)) { + const resolved = path.relative(ROOT, path.resolve(path.dirname(source), specifier!)); + pending.push(path.extname(resolved) ? resolved : `${resolved}.ts`); + } + } + const formatter = path.join(fixtureRoot, 'bin/gstack-office-hours-review'); + fs.chmodSync(formatter, 0o755); + return formatter; +} diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 4433c4fa0..2ad18b5ec 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -1099,7 +1099,7 @@ export const E2E_TOUCHFILES: Record = { 'test/skill-e2e-arm-benchmark.test.ts', 'ship/SKILL.md', 'test/helpers/e2e-gate.ts'], - 'office-hours-section-loading': [ 'office-hours/**', 'bin/gstack-office-hours-review', 'lib/office-hours-review.ts', 'lib/fs-atomic.ts', 'scripts/resolvers/review-dashboard.ts', 'scripts/resolvers/plan-gates.ts', 'scripts/resolvers/spec-review.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/review-scope.ts', 'scripts/resolvers/outside-voice.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/carve-guards.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/office-hours-completion.ts', 'test/helpers/llm-judge.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-office-hours-section-loading.test.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/auq-native-capture.ts', 'test/helpers/auto-decision-state.ts', 'test/helpers/autoplan-artifact-digest.ts', 'test/helpers/autoplan-artifact-permission.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/helpers/capture-parity-baseline.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty/**', 'test/helpers/dx-selected-navigation.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/eng-cache-writer-decision.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/helpers/native-auto-decide.ts', 'test/helpers/owned-claude-transcript.ts', 'test/helpers/parity-harness.ts', 'test/helpers/plan-count-artifacts.ts', 'test/helpers/plan-count-file-permission.ts', 'test/helpers/plan-count-fixture.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/helpers/plan-count-pending-question.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-floor-review.ts', 'test/helpers/plan-floor-target.ts', 'test/helpers/plan-scope-selection.ts', 'test/helpers/plan-seed-submission.ts', 'test/helpers/plan-skill-question-events.ts', 'test/helpers/plan-skill-question-hook-scope.ts', 'test/helpers/plan-skill-questions.ts', 'test/helpers/pty-screen.ts', 'test/helpers/pty-trust-dialog.ts', 'test/helpers/skill-census.ts'], + 'office-hours-section-loading': [ 'office-hours/**', 'bin/gstack-office-hours-review', 'lib/office-hours-review.ts', 'lib/fs-atomic.ts', 'lib/spec-review-cap.ts', 'test/helpers/office-hours-formatter-fixture.ts', 'scripts/resolvers/review-dashboard.ts', 'scripts/resolvers/plan-gates.ts', 'scripts/resolvers/spec-review.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/review-scope.ts', 'scripts/resolvers/outside-voice.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/carve-guards.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/office-hours-completion.ts', 'test/helpers/llm-judge.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-office-hours-section-loading.test.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/auq-native-capture.ts', 'test/helpers/auto-decision-state.ts', 'test/helpers/autoplan-artifact-digest.ts', 'test/helpers/autoplan-artifact-permission.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/helpers/capture-parity-baseline.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty/**', 'test/helpers/dx-selected-navigation.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/eng-cache-writer-decision.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/helpers/native-auto-decide.ts', 'test/helpers/owned-claude-transcript.ts', 'test/helpers/parity-harness.ts', 'test/helpers/plan-count-artifacts.ts', 'test/helpers/plan-count-file-permission.ts', 'test/helpers/plan-count-fixture.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/helpers/plan-count-pending-question.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-floor-review.ts', 'test/helpers/plan-floor-target.ts', 'test/helpers/plan-scope-selection.ts', 'test/helpers/plan-seed-submission.ts', 'test/helpers/plan-skill-question-events.ts', 'test/helpers/plan-skill-question-hook-scope.ts', 'test/helpers/plan-skill-questions.ts', 'test/helpers/pty-screen.ts', 'test/helpers/pty-trust-dialog.ts', 'test/helpers/skill-census.ts'], 'office-hours-design-draft': [ 'office-hours/**', 'lib/office-hours-review.ts', 'scripts/resolvers/review-dashboard.ts', 'scripts/resolvers/plan-gates.ts', 'scripts/resolvers/spec-review.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/review-scope.ts', 'scripts/resolvers/outside-voice.ts', 'scripts/resolvers/sections.ts', 'scripts/gen-skill-docs.ts', 'test/helpers/carve-guards.ts', 'test/helpers/auq-sdk-capture.ts', 'test/helpers/office-hours-completion.ts', 'test/helpers/llm-judge.ts', 'test/helpers/session-runner.ts', 'test/skill-e2e-office-hours-design-draft.test.ts', 'test/helpers/agent-sdk-runner.ts', 'test/helpers/auq-native-capture.ts', 'test/helpers/auto-decision-state.ts', 'test/helpers/autoplan-artifact-digest.ts', 'test/helpers/autoplan-artifact-permission.ts', 'test/helpers/autoplan-artifact-recorder.ts', 'test/helpers/capture-parity-baseline.ts', 'test/helpers/claude-pty-runner.ts', 'test/helpers/pty/**', 'test/helpers/dx-selected-navigation.ts', 'test/helpers/e2e-gate.ts', 'test/helpers/eng-cache-writer-decision.ts', 'test/helpers/hermetic-skill-runtime.ts', 'test/helpers/native-auto-decide.ts', 'test/helpers/owned-claude-transcript.ts', 'test/helpers/parity-harness.ts', 'test/helpers/plan-count-artifacts.ts', 'test/helpers/plan-count-file-permission.ts', 'test/helpers/plan-count-fixture.ts', 'test/helpers/plan-count-pending-exit.ts', 'test/helpers/plan-count-pending-question.ts', 'test/helpers/plan-count-transcript.ts', 'test/helpers/plan-floor-review.ts', 'test/helpers/plan-floor-target.ts', 'test/helpers/plan-scope-selection.ts', 'test/helpers/plan-seed-submission.ts', 'test/helpers/plan-skill-question-events.ts', 'test/helpers/plan-skill-question-hook-scope.ts', 'test/helpers/plan-skill-questions.ts', 'test/helpers/pty-screen.ts', 'test/helpers/pty-trust-dialog.ts', 'test/helpers/skill-census.ts'], 'dx-error-contract-review': ['test/skill-e2e-dx-error-contract.test.ts', 'test/fixtures/dx-error-contract-case.ts', 'lib/dx-error-contract.ts', 'scripts/resolvers/dx.ts', 'scripts/resolvers/index.ts', 'scripts/gen-skill-docs.ts', 'plan-devex-review/**', 'bin/gstack-autoplan-snapshot.ts', 'lib/autoplan-prompts.ts', 'lib/autoplan-reconcile.ts', 'lib/coordinator-contract.ts', 'test/helpers/session-runner.ts', 'test/helpers/eval-store.ts', 'test/helpers/eval-budgets.ts', 'lib/claude-bin.ts', 'lib/eval-model.ts'], 'plan-devex-peer-comparison-classification': [ diff --git a/test/office-hours-completion.test.ts b/test/office-hours-completion.test.ts index 156ff7dd3..cf73e5eaa 100644 --- a/test/office-hours-completion.test.ts +++ b/test/office-hours-completion.test.ts @@ -4,7 +4,7 @@ import { describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; -import { validateOfficeHoursCompletion, validateOfficeHoursDesignDraft, validateOfficeHoursReviewerHandoffs, validateOfficeHoursReviewArtifacts, validateOfficeHoursReviewPreservation, type OfficeHoursCompletionEvidence } from './helpers/office-hours-completion'; +import { observedSpecFixes, validateOfficeHoursCompletion, validateOfficeHoursDesignDraft, validateOfficeHoursReviewerHandoffs, validateOfficeHoursReviewArtifacts, validateOfficeHoursReviewPreservation, type OfficeHoursCompletionEvidence } from './helpers/office-hours-completion'; import { E2E_TOUCHFILES } from './helpers/touchfiles-data'; import { selectTests } from './helpers/test-selection'; import { expectMentions } from './helpers/prompt-structure'; @@ -954,6 +954,42 @@ Next: /plan-eng-review after observing the workflow. The user declined launching } finally { fs.rmSync(dir, { recursive: true, force: true }); } }); + test('cap fixes the agent saved and passed to finalize validate; without them the report mismatches (marathon rerun 2026-10-19)', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'office-hours-cap-fixes-')); + try { + const { evidence, artifacts } = JSON.parse(JSON.stringify(structured()) + .replaceAll('/tmp/office-hours-fixture', dir.replaceAll('\\', '/'))); + evidence.toolCalls.filter((call: { output?: string }) => call.output?.startsWith('OFFICE_HOURS_VERDICT ')) + .forEach((call: { output?: string }, i: number) => { call.output = officeHoursVerdictReceipt(i + 1, artifacts[i].path, artifacts[i].content); }); + const applied = 'An absent mapped column stops the import and names the missing column.'; + const fixes = [{ id: 'R2-1', applied_text: applied }]; + const reviewDir = path.dirname(artifacts[0].path); + const fixesPath = path.join(reviewDir, 'fixes.json'); + const reportPath = path.join(dir, 'REPORT.md'); + fs.mkdirSync(path.dirname(evidence.designPath), { recursive: true }); + fs.writeFileSync(evidence.designPath, evidence.designContent.replace('# Design: RosterCheck\n', `# Design: RosterCheck\n\n${applied}\n`)); + fs.writeFileSync(reportPath, evidence.output); + fs.mkdirSync(reviewDir, { recursive: true }); + for (const artifact of artifacts) fs.writeFileSync(artifact.path, artifact.content); + const snapshots = snapshotsIn(reviewDir); + for (const snapshot of snapshots) fs.writeFileSync(snapshot.path, snapshot.content); + fs.writeFileSync(fixesPath, JSON.stringify(fixes)); + const result = Bun.spawnSync([process.execPath, path.resolve(import.meta.dir, '../bin/gstack-office-hours-review'), + 'finalize', '--design', evidence.designPath, '--report', reportPath, '--fixes', fixesPath, ...artifacts.map((artifact: { path: string }) => artifact.path)], + { cwd: dir, timeout: 5000 }); + expect(result.exitCode, result.stderr.toString()).toBe(0); + expect(JSON.parse(result.stdout.toString()).cap.verified).toEqual(['R2-1']); + evidence.output = fs.readFileSync(reportPath, 'utf8'); + evidence.designContent = fs.readFileSync(evidence.designPath, 'utf8'); + evidence.toolCalls.push({ tool: 'Write', input: { file_path: fixesPath, content: JSON.stringify(fixes) } }); + expect(observedSpecFixes(evidence.toolCalls)).toEqual(fixes); + expect(() => checkArtifacts({ ...evidence, fixes: observedSpecFixes(evidence.toolCalls) }, artifacts, snapshots)).not.toThrow(); + expect(() => checkArtifacts(evidence, artifacts, snapshots)).toThrow('Spec Review does not match'); + expect(() => checkArtifacts({ ...evidence, fixes: [{ id: 'R2-2', applied_text: applied }] }, artifacts, snapshots)).toThrow('Spec Review does not match'); + expect(observedSpecFixes(evidence.toolCalls.filter((call: { input?: { file_path?: string } }) => call.input?.file_path !== fixesPath))).toBeNull(); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); + test('allows a Markdown separator after the exact block, without hiding extra outcome prose', () => { const { evidence, artifacts } = structured(); evidence.output = evidence.output.replace('## Handoff', '---\n\n## Handoff'); diff --git a/test/office-hours-review.test.ts b/test/office-hours-review.test.ts index 7694d2218..f0a44d82b 100644 --- a/test/office-hours-review.test.ts +++ b/test/office-hours-review.test.ts @@ -9,6 +9,7 @@ import { } from '../lib/office-hours-review'; import { parseSpecFixes, verifySpecFixesAtCap, renderSpecCap } from '../lib/spec-review-cap'; import { expectMentions } from './helpers/prompt-structure'; +import { copyOfficeHoursFormatter } from './helpers/office-hours-formatter-fixture'; const cli = path.resolve(import.meta.dir, '../bin/gstack-office-hours-review'); function review(round = 1, count = 1, severity: 'blocking' | 'minor' = 'blocking'): OfficeHoursReview { @@ -638,3 +639,17 @@ describe('office-hours review CLI', () => { } })); }); + +describe('office-hours formatter fixture copy', () => { + test('the copied formatter loads every local module it imports (marathon 39032308051)', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'office-hours-formatter-copy-')); + try { + const formatter = copyOfficeHoursFormatter(dir); + const result = Bun.spawnSync([process.execPath, formatter], { cwd: dir, stdout: 'pipe', stderr: 'pipe', timeout: 5000 }); + expect(result.stderr.toString()).not.toContain('Cannot find module'); + expect(result.stderr.toString()).toContain('gstack-office-hours-review: usage:'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/test/skill-e2e-office-hours-section-loading.test.ts b/test/skill-e2e-office-hours-section-loading.test.ts index dcdfc1aec..c50b70747 100644 --- a/test/skill-e2e-office-hours-section-loading.test.ts +++ b/test/skill-e2e-office-hours-section-loading.test.ts @@ -14,7 +14,8 @@ import * as path from 'node:path'; import { describeE2ETier } from './helpers/e2e-gate'; import { setupSkillDir, skillFromWorktree, captureSectionReads } from './helpers/auq-sdk-capture'; import { CARVE_GUARDS } from './helpers/carve-guards'; -import { validateOfficeHoursCompletion, validateOfficeHoursReviewerHandoffs, validateOfficeHoursReviewArtifacts, validateOfficeHoursReviewPreservation } from './helpers/office-hours-completion'; +import { copyOfficeHoursFormatter } from './helpers/office-hours-formatter-fixture'; +import { observedSpecFixes, validateOfficeHoursCompletion, validateOfficeHoursReviewerHandoffs, validateOfficeHoursReviewArtifacts, validateOfficeHoursReviewPreservation } from './helpers/office-hours-completion'; const describeMarathon = describeE2ETier('marathon'); const runId = `office-hours-section-loading-${process.env.EVALS_RUN_ID ?? 'local'}`; @@ -42,15 +43,9 @@ describeMarathon('/office-hours full section-loading workflow (marathon)', () => const planDir = setupSkillDir({ skillName: guard.skill, skillMd, sectionsFrom, tmpPrefix: 'gstack-office-hours-secload-', }); - // Copy the real formatter and its small dependency set into the fixture. + // Copy the real formatter and its local imports into the fixture. // This exercises production code while keeping all outputs in the sandbox. - for (const relative of ['bin/gstack-office-hours-review', 'lib/office-hours-review.ts', 'lib/fs-atomic.ts']) { - const target = path.join(planDir, relative); - fs.mkdirSync(path.dirname(target), { recursive: true }); - fs.copyFileSync(path.resolve(__dirname, '..', relative), target); - } - const formatter = path.join(planDir, 'bin/gstack-office-hours-review'); - fs.chmodSync(formatter, 0o755); + const formatter = copyOfficeHoursFormatter(planDir); // Two rounds keep the live run inside its budget: round 1 (full) and round 2 (delta) run here; round 3 and CONVERGENCE are covered by free validator tests. const reviewRounds = 2; const capture = await captureSectionReads({ @@ -83,6 +78,7 @@ Completion delivery, after the full workflow and design approval: const reviewEvidence = validateOfficeHoursReviewArtifacts({ ...capture, designPath, designContent: fs.existsSync(designPath) ? fs.readFileSync(designPath, 'utf-8') : null, + fixes: observedSpecFixes(capture.toolCalls), }, artifacts, snapshots, reviewRounds); validateOfficeHoursReviewerHandoffs({ ...capture, designPath, From 6237994016ffed000ca38744182d5aec37696c99 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 19 Oct 2026 15:55:42 -0700 Subject: [PATCH 5/5] v1.91.157.0 perf(gen-skill-docs): drop the "When to invoke" body section from Claude SKILL.md files (port of #3129) (#3305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(gen-skill-docs): stop emitting the "When to invoke" body section The Claude catalog trim moved each skill's routing prose ("Use when...", "Proactively invoke...", voice triggers) out of the frontmatter description into a "## When to invoke this skill" section at the top of the body. Claude Code reads only name + description to choose a skill and loads the body after the skill is invoked, so that text cannot route anything and costs tokens on every invocation. Keep the one-line trimmed description; drop the section. The routing prose stays in each template's description, which --catalog-mode=full and the other hosts still render. Tests that pinned the section now assert its absence, or check the template instead. (cherry picked from commit 707ccd6f38a69dd646edc4393845921d84cb6ec1) * feat(gen-skill-docs): keep voice triggers in the root routing block The Claude catalog trim drops each skill's "Voice triggers (speech-to-text aliases)" line from its description, and the body section that used to hold it is gone. {{VOICE_TRIGGER_ROUTING}} renders them in the root router's routing block instead: one "Voice trigger ... → invoke /" line per skill, read from the templates' voice-triggers lists, so disabled-skill filtering applies to them like any other routing line. Other hosts keep the line in each description and render nothing here. extractVoiceTriggers moves to the new resolver so the generator and the resolver share one parser. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 645e75b1e8d25554eaf3a54e234f584b07cb267a) * chore: regenerate SKILL.md files without the "When to invoke" section Output of `bun run gen:skill-docs --host all` after the previous commits. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit ff4c813337d71293dd4568f51abac709092413dc) * test: refresh context-budget ceilings for the smaller skill bodies `bun test/helpers/capture-context-budget.ts`. Against a recapture of the base commit, 55 skills that lost the section get a lower ceiling and the root gstack router rises by 532 tokens for the voice-trigger routing lines (5,369 tokens lower in total). devex-review and eval-plan differ from the committed fixture only because it already lagged main. alwaysOnTotal is unchanged: the catalog descriptions did not change. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit d886fcecc1c668825140ed6a9d4b3b5d9fc5eba8) * test: refresh the claude ship golden without the "When to invoke" section Co-Authored-By: Claude Opus 5.5 (cherry picked from commit f1789dca5c3e319e42aa12c1ea0e69c4c79bb538) * test: lower the claude design-review size pin The size guard (#3199) asks for it: claude/design-review shrank by 437 chars without its "When to invoke" section. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 3a446fcc62c6df05e6d803c74b39a44e1e7373ab) * test: skip alias symlinks in the voice-routing check, keep unrelated budget ceilings, add #3129 CHANGELOG entry Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: rebuild the historical cookie approval with the dropped section, record the skill-fixture change as non-behavioral Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * v1.91.157.0 perf(gen-skill-docs): drop the "When to invoke" body section from Claude SKILL.md files (port of #3129) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: thanhnguyentung95 <17350411+thanhnguyentung95@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 19 ++++ SKILL.md | 29 ++++-- SKILL.md.tmpl | 2 +- VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- autoplan/SKILL.md | 13 --- benchmark-models/SKILL.md | 12 --- benchmark/SKILL.md | 12 --- browse/SKILL.md | 8 -- canary/SKILL.md | 9 -- careful/SKILL.md | 9 -- codex/SKILL.md | 11 -- context-restore/SKILL.md | 9 -- context-save/SKILL.md | 8 -- cso/SKILL.md | 7 -- design-consultation/SKILL.md | 9 -- design-html/SKILL.md | 13 --- design-review/SKILL.md | 10 -- design-shotgun/SKILL.md | 9 -- deslop-shared-libs/SKILL.md | 7 -- devex-review/SKILL.md | 13 --- diagram/SKILL.md | 8 -- document-generate/SKILL.md | 9 -- document-release/SKILL.md | 11 -- freeze/SKILL.md | 9 -- gstack-upgrade/SKILL.md | 9 -- guard/SKILL.md | 8 -- health/SKILL.md | 9 -- investigate/SKILL.md | 11 -- ios-clean/SKILL.md | 12 --- ios-design-review/SKILL.md | 15 --- ios-fix/SKILL.md | 14 --- ios-qa/SKILL.md | 15 --- ios-sync/SKILL.md | 11 -- land-and-deploy/SKILL.md | 8 -- landing-report/SKILL.md | 9 -- learn/SKILL.md | 9 -- make-pdf/SKILL.md | 11 -- office-hours/SKILL.md | 15 --- open-gstack-browser/SKILL.md | 10 -- package.json | 2 +- pair-agent/SKILL.md | 13 --- plan-ceo-review/SKILL.md | 12 --- plan-design-review/SKILL.md | 10 -- plan-devex-review/SKILL.md | 14 --- plan-eng-review/SKILL.md | 12 --- plan-tune/SKILL.md | 15 --- qa-only/SKILL.md | 11 -- qa/SKILL.md | 12 --- retro/SKILL.md | 9 -- review/SKILL.md | 8 -- scrape/SKILL.md | 6 -- scripts/gen-skill-docs.ts | 59 ++--------- scripts/harness-version.json | 2 +- scripts/resolvers/index.ts | 2 + scripts/resolvers/voice-triggers.ts | 61 +++++++++++ setup-browser-cookies/SKILL.md | 7 -- setup-deploy/SKILL.md | 10 -- setup-gbrain/SKILL.md | 7 -- ship/SKILL.md | 8 -- skillify/SKILL.md | 10 -- spec/SKILL.md | 8 -- sync-gbrain/SKILL.md | 9 -- test-audit/SKILL.md | 5 - test/catalog-mode-full.test.ts | 4 +- test/catalog-trim.test.ts | 49 ++------- test/cso-preserved.test.ts | 1 - test/fixtures/context-budget.json | 110 ++++++++++---------- test/fixtures/golden/claude-ship-SKILL.md | 8 -- test/fixtures/skill-md-size-allowlist.json | 2 +- test/gen-skill-docs.test.ts | 34 ++++-- test/helpers/manual-judge-review-fixture.ts | 2 + test/helpers/parity-harness.ts | 2 +- test/helpers/skill-fixture.ts | 6 +- test/skill-e2e-retro.test.ts | 1 - test/skill-validation.test.ts | 17 ++- unfreeze/SKILL.md | 7 -- 77 files changed, 214 insertions(+), 757 deletions(-) create mode 100644 scripts/resolvers/voice-triggers.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cafe27eea..a2da8817b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## [1.91.157.0] - 2026-10-19 + +**Claude SKILL.md files no longer carry a "When to invoke" body section, which saves about 20 KB across 56 skills.** Claude Code reads a skill's body only after the skill is invoked, so routing prose there never routed anything. Routing still comes from the catalog description and the CLAUDE.md routing block, and voice triggers move into the root router's routing block. From #3129 by @thanhnguyentung95. + +### Changed +- **Catalog trim (`scripts/gen-skill-docs.ts`):** the Claude render keeps the one-line catalog description and no longer inserts the routing and voice-trigger prose into the body. `--catalog-mode=full` and the other hosts are unchanged: they keep the full description. +- **Voice triggers in the root router:** a new `{{VOICE_TRIGGER_ROUTING}}` resolver (`scripts/resolvers/voice-triggers.ts`) adds one `- Voice trigger "…" → invoke /` line per skill to the root `SKILL.md` routing block, Claude only. That's 21 lines, and the root router's eager ceiling rises from 4289 to 4821 tokens. +- **Context budget:** 54 per-skill eager ceilings go down (net −5,139 tokens). The `claude/design-review` size pin drops by 437 chars, and the claude ship golden is refreshed. +- **Gate integrity:** the tests that pinned the removed section now assert its absence, or check the template description instead: + - `test/catalog-trim.test.ts` (the three `buildWhenToInvokeSection` tests go with the function); + - `test/catalog-mode-full.test.ts` and `test/cso-preserved.test.ts`; + - `test/gen-skill-docs.test.ts` (the benchmark routing and CSO voice tests read the template); + - `test/skill-validation.test.ts` (trigger and proactive phrases are checked in `SKILL.md.tmpl`); + - `test/helpers/parity-harness.ts` (no `## When to invoke` heading required); + - `test/helpers/skill-fixture.ts` and `test/skill-e2e-retro.test.ts` (the section is dropped from the review, review-army and retro E2E section lists). `scripts/harness-version.json` records the `skill-fixture.ts` change as non-behavioral: `HARNESS_VERSION` stays 6, and the regenerated SKILL.md bytes already start those cases' new series. + - `test/helpers/manual-judge-review-fixture.ts` puts the removed section back when it rebuilds the historical cookie-workflow approval, so the approved prompt hash and line range still match. The approval itself is unchanged. + + One new free test checks that the root router routes every voice trigger, and it skips the `connect-chrome` alias symlink. No threshold, trial count, case kind or judge changed. No context-budget ceiling rose except the root router's. + ## [1.91.156.0] - 2026-10-19 **The weekly marathon's office-hours case can run its review formatter again, and its validator now checks cap fixes the way the formatter does.** Both defects came from #3164 and were in the test, not the skill. The marathon fixture copied `bin/gstack-office-hours-review` with a hand-kept list of two library files; #3164 added a third import (`lib/spec-review-cap.ts`), so every formatter command in the fixture failed at module load, the agent correctly declared the review UNREVIEWED, and the case went red in marathon run 39032308051 (#3246). With the module restored, a focused rerun showed the second defect: when the review stops at the round cap and the agent applies the remaining blocking fixes with `--fixes`, the formatter records them as verified, but the test never gave the validator that list, so it expected "fixes unconfirmed" and rejected a correct report. diff --git a/SKILL.md b/SKILL.md index 2508bc9a9..526670028 100644 --- a/SKILL.md +++ b/SKILL.md @@ -16,14 +16,6 @@ triggers: - -## When to invoke this skill - -Sends any gstack request to the right skill -(planning, review, QA, shipping, debugging, docs, security, design). Routes QA by -intent and browser interaction to /browse. Use when you invoke gstack without a specific -skill, or ask "which gstack skill fits this?". - ## Preamble (run first) ```bash @@ -230,6 +222,27 @@ Generic “second opinion”, “outside review”, or “cross-model review” - User asks what gstack has learned, "show learnings" → invoke `/learn` - 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 "auto plan", "automatic review" → invoke `/autoplan` +- Voice trigger "speed test", "check performance" → invoke `/benchmark` +- Voice trigger "compare models", "model shootout", "which model is best" → invoke `/benchmark-models` +- Voice trigger "open the browser", "look at this page" → invoke `/browse` +- Voice trigger "code x", "code ex", "get another opinion" → invoke `/codex` +- Voice trigger "see-so", "see so", "security review", "security check", "vulnerability scan", "run security" → invoke `/cso` +- Voice trigger "build the design", "code the mockup", "make it real" → invoke `/design-html` +- Voice trigger "dx audit", "test the developer experience", "try the onboarding", "developer experience test" → invoke `/devex-review` +- Voice trigger "upgrade the tools", "update the tools", "gee stack upgrade", "g stack upgrade" → invoke `/gstack-upgrade` +- Voice trigger "clean the iOS debug bridge", "remove DebugBridge", "strip the gstack iOS instrumentation" → invoke `/ios-clean` +- Voice trigger "review the iOS design", "audit the iPhone app's visuals", "design QA the iPhone app" → invoke `/ios-design-review` +- Voice trigger "fix the iOS bug", "patch the iPhone app", "auto-fix the iOS issue" → invoke `/ios-fix` +- Voice trigger "iOS quality check", "test the iPhone app", "test the iPad app", "run iOS QA" → invoke `/ios-qa` +- Voice trigger "resync the iOS debug bridge", "regenerate iOS accessors", "update the gstack iOS instrumentation" → invoke `/ios-sync` +- Voice trigger "make this a pdf", "make it a pdf", "export to pdf", "turn this into a pdf", "turn this markdown into a pdf", "generate a pdf", "make a pdf from", "pdf this markdown" → invoke `/make-pdf` +- Voice trigger "show me the browser" → invoke `/open-gstack-browser` +- Voice trigger "pair agent", "connect agent", "share my browser", "remote browser access" → invoke `/pair-agent` +- Voice trigger "dx review", "developer experience review", "devex review", "devex audit", "API design review", "onboarding review" → invoke `/plan-devex-review` +- Voice trigger "tech review", "technical review", "plan engineering review" → invoke `/plan-eng-review` +- Voice trigger "quality check", "test the app", "run QA" → invoke `/qa` +- Voice trigger "bug report", "just check for bugs" → invoke `/qa-only` **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 8065cd9c7..3326854f2 100644 --- a/SKILL.md.tmpl +++ b/SKILL.md.tmpl @@ -97,7 +97,7 @@ quality gates that produce better results than answering inline. - User asks to plan, preregister, price or pilot a paid eval → invoke `/eval-plan` - User asks what gstack has learned, "show learnings" → invoke `/learn` - User asks to tune question sensitivity, "stop asking me that" → invoke `/plan-tune` -- User asks for code quality dashboard, "health check" → invoke `/health` +- User asks for code quality dashboard, "health check" → invoke `/health`{{VOICE_TRIGGER_ROUTING}} **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 59c1b7f37..6c4ab49a6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.156.0 +1.91.157.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index b5dfb1d5f..bcc293a95 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.156.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.157.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/autoplan/SKILL.md b/autoplan/SKILL.md index ace175323..e1bed2255 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -32,19 +32,6 @@ hooks: - -## When to invoke this skill - -Surfaces -taste decisions (close approaches, borderline scope, outside-review disagreements) at a final -approval gate. One command, fully reviewed plan out. -Use when asked to "auto review", "autoplan", "run all reviews", "review this plan -automatically", or "make the decisions for me". -Proactively suggest when the user has a plan file and wants to run the full review -gauntlet without answering 15-30 intermediate questions. - -Voice triggers (speech-to-text aliases): "auto plan", "automatic review". - ## Preamble (run first) ```bash diff --git a/benchmark-models/SKILL.md b/benchmark-models/SKILL.md index 359eb40ed..27893eb44 100644 --- a/benchmark-models/SKILL.md +++ b/benchmark-models/SKILL.md @@ -16,18 +16,6 @@ allowed-tools: - -## When to invoke this skill - -Runs the same prompt through Claude, -GPT (via Codex CLI), and Gemini side-by-side — compares latency, tokens, cost, -and optionally quality via LLM judge. Answers "which model is actually best -for this skill?" with data instead of vibes. Separate from /benchmark, which -measures web page performance. Use when: "benchmark models", "compare models", -"which model is best for X", "cross-model comparison", "model shootout". - -Voice triggers (speech-to-text aliases): "compare models", "model shootout", "which model is best". - ## Preamble (run first) ```bash diff --git a/benchmark/SKILL.md b/benchmark/SKILL.md index df25dcc57..0f12040c1 100644 --- a/benchmark/SKILL.md +++ b/benchmark/SKILL.md @@ -17,18 +17,6 @@ allowed-tools: - -## When to invoke this skill - -Establishes -baselines for page load times, Core Web Vitals, and resource sizes. -Compares before/after on every PR. Tracks performance trends over time. -Use when: "web performance benchmark", "page speed", "lighthouse", "web vitals", -"bundle size", "load time". Not for AI model, agent eval or business -benchmarks; use /benchmark-models for cross-model skill comparisons. - -Voice triggers (speech-to-text aliases): "speed test", "check performance". - ## Preamble (run first) ```bash diff --git a/browse/SKILL.md b/browse/SKILL.md index 1505815f6..7f1842b0a 100644 --- a/browse/SKILL.md +++ b/browse/SKILL.md @@ -16,14 +16,6 @@ allowed-tools: - -## When to invoke this skill - -Use when asked to open a site, test a page, take a -screenshot, or dogfood a flow. - -Voice triggers (speech-to-text aliases): "open the browser", "look at this page". - ## Preamble (run first) ```bash diff --git a/canary/SKILL.md b/canary/SKILL.md index d0b70af71..dd2e3c572 100644 --- a/canary/SKILL.md +++ b/canary/SKILL.md @@ -17,15 +17,6 @@ triggers: - -## When to invoke this skill - -Watches the live app for console errors, -performance regressions, and page failures. Takes -periodic screenshots, compares against pre-deploy baselines, and alerts -on anomalies. Use when: "monitor deploy", "canary", "post-deploy check", -"watch production", "verify deploy". - ## Preamble (run first) ```bash diff --git a/careful/SKILL.md b/careful/SKILL.md index a92bf953f..bf5aaca88 100644 --- a/careful/SKILL.md +++ b/careful/SKILL.md @@ -25,15 +25,6 @@ hooks: - -## When to invoke this skill - -Warns before rm -rf, DROP TABLE, -force-push, git reset --hard, kubectl delete, and similar destructive operations. -User can override each warning. Use when touching prod, debugging live systems, -or working in a shared environment. Use when asked to "be careful", "safety mode", -"prod mode", or "careful mode". - # /careful — Destructive Command Guardrails Safety mode is now **active**. Every Bash and PowerShell command will be checked diff --git a/codex/SKILL.md b/codex/SKILL.md index 72c04a24a..033ecb507 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -18,17 +18,6 @@ allowed-tools: - -## When to invoke this skill - -Code review: independent diff review via -codex review with pass/fail gate. Challenge: adversarial mode that tries to break -your code. Consult: ask codex anything with session continuity for follow-ups. -The "200 IQ autistic developer" second opinion. Use when asked to "codex review", -"codex challenge", "ask codex", "second opinion", or "consult codex". - -Voice triggers (speech-to-text aliases): "code x", "code ex", "get another opinion". - ## Preamble (run first) ```bash diff --git a/context-restore/SKILL.md b/context-restore/SKILL.md index ea94985f4..4238273ad 100644 --- a/context-restore/SKILL.md +++ b/context-restore/SKILL.md @@ -19,15 +19,6 @@ triggers: - -## When to invoke this skill - -Loads the most recent -saved state (preferring the current branch, falling back across branches) so -you can pick up where you left off — even across Conductor workspace handoffs. -Use when asked to "resume", "restore context", "where was I", or -"pick up where I left off". Pair with /context-save. - ## Preamble (run first) ```bash diff --git a/context-save/SKILL.md b/context-save/SKILL.md index d68858b9c..851260191 100644 --- a/context-save/SKILL.md +++ b/context-save/SKILL.md @@ -19,14 +19,6 @@ triggers: - -## When to invoke this skill - -Captures git state, decisions made, and remaining work -so any future session can pick up without losing a beat. -Use when asked to "save progress", "save state", "context save", or -"save my work". Pair with /context-restore to resume later. - ## Preamble (run first) ```bash diff --git a/cso/SKILL.md b/cso/SKILL.md index 1b8030423..9b3255828 100644 --- a/cso/SKILL.md +++ b/cso/SKILL.md @@ -13,13 +13,6 @@ triggers: - -## When to invoke this skill - -Use when: "security audit", "threat model", "OWASP", "CSO review", "recheck a vulnerability". - -Voice triggers (speech-to-text aliases): "see-so", "see so", "security review", "security check", "vulnerability scan", "run security". - # /cso — evidence before assurance Find exploitable defects. State attacker, boundary, impact, and challenge. Static assessment remains available without runtime or scanner profiles. Qualified comprehensive profiles add reproduction and repair candidates without changing the branch. Trusted `gstack-cso` owns execution, persistence, and proof labels. diff --git a/design-consultation/SKILL.md b/design-consultation/SKILL.md index 61e47c0a8..48a8b38c6 100644 --- a/design-consultation/SKILL.md +++ b/design-consultation/SKILL.md @@ -43,15 +43,6 @@ gbrain: - -## When to invoke this skill - -Creates DESIGN.md as your project's design source -of truth. For existing sites, use /plan-design-review to infer the system instead. -Use when asked to "design system", "brand guidelines", or "create DESIGN.md". -Proactively suggest when starting a new project's UI with no existing -design system or DESIGN.md. - ## Preamble (run first) ```bash diff --git a/design-html/SKILL.md b/design-html/SKILL.md index 28924e91a..cc8ec3b50 100644 --- a/design-html/SKILL.md +++ b/design-html/SKILL.md @@ -20,19 +20,6 @@ allowed-tools: - -## When to invoke this skill - -Works with approved mockups from /design-shotgun, CEO plans from /plan-ceo-review, -design review context from /plan-design-review, or from scratch with a user -description. Text actually reflows, heights are computed, layouts are dynamic. -30KB overhead, zero deps. Smart API routing: picks the right Pretext patterns -for each design type. Use when: "finalize this design", "turn this into HTML", -"build me a page", "implement this design", or after any planning skill. -Proactively suggest when user has approved a design or has a plan ready. - -Voice triggers (speech-to-text aliases): "build the design", "code the mockup", "make it real". - ## Preamble (run first) ```bash diff --git a/design-review/SKILL.md b/design-review/SKILL.md index 1df794548..894703d60 100644 --- a/design-review/SKILL.md +++ b/design-review/SKILL.md @@ -20,16 +20,6 @@ triggers: - -## When to invoke this skill - -Iteratively fixes issues -in source code, committing each fix atomically and re-verifying with before/after -screenshots. For plan-mode design review (before implementation), use /plan-design-review. -Use when asked to "audit the design", "visual QA", "check if it looks good", or "design polish". -Proactively suggest when the user mentions visual inconsistencies or -wants to polish the look of a live site. - ## Preamble (run first) ```bash diff --git a/design-shotgun/SKILL.md b/design-shotgun/SKILL.md index 0ae07b42f..3287bb48d 100644 --- a/design-shotgun/SKILL.md +++ b/design-shotgun/SKILL.md @@ -37,15 +37,6 @@ gbrain: - -## When to invoke this skill - -Standalone design exploration you can -run anytime. Use when: "explore designs", "show me options", "design variants", -"visual brainstorm", or "I don't like how this looks". -Proactively suggest when the user describes a UI feature but hasn't seen -what it could look like. - ## Preamble (run first) ```bash diff --git a/deslop-shared-libs/SKILL.md b/deslop-shared-libs/SKILL.md index 64a7b68e2..6c65869de 100644 --- a/deslop-shared-libs/SKILL.md +++ b/deslop-shared-libs/SKILL.md @@ -14,13 +14,6 @@ triggers: - -## When to invoke this skill - -Use for -/deslop-shared-libs, $deslop-shared-libs, or requests to find code worth sharing. -Recommendations only. - # Find code worth sharing Find up to five new opportunities to share code, then recommend the best three. diff --git a/devex-review/SKILL.md b/devex-review/SKILL.md index 22866c372..80b2bb0ad 100644 --- a/devex-review/SKILL.md +++ b/devex-review/SKILL.md @@ -19,19 +19,6 @@ allowed-tools: - -## When to invoke this skill - -Actually TESTS the developer experience -in the Aside browser: navigates docs, tries the getting started flow, times -TTHW, screenshots error messages, evaluates CLI help text. Produces a DX -scorecard with evidence. Compares against /plan-devex-review scores if they -exist (the boomerang: plan said 3 minutes, reality says 8). Use when asked to -"test the DX", "DX audit", "developer experience test", or "try the -onboarding". Proactively suggest after shipping a developer-facing feature. - -Voice triggers (speech-to-text aliases): "dx audit", "test the developer experience", "try the onboarding", "developer experience test". - ## Preamble (run first) ```bash diff --git a/diagram/SKILL.md b/diagram/SKILL.md index 839fd3b45..31d2b36ec 100644 --- a/diagram/SKILL.md +++ b/diagram/SKILL.md @@ -19,14 +19,6 @@ triggers: - -## When to invoke this skill - -The SVG/PNG use clean mermaid style; the -.excalidraw carries the hand-drawn aesthetic. Fully offline. -Use when asked to "make a diagram", "draw the architecture", "create a -flowchart", "diagram this", or "visualize this flow". - ## Preamble (run first) ```bash diff --git a/document-generate/SKILL.md b/document-generate/SKILL.md index 7d63c448c..f262eb2f9 100644 --- a/document-generate/SKILL.md +++ b/document-generate/SKILL.md @@ -23,15 +23,6 @@ triggers: - -## When to invoke this skill - -Uses the Diataxis framework (tutorial / how-to / reference / explanation) to produce -complete, structured documentation. Can be invoked standalone or called by -/document-release when it finds coverage gaps. Use when asked to "write docs", -"generate documentation", "document this feature", "create a tutorial", or -"explain this module". - ## Preamble (run first) ```bash diff --git a/document-release/SKILL.md b/document-release/SKILL.md index 70e56147f..8c1690ca4 100644 --- a/document-release/SKILL.md +++ b/document-release/SKILL.md @@ -19,17 +19,6 @@ triggers: - -## When to invoke this skill - -Reads relevant project docs, cross-references the -diff, builds a Diataxis coverage map (reference/how-to/tutorial/explanation), -updates README/ARCHITECTURE/CONTRIBUTING/CLAUDE.md to match what shipped, -detects architecture diagram drift, polishes CHANGELOG voice with a sell-test -rubric, cleans up TODOS, and optionally bumps VERSION. Surfaces documentation -debt in the PR body. Use when asked to "update the docs", "sync documentation", -or "post-ship docs". Proactively suggest a documentation audit before merge. - ## Preamble (run first) ```bash diff --git a/freeze/SKILL.md b/freeze/SKILL.md index 23dd76f48..02f85237b 100644 --- a/freeze/SKILL.md +++ b/freeze/SKILL.md @@ -31,15 +31,6 @@ hooks: - -## When to invoke this skill - -Blocks Edit, -Write and NotebookEdit outside the allowed path. Use when debugging to prevent accidentally -"fixing" unrelated code, or when you want to scope changes to one module. -Use when asked to "freeze", "restrict edits", "only edit this folder", -or "lock down edits". - # /freeze — Restrict Edits to a Directory Lock file edits to a specific directory. Any Edit, Write or NotebookEdit diff --git a/gstack-upgrade/SKILL.md b/gstack-upgrade/SKILL.md index d95b649e6..2af64b6ce 100644 --- a/gstack-upgrade/SKILL.md +++ b/gstack-upgrade/SKILL.md @@ -15,15 +15,6 @@ allowed-tools: - -## When to invoke this skill - -Detects global vs vendored install, -runs the upgrade, and shows what's new. Use when asked to "upgrade gstack", -"update gstack", or "get latest version". - -Voice triggers (speech-to-text aliases): "upgrade the tools", "update the tools", "gee stack upgrade", "g stack upgrade". - # /gstack-upgrade Upgrade gstack to the latest version and show what's new. diff --git a/guard/SKILL.md b/guard/SKILL.md index 2b2117db8..ef4b7bd79 100644 --- a/guard/SKILL.md +++ b/guard/SKILL.md @@ -41,14 +41,6 @@ hooks: - -## When to invoke this skill - -Combines /careful (warns before rm -rf, DROP TABLE, force-push, etc.) with -/freeze (blocks edits outside a specified directory). Use for maximum safety -when touching prod or debugging live systems. Use when asked to "guard mode", -"full safety", "lock it down", or "maximum safety". - # /guard — Full Safety Mode Activates both destructive command warnings and directory-scoped edit restrictions. diff --git a/health/SKILL.md b/health/SKILL.md index e90b83647..ae587a8d5 100644 --- a/health/SKILL.md +++ b/health/SKILL.md @@ -19,15 +19,6 @@ allowed-tools: - -## When to invoke this skill - -Wraps existing project tools (type checker, linter, -test runner, dead code detector, shell linter), computes a weighted composite -0-10 score, and tracks trends over time. Use when: "health check", -"code quality", "how healthy is the codebase", "run all checks", -"quality score". - ## Preamble (run first) ```bash diff --git a/investigate/SKILL.md b/investigate/SKILL.md index 7e5fb7a8f..659937b9f 100644 --- a/investigate/SKILL.md +++ b/investigate/SKILL.md @@ -61,17 +61,6 @@ gbrain: - -## When to invoke this skill - -Four phases: investigate, -analyze, hypothesize, implement. Iron Law: no fixes without root cause. -Use when asked to "debug this", "fix this bug", "why is this broken", -"investigate this error", or "root cause analysis". -Proactively invoke this skill (do NOT debug directly) when the user reports -errors, 500 errors, stack traces, unexpected behavior, "it was working -yesterday", or is troubleshooting why something stopped working. - ## Preamble (run first) ```bash diff --git a/ios-clean/SKILL.md b/ios-clean/SKILL.md index 9658e6447..5e176a996 100644 --- a/ios-clean/SKILL.md +++ b/ios-clean/SKILL.md @@ -18,18 +18,6 @@ triggers: - -## When to invoke this skill - -Cleans up StateServer, DebugOverlay, accessor codegen output, and -app-side hooks installed by /ios-qa. This is a convenience wrapper — -the structural Release-build guard (Package.swift conditional + CI -swift build -c release check) is the safety-critical path. -Use when asked to "clean the iOS debug bridge", "remove DebugBridge", -or "strip the gstack iOS instrumentation". - -Voice triggers (speech-to-text aliases): "clean the iOS debug bridge", "remove DebugBridge", "strip the gstack iOS instrumentation". - ## Preamble (run first) ```bash diff --git a/ios-design-review/SKILL.md b/ios-design-review/SKILL.md index 67f0bae28..bc25c0188 100644 --- a/ios-design-review/SKILL.md +++ b/ios-design-review/SKILL.md @@ -17,21 +17,6 @@ triggers: - -## When to invoke this skill - -Connects to a real -iPhone via the same StateServer as /ios-qa, screenshots every screen, -evaluates against Apple HIG, DESIGN.md, and design best practices. Scores -each dimension 0-10 with "what would make it a 10" framing — mirrors -/plan-design-review for browser. For plan-stage design review (before -implementation), use /plan-design-review. For live web visual audits, use -/design-review. -Use when asked to "review the iOS design", "audit the iPhone app's -visuals", or "design QA the iOS app". - -Voice triggers (speech-to-text aliases): "review the iOS design", "audit the iPhone app's visuals", "design QA the iPhone app". - ## Preamble (run first) ```bash diff --git a/ios-fix/SKILL.md b/ios-fix/SKILL.md index 7e92bbe68..4266f9a72 100644 --- a/ios-fix/SKILL.md +++ b/ios-fix/SKILL.md @@ -19,20 +19,6 @@ triggers: - -## When to invoke this skill - -Takes a bug found by /ios-qa, reads the source, -writes the fix, rebuilds, redeploys, and verifies the fix on the real -device. Closes the loop: find bug → fix bug → confirm fix — zero human -intervention. Captures the pre-bug state snapshot as a regression test -fixture, so the bug can never recur silently. -Use when /ios-qa reports a bug and you want it fixed automatically, or -when asked to "fix this iOS bug", "patch the iPhone app", or "auto-fix -the iOS issue". - -Voice triggers (speech-to-text aliases): "fix the iOS bug", "patch the iPhone app", "auto-fix the iOS issue". - ## Preamble (run first) ```bash diff --git a/ios-qa/SKILL.md b/ios-qa/SKILL.md index fdaa7559d..421abc1d2 100644 --- a/ios-qa/SKILL.md +++ b/ios-qa/SKILL.md @@ -22,21 +22,6 @@ triggers: - -## When to invoke this skill - -Connects to a real iPhone or iPad via -USB CoreDevice IPv6 tunnel, reads Swift source to understand every screen, then -runs a vision-driven agent loop: screenshot → analyze → decide → act → -verify → repeat. All interaction happens via HTTP to an embedded -StateServer in the app under test. Optionally exposes the device over -Tailscale so remote agents (OpenClaw, Codex, any HTTP-capable agent) can -run iOS QA from anywhere without touching the hardware. -Use when asked to "ios qa", "test my iPhone app", "test my iPad app", -"find bugs on the device", or "qa the iOS app". - -Voice triggers (speech-to-text aliases): "iOS quality check", "test the iPhone app", "test the iPad app", "run iOS QA". - ## Preamble (run first) ```bash diff --git a/ios-sync/SKILL.md b/ios-sync/SKILL.md index e2388d078..acc3e9cc9 100644 --- a/ios-sync/SKILL.md +++ b/ios-sync/SKILL.md @@ -19,17 +19,6 @@ triggers: - -## When to invoke this skill - -Updates StateServer.swift, DebugOverlay.swift, Package.swift, -and the typed @Observable state accessors. Use after you upgrade gstack -or add new ViewModels/properties that need accessor coverage. -Use when asked to "resync the iOS debug bridge", "regenerate iOS -accessors", or "update the gstack iOS instrumentation". - -Voice triggers (speech-to-text aliases): "resync the iOS debug bridge", "regenerate iOS accessors", "update the gstack iOS instrumentation". - ## Preamble (run first) ```bash diff --git a/land-and-deploy/SKILL.md b/land-and-deploy/SKILL.md index a3f85747b..5e0edd2af 100644 --- a/land-and-deploy/SKILL.md +++ b/land-and-deploy/SKILL.md @@ -17,14 +17,6 @@ triggers: - -## When to invoke this skill - -Merges the PR, waits for CI and deploy, -verifies production health via canary checks. Takes over after /ship -creates the PR. Use when: "merge", "land", "deploy", "merge and verify", -"land it", "ship it to production". - ## Preamble (run first) ```bash diff --git a/landing-report/SKILL.md b/landing-report/SKILL.md index 1357bed16..6571b4ffb 100644 --- a/landing-report/SKILL.md +++ b/landing-report/SKILL.md @@ -16,15 +16,6 @@ allowed-tools: - -## When to invoke this skill - -Shows which VERSION slots -are currently claimed by open PRs, which sibling Conductor workspaces have -WIP work likely to ship soon, and what slot /ship would pick next. No -mutations — just a snapshot. Use when asked to "landing report", "what's in -the queue", "show me open PRs", or "which version do I claim next". - # /landing-report — Version Queue Dashboard ## Preamble (run first) diff --git a/learn/SKILL.md b/learn/SKILL.md index aad4a1d99..efbccae5d 100644 --- a/learn/SKILL.md +++ b/learn/SKILL.md @@ -19,15 +19,6 @@ allowed-tools: - -## When to invoke this skill - -Review, search, prune, and export what gstack -has learned across sessions. Use when asked to "what have we learned", -"show learnings", "prune stale learnings", or "export learnings". -Proactively suggest when the user asks about past patterns or wonders -"didn't we fix this before?" - ## Preamble (run first) ```bash diff --git a/make-pdf/SKILL.md b/make-pdf/SKILL.md index 7c07e515e..92b788f8e 100644 --- a/make-pdf/SKILL.md +++ b/make-pdf/SKILL.md @@ -16,17 +16,6 @@ allowed-tools: - -## When to invoke this skill - -Proper 1in margins, -intelligent page breaks, page numbers, cover pages, running headers, curly -quotes and em dashes, clickable TOC, diagonal DRAFT watermark. Not a draft -artifact — a finished artifact. Use when asked to "make a PDF", "export to -PDF", "turn this markdown into a PDF", or "generate a document". - -Voice triggers (speech-to-text aliases): "make this a pdf", "make it a pdf", "export to pdf", "turn this into a pdf", "turn this markdown into a pdf", "generate a pdf", "make a pdf from", "pdf this markdown". - ## Preamble (run first) ```bash diff --git a/office-hours/SKILL.md b/office-hours/SKILL.md index 81ab5eb42..190afe54d 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -47,21 +47,6 @@ gbrain: - -## When to invoke this skill - -Startup mode: six forcing questions that expose -demand reality, status quo, desperate specificity, narrowest wedge, observation, -and future-fit. Builder mode: design thinking brainstorming for side projects, -hackathons, learning, and open source. Saves a design doc. -Use when asked to "brainstorm this", "I have an idea", "help me think through -this", "office hours", or "is this worth building". -Proactively invoke this skill (do NOT answer directly) when the user describes -a new product idea, asks whether something is worth building, wants to think -through design decisions for something that doesn't exist yet, or is exploring -a concept before any code is written. -Use before /plan-ceo-review or /plan-eng-review. - ## Preamble (run first) ```bash diff --git a/open-gstack-browser/SKILL.md b/open-gstack-browser/SKILL.md index b69670f12..a46632af4 100644 --- a/open-gstack-browser/SKILL.md +++ b/open-gstack-browser/SKILL.md @@ -16,16 +16,6 @@ allowed-tools: - -## When to invoke this skill - -Opens a visible browser window where you can watch every action in real time. -The sidebar shows a live activity feed and chat. Anti-bot stealth built in. -Use when asked to "open gstack browser", "launch browser", "connect chrome", -"open chrome", "real browser", "launch chrome", "side panel", or "control my browser". - -Voice triggers (speech-to-text aliases): "show me the browser". - ## Preamble (run first) ```bash diff --git a/package.json b/package.json index 603e2ffd8..dce145af6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.156", + "version": "1.91.157", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/pair-agent/SKILL.md b/pair-agent/SKILL.md index 1302a9bcd..58ee1bc8c 100644 --- a/pair-agent/SKILL.md +++ b/pair-agent/SKILL.md @@ -16,19 +16,6 @@ allowed-tools: - -## When to invoke this skill - -One command generates a setup key and -prints instructions the other agent can follow to connect. Works with OpenClaw, -Hermes, Codex, Cursor, or any agent that can make HTTP requests. The remote agent -gets its own tab with full page access by default (the pairing ceremony is the -trust boundary; --restrict narrows it). -Use when asked to "pair agent", "connect agent", "share browser", "remote browser", -"let another agent use my browser", or "give browser access". - -Voice triggers (speech-to-text aliases): "pair agent", "connect agent", "share my browser", "remote browser access". - ## Preamble (run first) ```bash diff --git a/plan-ceo-review/SKILL.md b/plan-ceo-review/SKILL.md index ca165e713..2324db2f8 100644 --- a/plan-ceo-review/SKILL.md +++ b/plan-ceo-review/SKILL.md @@ -49,18 +49,6 @@ gbrain: - -## When to invoke this skill - -Rethink the problem, find the 10-star product, -challenge premises, expand scope when it creates a better product. Four modes: -SCOPE EXPANSION (dream big), SELECTIVE EXPANSION (hold scope + cherry-pick -expansions), HOLD SCOPE (maximum rigor), SCOPE REDUCTION (strip to essentials). -Use when asked to "think bigger", "expand scope", "strategy review", "rethink this", -or "is this ambitious enough". -Proactively suggest when the user is questioning scope or ambition of a plan, -or when the plan feels like it could be thinking bigger. - ## Preamble (run first) ```bash diff --git a/plan-design-review/SKILL.md b/plan-design-review/SKILL.md index 0d4ee094d..1ee12b84b 100644 --- a/plan-design-review/SKILL.md +++ b/plan-design-review/SKILL.md @@ -18,16 +18,6 @@ triggers: - -## When to invoke this skill - -Rates each design dimension 0-10, explains what would make it a 10, -then fixes the plan to get there. Works in plan mode. For live site -visual audits, use /design-review. Use when asked to "review the design plan" -or "design critique". -Proactively suggest when the user has a plan with UI/UX components that -should be reviewed before implementation. - # /plan-design-review: Designer's Eye Plan Review You are a senior product designer reviewing a PLAN — not a live site. Your job is diff --git a/plan-devex-review/SKILL.md b/plan-devex-review/SKILL.md index c3a09395d..f0050d3cf 100644 --- a/plan-devex-review/SKILL.md +++ b/plan-devex-review/SKILL.md @@ -19,20 +19,6 @@ triggers: - -## When to invoke this skill - -Explores developer personas, -benchmarks against competitors, designs magical moments, and traces friction -points before scoring. Three modes: DX EXPANSION (competitive advantage), -DX POLISH (bulletproof every touchpoint), DX TRIAGE (critical gaps only). -Use when asked to "DX review", "developer experience audit", "devex review", -or "API design review". -Proactively suggest when the user has a plan for developer-facing products -(APIs, CLIs, SDKs, libraries, platforms, docs). - -Voice triggers (speech-to-text aliases): "dx review", "developer experience review", "devex review", "devex audit", "API design review", "onboarding review". - ## Preamble (run first) ```bash diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index 1d86bb703..1dd4d34c4 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -19,18 +19,6 @@ triggers: - -## When to invoke this skill - -Lock in the execution plan — architecture, -data flow, diagrams, edge cases, test coverage, performance. Walks through -issues interactively with opinionated recommendations. Use when asked to -"review the architecture", "engineering review", or "lock in the plan". -Proactively suggest when the user has a plan or design doc and is about to -start coding — to catch architecture issues before implementation. - -Voice triggers (speech-to-text aliases): "tech review", "technical review", "plan engineering review". - # Plan Review Mode Review the selected target. Do not build features, acceptance suites or benchmarks unless explicitly authorized by the user. Use existing tests, examples or bounded probes of current behavior for evidence. diff --git a/plan-tune/SKILL.md b/plan-tune/SKILL.md index a557c3f26..dee752e2c 100644 --- a/plan-tune/SKILL.md +++ b/plan-tune/SKILL.md @@ -23,21 +23,6 @@ allowed-tools: - -## When to invoke this skill - -Review which AskUserQuestion prompts fire across gstack skills, set per-question preferences -(never-ask / always-ask / ask-only-for-one-way), inspect the dual-track -profile (what you declared vs what your behavior suggests), and enable/disable -question tuning. Conversational interface — no CLI syntax required. - -Use when asked to "tune questions", "stop asking me that", "too many questions", -"show my profile", "what questions have I been asked", "show my vibe", -"developer profile", or "turn off question tuning". - -Proactively suggest when the user says the same gstack question has come up before, -or when they explicitly override a recommendation for the Nth time. - ## Preamble (run first) ```bash diff --git a/qa-only/SKILL.md b/qa-only/SKILL.md index 960d54419..2a662251f 100644 --- a/qa-only/SKILL.md +++ b/qa-only/SKILL.md @@ -17,17 +17,6 @@ triggers: - -## When to invoke this skill - -Produces a -structured report with contract evidence or browser scores and repro steps — but never -fixes anything. Use when asked to "just report bugs", "qa report only", or -"test but don't fix". For the full test-fix-verify loop, use /qa instead. -Proactively suggest when the user wants a bug report without any code changes. - -Voice triggers (speech-to-text aliases): "bug report", "just check for bugs". - ## Preamble (run first) ```bash diff --git a/qa/SKILL.md b/qa/SKILL.md index 3876849b4..5d4bd506b 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -20,18 +20,6 @@ triggers: - -## When to invoke this skill - -Commit verified fixes atomically. Use when asked to "qa", "QA", "test this site", "find bugs", -"test and fix", or "fix what's broken". -Proactively suggest when the user says a feature is ready for testing -or asks "does this work?". Three tiers: Quick (critical/high only), -Standard (+ medium), Exhaustive (+ cosmetic). Produces contract outcomes or browser health scores, -fix evidence, and a ship-readiness summary. For report-only mode, use /qa-only. - -Voice triggers (speech-to-text aliases): "quality check", "test the app", "run QA". - ## Preamble (run first) ```bash diff --git a/retro/SKILL.md b/retro/SKILL.md index b0650d40f..505bd77c3 100644 --- a/retro/SKILL.md +++ b/retro/SKILL.md @@ -37,15 +37,6 @@ gbrain: - -## When to invoke this skill - -Analyzes commit history, work patterns, -and code quality metrics with persistent history and trend tracking. -Team-aware: breaks down per-person contributions with praise and growth areas. -Use when asked to "weekly retro", "what did we ship", or "engineering retrospective". -Proactively suggest at the end of a work week or sprint. - ## Preamble (run first) ```bash diff --git a/review/SKILL.md b/review/SKILL.md index f5786affa..c6e3509b5 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -22,14 +22,6 @@ triggers: - -## When to invoke this skill - -Analyzes diff against the base branch for SQL safety, LLM trust -boundary violations, conditional side effects, and other structural issues. Use when -asked to "review this PR", "code review", "pre-landing review", or "check my diff". -Proactively suggest when the user is about to merge or land code changes. - ## Preamble (run first) ```bash diff --git a/scrape/SKILL.md b/scrape/SKILL.md index a5eeb60a6..69e7e7aee 100644 --- a/scrape/SKILL.md +++ b/scrape/SKILL.md @@ -17,12 +17,6 @@ triggers: - -## When to invoke this skill - -Read-only; returns one JSON document. Use when asked to -"scrape", "get data from", "pull", "extract from", or "what's on" a page. - ## Preamble (run first) ```bash diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index e4718441e..bcddc1d61 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -25,6 +25,7 @@ import { HOST_PATHS } from './resolvers/types'; import { RESOLVERS } from './resolvers/index'; import { rewriteCarvedSectionRefs, SKILL_BYTE_CEILING, usesLazySections } from './resolvers/sections'; import { insertRuntimePreludes } from './resolvers/runtime-root'; +import { extractVoiceTriggers } from './resolvers/voice-triggers'; import { ALL_HOST_NAMES, resolveHostArg, getHostConfig } from '../hosts/index'; import type { HostConfig } from './host-config'; @@ -209,30 +210,6 @@ function rewriteSectionBase(content: string, linkRoot: string | null): string { // ─── Voice Trigger Processing ──────────────────────────────── -/** - * Extract voice-triggers YAML list from frontmatter. - * Returns an array of trigger strings, or [] if no voice-triggers field. - */ -function extractVoiceTriggers(content: string): string[] { - const fmStart = content.indexOf('---\n'); - if (fmStart !== 0) return []; - const fmEnd = content.indexOf('\n---', fmStart + 4); - if (fmEnd === -1) return []; - const frontmatter = content.slice(fmStart + 4, fmEnd); - - const triggers: string[] = []; - let inVoice = false; - for (const line of frontmatter.split('\n')) { - if (/^voice-triggers:/.test(line)) { inVoice = true; continue; } - if (inVoice) { - const m = line.match(/^\s+-\s+"(.+)"$/); - if (m) triggers.push(m[1]); - else if (!/^\s/.test(line)) break; - } - } - return triggers; -} - /** * Preprocess voice triggers: fold voice-triggers YAML field into description, * then strip the field from frontmatter. Must run BEFORE transformFrontmatter @@ -271,8 +248,10 @@ export { extractVoiceTriggers, processVoiceTriggers }; // "(gstack)" tag. This pile is the always-loaded catalog surface — every // session pays for the full text. The catalog trim splits the description // into a one-line catalog entry (lead sentence + "(gstack)") that stays in -// the frontmatter, and a "## When to invoke" body section that holds the -// routing/voice triggers prose for in-skill discovery. +// the frontmatter and drops the routing/voice prose. The body is not a home +// for it: Claude Code loads a skill body only after the skill is invoked, so +// routing text there can no longer route anything. Voice triggers live on in +// the root router's routing block ({{VOICE_TRIGGER_ROUTING}}). // // Opt-out: `--catalog-mode=full` keeps v1.44 behavior (no trim, full // description in frontmatter). Use when debugging routing regressions or @@ -358,20 +337,6 @@ export function buildTrimmedDescription(parts: CatalogParts): string { return `${lead}${suffix}`; } -/** Build the body section that holds the routing/voice prose. */ -export function buildWhenToInvokeSection(parts: CatalogParts): string { - const lines: string[] = ['## When to invoke this skill', '']; - if (parts.routingProse) { - lines.push(parts.routingProse); - lines.push(''); - } - if (parts.voiceLine) { - lines.push(parts.voiceLine); - lines.push(''); - } - return lines.join('\n'); -} - /** * Render a string as a YAML inline scalar value (the text after `key: `), * quoting only when a plain scalar would be invalid or ambiguous. @@ -398,9 +363,6 @@ export function toYamlInlineScalar(s: string): string { /** * Apply catalog trim to a SKILL.md body: * - shorten frontmatter `description:` to lead + (gstack) - * - insert "## When to invoke" body section AFTER the generated header - * (so it lands near the top of body content, where routing guidance - * belongs) * * Returns the rewritten content plus the extracted parts. */ @@ -445,16 +407,7 @@ export function applyCatalogTrim(content: string, skillName: string): { content: // pattern and silently corrupt the frontmatter. const newDescLine = `description: ${toYamlInlineScalar(newDesc)}\n`; const newFrontmatter = frontmatter.replace(descMatch[0], () => newDescLine); - let newContent = '---\n' + newFrontmatter + content.slice(fmEnd); - - // Insert body section after frontmatter (after the closing ---\n and any - // existing GENERATED header). We insert before the first non-comment line. - const bodyStart = newContent.indexOf('\n---\n') + 5; - const whenToInvoke = '\n' + buildWhenToInvokeSection(parts).trim() + '\n'; - // Skip past the generated header if present (it lives after frontmatter close) - const headerMatch = newContent.slice(bodyStart).match(/^(\s*\n)+/); - const insertAt = bodyStart + (headerMatch ? headerMatch[0].length : 0); - newContent = newContent.slice(0, insertAt) + whenToInvoke + '\n' + newContent.slice(insertAt); + const newContent = '---\n' + newFrontmatter + content.slice(fmEnd); return { content: newContent, parts }; } diff --git a/scripts/harness-version.json b/scripts/harness-version.json index 4e9d5794e..e5614ecee 100644 --- a/scripts/harness-version.json +++ b/scripts/harness-version.json @@ -48,7 +48,7 @@ "test/helpers/session-drain-policy.ts": {"blob":"6211239dd0485e6495a700b824a7e0e61d1bcd97","since":6}, "test/helpers/session-ledger.ts": {"blob":"425323284a368f5e77937e03884db4dea4e6134e","since":6,"nonBehavioral":"Claude-host verdicts are unchanged: the Codex host (EVALS_HOST=codex, codex-host-runner) runs only when selected and records its own per-model series; the Codex runner refactor (execCodex extraction, onSpawn hook) keeps its events and verdicts and adds token cost accounting; host fields on the plan/report are absent for claude"}, "test/helpers/session-runner.ts": {"blob":"cc65a66adafa9c053e3da3c6f17b5ac86c0eade7","since":6,"nonBehavioral":"Claude-host verdicts are unchanged: the Codex host (EVALS_HOST=codex, codex-host-runner) runs only when selected and records its own per-model series; the Codex runner refactor (execCodex extraction, onSpawn hook) keeps its events and verdicts and adds token cost accounting; host fields on the plan/report are absent for claude"}, - "test/helpers/skill-fixture.ts": {"blob":"b188064594eb4201a54a80ba1bc6f28e31d7f49e","since":6}, + "test/helpers/skill-fixture.ts": {"blob":"ffece830f35b0f1562005105e9296dc4d8ca50c2","since":6,"nonBehavioral":"skill-fixture.ts drops 'When to invoke this skill' from the review, review-army and retro section lists because #3129 stops generating that heading; extraction would throw otherwise. The fixture bytes follow the regenerated SKILL.md, whose owned prompt bytes already start those cases' new series."}, "test/helpers/test-selection.ts": {"blob":"51655f89f66be7a5ec487e2abfe4744300ebd5f6","since":6}, "test/helpers/touchfiles.ts": {"blob":"15f369056d41b9dc42dcd86f81f60c878074704c","since":6} } diff --git a/scripts/resolvers/index.ts b/scripts/resolvers/index.ts index cf80ccd40..d02bdae93 100644 --- a/scripts/resolvers/index.ts +++ b/scripts/resolvers/index.ts @@ -54,12 +54,14 @@ import { generateSidewaysSweep, generateFlakeFixNumber } from './sideways'; import { generateCoordinatorContract } from './coordinator-contract'; import { generateAutoplanDriverCodes, generateAutoplanDriverCommands, generateAutoplanDriverFiles, generateAutoplanDriverGate, generateAutoplanDriverPhases, generateAutoplanDriverResultFormat } from './autoplan-driver'; import { generateQAScope, generateQAExploratory, generateQAFunctional, generateQAResource, generateQAReview, generateQAReviewPreflight, generateQAMethodReads } from './qa'; +import { generateVoiceTriggerRouting } from './voice-triggers'; export const RESOLVERS: Record = { AUTOPLAN_PUBLICATION_HOOK: generateAutoplanPublicationHook, CEO_MODE_HANDOFF_HOOK: generateCeoModeHandoffHook, OUTSIDE_SELF_GUARD: (ctx, args) => outsideVoiceGuard({ ...ctx, host: args?.[0] === 'claude-code' ? 'codex' : 'claude' }), OUTSIDE_VOICE_ROUTING: generateOutsideVoiceRouting, + VOICE_TRIGGER_ROUTING: generateVoiceTriggerRouting, OUTSIDE_LABEL: (ctx) => outsideVoiceFor(ctx).label, NATIVE_LABEL: (ctx) => outsideVoiceFor(ctx).nativeLabel, OUTSIDE_PROVIDER: (ctx) => outsideVoiceFor(ctx).id, diff --git a/scripts/resolvers/voice-triggers.ts b/scripts/resolvers/voice-triggers.ts new file mode 100644 index 000000000..da652833b --- /dev/null +++ b/scripts/resolvers/voice-triggers.ts @@ -0,0 +1,61 @@ +/** + * Voice triggers: speech-to-text aliases a template lists under + * `voice-triggers:` in its frontmatter. + * + * Every host folds them into the skill's description, but Claude's catalog trim + * keeps only the lead sentence. {{VOICE_TRIGGER_ROUTING}} gives them a cheap home + * in the root router's routing block instead: one routing line per skill. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import type { TemplateContext } from './types'; +import { discoverTemplates, includesSkill } from '../discover-skills'; +import { getHostConfig } from '../../hosts/index'; + +const ROOT = path.resolve(import.meta.dir, '..', '..'); + +/** + * Extract voice-triggers YAML list from frontmatter. + * Returns an array of trigger strings, or [] if no voice-triggers field. + */ +export function extractVoiceTriggers(content: string): string[] { + const fmStart = content.indexOf('---\n'); + if (fmStart !== 0) return []; + const fmEnd = content.indexOf('\n---', fmStart + 4); + if (fmEnd === -1) return []; + const frontmatter = content.slice(fmStart + 4, fmEnd); + + const triggers: string[] = []; + let inVoice = false; + for (const line of frontmatter.split('\n')) { + if (/^voice-triggers:/.test(line)) { inVoice = true; continue; } + if (inVoice) { + const m = line.match(/^\s+-\s+"(.+)"$/); + if (m) triggers.push(m[1]); + else if (!/^\s/.test(line)) break; + } + } + return triggers; +} + +/** + * Claude only: other hosts keep the voice triggers in each skill's description. + * The template places this at the end of the last routing line, so each line + * starts with its own newline and an empty result leaves the list unchanged. + */ +export function generateVoiceTriggerRouting(ctx: TemplateContext): string { + if (ctx.host !== 'claude') return ''; + const hostConfig = getHostConfig(ctx.host); + let out = ''; + // Sorted so the render doesn't depend on filesystem iteration order. + const skillDirs = discoverTemplates(ROOT).map(t => path.dirname(t.tmpl)).sort(); + for (const skillDir of skillDirs) { + if (skillDir === '.' || !includesSkill(hostConfig, skillDir)) continue; + const content = fs.readFileSync(path.join(ROOT, skillDir, 'SKILL.md.tmpl'), 'utf-8').replace(/\r\n/g, '\n'); + const triggers = extractVoiceTriggers(content); + if (triggers.length === 0) continue; + out += `\n- Voice trigger ${triggers.map(t => `"${t}"`).join(', ')} → invoke \`/${skillDir}\``; + } + return out; +} diff --git a/setup-browser-cookies/SKILL.md b/setup-browser-cookies/SKILL.md index 830f9c7c2..149f9e584 100644 --- a/setup-browser-cookies/SKILL.md +++ b/setup-browser-cookies/SKILL.md @@ -15,13 +15,6 @@ allowed-tools: - -## When to invoke this skill - -Opens an interactive picker UI where you select which cookie domains to import. -Use before QA testing authenticated pages. Use when asked to "import cookies", -"login to the site", or "authenticate the browser". - ## Preamble (run first) ```bash diff --git a/setup-deploy/SKILL.md b/setup-deploy/SKILL.md index dcdb7b084..969637b69 100644 --- a/setup-deploy/SKILL.md +++ b/setup-deploy/SKILL.md @@ -19,16 +19,6 @@ allowed-tools: - -## When to invoke this skill - -Detects your deploy -platform (Fly.io, Render, Vercel, Netlify, Heroku, GitHub Actions, custom), -production URL, health check endpoints, and deploy status commands. Writes -the configuration to CLAUDE.md so all future deploys are automatic. -Use when: "setup deploy", "configure deployment", "set up land-and-deploy", -"how do I deploy with gstack", "add deploy config". - ## Preamble (run first) ```bash diff --git a/setup-gbrain/SKILL.md b/setup-gbrain/SKILL.md index f24634b53..3ee4a2a41 100644 --- a/setup-gbrain/SKILL.md +++ b/setup-gbrain/SKILL.md @@ -21,13 +21,6 @@ allowed-tools: - -## When to invoke this skill - -One command from zero to "gbrain is running, and this agent -can call it." Use when: "setup gbrain", "connect gbrain", "start -gbrain", "install gbrain", "configure gbrain for this machine". - ## Preamble (run first) ```bash diff --git a/ship/SKILL.md b/ship/SKILL.md index 961d95705..f159bc05f 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -22,14 +22,6 @@ triggers: - -## When to invoke this skill - -Use when asked to "ship", "deploy", -"push to main", "create a PR", "merge and push", or "get it deployed". -Proactively invoke this skill (do NOT push/PR directly) when the user says code -is ready, asks about deploying, wants to push code up, or asks to create a PR. - ## Preamble (run first) ```bash diff --git a/skillify/SKILL.md b/skillify/SKILL.md index fcedc1f49..6a32fc9e2 100644 --- a/skillify/SKILL.md +++ b/skillify/SKILL.md @@ -17,16 +17,6 @@ triggers: - -## When to invoke this skill - -Future /scrape calls with the same intent run -the codified script in ~200ms instead of re-driving the page. Walks -back through the conversation, synthesizes script.ts + script.test.ts -+ fixture, runs the test in a temp dir, and asks before committing. -Use when asked to "skillify", "codify", "save this scrape", or -"make this permanent". - ## Preamble (run first) ```bash diff --git a/spec/SKILL.md b/spec/SKILL.md index 67061b932..da4b6b04c 100644 --- a/spec/SKILL.md +++ b/spec/SKILL.md @@ -20,14 +20,6 @@ triggers: - -## When to invoke this skill - -Files the issue, -optionally spawns a Claude Code agent in a fresh worktree, and lets /ship close -the source issue on merge. Use when asked to "spec this out", "file an issue", -"write up a ticket", "make this a GitHub issue", or "turn this into a backlog item". - ## Preamble (run first) ```bash diff --git a/sync-gbrain/SKILL.md b/sync-gbrain/SKILL.md index 134892a83..f68bb0c71 100644 --- a/sync-gbrain/SKILL.md +++ b/sync-gbrain/SKILL.md @@ -20,15 +20,6 @@ allowed-tools: - -## When to invoke this skill - -Wraps the gstack-gbrain-sync orchestrator with -state probing, native code-surface registration, capability checks, -and a verdict block. Re-runnable, idempotent. Use when: "sync gbrain", -"refresh gbrain", "re-index this repo", "gbrain search isn't finding -things". - ## Preamble (run first) ```bash diff --git a/test-audit/SKILL.md b/test-audit/SKILL.md index a385dbd29..d311fe941 100644 --- a/test-audit/SKILL.md +++ b/test-audit/SKILL.md @@ -19,11 +19,6 @@ allowed-tools: - -## When to invoke this skill - -Report-only unless you approve a batch. Use for /test-audit. - ## Preamble (run first) ```bash diff --git a/test/catalog-mode-full.test.ts b/test/catalog-mode-full.test.ts index 43d7b9b96..1f5432c0e 100644 --- a/test/catalog-mode-full.test.ts +++ b/test/catalog-mode-full.test.ts @@ -40,7 +40,7 @@ describe('catalog mode CLI behavior', () => { test('omitting the flag defaults to trim', () => { expect(frontmatter(defaultContent)).toMatch(/^description: "?Ship workflow:[^\n]*\(gstack\)"?$/m); expect(frontmatter(defaultContent)).not.toMatch(/Use when asked to/i); - expect(defaultContent).toContain('## When to invoke this skill'); + expect(defaultContent).not.toContain('## When to invoke this skill'); }); for (const form of ['equals', 'separate'] as const) { @@ -53,14 +53,12 @@ describe('catalog mode CLI behavior', () => { expect(fm).toMatch(/^description: \|\s*$/m); expect(fm).toMatch(/Use when asked to/i); expect(fm).not.toBe(frontmatter(defaultContent)); - expect(result.content.slice(fm.length)).not.toContain('## When to invoke this skill'); }); test(`${form} flag form accepts explicit trim and matches the default catalog`, () => { const result = render(args('trim')); expect(result.status, result.stderr).toBe(0); expect(frontmatter(result.content)).toBe(frontmatter(defaultContent)); - expect(result.content).toContain('## When to invoke this skill'); }); test(`${form} flag form rejects invalid modes before writing output`, () => { diff --git a/test/catalog-trim.test.ts b/test/catalog-trim.test.ts index 79380c341..db130d816 100644 --- a/test/catalog-trim.test.ts +++ b/test/catalog-trim.test.ts @@ -1,8 +1,7 @@ /** * Unit tests for catalog-trim helpers (gen-skill-docs.ts T4 functions). * - * splitCatalogDescription, buildTrimmedDescription, buildWhenToInvokeSection, - * applyCatalogTrim — these handle every skill's frontmatter rewrite at gen + * splitCatalogDescription, buildTrimmedDescription, applyCatalogTrim — these handle every skill's frontmatter rewrite at gen * time. Two bugs already shipped here: * * v1.45.0.0 design-consultation: when the first sentence exceeded 200 chars, @@ -21,7 +20,6 @@ import { describe, test, expect } from 'bun:test'; import { splitCatalogDescription, buildTrimmedDescription, - buildWhenToInvokeSection, applyCatalogTrim, toYamlInlineScalar, } from '../scripts/gen-skill-docs'; @@ -229,41 +227,6 @@ describe('buildTrimmedDescription', () => { }); }); -describe('buildWhenToInvokeSection', () => { - test('produces markdown H2 with routing prose and voice line', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: 'Use when asked to ship.', - voiceLine: 'Voice triggers (speech-to-text aliases): "ship it".', - hasGstackTag: true, - }); - expect(out).toContain('## When to invoke this skill'); - expect(out).toContain('Use when asked to ship.'); - expect(out).toContain('Voice triggers'); - }); - - test('omits routing block when routingProse is empty', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: '', - voiceLine: null, - hasGstackTag: true, - }); - expect(out).toContain('## When to invoke this skill'); - expect(out).not.toContain('Use when'); - }); - - test('emits even when only voice line is present', () => { - const out = buildWhenToInvokeSection({ - lead: 'Lead.', - routingProse: '', - voiceLine: 'Voice triggers: x.', - hasGstackTag: true, - }); - expect(out).toContain('Voice triggers: x.'); - }); -}); - describe('applyCatalogTrim', () => { const minimalSkill = `--- name: example @@ -280,7 +243,7 @@ preamble-tier: 2 Original body content here. `; - test('rewrites multi-line description into one-line + body section', () => { + test('rewrites multi-line description into one line and adds no body section', () => { const result = applyCatalogTrim(minimalSkill, 'example'); expect(result).not.toBeNull(); const { content, parts } = result!; @@ -288,10 +251,10 @@ Original body content here. // description with an interior colon ("Example skill:") is YAML-quoted, so // the value is wrapped in double quotes — tolerate the optional quotes. expect(content).toMatch(/^description: "?Example skill:[^\n]*\(gstack\)"?\n/m); - // Body has the When to invoke section - expect(content).toContain('## When to invoke this skill'); - expect(content).toContain('Use when asked to do an example task.'); - expect(content).toContain('Proactively suggest when'); + // The body gains no routing section: Claude Code reads a skill body only + // after invocation, so routing prose there routes nothing. + expect(content).not.toContain('## When to invoke this skill'); + expect(content).not.toContain('Use when asked to do an example task.'); // Original body still present expect(content).toContain('# Example body'); expect(content).toContain('Original body content here.'); diff --git a/test/cso-preserved.test.ts b/test/cso-preserved.test.ts index 04c25022e..5d45f7576 100644 --- a/test/cso-preserved.test.ts +++ b/test/cso-preserved.test.ts @@ -103,6 +103,5 @@ describe('CSO v3 always-loaded policy', () => { const description = frontmatter.match(/^description:\s+(.+)$/m)![1].trim(); expect(description.length).toBeLessThanOrEqual(200); expect(description).toContain('(gstack)'); - expect(controller()).toContain('## When to invoke this skill'); }); }); diff --git a/test/fixtures/context-budget.json b/test/fixtures/context-budget.json index e37eb8721..3156dd309 100644 --- a/test/fixtures/context-budget.json +++ b/test/fixtures/context-budget.json @@ -2,68 +2,68 @@ "_comment": "Context-budget ratchet ceilings (~tokens). Regenerate: bun test/helpers/capture-context-budget.ts. Headroom: alwaysOnTotal x1.05, eagerPerInvocation x1.1. Graded by test/context-budget-ratchet.test.ts via lib/context-bill.ts checkBudget.", "alwaysOnTotal": 7458, "eagerPerInvocation": { - "autoplan": 17136, - "benchmark": 7938, - "benchmark-models": 4231, - "browse": 8961, + "autoplan": 17005, + "benchmark": 7813, + "benchmark-models": 4086, + "browse": 8943, "browser-skills/hackernews-frontpage": 371, - "canary": 13520, + "canary": 13441, "capy/skill/gstack": 2923, - "careful": 1328, - "codex": 15717, - "context-restore": 10995, - "context-save": 10588, - "cso": 4715, - "design-consultation": 16277, - "design-html": 14555, - "design-review": 35921, - "design-shotgun": 14980, - "deslop-shared-libs": 3448, + "careful": 1239, + "codex": 15594, + "context-restore": 10909, + "context-save": 10517, + "cso": 4660, + "design-consultation": 16188, + "design-html": 14380, + "design-review": 35808, + "design-shotgun": 14901, + "deslop-shared-libs": 3411, "devex-review": 20161, - "diagram": 4336, - "document-generate": 11809, - "document-release": 10235, + "diagram": 4268, + "document-generate": 11713, + "document-release": 10092, "eval-plan": 9811, - "freeze": 1194, - "gstack": 4289, - "gstack-upgrade": 6407, - "guard": 1082, - "health": 11521, - "investigate": 13465, - "ios-clean": 8280, - "ios-design-review": 8477, - "ios-fix": 8170, - "ios-qa": 11742, - "ios-sync": 8295, - "land-and-deploy": 19061, - "landing-report": 8993, - "learn": 8708, - "make-pdf": 5348, - "office-hours": 24993, - "open-gstack-browser": 4981, + "freeze": 1115, + "gstack": 4821, + "gstack-upgrade": 6212, + "guard": 1000, + "health": 11440, + "investigate": 13344, + "ios-clean": 8139, + "ios-design-review": 8305, + "ios-fix": 8009, + "ios-qa": 11551, + "ios-sync": 8167, + "land-and-deploy": 18991, + "landing-report": 8901, + "learn": 8626, + "make-pdf": 5204, + "office-hours": 24792, + "open-gstack-browser": 4872, "openclaw/skills/gstack-openclaw-ceo-review": 2764, "openclaw/skills/gstack-openclaw-investigate": 1429, "openclaw/skills/gstack-openclaw-office-hours": 4433, "openclaw/skills/gstack-openclaw-retro": 2542, - "pair-agent": 11156, - "plan-ceo-review": 21151, - "plan-design-review": 22384, - "plan-devex-review": 17595, - "plan-eng-review": 13663, - "plan-tune": 14773, - "qa": 13497, - "qa-only": 11543, - "retro": 19033, - "review": 21135, - "scrape": 7354, - "setup-browser-cookies": 3628, - "setup-deploy": 11549, - "setup-gbrain": 15760, - "ship": 21164, - "skillify": 11648, - "spec": 14922, - "sync-gbrain": 15477, - "test-audit": 10348, - "unfreeze": 457 + "pair-agent": 10995, + "plan-ceo-review": 21007, + "plan-design-review": 22284, + "plan-devex-review": 17556, + "plan-eng-review": 13521, + "plan-tune": 14589, + "qa": 13348, + "qa-only": 11429, + "retro": 18938, + "review": 21047, + "scrape": 7312, + "setup-browser-cookies": 3565, + "setup-deploy": 11440, + "setup-gbrain": 15702, + "ship": 21081, + "skillify": 11551, + "spec": 14840, + "sync-gbrain": 15399, + "test-audit": 10324, + "unfreeze": 409 } } diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 961d95705..f159bc05f 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -22,14 +22,6 @@ triggers: - -## When to invoke this skill - -Use when asked to "ship", "deploy", -"push to main", "create a PR", "merge and push", or "get it deployed". -Proactively invoke this skill (do NOT push/PR directly) when the user says code -is ready, asks about deploying, wants to push code up, or asks to create a PR. - ## Preamble (run first) ```bash diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index b51d87adb..9ed135902 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -10,7 +10,7 @@ "agy/gstack-plan-devex-review": 135727, "agy/gstack-plan-eng-review": 152669, "agy/gstack-review": 124266, - "claude/design-review": 136826, + "claude/design-review": 136389, "codex/gstack-autoplan": 130298, "codex/gstack-design-consultation": 109239, "codex/gstack-design-review": 144820, diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 645b947fa..91d50d4c1 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -437,14 +437,13 @@ describe('gen-skill-docs', () => { }); test('benchmark routing is scoped to web page performance', () => { - const generated = fs.readFileSync(path.join(ROOT, 'benchmark', 'SKILL.md'), 'utf-8'); + // The template description is the routing source; the Claude catalog trim + // drops its tail from benchmark/SKILL.md. const template = fs.readFileSync(path.join(ROOT, 'benchmark', 'SKILL.md.tmpl'), 'utf-8'); - for (const content of [generated, template]) { - const useWhen = content.match(/Use when:[\s\S]*?(?=\n\n|\(gstack\))/)?.[0] ?? ''; - expect(useWhen).toContain('"web performance benchmark"'); - expect(useWhen).not.toMatch(/"benchmark"|"performance"/); - expect(content).toContain('/benchmark-models'); - } + const useWhen = template.match(/Use when:[\s\S]*?(?=\n\n|\(gstack\))/)?.[0] ?? ''; + expect(useWhen).toContain('"web performance benchmark"'); + expect(useWhen).not.toMatch(/"benchmark"|"performance"/); + expect(template).toContain('/benchmark-models'); const router = fs.readFileSync(path.join(ROOT, 'SKILL.md.tmpl'), 'utf-8'); expect(router).toContain('web performance regression → invoke `/benchmark`'); expect(router).toContain('model or skill benchmarks → invoke `/benchmark-models`'); @@ -4444,12 +4443,29 @@ describe('voice-triggers processing', () => { expect(processVoiceTriggers(content)).toBe(content); }); - test('generated CSO SKILL.md contains voice triggers in description', () => { - const content = fs.readFileSync(path.join(ROOT, 'cso', 'SKILL.md'), 'utf-8'); + test('CSO voice triggers fold into the full description', () => { + // The Claude catalog trim drops this line from cso/SKILL.md; full-mode and + // other-host renders still carry it. + const content = processVoiceTriggers(fs.readFileSync(path.join(ROOT, 'cso', 'SKILL.md.tmpl'), 'utf-8')); expect(content).toContain('"see-so"'); expect(content).toContain('Voice triggers (speech-to-text aliases):'); }); + test('Claude root router routes every skill voice trigger the catalog trim drops', () => { + const routing = between(fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8'), '**Routing rules', '**Route only to skills'); + const skills = fs.readdirSync(ROOT).filter(d => !fs.lstatSync(path.join(ROOT, d)).isSymbolicLink() && fs.existsSync(path.join(ROOT, d, 'SKILL.md.tmpl'))); + let routed = 0; + for (const skill of skills) { + const triggers = extractVoiceTriggers(fs.readFileSync(path.join(ROOT, skill, 'SKILL.md.tmpl'), 'utf-8').replace(/\r\n/g, '\n')); + if (triggers.length === 0) continue; + const line = routing.split('\n').find(l => l.endsWith(`→ invoke \`/${skill}\``) && l.startsWith('- Voice trigger ')); + expect(line, skill).toBeDefined(); + for (const t of triggers) expect(line, skill).toContain(`"${t}"`); + routed++; + } + expect(routed).toBeGreaterThan(0); + }); + test('generated CSO SKILL.md does NOT contain raw voice-triggers field', () => { const content = fs.readFileSync(path.join(ROOT, 'cso', 'SKILL.md'), 'utf-8'); const fmEnd = content.indexOf('\n---', 4); diff --git a/test/helpers/manual-judge-review-fixture.ts b/test/helpers/manual-judge-review-fixture.ts index 0acb4f5d5..9d283e02f 100644 --- a/test/helpers/manual-judge-review-fixture.ts +++ b/test/helpers/manual-judge-review-fixture.ts @@ -35,6 +35,8 @@ const LATER_WAVE_EDITS: Array<[string, string]> = [ "The skill-start output above already ran artifacts sync. Act on its lines:\nGBrain hint text (if present) tells you when to prefer `gbrain` over Grep;\n`ARTIFACTS_SYNC:` reports sync health (`off`, `mode=... | queue=N`,\n`remote-mode`, or a restore hint naming `gstack-brain-restore`).\n\nThe one-time privacy stop-gate (artifacts-sync consent) arrives as a\n`GSTACK_INSTRUCTION` block from skill-start when consent is actually pending\n\u2014 fire it via AskUserQuestion exactly as the block instructs."], // Oct 8 wave: the tier-1 voice directive gained the reply-language rule (two lines longer). ["Reply in the language of the user's latest message unless asked otherwise. Code, commands, paths, identifiers and quoted output stay verbatim.\n\n", ''], + // #3129 dropped the Claude body's "When to invoke" section above the preamble (seven lines shorter). + ['\n\n## Preamble (run first)\n', '\n\n\n## When to invoke this skill\n\nOpens an interactive picker UI where you select which cookie domains to import.\nUse before QA testing authenticated pages. Use when asked to "import cookies",\n"login to the site", or "authenticate the browser".\n\n## Preamble (run first)\n'], ]; // The honest-install wave (lazy Chromium) made the fallback probe call diff --git a/test/helpers/parity-harness.ts b/test/helpers/parity-harness.ts index 6c1caf4dd..c0382c8da 100644 --- a/test/helpers/parity-harness.ts +++ b/test/helpers/parity-harness.ts @@ -229,7 +229,7 @@ const CARVED_INVARIANTS: ParityInvariant[] = Object.values(CARVE_GUARDS).map((g) mustContain: g.mustContain, // CSO's helper trust boundary requires its private startup; demanding the // shared Preamble here would silently reintroduce conflicting policy. - mustHaveHeadings: g.skill === 'cso' ? ['## When to invoke'] : ['## Preamble', '## When to invoke'], + mustHaveHeadings: g.skill === 'cso' ? [] : ['## Preamble'], maxSizeRatio: g.maxSizeRatio ?? 1.05, })); diff --git a/test/helpers/skill-fixture.ts b/test/helpers/skill-fixture.ts index b18806459..ffece830f 100644 --- a/test/helpers/skill-fixture.ts +++ b/test/helpers/skill-fixture.ts @@ -39,7 +39,6 @@ import * as path from 'path'; /** /review E2E (sql-injection, enum-completeness, design-lite): the core * review workflow without the shared preamble, Review Army, or Fix-First. */ export const REVIEW_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'Step 1: Check branch', 'Step 2: Read the checklist', @@ -54,7 +53,6 @@ export const REVIEW_E2E_SECTIONS = [ * (delivery-audit test) + Step 4.5 specialist dispatch (quality score, * JSON findings schema, MULTI-SPECIALIST consensus, Red Team). */ export const REVIEW_ARMY_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'Step 1: Check branch', 'Step 1.5: Scope Drift Detection', @@ -72,7 +70,6 @@ export const REVIEW_ARMY_E2E_SECTIONS = [ * + the narrative report template. Global mode and Compare mode are not * exercised by the E2E tests and are dropped. */ export const RETRO_E2E_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'User-invocable', 'Arguments', @@ -86,8 +83,7 @@ export const RETRO_E2E_SECTIONS = [ /** codex-review-findings E2E against the Codex host variant * (.agents/skills/gstack-review/SKILL.md). Same core workflow as - * REVIEW_E2E_SECTIONS, minus "When to invoke this skill" (the Codex host - * adapter does not emit that section). */ + * REVIEW_E2E_SECTIONS. */ export const CODEX_REVIEW_E2E_SECTIONS = [ 'Step 0: Detect platform and base branch', 'Step 1: Check branch', diff --git a/test/skill-e2e-retro.test.ts b/test/skill-e2e-retro.test.ts index 29aa7e853..7d0324eee 100644 --- a/test/skill-e2e-retro.test.ts +++ b/test/skill-e2e-retro.test.ts @@ -23,7 +23,6 @@ const evalCollector = createEvalCollector('e2e-retro'); // still an extraction, not a full-file copy (sections ARE the minimal // on-demand units; same pattern as skill-e2e-review-army.test.ts). const RETRO_SKELETON_SECTIONS = [ - 'When to invoke this skill', 'Step 0: Detect platform and base branch', 'User-invocable', 'Arguments', diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 4f8c73cde..3a003970f 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1779,14 +1779,13 @@ describe('Skill trigger phrases', () => { ]; for (const skill of SKILLS_REQUIRING_TRIGGERS) { - test(`${skill}/SKILL.md has "Use when" trigger phrases`, () => { - const skillPath = path.join(ROOT, skill, 'SKILL.md'); + test(`${skill}/SKILL.md.tmpl has "Use when" trigger phrases`, () => { + const skillPath = path.join(ROOT, skill, 'SKILL.md.tmpl'); if (!fs.existsSync(skillPath)) return; const content = fs.readFileSync(skillPath, 'utf-8'); - // v1.45.0.0 catalog trim moved trigger prose out of frontmatter into a - // body "## When to invoke" section. Search the full file content, not - // just frontmatter. The trigger phrase must still appear somewhere in - // the skill so agents can match user requests to the skill. + // The catalog trim drops trigger prose from the Claude SKILL.md, so check + // the template description: --catalog-mode=full and the other hosts + // still route on it. expect(content).toMatch(/Use when/i); }); } @@ -1799,11 +1798,11 @@ describe('Skill trigger phrases', () => { ]; for (const skill of SKILLS_REQUIRING_PROACTIVE) { - test(`${skill}/SKILL.md has proactive routing phrase`, () => { - const skillPath = path.join(ROOT, skill, 'SKILL.md'); + test(`${skill}/SKILL.md.tmpl has proactive routing phrase`, () => { + const skillPath = path.join(ROOT, skill, 'SKILL.md.tmpl'); if (!fs.existsSync(skillPath)) return; const content = fs.readFileSync(skillPath, 'utf-8'); - // Same catalog-trim consideration — search the full file content. + // Same catalog-trim consideration — check the template description. expect(content).toMatch(/Proactively (suggest|invoke)/i); }); } diff --git a/unfreeze/SKILL.md b/unfreeze/SKILL.md index 9fbd4ade6..ca32654bd 100644 --- a/unfreeze/SKILL.md +++ b/unfreeze/SKILL.md @@ -13,13 +13,6 @@ allowed-tools: - -## When to invoke this skill - -Use when you want to widen edit scope without ending the session. -Use when asked to "unfreeze", "unlock edits", "remove freeze", or -"allow all edits". - # /unfreeze — Clear Freeze Boundary Remove the edit restriction set by `/freeze`, allowing edits to all directories.