fix(review): count the script-lint gate's Criticals in the convergence signal (#9526)

The persistently-critical signal read `thisCriticals` before the gate
pushed its Criticals into `bodyCriticals`, and the ledger work-list
feeding the next round's persistence half omitted them too. A loop
whose standing blocker is the deterministic [lint] gate — the exact
shape the signal exists to name — held the whole conjunction
semantically while the advisory stayed silent: the count was taken
before the array was complete, and the gate-only round recorded no
sev 'C' for its successor to recover.

The assessment now runs after the relocated and gate pushes and reads
the completed array with the same semantics as the verdict's own `c`
(the explicit relocated term drops with the push that already carries
it), and the gate's Criticals join the marker work-list. Adds the
handler fixture arming the gate end to end — advisory fires, marker
records sev 'C' — and pins both branches of the trim notice's
copy-location conditional, which had no oracle on either side.
This commit is contained in:
qwen-code-dev-bot 2026-08-20 14:00:02 +00:00
parent 7389a108df
commit d72287cc6d
2 changed files with 178 additions and 25 deletions

View file

@ -3133,6 +3133,135 @@ describe('composeReviewCommand handler (the CLI glue)', () => {
}
});
it("counts the script-lint gate's standing Critical — advisory and work-list (#9526)", async () => {
// The deterministic gate posts a body-only [lint] Critical every round
// while the model drafts nothing — the standing-blocker loop the signal
// exists to name. The count must see the gate's Critical exactly like
// the verdict's own `c` does, and the carried work-list must record
// sev 'C' for it, or the whole conjunction holds semantically while the
// advisory stays silent and the next round's persistence half is blind.
const dir = mkdtempSync(join(tmpdir(), 'compose-converge-gate-'));
const inputPath = join(dir, 'compose.json');
const commentsPath = join(dir, 'comments.json');
const planPath = join(dir, 'plan.json');
// A worktree arms the gate (pr-worktree, not diff-only); the report
// binds to the plan diff's hash so the gate reads it as fresh.
const diffPath = join(dir, 'the.diff');
writeFileSync(
diffPath,
'diff --git a/deploy.sh b/deploy.sh\n@@ -0,0 +1 @@\n+x\n',
'utf8',
);
const diffHash = createHash('sha256')
.update(readFileSync(diffPath))
.digest('hex');
writeFileSync(
planPath,
JSON.stringify({
prNumber: 8255,
worktreePath: '.qwen/tmp/review-pr-8255',
diffPathAbsolute: diffPath,
}),
'utf8',
);
writeFileSync(
join(dir, 'qwen-review-pr-8255-script-lint.json'),
JSON.stringify({
checked: [
{
path: 'deploy.sh',
tool: 'shellcheck',
findings: [
{
line: 1,
code: 'SC2086',
level: 'info',
message: 'quote the variable',
inDiff: true,
},
],
},
],
skipped: [],
errored: [],
deferred: [],
ok: false,
note: '',
diffHash,
}),
'utf8',
);
writeFileSync(
inputPath,
JSON.stringify({ modelId: MODEL, planPath, severityFloor: 'auto' }),
'utf8',
);
// The model drafts nothing: the gate's [lint] blocker is the round's
// only Critical and posts body-only, so the inline volume is 0.
writeFileSync(commentsPath, '[]', 'utf8');
const stderr = () =>
(writeStderrLine as ReturnType<typeof vi.fn>).mock.calls.map((c) =>
String(c[0]),
);
const stdoutJson = () =>
JSON.parse(
(writeStdoutLine as ReturnType<typeof vi.fn>).mock.calls
.map((c) => String(c[0]))
.join('\n'),
) as {
event?: string;
convergence?: {
shape: string;
recommendation: string;
criticals: number;
posted: number;
prevPosted: number;
};
body?: string;
};
try {
// The predecessor carried a Critical and posted 0; this round posts 0
// inline (the gate blocker rides the body) — flat, not shrinking.
// Round 7 of `auto`: the floor is engaged.
(writeStderrLine as ReturnType<typeof vi.fn>).mockClear();
(writeStdoutLine as ReturnType<typeof vi.fn>).mockClear();
writeFileSync(
join(dir, 'qwen-review-pr-8255-prev-ledger.json'),
JSON.stringify({
v: 1,
round: 6,
findings: [{ id: 'R6-1', sev: 'C', file: 'x.ts', title: 'blocker' }],
posted: 0,
}),
'utf8',
);
await runComposeReviewCommand({
input: inputPath,
comments: commentsPath,
});
const composed = stdoutJson();
expect(composed.event).toBe('REQUEST_CHANGES');
expect(composed.convergence).toMatchObject({
shape: 'persistently-critical',
recommendation: 'land-with-residual-risk',
criticals: 1,
posted: 0,
prevPosted: 0,
});
expect(composed.body).toContain('land-with-residual-risk');
expect(
stderr().filter((l) => l.startsWith('CONVERGENCE: ')),
).toHaveLength(1);
// The marker records the gate Critical as sev 'C' in the work-list,
// so a second gate-only round recovers the persistence half instead
// of reading "no prior Critical" over a round that posted one.
const ledger = parseLedger(composed.body ?? '');
expect(ledger?.findings.some((f) => f.sev === 'C')).toBe(true);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it('honours review.attribution=false through the handler (wiring)', async () => {
// Third wiring leg: deleting the attribution argument from the
// composeReviewCommand call leaves the direct composeReview test and the
@ -7777,6 +7906,16 @@ describe("composeReview — the composed body fits GitHub's limit", () => {
// operator sent to a list that does not exist is the same false record
// in the channel the operator actually reads.
expect(r.remediation.join('\n')).not.toContain('findings artifact');
// The rank-3-only tail clause: a trimmed disclosure section survives
// nowhere but the terminal summary, and the line must say exactly
// that — naming an advisory copy for an advisory that was never
// trimmed is the same false record in the other direction.
expect(
r.remediation.some(
(l) =>
l.startsWith('body budget:') && l.includes('their only other copy'),
),
).toBe(true);
});
it('names the trimmed advisory for itself — never a deferral list that does not exist (#9410)', () => {
@ -7857,7 +7996,13 @@ describe("composeReview — the composed body fits GitHub's limit", () => {
r.remediation.some(
(l) =>
l.startsWith('body budget:') &&
l.includes('persistently-critical convergence advisory'),
l.includes('persistently-critical convergence advisory') &&
// The tail clause is the branch under test: the advisory keeps
// two more durable copies, so "their only other copy" (true only
// of rank-3 disclosures) would be a false record here.
l.includes(
'another copy — the advisory also rides the composed JSON',
),
),
).toBe(true);
});

View file

@ -1515,6 +1515,11 @@ function ledgerMarkerFor(
// The same split the body performed: a relocated Critical is a
// posted, counted blocker and must enter the work list.
...splitDeferralChannel(input.deferredSuggestions).relocated,
// The same gate the body ran: a gate Critical is a posted,
// counted blocker too — leaving it out let the next round's
// persistence half read "no prior Critical" over a round that
// posted one (#9526).
...scriptLintGate(input.planPath).criticals,
],
),
// The pair falls together: a sha with no model reads to the next
@ -1679,30 +1684,6 @@ function composeReviewBody(
relocated: relocatedCriticals,
relocatedDeterministic,
} = splitDeferralChannel(input.deferredSuggestions);
// The persistently-critical convergence signal (#9410): computed, never
// decided. "This round's Criticals" counts every Critical this round
// stands behind — the inline count, the body-only Criticals, and the
// relocated ones (a Critical the model deferred is relocated back to a
// posted blocker, so it still stands). The persistence half and the volume
// window come from the carried ledger (`prevConvergence`). Advisory only:
// it cannot move the event or cap the verdict; it only surfaces, and every
// input degrades open to "no assessment".
const convergence = convergenceAssessment({
prevHadCritical: prevConvergence.hadCritical,
// The floor-engagement conjunct is computed by the SAME predicate the
// enforcement backstop keys on (#9410): the advisory's "the floor will
// not converge it" claim is provable only where the floor is actually
// running, so a pre-engagement round degrades open to silence.
floorEngaged: severityFloorEngaged(
input.severityFloor,
input.contextUnavailable === true,
prevRound,
),
thisCriticals:
criticalsInline + bodyCriticals.length + relocatedCriticals.length,
posted: postedInline,
prevPosted: prevConvergence.posted,
});
// The floor-enforced reroutes join the model's deferrals AFTER the split:
// they are constructed typed by this module's own code (see
// `floorEnforcedReroute`), so routing them through the model-channel
@ -1990,6 +1971,33 @@ function composeReviewBody(
// dimension nobody reviewed. Inert on every diff the manifest does not mark.
unreviewed.push(...layerAuditGate(input.planPath, input.env).unreviewed);
}
// The persistently-critical convergence signal (#9410): computed, never
// decided. Computed HERE — after the relocated push above and the gate
// push inside this block — because it counts every Critical this round
// stands behind, and `bodyCriticals` is only complete once both have
// joined it: the SAME array, with the SAME semantics, the verdict's `c`
// counts below. A count taken before the pushes read a gate-only round —
// a standing deterministic [lint] blocker the floor can never converge —
// as standing behind zero Criticals, so the advisory the shape exists to
// surface never fired on it (#9526). The persistence half and the volume
// window come from the carried ledger (`prevConvergence`). Advisory only:
// it cannot move the event or cap the verdict; it only surfaces, and every
// input degrades open to "no assessment".
const convergence = convergenceAssessment({
prevHadCritical: prevConvergence.hadCritical,
// The floor-engagement conjunct is computed by the SAME predicate the
// enforcement backstop keys on (#9410): the advisory's "the floor will
// not converge it" claim is provable only where the floor is actually
// running, so a pre-engagement round degrades open to silence.
floorEngaged: severityFloorEngaged(
input.severityFloor,
input.contextUnavailable === true,
prevRound,
),
thisCriticals: criticalsInline + bodyCriticals.length,
posted: postedInline,
prevPosted: prevConvergence.posted,
});
// The Criticals a verifier must have ruled on before this review may post them as
// blockers. Only the MODEL's criticals are candidates — the gate's are excluded by