Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
186 changes: 186 additions & 0 deletions packages/cli/src/commands/review/check-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
findingsFilePath,
} from './lib/prompt-record.js';
import { requiredAgents, type RosterPlan } from './lib/roster.js';
import { BRIEFS } from './lib/agent-briefs.js';
import { checkCoverageCommand } from './check-coverage.js';
import { writeStderrLine } from '../../utils/stdioHelpers.js';

Expand Down Expand Up @@ -858,6 +859,191 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => {
expect(r.budgetGaps).toEqual([]);
});

it("names a rostered discloser by its brief's publicLabel, not its prompt", () => {
// The fallback label is the launch prompt's first line, and a real
// posted body rendered a disclosure as "You are review agent
// `reverse-audit` — Reverse audit agen...:" — plumbing, truncated, on a
// public PR page. A record matching a built role prompt gets the
// author-register name instead. The key is spelled the way
// agent-prompt records a findings-taking role — with its digest
// suffix — because that is the shape the lookup has to survive.
transcript('a1', good(1), { calls: 3, range: [0, 100] });
transcript('a2', good(2), { calls: 2, range: [100, 100] });
const p = plan();
const role = 'reverse-audit';
const key = `${role}--round-1--abc123def456`;
const d = promptRecordDir(p);
const brief = briefPath(p, key);
const prompt =
`You are ${role}.\n` +
`read_file(file_path="${brief}")\n` +
`read_file(file_path="${DIFF}")`;
writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt);
transcript('tm-gap', prompt, {
calls: 3,
opens: [brief],
text:
'No issues found — mapped the behaviours.\n' +
'Budget gap: the negative-path matrix rows',
});

const gaps = coverageFromTranscripts(p, ENV).budgetGaps;
const entry = gaps.find((g) =>
g.gaps.includes('the negative-path matrix rows'),
);
expect(entry?.agent).toBe(BRIEFS[role].publicLabel);
expect(entry?.agent).not.toContain('You are');
});

it('keeps the launch first line for a discloser no built prompt matches', () => {
// The boundary of the rename above: only a record matching a BUILT
// role prompt escapes the fallback, and the fallback is the launch
// prompt's first line, truncated — the exact register the production
// spill wore. A discloser whose prompt the run wrote itself keeps
// that name; a regression to a worse default fails right here, in
// the channel where the spill landed.
transcript('a1', good(1), { calls: 3, range: [0, 100] });
transcript('a2', good(2), { calls: 2, range: [100, 100] });
const p = plan();
const prompt =
'You are review agent `free-lance`, an extra pass this run wrote for itself.\n' +
`read_file(file_path="${DIFF}", offset=0, limit=100)`;
transcript('stray', prompt, {
calls: 2,
range: [0, 100],
text: 'Walked what I could.\nBudget gap: the stray pass',
});

const gaps = coverageFromTranscripts(p, ENV).budgetGaps;
const entry = gaps.find((g) => g.gaps.includes('the stray pass'));
expect(entry?.agent).toBe(
'You are review agent `free-lance`, an extra pass this run...',
);
});

it('names idle and unopened rostered agents in the same register', () => {
// The fallback name rides the posted body's coverage lines too: a
// rostered whole-diff agent that made no tool call, or none against
// the diff, must not read "You are ..." there either.
const p = plan();
const d = promptRecordDir(p);
const launch = (key: string, offset: number): string => {
const brief = briefPath(p, key);
const prompt =
`You are ${key}.\n` +
`read_file(file_path="${brief}")\n` +
`read_file(file_path="${DIFF}", offset=${offset}, limit=100)`;
writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt);
return prompt;
};
transcript('tm-idle', launch('reverse-audit--round-1--abc123def456', 0), {
calls: 0,
});
const unopenedKey = 'verify--round-1--fed456abc123';
transcript('tm-unopened', launch(unopenedKey, 100), {
opens: [briefPath(p, unopenedKey)],
});

const r = coverageFromTranscripts(p, ENV);
expect(r.idleAgents).toEqual([BRIEFS['reverse-audit'].publicLabel]);
expect(r.unopenedAgents).toEqual([BRIEFS['verify'].publicLabel]);
});

