Skip to content

Commit 23a2d98

Browse files
committed
fix(agent): every run of a command is reviewed — an earlier pass is never reused
The reviewer's pass was reused for the same command line in the same folder while the agent's edit count had not moved. That count sees only the agent's file tools: `sh check.sh`, then `git switch other-branch`, then `sh check.sh` reused the first pass although check.sh may have changed — as it may through any command, or through the person's own terminal. Nothing cheap can establish that the workspace is unchanged, so the reuse is gone: each run is one review. The design note says so and what it costs.
1 parent 5f3f368 commit 23a2d98

3 files changed

Lines changed: 16 additions & 27 deletions

File tree

‎docs/ASK-WHEN-RISKY.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,12 @@ Command line:
9999
read back from `sessions.dir`, which a folder setting can point anywhere. The earlier goals still
100100
reach the reviewer, as activity. This errs toward asking.
101101

102-
### Reviewed once, until something changes
102+
### Every run is reviewed
103103

104-
The same command line in the same folder, with no file edited since it last passed, passes again
105-
without a second call (`npm test` five times in a row is one review). An edit in between makes it a
106-
new question, because the command may now run code the reviewer has not seen.
104+
A command that passed a moment ago is reviewed again. What `sh check.sh` does depends on `check.sh`, which an
105+
earlier command, a `git switch` or the person in their own terminal may have changed, and nothing the
106+
extension can cheaply observe says it has not. Reusing a pass would vouch for code the reviewer never saw.
107+
The price is one review call per run, `npm test` included.
107108

108109
## Cost and wait
109110

‎extensions/levelcode-ai/agent.js‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -862,23 +862,16 @@ function isAgentAuthError(e) {
862862
/**
863863
* Ask When Risky: show one command to the reviewer (agentReview.js) and report what it decided.
864864
*
865-
* The same command in the same folder, with no file edited since it last passed, passes again without
866-
* a second review: `npm test` five times in a row is one review. An edit in between makes it a new
867-
* question, because the command may now run code the reviewer has not seen.
865+
* Every run is reviewed, including a command that passed a moment ago. What a command does depends on the
866+
* files it runs, and those change in ways no counter here can see: an earlier command, a `git switch`, the
867+
* person in their own terminal. Reusing an earlier pass would vouch for code the reviewer has not seen.
868868
*
869869
* The review is a real model call on the run's own provider, so in gateway mode it costs credits. Its
870870
* cost is kept on ctx and added to the run's total, so the response bar's figure includes it.
871871
* @returns {Promise<{outcome:'allow'|'ask'|'stopped', verdict:any, reason:string, failure:string|null}>}
872872
*/
873873
async function reviewGate(ctx, action) {
874874
const stats = ctx.reviewStats || (ctx.reviewStats = { ran: 0, asked: 0 });
875-
const passed = ctx.reviewPassed || (ctx.reviewPassed = new Set());
876-
const key = action.cwd + '\n' + (ctx.editCount || 0) + '\n' + action.command;
877-
if (passed.has(key)) {
878-
stats.ran++;
879-
ctx.post({ type: 'agentTool', icon: 'workspace-trusted', text: 'Reviewer · passed this exact command earlier, no edits since' });
880-
return { outcome: 'allow', verdict: null, reason: '', failure: null };
881-
}
882875
const model = ctx.reviewModel || ctx.model;
883876
const reasons = !!catalog.modelCaps(model).reasoning;
884877
ctx.post({ type: 'agentStatus', text: 'reviewing the command…' });
@@ -903,7 +896,6 @@ async function reviewGate(ctx, action) {
903896
if (ctx.dbg) { ctx.dbg('review.done', { outcome: r.outcome, failure: r.failure, risk: r.verdict && r.verdict.risk, authorization: r.verdict && r.verdict.authorization, injection: r.verdict && r.verdict.injection, model, costMicros: r.usage && r.usage.cost_micros }); }
904897
if (r.outcome === 'allow') {
905898
stats.ran++;
906-
passed.add(key);
907899
ctx.post({ type: 'agentStatus', text: 'running command…' });
908900
ctx.post({ type: 'agentTool', icon: 'workspace-trusted', text: 'Reviewer · ' + r.verdict.risk + ' risk — ' + host.displaySafe(r.reason || 'no reason given') });
909901
} else if (r.outcome === 'ask') {

‎extensions/levelcode-ai/test/agentReviewGate.test.js‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* 1. Ask When Risky is never looser than Autopilot: a danger-set command asks the person, and the
1212
* reviewer is not consulted at all.
1313
* 2. A command the reviewer does not pass, or cannot judge, asks the person, with the reason on the card.
14-
* 3. A command the reviewer passes runs without a card, and the run says so.
14+
* 3. A command the reviewer passes runs without a card, and the run says so — and every run is reviewed.
1515
* 4. Default Approvals and Autopilot behave exactly as before: no reviewer call in either.
1616
*--------------------------------------------------------------------------------------------*/
1717
// @ts-check
@@ -211,18 +211,14 @@ const reviewerRows = (posted) => posted.filter((m) => m.type === 'agentTool' &&
211211
// two agent turns at 1000 each, one review at 250
212212
assert.strictEqual(r.done.costMicros, 2250);
213213
});
214-
await testAsync('REVIEW: the same command twice with no edit between is reviewed once', async () => {
214+
await testAsync('REVIEW: the same command twice is reviewed twice — an earlier pass is never reused', async () => {
215+
// What `sh check.sh` does depends on check.sh, which an earlier command, a git switch or the person
216+
// may have changed with no edit the agent made: nothing here can tell, so nothing is reused.
215217
reviewer = verdict('low', 'implied', 'ok');
216-
const r = await run('review', [cmd('npm test'), cmd('npm test')]);
217-
assert.strictEqual(r.reviews, 1);
218-
assert.strictEqual(r.ran.length, 2);
219-
assert.match(reviewerRows(r.posted).join('\n'), /passed this exact command earlier, no edits since/);
220-
});
221-
await testAsync('REVIEW: an edit between them makes it a new question', async () => {
222-
reviewer = verdict('low', 'implied', 'ok');
223-
const r = await run('review', [cmd('npm test'), { name: 'write_file', input: { path: 'a.js', content: 'x' } }, cmd('npm test')],
224-
{ ctx: { applyEdit: async () => true, touched: new Set() } });
225-
assert.strictEqual(r.reviews, 2);
218+
const r = await run('review', [cmd('sh check.sh'), cmd('git switch other-branch'), cmd('sh check.sh')]);
219+
assert.strictEqual(r.reviews, 3);
220+
assert.strictEqual(r.ran.length, 3);
221+
assert.ok(!reviewerRows(r.posted).some((t) => /earlier/.test(t)), 'a pass was reused');
226222
});
227223
await testAsync('REVIEW: the person\'s ask_user choices reach the reviewer as theirs', async () => {
228224
reviewer = verdict('medium', 'explicit', 'You chose to install it.');

0 commit comments

Comments
 (0)