From a305302963372231ce8db901b9515c30efa6dc1c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 31 Oct 2026 05:28:56 -0700 Subject: [PATCH] v1.91.365.0 fix(outside-review): an unclosed code fence no longer hides a later [P1] or not-ready verdict (GST-436) (#3681) * fix(outside-review): an unclosed code fence no longer hides a later [P1] or not-ready verdict (GST-436) withoutCode() blanked every line after a fence that never closed, so a reply with a no-findings line, an unclosed fence and then a [P1] or 'Verdict: not ready' classified clean. A fence that never closes is now not code. Test fails first on main 89f20330. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(outside-review): an unclosed code fence no longer hides a later [P1] or not-ready verdict (GST-436) withoutCode() blanked every line after a fence that never closed, so a reply with a no-findings line, an unclosed fence and then a [P1] or 'Verdict: not ready' classified clean. A fence that never closes is now not code. Test fails first on main 89f20330. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(outside-review): text in an unclosed fence blocks but never makes a reply clean; one-pass fence matching (GST-436 review) Review pass 1 found the first commit read an unclosed fence's remainder for every signal: a quoted Verdict: ready, [P3] or No issues found. there turned a not-ready or unverified reply clean, and each unmatched fence rescanned the rest of the reply (3,000 fences: 5.9 s, 4.7 GB). Blocking signals now read the remainder; the evidence for clean keeps it blanked; fence matching is one linear pass. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: outside-review verdict rules name the unclosed-fence behavior (GST-436) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: unclosed-fence wording matches the classifier: unparsed label is unverified, only P0/P1 block (GST-436) 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 | 15 +++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- docs/troubleshooting.md | 6 +- lib/outside-review-result.ts | 37 +++++++++--- package.json | 2 +- test/outside-review-unclosed-fence.test.ts | 65 ++++++++++++++++++++++ 7 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 test/outside-review-unclosed-fence.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cf72afddc..05b172940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [1.91.365.0] - 2026-10-31 + +**An unclosed code fence in an outside review no longer hides a later `[P1]` or not-ready verdict (GST-436).** Before, an outside-voice reply that said no issues were found, opened a code fence it never closed, and then listed a `[P1]` finding or ended with `Verdict: not ready` was classified clean, because everything after the fence was treated as quoted code. Now a P0 or P1 severity or a last `Verdict: not ready` after an unclosed fence still blocks. Text inside that fence still can't make a reply clean, so a quoted `Verdict: ready`, `[P3]` or `No issues found.` there is ignored, as before. + +### Fixed +- `lib/outside-review-result.ts` reads the reply after an unclosed fence's opening line for severities (P0 and P1, or Critical and High words, block), an unparsed label (which makes the review `unverified`) and a last `Verdict: not ready`. The evidence for clean (a parsed severity, the last verdict line, the no-findings conclusion) keeps that text blanked. Closed fences still hide quoted severities. +- Fence matching is one pass, and an opener counts only when a later marker of its kind is at least as long. A reply with thousands of unmatched fences classifies in milliseconds instead of rescanning the rest once per fence. + +### Tests +- `test/outside-review-unclosed-fence.test.ts` covers: + - a `[P1]` and a `Verdict: not ready` after an unclosed fence + - a closed fence that still hides a quoted `[P1]` + - a quoted `Verdict: ready`, `[P3]` or `No issues found.` inside an unclosed fence, which stays findings or unverified + - 3,000 unmatched fences + ## [1.91.364.0] - 2026-10-31 **`--policy-from head` now only adds test lanes, and the repo can set pregate's per-process timeout.** Two /ship tooling gaps cost other crews a rerun each. GEVA-2 (gstack-evals#93) had a PR that added its own pregate lane, but `gstack-ship-preflight` could not pass `--policy-from head`, so Step 5 failed with `PREGATE_NO_LANE` and the lane called runLanes by hand (GST-419). HARN-1 (HARN-12) had lanes of about 780 s, while preflight and the `finish` fallback ran pregate at its 600 s default, so the result was always `PREGATE: incomplete` (GST-420). There was also a hole behind the first gap: `--policy-from head` took the branch's whole `.gstack/pregate.json`, so a branch could delete its own lanes. diff --git a/VERSION b/VERSION index a2f1e59ed..d04265b19 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.364.0 +1.91.365.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 4886998eb..f731d61ff 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.364.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.365.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 64b9f3bf1..2a905e71d 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -160,7 +160,11 @@ the readiness dashboard and the PR body, and never count it as a pass. cell `| P1 |`. P0 and P1 block; P2 and P3 are advisory. A P0-P3 token in that label position that none of these forms reads (`## P1/P2 findings`) makes the review [`unverified`](#outside-review-unparsed-severity), never `clean`. Code - fences and inline code are quotes, not labels. + fences and inline code are quotes, not labels. A fence that never closes + still hides its text from the evidence for `clean`, but after its opening + line a P0 or P1 label (or a Critical or High word) and a last + `Verdict: not ready` still block, and an unreadable label still makes the + review `unverified`. - **Severity words in label position.** `Severity: High`, `Priority: low`, a line that starts with `High:`, `High —`, `[High]` or `**High**` (after an optional heading, bullet or number), a bold `**High**` anywhere, or a table diff --git a/lib/outside-review-result.ts b/lib/outside-review-result.ts index ad76233ab..c09ffb9a5 100644 --- a/lib/outside-review-result.ts +++ b/lib/outside-review-result.ts @@ -195,17 +195,33 @@ function plainReview(text: string): string { * Code quotes text, so a severity inside a fence or an inline code span is not * a label. An inline span holding only a severity token in label position * (`- \`P1\`: x`, `Severity: \`High\``, a table cell) is formatting and stays. + * `readUnclosed` reads a fence that never closes as prose instead (GST-436): the + * blocking signals use that view, so an unclosed fence cannot hide a later `[P1]` + * or `Verdict: not ready`, while the evidence for clean keeps it blanked. One pass: + * an opener closes only when a later marker of its kind is at least as long. */ const CODE_LABEL_BEFORE = /^[\t ]*(?:>[\t ]*)?(?:#{1,6}[\t ]+|[-+*][\t ]+|\(?\d{1,3}[.)][\t ]+)?(?:\*\*|__)?$|\b(?:severity|priority)[\t ]*[:=][\t ]*(?:\*\*|__)?$|\|[\t ]*(?:\*\*)?$/i; -function withoutCode(text: string): string { +function withoutCode(text: string, readUnclosed = false): string { + const lines = text.split(/\r?\n/); + const markers = lines.map(line => /^[\t ]*(`{3,}|~{3,})/.exec(line)?.[1]); + const closes = markers.map(() => true); + if (readUnclosed) { + const longestLater: Record = { '`': 0, '~': 0 }; + for (let i = lines.length - 1; i >= 0; i--) { + const m = markers[i]; + if (!m) continue; + closes[i] = longestLater[m[0]!]! >= m.length; + longestLater[m[0]!] = Math.max(longestLater[m[0]!]!, m.length); + } + } let fence: string | undefined; - return text.split(/\r?\n/).map(line => { - const marker = /^[\t ]*(`{3,}|~{3,})/.exec(line)?.[1]; + return lines.map((line, i) => { + const marker = markers[i]; if (fence) { if (marker && marker[0] === fence[0] && marker.length >= fence.length) fence = undefined; return ''; } - if (marker) { fence = marker; return ''; } + if (marker) { if (closes[i]) fence = marker; return ''; } return line.replace(/`([^`]+)`/g, (span, inner: string, at: number) => 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'); @@ -243,9 +259,12 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC const plain = plainReview(input.text); const reviewed = input.gate !== 'proposal'; const code = withoutCode(input.text), plainCode = plainReview(code); - const levels = [...[...plainCode.matchAll(/\[(P[0-3])\]|^(P[0-3]):/gm)].map(m => (m[1] ?? m[2]) as Severity), - ...severityTokens(code), ...(reviewed ? severityWords(code) : [])]; + const read = withoutCode(input.text, true), plainRead = plainReview(read); + const levelsOf = (c: string, p: string) => [...[...p.matchAll(/\[(P[0-3])\]|^(P[0-3]):/gm)].map(m => (m[1] ?? m[2]) as Severity), + ...severityTokens(c), ...(reviewed ? severityWords(c) : [])]; + const levels = levelsOf(read, plainRead); const findings = { highest: levels.length ? levels.sort()[0]! : null }; + const cleanEvidence = levelsOf(code, plainCode).length > 0; const ran = execution(input); if (ran.state === 'unavailable') return { execution: ran, findings, verdict: 'unavailable', reason: ran.reason, detail: ran.detail }; const blocking = findings.highest === 'P0' || findings.highest === 'P1'; @@ -262,12 +281,12 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC return result('unavailable', 'missing_markers', 'missing review completion recommendation'); } if (!reviewed) return result(blocking ? 'findings' : 'clean'); - const unparsed = firstUnparsed(code); + const unparsed = firstUnparsed(read); // A last explicit not-ready blocks even when the only parsed severities are advisory. - const notReady = verdictNotReady(plainCode); + const notReady = verdictNotReady(plainCode) || verdictNotReady(plainRead); if (blocking || notReady) return { ...result('findings'), ...(unparsed ? { unparsed } : {}) }; if (unparsed) return { ...result('unverified', 'unparsed_severity', unparsed), unparsed }; - if ((input.gate === 'structured' || input.gate === 'review') && !findings.highest && !concludesNoFindings(code)) { + if ((input.gate === 'structured' || input.gate === 'review') && !cleanEvidence && !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 fad29a377..521d596fe 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.364", + "version": "1.91.365", "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-unclosed-fence.test.ts b/test/outside-review-unclosed-fence.test.ts new file mode 100644 index 000000000..ba8285e34 --- /dev/null +++ b/test/outside-review-unclosed-fence.test.ts @@ -0,0 +1,65 @@ +/** + * GST-436: an unclosed code fence must not hide the severity lines and verdict after it. + * withoutCode() blanks fenced text so a quoted severity is not read as a label; a fence that + * never closes used to blank the rest of the reply, so a no-findings line before it read as clean. + */ +import { describe, expect, test } from 'bun:test'; +import { classifyOutsideReview } from '../lib/outside-review-result'; + +const NO_ISSUES = 'No issues found in changed.ts.'; +const RECOMMEND = 'Recommendation: approve because changed.ts keeps its data.'; + +describe('an unclosed fence does not hide what follows it (GST-436)', () => { + // Value: protects=the outside-voice verdict on review gates; fails_when=a reply with NO_FINDINGS, an unclosed + // fence and then a [P1] or a not-ready verdict classifies clean; seam=classifyOutsideReview + test('NO_FINDINGS, an unclosed fence, then [P1] is not clean', () => { + const text = [NO_ISSUES, '```ts', 'const x = 1;', '[P1] changed.ts drops the last row', RECOMMEND].join('\n'); + const r = classifyOutsideReview({ text, gate: 'review' }); + expect(r.verdict).not.toBe('clean'); + expect(r.verdict).toBe('findings'); + expect(r.findings.highest).toBe('P1'); + }); + + test('NO_FINDINGS, an unclosed fence, then Verdict: not ready is not clean', () => { + const text = [NO_ISSUES, '~~~', 'quoted', RECOMMEND, 'Verdict: not ready'].join('\n'); + expect(classifyOutsideReview({ text, gate: 'review' }).verdict).toBe('findings'); + }); + + test('a closed fence still hides a quoted severity', () => { + const text = [NO_ISSUES, '```', '[P1] quoted from the diff', '```', RECOMMEND].join('\n'); + const r = classifyOutsideReview({ text, gate: 'review' }); + expect(r.verdict).toBe('clean'); + expect(r.findings.highest).toBeNull(); + }); +}); + +describe('an unclosed fence never makes a reply clean (GST-436 review)', () => { + // Value: protects=the outside-voice verdict on review gates; fails_when=text quoted inside an unclosed fence turns a + // not-ready or unverified reply clean; seam=classifyOutsideReview + test('a quoted Verdict: ready in an unclosed fence does not override a real not-ready', () => { + const text = ['[P2] parser.ts drops the trailing row.', 'Verdict: not ready', 'Recommendation: fix because parser.ts loses data.', '```md', 'Verdict: ready'].join('\n'); + expect(classifyOutsideReview({ text, gate: 'review' }).verdict).toBe('findings'); + }); + + test('a quoted advisory severity in an unclosed fence does not stand in for a tagged review', () => { + const text = ['Recommendation: hold because I ran out of time.', 'Looking at the fixture:', '```', '[P3] example finding text'].join('\n'); + expect(classifyOutsideReview({ text, gate: 'review' }).verdict).toBe('unverified'); + expect(classifyOutsideReview({ text, gate: 'structured' }).verdict).toBe('unverified'); + }); + + test('a quoted no-findings line in an unclosed fence does not conclude the review', () => { + const text = ['The retry loop in fetch.ts has no cap.', 'Recommendation: fix before merge because the retry loop has no cap.', 'Output of the lint run:', '```text', '$ bun run lint', 'No issues found.'].join('\n'); + expect(classifyOutsideReview({ text, gate: 'review' }).verdict).toBe('unverified'); + expect(classifyOutsideReview({ text, gate: 'structured' }).verdict).toBe('unverified'); + }); + + test('many unmatched fences classify in one linear pass', () => { + const lines: string[] = []; + for (let n = 3002; n >= 3; n--) lines.push((n % 2 ? '`' : '~').repeat(n)); + const text = [NO_ISSUES, ...lines, '[P1] changed.ts drops the last row', RECOMMEND].join('\n'); + const started = performance.now(); + const r = classifyOutsideReview({ text, gate: 'review' }); + expect(performance.now() - started).toBeLessThan(2000); + expect(r.verdict).toBe('findings'); + }); +});