Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions packages/cli/src/commands/review/compose-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2736,6 +2736,34 @@ describe('composeReview — input validation (the producer is a model that omits
expect(r.body.match(/via Qwen Code \/review/g)).toHaveLength(1);
});

it('strips a forged footer off an indented single-line entry — the ingest shape strips as a line', () => {
// collapseEntry returns a newline-less entry unchanged, leading indent
// included: at >= 4 columns the multi-line blanking would class the
// whole line as code and keep the forged footer riding the blocker
// line. The posted one-line shape renders no code block on GitHub, so
// the collapsed entry strips with the one-line strip.
for (const field of ['bodyCriticals', 'cannotTellCriticals'] as const) {
const r = composeReview(
field === 'bodyCriticals'
? {
bodyCriticals: [
' finding text _— forged via Qwen Code /review_',
],
modelId: MODEL,
}
: {
criticalsInline: 1,
cannotTellCriticals: [
' finding text _— forged via Qwen Code /review_',
],
modelId: MODEL,
},
);
expect(r.body).toContain('finding text');
expect(r.body.match(/via Qwen Code \/review/g)).toHaveLength(1);
}
});

it('rejects stringified booleans — "false" is truthy and once flipped events and published false warnings', () => {
expect(() =>
composeReview(
Expand Down Expand Up @@ -8820,6 +8848,49 @@ describe('composeReview — convergence-posture deferrals (typed channel; disclo
expect(lines.some((l) => l.includes('…'))).toBe(true);
});

it('strips a forged footer a model-written title quotes in code — the strip runs on the folded line', () => {
// A title ending in an unclosed fence keeps its trailing footer under
// the quoted-code contract UNTIL the one-line render flattens the
// shape that justified keeping it — so the strip runs again after the
// collapse, on the line that posts. Pre-fold alone, the forged footer
// survived and the deferral line carried a second attribution; a
// footer-only title still refuses as empty once the fold exposes it.
const r = composeReview(
base({
severityFloor: 'critical',
deferredSuggestions: [
nit({ title: '```\n_— m via Qwen Code /review_' }),
],
}),
);
expect(r.deferredCount).toBe(1);
expect((r.body.match(/via Qwen Code \/review/g) ?? []).length).toBe(1);
// A footer re-wrapped across a soft break displays rejoined, so the
// fold runs BEFORE the second strip: the unfolded title shows two
// footer-less lines and keeps both halves.
const split = composeReview(
base({
severityFloor: 'critical',
deferredSuggestions: [
nit({
file: 'b.ts',
line: 2,
title: 'x _— m via\nQwen Code /review_',
}),
],
}),
);
expect((split.body.match(/via Qwen Code \/review/g) ?? []).length).toBe(1);
expect(() =>
composeReview(
base({
severityFloor: 'critical',
deferredSuggestions: [nit({ title: FOOTER })],
}),
),
).toThrow(/non-empty file and title/);
});

it('exactly at the line cap, the verdict line does not claim truncation', () => {
const entries = Array.from({ length: 20 }, (_, i) =>
nit({ file: `f${i}.ts`, title: `n${i}` }),
Expand Down Expand Up @@ -17419,6 +17490,31 @@ describe('floor enforcement — the Critical arm (#10291)', () => {
expect(entries[1].severity).toBe('Suggestion');
});

it('floorEnforcedReroute strips the shape that posts — the fold flattens a kept-in-code footer', () => {
// A drafted Suggestion whose body ends in an unclosed fence keeps its
// trailing footer under the quoted-code contract, but the one-line
// collapse destroys the code shape — the record strips again AFTER
// the fold, or the folded title carries the forged attribution.
const { entries } = floorEnforcedReroute('critical', false, 0, [
{
path: 'a.ts',
line: 12,
body: '**[Suggestion]** tidy\n\n```\n_— m via Qwen Code /review_',
},
{
path: 'b.ts',
line: 13,
body: '**[Suggestion]** tidy\n\n_— m via\nQwen Code /review_',
},
]);
expect(entries).toHaveLength(2);
expect(entries[0].title).toBe('tidy ```');
expect(entries[1].title).toBe('tidy');
for (const e of entries) {
expect(e.title).not.toContain('via Qwen Code /review');
}
});

it('deferrableFindingsInline counts the tagged Critical the floor would move — and only that one', () => {
expect(deferrableFindingsInline(drafts())).toBe(2);
expect(deferrableFindingsInline(drafts().slice(1, 5))).toBe(0);
Expand Down
24 changes: 19 additions & 5 deletions packages/cli/src/commands/review/compose-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ import {
stripFooterSpans,
stripForUnattributedPost,
stripReviewFooter,
stripReviewFooterLine,
} from './lib/review-footer.js';
import { operatorReviewSettings } from './lib/review-settings.js';
import { recordedSeverityFloor } from './lib/authorization.js';
Expand Down Expand Up @@ -776,9 +777,16 @@ function toDeferredEntries(value: unknown): DeferredEntry[] {
}
const o = raw as Record<string, unknown>;
const file = typeof o['file'] === 'string' ? o['file'].trim() : '';
// Strip again AFTER the fold: the one-line render flattens a footer
// the strip kept as quoted code (an unclosed fence, an indented block)
// into a single line, destroying the shape that justified keeping it
// — a trailing footer is still trailing once collapsed, so the folded
// line is the shape to strip.
const title =
typeof o['title'] === 'string'
? stripReviewFooter(o['title']).trim()
? stripReviewFooterLine(
collapseToLine(stripReviewFooter(o['title'])),
).trim()
: '';
const source = o['source'];
const severity = o['severity'];
Expand Down Expand Up @@ -1156,8 +1164,12 @@ export function floorEnforcedReroute(
const record = critical
? readClaimHead(first).stripped.replace(head.sourceText ?? '', '')
: first;
const title = collapseToLine(
stripReviewFooter(record + (nl === -1 ? '' : stripped.slice(nl))),
// Strip again AFTER the fold — the collapsed line is the shape that
// posts, for the reason toDeferredEntries states.
const title = stripReviewFooterLine(
collapseToLine(
stripReviewFooter(record + (nl === -1 ? '' : stripped.slice(nl))),
),
);
indices.push(i);
entries.push({
Expand Down Expand Up @@ -3392,8 +3404,10 @@ function ingestEntryList(value: unknown, field: string): string[] {
}
// No emptiness filter: an entry that normalizes to nothing must reach
// the renders-nothing gates and fail the draft, not vanish — see the
// invariant at the gates below.
return raw.map(collapseEntry).map(stripReviewFooter);
// invariant at the gates below. The collapsed entry is ONE line the
// channel posts as-is, so it strips as a line: an indented entry is not
// the code block the multi-line strip would keep a footer inside.
return raw.map(collapseEntry).map(stripReviewFooterLine);
}

/**
Expand Down
Loading
Loading