Skip to content
Closed
92 changes: 91 additions & 1 deletion packages/cli/src/commands/review/check-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,20 @@ function transcript(
* credit narrows to its ranged reads.
*/
range?: [number, number];
/**
* Launch-path provenance stamped on the records — `workflow` for a
* workflow agent() dispatch, which shares this directory but is not an
* agent this review launched.
*/
agentKind?: string;
} = {},
): void {
const base = { agentId: id, agentName: 'general-purpose', sessionId: 'S1' };
const base = {
agentId: id,
agentName: 'general-purpose',
sessionId: 'S1',
...(opts.agentKind ? { agentKind: opts.agentKind } : {}),
};
const pointedAtBriefs = [
...launchPrompt.matchAll(/read_file\(file_path="([^"]*\.brief\.md)"\)/g),
].map((m) => m[1]);
Expand Down Expand Up @@ -433,6 +444,56 @@ describe('coverage — from the harness, not from the caller', () => {
expect(r.ok).toBe(false);
});

it('ignores workflow-dispatch transcripts, however they read', () => {
// Workflow agent() dispatches write their transcripts into this same
// directory. A pre-launch failure or an abandoned attempt leaves a
// seed-only record — zero tool calls — that used to be classified idle
// BEFORE the "not launched by this review" escape could see it, failing
// a compliant review. Provenance, not prompt shape, decides.
transcript('a1', good(1), { calls: 2 });
transcript('a2', good(2), { calls: 2 });
// Seed-only: the prompt carries no diff path.
transcript('workflow-agent-deadbeef01', 'review the widget cache', {
calls: 0,
agentKind: 'workflow',
});
// Prompt that happens to name the diff — `given` would be true, and
// zero tool calls would read as an idled review agent.
transcript('workflow-agent-deadbeef02', `read ${DIFF} and summarize`, {
calls: 0,
agentKind: 'workflow',
});
// Prompt that happens to say `chunk 1 of 2` — would otherwise be
// adopted as that chunk's agent and reported blind.
transcript('workflow-agent-deadbeef03', 'chunk 1 of 2 of my own work', {
calls: 0,
agentKind: 'workflow',
});

const r = coverageFromTranscripts(plan(), ENV);
expect(r.ok).toBe(true);
expect(r.idleAgents).toEqual([]);
expect(r.blindAgents).toEqual([]);
// The foreign records do not even count as agents of this review:
// the two chunk agents plus the roster's test-matrix stand-in, and
// none of the three workflow transcripts.
expect(r.agents).toBe(3);
});

it('skips zero-tool-call records this review never launched', () => {
// The "not launched" escape runs BEFORE the idle classification: a
// foreign record with no diff in its prompt owes the review nothing,
// whatever it did or did not call — including agents the review's own
// agents spawned.
transcript('a1', good(1), { calls: 2 });
transcript('a2', good(2), { calls: 2 });
transcript('nested-spawn', 'check the build logs', { calls: 0 });

const r = coverageFromTranscripts(plan(), ENV);
expect(r.ok).toBe(true);
expect(r.idleAgents).toEqual([]);
});

it('names a blind launch as itself — the prompt is the defect, not the agent', () => {
// The real failure, 23 times over: the agent was handed a description of a
// chunk it had no way to open. Calling this a whiff sends the reader off to
Expand Down Expand Up @@ -1981,6 +2042,35 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', ()
expect(gapText(r)).not.toMatch(/verification/);
});

it('ignores workflow-dispatch transcripts, however they match the floor', () => {
// Same provenance fence as coverageFromTranscripts, asserted where it
// bites: a workflow agent() dispatch launched with the VERBATIM built
// prompt, which opened the brief and read the findings file, is the
// exact record shape that satisfies the verifier floor. Without the
// filter this one record flips the gate to ok while nothing this
// review launched ever verified anything.
const p = plan();
step45(p, 'reverse-audit'); // Step 5 compliant; verification is the subject
const key = 'verify--abc123def456';
step45(p, key, { findings: true, launch: false });
const recordedPrompt = readFileSync(
join(promptRecordDir(p), `${key}.txt`),
'utf8',
);
transcript('workflow-agent-deadbeef01', recordedPrompt, {
calls: 2,
opens: [briefPath(p, key), findingsFilePath(p, key)],
agentKind: 'workflow',
});

const r = verificationGaps(p, { postsFindings: true }, ENV);
expect(r.ok).toBe(false);
expect(r.unverifiedFindings).toBe(true);
expect(gapText(r)).toMatch(
/verification — its prompt was built, but no agent was launched with it/,
);
});

it('flags a verifier built but whose agent never opened its brief', () => {
const p = plan();
step45(p, 'reverse-audit');
Expand Down
35 changes: 26 additions & 9 deletions packages/cli/src/commands/review/lib/coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,16 @@ export function coverageFromTranscripts(
env: NodeJS.ProcessEnv = process.env,
): CoverageFromTranscripts {
const { plan, mtimeMs } = readPlan(planPath);
const records = readTranscripts(mtimeMs, env, plan.diffPathAbsolute);
const records = readTranscripts(mtimeMs, env, plan.diffPathAbsolute).filter(
// Workflow `agent()` dispatches write their transcripts into this same
// directory, but they are not agents this review launched. A seed-only
// workflow record made zero tool calls and would be classified idle
// before the "not launched by this review" escape below could see it,
// and a workflow prompt that happens to say `chunk N of M` would be
// adopted as that chunk's agent. Provenance, not prompt shape, decides
// whose evidence this gate reads.
(rec) => rec.agentKind !== 'workflow',
);
const built = readRecordedPrompts(planPath);

const blindAgents: string[] = [];
Expand Down Expand Up @@ -565,6 +574,16 @@ export function coverageFromTranscripts(
continue; // Its silence proves nothing about the diff; the prompt failed.
}

// Not a diff reader, and not required to be. Two review agents legitimately
// never open the diff — Build & Test runs the build, Issue Fidelity reads the
// issue — and the session's transcript directory also holds agents this review
// did not launch, including ones its own agents spawned. None of them owes the
// diff anything; none of them may be credited with having read it either.
// Runs BEFORE the idle check, on purpose: a zero-tool-call record this
// review never launched is not a review agent that idled, and classifying
// it as one is how a foreign transcript fails a compliant review.
if (!given) continue;

// Did it work? Zero successful tool calls means it read nothing — whatever
// its prose says. This is checked BEFORE the Uncoverable claim below, and the
// order is load-bearing: `Uncoverable: chunk N` is a line the prompt hands the
Expand All @@ -576,13 +595,6 @@ export function coverageFromTranscripts(
continue;
}

// Not a diff reader, and not required to be. Two review agents legitimately
// never open the diff — Build & Test runs the build, Issue Fidelity reads the
// issue — and the session's transcript directory also holds agents this review
// did not launch, including ones its own agents spawned. None of them owes the
// diff anything; none of them may be credited with having read it either.
if (!given) continue;

// The prompt the CLI built for this chunk, against the prompt the harness
// recorded the agent being launched with. Nothing else in the run can see the
// difference: a paraphrase keeps the diff path, so every other check passes.
Expand Down Expand Up @@ -1327,7 +1339,12 @@ export function verificationGaps(
env: NodeJS.ProcessEnv = process.env,
): VerificationReport {
const { plan, mtimeMs } = readPlan(planPath);
const records = readTranscripts(mtimeMs, env, plan.diffPathAbsolute);
// Same provenance fence as coverageFromTranscripts: workflow dispatches
// share the transcript directory but are not agents this review launched,
// so they neither satisfy a delivery floor nor count against it.
const records = readTranscripts(mtimeMs, env, plan.diffPathAbsolute).filter(
(rec) => rec.agentKind !== 'workflow',
);
const built = readRecordedPrompts(planPath);
const gaps: VerificationReport['gaps'] = [];
const remediation: string[] = [];
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/commands/review/lib/transcripts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,28 @@ describe('readTranscripts — defensive parsing', () => {
expect(recs[0].launchPrompt).toBe('chunk 1 of 1');
});

it('reads the launch-path provenance stamped on the records', () => {
// Workflow agent() dispatches share this directory with Agent-tool
// launches; readers filter on the stamped field, so the parse must
// surface it.
const b = {
agentId: 'workflow-agent-deadbeef',
agentName: 'workflow-agent',
agentKind: 'workflow',
sessionId: 'S1',
};
file(
'agent-workflow-agent-deadbeef.jsonl',
JSON.stringify({
...b,
type: 'user',
message: { role: 'user', parts: [{ text: 'do the thing' }] },
}) + '\n',
);
const [rec] = readTranscripts(undefined, ENV);
expect(rec.agentKind).toBe('workflow');
});

it('counts only successful tool calls', () => {
const b = { agentId: 'a1', agentName: 'general-purpose', sessionId: 'S1' };
const call = {
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/commands/review/lib/transcripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ import { join } from 'node:path';
export interface AgentRecord {
agentId: string;
agentName: string;
/**
* Launch-path provenance stamped on the records, when present.
* `'workflow'` marks a workflow `agent()` dispatch — a transcript that
* shares this directory but is not an agent the review launched.
*/
agentKind?: string;
/** The prompt the agent was launched with — the transcript's first record. */
launchPrompt: string;
/** Tool calls that came back without an error. */
Expand Down Expand Up @@ -211,6 +217,7 @@ function parseTranscript(file: string, diffPath?: string): AgentRecord | null {

let agentId = '';
let agentName = '';
let agentKind = '';
let launchPrompt = '';
let finalText = '';
let successfulToolCalls = 0;
Expand Down Expand Up @@ -245,6 +252,9 @@ function parseTranscript(file: string, diffPath?: string): AgentRecord | null {
if (!agentName && typeof rec['agentName'] === 'string') {
agentName = rec['agentName'];
}
if (!agentKind && typeof rec['agentKind'] === 'string') {
agentKind = rec['agentKind'];
}

const type = rec['type'];

Expand Down Expand Up @@ -325,6 +335,7 @@ function parseTranscript(file: string, diffPath?: string): AgentRecord | null {
return {
agentId,
agentName,
...(agentKind ? { agentKind } : {}),
launchPrompt,
successfulToolCalls,
diffToolCalls,
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/agents/agent-transcript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,13 @@ export interface AttachJsonlOptions {
agentId: string;
/** Display name (subagent type), e.g. "explore". */
agentName?: string;
/**
* Launch path provenance, stamped on every record — see
* `ChatRecord.agentKind`. Workflow dispatches pass `'workflow'` so
* readers of the shared subagents directory can tell them apart from
* Agent-tool launches.
*/
agentKind?: 'workflow';
/** UI hint. */
agentColor?: string;
/** Parent user-session UUID — recorded as `sessionId` on every record. */
Expand Down Expand Up @@ -397,6 +404,7 @@ export function attachJsonlTranscriptWriter(
agentId: options.agentId,
agentName: options.agentName,
agentColor: options.agentColor,
agentKind: options.agentKind,
isSidechain: true,
});

Expand Down
Loading
Loading