it('keeps the file on a file-scoped rostered label', () => {
// A `${role}--${file}` launch is rostered per heavy file; dropping the
// file makes N per-file agents of one role read as one repeated line in
// the posted body, indistinguishable — the author cannot tell which
// file's check stopped. The label keeps the file, the way
// `publicRoleLabel` renders these same roles elsewhere. Not `plan()`:
// its files are not heavy, so its roster carries no invariant agents
// for the label lookup to resolve against.
const p = join(dir, 'plan.json');
writeFileSync(
p,
JSON.stringify({
diffPathAbsolute: DIFF,
srcDiffLines: 5000,
diffLines: 5000,
files: [
{ path: 'src/a.ts', kind: 'source', removedLines: 0, heavy: true },
{ path: 'src/b.ts', kind: 'source', removedLines: 0, heavy: true },
],
chunks: [
{ id: 1, startLine: 1, endLine: 100 },
{ id: 2, startLine: 101, endLine: 200 },
],
}),
);
const d = promptRecordDir(p);
mkdirSync(d, { recursive: true });
const role = 'invariant-a';
const launch = (file: string): string => {
const key = `${role}--${file}`;
const brief = briefPath(p, key);
const prompt =
`You are ${key}.\n` +
`read_file(file_path="${brief}")\n` +
`read_file(file_path="${DIFF}", offset=0, limit=100)`;
writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt);
return prompt;
};
transcript('inv-idle-a', launch('src/a.ts'), { calls: 0 });
transcript('inv-idle-b', launch('src/b.ts'), { calls: 0 });
const old = new Date(2020, 0, 1);
utimesSync(p, old, old);

const r = coverageFromTranscripts(p, ENV);
const base = BRIEFS[role].publicLabel;
expect(r.idleAgents).toHaveLength(2);
expect(r.idleAgents).toContain(`${base} on src/a.ts`);
Comment on lines +996 to +998

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R7-P1 (instance 4/9): Pattern — nine behaviors this diff adds are pinned by NO discriminating test; for each, an executed mutant survives the entire suite and a probe demonstrates the behavioral flip. This instance: the file-scoped test pins only the English half of the per-file label — the Chinese twin computed alongside it (publicRoleLabelZh${base}(${req.file}), carried out via publicLabelsZh) is pinned by no test. Executed mutation: dropping the file from publicRoleLabelZh leaves all 304 tests green, and two per-file invariant agents stopping in a bilingual run then render two identical zh clauses with no file discriminator — the exact indistinguishability this test's own comment says it exists to prevent, surviving on the zh side (probe flips). Suggested fix: also assert expect(r.publicLabelsZh[${base} on src/a.ts]).toBe(${BRIEFS[role].publicLabelZh}(src/a.ts)) (and likewise for src/b.ts).

中文说明

[Suggestion] R7-P1(实例 4/9):模式——本 diff 新增的九处行为均无任何判别性测试固定;每一处执行变异后整个套件仍然全绿,且探针演示了行为翻转。本实例:file-scoped 测试只固定了按文件标签的英文半区——与之同时计算的中文孪生(publicRoleLabelZh${base}(${req.file}),经 publicLabelsZh 输出)没有任何测试固定。已执行变异:从 publicRoleLabelZh 中去掉文件后全部 304 个测试仍绿,双语运行中两个按文件的 invariant agent 停止时会渲染出两条逐字相同、无文件区分的中文子句——正是本测试自身注释声称要防止的不可区分性,在中文侧存活(探针翻转)。建议修复:同时断言 expect(r.publicLabelsZh[${base} on src/a.ts]).toBe(${BRIEFS[role].publicLabelZh}(src/a.ts))src/b.ts 同理)。

— qwen3.8-max via Qwen Code /review (v0.21.10)

expect(r.idleAgents).toContain(`${base} on src/b.ts`);
// The Chinese twin keeps the file too: dropping it renders two
// identical zh clauses with no file discriminator — the exact
// indistinguishability this test exists to prevent, on the zh side.
expect(r.publicLabelsZh[`${base} on src/a.ts`]).toBe(
`${BRIEFS[role].publicLabelZh}(src/a.ts)`,
);
expect(r.publicLabelsZh[`${base} on src/b.ts`]).toBe(
`${BRIEFS[role].publicLabelZh}(src/b.ts)`,
);
});

it('never reads a digest or chunk suffix as a file on a rostered label', () => {
// Two-segment keys are not all file-scoped: `verify--<digest>` and
// `reverse-audit--chunk-N` are Step 3B/4 plumbing. Only the roster's
// per-file requirements carry a file into the label; anything else
// keeps the bare publicLabel, so no digest or chunk id reaches the
// posted body.
const p = plan();
const d = promptRecordDir(p);
const brief = briefPath(p, 'verify--abc123def456');
const prompt =
`You are verify--abc123def456.\n` +
`read_file(file_path="${brief}")\n` +
`read_file(file_path="${DIFF}")`;
writeFileSync(
join(d, `${encodeURIComponent('verify--abc123def456')}.txt`),
prompt,
);
transcript('tm-digest', prompt, { calls: 0 });
// The chunk-suffixed findings-role key shape: a key-shape heuristic in
// place of the roster lookup would read `chunk-1` as a file and leak
// the chunk id onto the public label, so it keeps the bare publicLabel.
const chunkKey = 'reverse-audit--chunk-1--round-1--abc123def456';
const chunkBrief = briefPath(p, chunkKey);
const chunkPrompt =
`You are ${chunkKey}.\n` +
`read_file(file_path="${chunkBrief}")\n` +
`read_file(file_path="${DIFF}")`;
writeFileSync(join(d, `${encodeURIComponent(chunkKey)}.txt`), chunkPrompt);
transcript('tm-chunk-key', chunkPrompt, { calls: 0 });

const r = coverageFromTranscripts(p, ENV);
expect(r.idleAgents).toHaveLength(2);
expect(r.idleAgents).toContain(BRIEFS['verify'].publicLabel);
expect(r.idleAgents).toContain(BRIEFS['reverse-audit'].publicLabel);
});

it('reports none when nobody disclosed one', () => {
transcript('a1', good(1), { calls: 3 });
transcript('a2', good(2), { calls: 2 });
Expand Down
Loading
Loading