mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 14:34:42 +01:00
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 main89f20330. 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 main89f20330. 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>
This commit is contained in:
1 parent
088bfc5914
commit
a305302963
7 files changed
+116
-13
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, number> = { '`': 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');
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user