Outside reviews that mention a problem no longer pass as clean (#4136)

* test(outside-review): failing-first cases for false-clean conclusions (GST-1808) and the preflight state-root leak (GST-1809)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(outside-voice-preflight): spawn fixtures on an isolated state root (GST-1809)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(outside-review): fixtures that read clean only through the lax scope and colon rules name the whole change (GST-1808)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(outside-review): read the no-findings conclusion as one closed sentence (GST-1808)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(outside-review): failing-first: a whole-change scope may name the branch or its base (GST-1808 review p1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(outside-review): a whole-change scope may name the branch or the base it is diffed against (GST-1808 review p1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(outside-review): failing-first: a base-name tail never accepts another origin ref (GST-1808 review p2)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(outside-review): the base tail names only main or master, with or without origin/ (GST-1808 review p2)

Review-Finding: p2/red-team#1 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(changelog): scope the toward-clean claim to captured fixture replies (GST-1808 review v2)

Review-Finding: v2/adversarial-native#3 reader=adversarial-native
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(changelog): name the fragment after the branch and end the colon sentence (GST-1808, /ship Step 13)

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:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-07 19:49:19 +00:00
1 parent 1a29fbc344
commit f97c0dde02
10 files changed
+145 -64

No files matched your search

@@ -0,0 +1 @@
- **An outside review's no-findings conclusion must now be one complete sentence, so a review that reports a problem after its no-findings statement can no longer read as clean.** Until now a few reply shapes read `VERDICT: clean` on a merge gate while saying something was wrong: a Recommendation-only reply ("Recommendation: Ship because no issues in the diff, so the change disables auth."), an exception after the scope ("No issues in the diff with the exception of the auth change."), a question ("No issues in the diff?"), and a colon or symbol after the scope ("No issues in the diff: the fallback path now swallows ENOENT.", also with ` & `, ` + `, ` -> `, ` | ` and similar). The parser now reads the conclusion as a closed sentence. It may hold an optional lead, the no-findings statement, an optional "were found", an optional scope that names the whole change ("in the diff", "in the changes on this branch", "in the diff against main"), then at most one `.`, `!` or `;`. Anything else is `VERDICT: unverified` / `REASON: untagged_review`, so the rule no longer depends on lists of exception words and joiners. This fails closed in a few clean-looking shapes too: a scope that names part of the change ("No issues in the parser.", "in changed.ts") and a colon explanation ("No findings: the cap is enforced.") now read `unverified` even as the reply's only sentence. No captured fixture reply moves toward clean. Whole-change scopes that #4130 left fail-closed after other prose now read `clean`: "in the change as a whole", "in the committed and uncommitted changes" and "in the staged / unstaged changes". See docs/troubleshooting.md#outside-review-verdict. Also, test/outside-voice-preflight.test.ts no longer writes the machine's real ~/.gstack/config.yaml through an inherited state root. (GST-1808, GST-1809)
+20 -16
View File
@@ -198,22 +198,26 @@ the readiness dashboard and the PR body, and never count it as a pass.
`gstack-outside-voice run`); the verdict still comes from the rules above.
- **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", "I did not find
any bugs" or "The changes introduce no regressions", with no "but",
"however", "while", "whereas", "except", "other than" or "elsewhere". Closing sentences that
only report passing checks ("Tests, lint, and build all pass.") sign the
review off and are skipped, so the sentence before them is the conclusion;
a check sentence that quotes code, fails, is cut off, continues a wrapped
line, follows a colon or is the reply's only sentence is never skipped.
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, and neither does a
clause joined after that scope ("No issues in the diff, so it is safe.").
A statement with the change as its subject ("The changes introduce no
regressions") is held to this scope rule even as the reply's only sentence. A "no issues" phrase
anywhere else in the reply does not count. Otherwise the review is
`because` clause of its `Recommendation:` line, is a complete no-findings
sentence and nothing more. It may open with "Overall," or "After reviewing
the patch,", then states `NO_FINDINGS`, "No issues found", "I found no
actionable regressions", "I did not find any bugs" or "The changes introduce
no regressions", then may add "were found" or "is evident", then may name the
whole change ("in this change", "in the diff", "in the changed files", "in
the staged, unstaged, or untracked changes", "in the change as a whole",
"in the changes on this branch", "in the diff against main"), and ends
there with at most one `.`, `!` or `;`. Anything else makes the
review `unverified`: a clause after the scope ("No issues in the diff, so
it is safe.", "No issues in the diff: the fallback swallows ENOENT."), an
exception ("with the exception of", "save for"), a question ("No issues in
the diff?"), or a scope that names part of the change ("No issues in the
parser.", "in changed.ts"), whether it is the reply's only sentence or not.
Closing sentences that only report passing checks ("Tests, lint, and build
all pass.") sign the review off and are skipped, so the sentence before
them is the conclusion; a check sentence that quotes code, fails, is cut
off, continues a wrapped line, follows a colon or is the reply's only
sentence is never skipped. 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`.
+28 -41
View File
@@ -137,54 +137,41 @@ function severityWords(text: string): Severity[] {
return SEVERITY_LABELS.flatMap(re => [...text.matchAll(re)].map(m => SEVERITY_WORDS[m[1]!.toLowerCase()]!));
}
/**
* 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", "The changes introduce no definite,
* actionable regressions" (the whole change as subject). Never a phrase inside
* another clause ("renaming the flag is no problem for callers, but ...").
* The review's conclusion says it found nothing when its last prose sentence or
* ", and" clause (a wrapped line that continues in lowercase joined), or the
* because-clause of its Recommendation line, is one closed sentence (GST-1808):
* an optional lead ("Overall," or "After reviewing the patch,"), a no-findings
* statement (NO_FINDINGS, its underscore gone after plainReview; "No issues
* found"; "I did not find any bugs"; "The changes introduce no definite,
* actionable regressions"), an optional predicate ("were found", "is evident"),
* an optional scope that names the whole change ("in the diff", "in the changed
* files", "while reviewing the staged, unstaged, or untracked changes", "in the
* change as a whole", "in the changes on this branch", "in the diff against main"), then
* at most one ".", "!" or ";". Nothing else may follow: a clause after the scope,
* an exception, a colon explanation or a question never concludes, and a scope
* that names part of the change ("in the parser") says nothing about the rest of
* it, even as the reply's only sentence.
*/
const FINDING_NOUN = '(?:(?:actionable|significant|new|concrete|definite),?\\s+){0,2}(?: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 CHANGE_SUBJECT = '(?:the|this|these)\\s+(?:changes?|diffs?|patch(?:es)?)\\s+(?:introduces?|adds?|contains?|ha(?:s|ve))\\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+|${CHANGE_SUBJECT})?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');
/** The whole change as subject is held to the scope rule even as the reply's only sentence. */
const CHANGE_SUBJECT_STATEMENT = new RegExp(`^${NO_FINDINGS_LEAD}${CHANGE_SUBJECT}`, 'i');
const CHANGE_NOUN = '(?:change(?:s|sets?)?|diffs?|patch(?:es)?|branch|commits?|pull\\s+request|PR|(?:changed|modified|touched)\\s+(?:files?|code|lines))';
const CHANGE_KIND = '(?:staged|unstaged|untracked|committed|uncommitted|current|proposed|submitted|pending|whole|entire|full|complete|combined|new|latest|recent|local|read-only|code|branch)';
const WHOLE_CHANGE = `(?:(?:the|this|these|your|both|all(?:\\s+(?:of\\s+)?the)?)\\s+)?(?:${CHANGE_KIND}(?:(?:\\s*,\\s*(?:(?:and|or)\\s+)?|\\s*\\/\\s*|\\s+(?:and|or)\\s+)${CHANGE_KIND})*\\s+)?${CHANGE_NOUN}(?:\\s+as\\s+a\\s+whole)?(?:\\s+(?:on|in)\\s+(?:this|the)\\s+branch)?(?:\\s+against\\s+(?:the\\s+)?(?:base(?:\\s+branch)?|(?:origin\\/)?(?:main|master)))?`;
const READING = '(?:reviewing|reading|checking|inspecting)';
const NO_FINDINGS_LEAD = `(?:(?:overall|in summary|in short|summary|conclusion|result)[,:]?\\s+|(?:after|having|on|upon)\\s+(?:${READING}|reviewed|read|checked|inspected)\\s+${WHOLE_CHANGE},\\s+)?`;
const NO_FINDINGS_STATEMENT = `(?:NO_?FINDINGS\\b|(?:(?:i|we)\\s+(?:found|see|saw|identified)\\s+|there\\s+(?:are|were|is)\\s+|(?:the|this|these)\\s+(?:changes?|diffs?|patch(?:es)?)\\s+(?:introduces?|adds?|contains?|ha(?:s|ve))\\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})`;
const NO_FINDINGS_PREDICATE = '(?:\\s+(?:(?:was|were|is|are|has been|have been)\\s+)?(?:found|identified|detected|evident|apparent|introduced|remain|exist|to report))?';
const NO_FINDINGS_SCOPE = `(?:\\s+(?:in|within|across|throughout|with|from|for|(?:while|when|after|on|upon)\\s+${READING})\\s+${WHOLE_CHANGE})?`;
const NO_FINDINGS_CONCLUSION = new RegExp(`^${NO_FINDINGS_LEAD}${NO_FINDINGS_STATEMENT}${NO_FINDINGS_PREDICATE}${NO_FINDINGS_SCOPE}[.!;]?$`, 'i');
/**
* A whole sentence that only reports passing checks ("Tests, lint, and build all
* pass."). Closing the reply, it signs the review off and never replaces its conclusion (#3469).
*/
const CHECK_NAME = '(?:(?:unit|existing|targeted|full)\\s+)?(?:tests?|test suite|lint(?:ing)?|build|typecheck|type[- ]check|format check)';
const CHECK_RESULTS = new RegExp(`^(?:the\\s+|all\\s+)?${CHECK_NAME}(?:(?:,\\s*(?:and\\s+)?|\\s+and\\s+)${CHECK_NAME})*\\s+(?:all\\s+|both\\s+)?(?:pass(?:es|ed)?|succeed(?:s|ed)?|(?:is|are)\\s+green)[.!]$`, 'i');
/** A contrast or exception qualifies the claim, so the sentence does not conclude that nothing was found. */
const NO_FINDINGS_EXCEPTION = /(?<![\w-])(?:but|however|although|though|whereas|while(?!\s+(?:reviewing|reading|checking|inspecting)\b)|yet|except|excluding|unless|only|apart from|aside from|other than|besides|beyond|elsewhere|otherwise)(?![\w-])/i;
const NO_FINDINGS_PREDICATE = /^\s*(?:(?:was|were|is|are|has been|have been)\s+)?(?:found|identified|detected|evident|apparent|introduced|remain|exist|to report)?/i;
const WHOLE_CHANGE_SCOPE = /^(?:in|within|across|throughout|with|from|for)\s.*\b(?:chang(?:e|es|ed)|diffs?|patch(?:es)?|staged|unstaged|untracked|branch|commits?|pull request|PR)\b/i;
/** A list of change kinds ("the staged, unstaged, or untracked changes") is one scope; any other joiner starts a clause the scope does not cover. */
const SCOPE_LIST_JOIN = /(?:,\s*(?:(?:and|or)\s+)?|\s+(?:and|or)\s+)(?=(?:staged|unstaged|untracked|committed)\b)/gi;
const SCOPE_CLAUSE_BREAK = /[,;()[\]{}\u2012-\u2015]|--|\s[-/]\s|\b(?:and|or|plus|so|since|because|as|while|whereas|despite|then|which|that|where|when|though|but|yet|until|unless|if)\b/i;
/**
* The review's conclusion says it found nothing: its last prose sentence or
* ", and" clause (a wrapped line that continues in lowercase joined), or the because-clause of its
* Recommendation line, is an unqualified no-findings statement. When other
* prose precedes it, 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: "No issues in the parser." or "No regressions were found
* by the tests." says nothing about the rest of the reply. The conclusion is
* read from `concluding`, the reply without its closing check-results
* sentences, while the sentence count comes from the whole reply, so dropping
* a sign-off never makes a scoped claim the reply's only sentence.
*/
function concludesNoFindings(code: string, concluding: string): boolean {
const all = reviewProse(code), { sentences, because } = reviewProse(concluding);
const total = all.sentences.length + (all.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 && !CHANGE_SUBJECT_STATEMENT.test(conclusion)) || !qualifier || qualifier.startsWith(':')
|| (WHOLE_CHANGE_SCOPE.test(qualifier) && !SCOPE_CLAUSE_BREAK.test(qualifier.replace(SCOPE_LIST_JOIN, ' ')));
});
/** The conclusion is read from the reply without its closing check-results sentences, so a sign-off never stands in for it. */
function concludesNoFindings(concluding: string): boolean {
const { sentences, because } = reviewProse(concluding);
return [sentences.at(-1), because].some(conclusion => conclusion !== undefined && NO_FINDINGS_CONCLUSION.test(conclusion));
}
/**
@@ -317,7 +304,7 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC
const code = withoutCode(input.text), plainCode = plainReview(code);
const read = withoutCode(input.text, true), plainRead = plainReview(read);
const marked = recommendationHeading(code);
const concludes = concludesNoFindings(marked, recommendationHeading(withoutCode(withoutClosingCheckResults(input.text))));
const concludes = concludesNoFindings(recommendationHeading(withoutCode(withoutClosingCheckResults(input.text))));
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);
@@ -1,5 +1,5 @@
I read the diff, ran `bun test test/retry.test.ts`, and checked every caller of
`retry()`. No findings: the cap is enforced and the tests cover the boundary.
`retry()`. The cap is enforced and the tests cover the boundary. No findings.
Recommendation: ship because the change is covered and correct.
@@ -7,6 +7,6 @@ P1: The cap is ignored.
1. **P1 — The cap is ignored.**
```
No issues found in the changed parser.
No issues found in the changed files.
Recommendation: ship because the parser handles every documented form.
+1 -1
View File
@@ -1,5 +1,5 @@
This is a high-level plan with low-risk changes and a medium-term follow-up. The design is highly consistent, and there are no critical findings. A high number of the steps are already covered by tests, and the low cost of the rollback keeps the change safe.
No issues found in the changed helpers.
No issues found in the changed files.
Recommendation: proceed because the high-quality tests cover every changed path.
+89 -1
View File
@@ -28,7 +28,7 @@ function cli(args: string[]) {
}
// A clean review states that it found nothing; an untagged finding is unverified (DX-11).
const RECOMMEND = 'No issues found in changed.ts.\nRecommendation: approve because changed.ts keeps its data.';
const RECOMMEND = 'No issues found in this change.\nRecommendation: approve because changed.ts keeps its data.';
const UNTAGGED = 'Recommendation: fix the guard because changed.ts loses data.';
// A model may write the completion recommendation as a Markdown section instead of an inline line (#3980).
@@ -261,6 +261,94 @@ describe('a closing check-results sentence keeps the no-findings conclusion (#34
});
});
// GST-1808: the no-findings conclusion is one closed sentence: an optional lead
// ("Overall," or "After reviewing the patch,"), the statement, an optional predicate
// ("were found") and an optional scope naming the whole change, then at most one
// "." "!" or ";". Anything after that, a question, or a scope naming only part of
// the change leaves the review unverified, as the reply's only sentence, its last
// sentence or its Recommendation because-clause alike.
describe('the no-findings conclusion is a closed sentence (GST-1808)', () => {
const FINDING = 'the fallback path now swallows ENOENT.';
const verdictOf = (text: string, gate: OutsideGate) => {
const r = classifyOutsideReview({ text, gate });
return { text, gate, verdict: r.verdict, reason: r.reason };
};
const unverified = (text: string, gate: OutsideGate = 'structured') =>
expect(verdictOf(text, gate)).toEqual({ text, gate, verdict: 'unverified', reason: 'untagged_review' });
const clean = (text: string, gate: OutsideGate = 'structured') =>
expect(verdictOf(text, gate)).toEqual({ text, gate, verdict: 'clean', reason: undefined });
test('a Recommendation-only reply is held to the same rule as any other conclusion', () => {
for (const text of ['Recommendation: Ship because no issues in the diff, so the change disables auth.',
'Recommendation: ship because no issues, so the change disables auth.',
'Recommendation: ship because I found no regressions in this change, which now disables auth.',
'Recommendation: ship because no issues in the diff\nso the change disables auth.']) unverified(text, 'review');
});
test('an exception after the statement or its whole-change scope is not a conclusion', () => {
for (const tail of ['with the exception of the auth change', 'outside of the auth path', 'save for the auth change',
'after the change removed the auth check', 'bar the auth path', 'excepting the auth path']) {
for (const text of [`No issues in the diff ${tail}.`, `I reviewed the parser.\nNo issues in the diff ${tail}.`,
`No issues found ${tail}.`, `I reviewed the parser.\nNo actionable regressions were found in this change ${tail}.`]) unverified(text);
}
});
test('a question is not a conclusion', () => {
for (const text of ['Prose here. No issues in the diff?', 'No issues in the diff?', 'No issues?', 'NO_FINDINGS?',
'I reviewed the parser.\nNo issues in the diff?!', 'I reviewed the parser.\nNo issues found in this change?\nTests pass.']) unverified(text);
unverified('Recommendation: ship because no issues remain?', 'review');
});
test('a joiner after the scope starts a clause the conclusion does not cover, with or without a sign-off (fc3#1)', () => {
for (const joiner of [': ', ' & ', ' + ', ' -> ', ' \u2192 ', ' | ', '\u2026 ', '/', ' ~ ', ' \u2010 ', ' \u2212 ', '- ', ' with ', ' now ', '. so ']) {
for (const signoff of ['', ' Tests pass.']) unverified(`I reviewed the parser.\nNo issues in the diff${joiner}${FINDING}${signoff}`);
}
for (const text of [`No issues found: ${FINDING}`, `I reviewed the parser.\nNo issues found: ${FINDING}`, `NO_FINDINGS: ${FINDING}`]) unverified(text);
});
test('a scope that names only part of the change does not conclude, even as the only sentence', () => {
for (const text of ['No issues in the parser.', 'I found no issues in the retry helper.', 'After reviewing the parser, I found no issues.',
'I reviewed the parser.\nNo issues in the auth module of this change.', 'No issues in the diff against the auth module.',
// Former fixtures that read clean only through the lax scope and colon rules (RECOMMEND, GST-436's NO_ISSUES, prose-negative.txt,
// codex-3105/p1-in-code.txt and codex-3105/no-findings.txt).
'No issues found in changed.ts.\nRecommendation: approve because changed.ts keeps its data.', 'No issues found in the changed helpers.',
'Reviewed the parser.\n\nNo issues found in the changed parser.',
'I read the diff and checked every caller.\nNo findings: the cap is enforced and the tests cover the boundary.']) unverified(text, text.includes('Recommendation:') ? 'review' : 'structured');
});
test('real clean conclusions stay clean (negative controls)', () => {
for (const text of [
// #3469's two clean tails (codex review --base main, codex-cli 0.163.0-alpha, gpt-6-luna), with and without their sign-off.
'No actionable regressions were identified in the diff. Tests, lint, and build all pass.',
'The changes introduce no definite, actionable regressions. Tests, lint, and build all pass.',
'No actionable regressions were identified in the diff.', 'The changes introduce no definite, actionable regressions.',
// #3294's reply and the codex-cli 0.160.0 captures.
'I found no actionable regressions in the staged, unstaged, or untracked changes.',
'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.',
'NO_FINDINGS', '**NO_FINDINGS**', 'No issues found.', 'No issues found!', 'There are no actionable regressions.', 'I did not find any defects.',
'Overall, no issues found.', 'After reviewing the patch, I found no issues.', 'Having reviewed the diff, I found no issues.',
'No issues found while reviewing the diff.', 'I found no issues in this read-only change.', 'No issues to report in this PR.',
'Reviewed the retry helper.\n\nNo issues found in this change.', 'No regressions were found in the change as a whole.',
'No issues found across the committed and uncommitted changes.', 'No issues found in the staged / unstaged changes.', 'No issues found in the changed files.',
'The change validates the retry limit.\nNo actionable regressions were identified in the diff; tests, lint, and build all pass.',
]) clean(text);
for (const text of ['Recommendation: ship because I found no regressions in this change.', 'Recommendation: proceed because no findings remain.',
'The change adds a retry cap.\nRecommendation: ship because no issues remain.', '## Recommendation\n\nApprove because I found no issues.']) clean(text, 'review');
});
test('a whole-change scope may name the branch or the base it is diffed against, and nothing more', () => {
for (const text of ['No issues found in the changes on this branch.', 'I found no actionable regressions in the branch diff.',
'No issues found in the diff against main.', 'No issues found in the diff against the base branch.',
'No issues found in the changes on this branch against origin/main.', 'After reviewing the changes on this branch, I found no issues.']) clean(text);
clean('Recommendation: ship because no issues in the changes on this branch.', 'review');
for (const text of ['No issues found in the changes on this branch, except the auth path.', 'No issues found in the diff against main, so ship it.',
'No issues found in the changes on this branch?', 'No issues found in the diff against the auth module.',
'No issues found in the branch diff against main except the auth path.', 'No issues found in the diff against origin/main-except-the-auth-path.',
'No issues found in the diff against origin/feature/auth-only.']) unverified(text);
});
});
describe('validateOutsideReview keeps the text-only shape for direct importers', () => {
test('existing shapes are unchanged', () => {
expect(validateOutsideReview('', 'review')).toEqual({ completed: false, reason: 'empty response' });
+1 -1
View File
@@ -6,7 +6,7 @@
import { describe, expect, test } from 'bun:test';
import { classifyOutsideReview } from '../lib/outside-review-result';
const NO_ISSUES = 'No issues found in changed.ts.';
const NO_ISSUES = 'No issues found in this change.';
const RECOMMEND = 'Recommendation: approve because changed.ts keeps its data.';
describe('an unclosed fence does not hide what follows it (GST-436)', () => {
+2 -1
View File
@@ -10,6 +10,7 @@ import { validateOutsideReview } from '../lib/outside-review-result';
import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types';
import { ALL_HOST_CONFIGS } from '../hosts';
import { expectMentions, expectTokens } from './helpers/prompt-structure';
import { isolatedEnv } from './helpers/state-root-isolation';
const ROOT = path.resolve(import.meta.dir, '..');
const TEMP = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-outside-preflight-'));
@@ -182,7 +183,7 @@ function fixture() {
const capture = path.join(dir, 'capture.json');
const fake = path.join(dir, 'fake-claude.ts');
fs.writeFileSync(fake, `const prompt=await Bun.stdin.text(); await Bun.write(process.env.FIXTURE_CAPTURE!,JSON.stringify({prompt,args:process.argv.slice(2)})); console.log(JSON.stringify({result:'No issues found.\\nRecommendation: ship because the isolated fixture completed its review.'}));`);
const env = { ...process.env, HOME: home, CODEX_HOME: '', GSTACK_HOME: path.join(home, '.gstack'),
const env = { ...isolatedEnv(home), CODEX_HOME: '', GSTACK_HOME: path.join(home, '.gstack'),
GSTACK_ROOT: '', GSTACK_BIN: '', GSTACK_ACTIVE_HOST: 'codex', CODEX_THREAD_ID: 'fixture', CODEX_SANDBOX: '', CLAUDECODE: '',
GSTACK_CLAUDE_BIN: process.execPath, GSTACK_CLAUDE_BIN_ARGS: JSON.stringify([fake]), FIXTURE_CAPTURE: capture };
const install = (destination: string) => {
+1 -1
View File
@@ -17,7 +17,7 @@ import { bunIsolationArgs } from '../lib/bun-isolation';
import { tree } from './helpers/install-fixture';
const ROOT = path.resolve(import.meta.dir, '..');
const GUARDED = ['test/update-channel.test.ts', 'test/update-channel-setup.test.ts'];
const GUARDED = ['test/update-channel.test.ts', 'test/update-channel-setup.test.ts', 'test/outside-voice-preflight.test.ts'];
const SENTINELS: Record<string, string> = { GSTACK_STATE_ROOT: 'state-root', GSTACK_HOME: 'gstack-home', GSTACK_STATE_DIR: 'state-dir', CLAUDE_PLUGIN_DATA: 'plugin-data', HOME: 'home' };
const bases: string[] = [];