From 7ce083954ff2d8b53bdabbdc5bf41502c70556a3 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 21 Aug 2026 01:38:40 +0800 Subject: [PATCH] =?UTF-8?q?fix(review):=20close=20round=207=20=E2=80=94=20?= =?UTF-8?q?two=20blockers=20on=20values=20this=20pipeline=20re-reads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `mdField` neutralised the code-span grammar and left the ledger marker's. The span makes Markdown and HTML inert to a RENDERER; this pipeline's own readers scan the raw body, and `stripLedgerMarker` takes the FIRST ``, deleting that round's prose AND its marker from the model-facing rendering. A complete forged pair additionally parses as the recovered ledger on any round the real marker is missing, which this account then trusts as its own. The delimiters are broken in the strip now. - The anonymous whole-write branch persisted the winner's volume verbatim under `foreign: false`. Without a `me` every marker walks as foreign, so the upstream strip never fires and `ownMax` is 0 — any marker inside the headroom wins. Kept, a stranger's counts became this loop's baseline: the trend evaluated against them, the paragraph cited them as own history, and they were stamped into this account's own next marker as `prevPosted`, which later recovery trusts. It takes the same "not recorded" degradation the counter-advance branch already accepts, through the same projection. Two mutations, each verified to turn a named test red. --- .../src/commands/review/lib/md-field.test.ts | 20 +++++++++++++++++++ .../cli/src/commands/review/lib/md-field.ts | 12 ++++++++++- .../review/pr-context-persist.test.ts | 17 +++++++++++++++- .../cli/src/commands/review/pr-context.ts | 16 ++++++++++++++- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/review/lib/md-field.test.ts b/packages/cli/src/commands/review/lib/md-field.test.ts index 293d1e043b..7aa68eac7d 100644 --- a/packages/cli/src/commands/review/lib/md-field.test.ts +++ b/packages/cli/src/commands/review/lib/md-field.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect } from 'vitest'; import { mdField } from './md-field.js'; +import { parseLedger, stripLedgerMarker } from './ledger.js'; describe('mdField — a PR-controlled value, made inert', () => { it('holds a value inside one code span', () => { @@ -34,6 +35,25 @@ describe('mdField — a PR-controlled value, made inert', () => { expect(paragraph).not.toContain('`` '); }); + it('breaks the ledger marker grammar, which the span does not neutralise', () => { + // The code span makes Markdown and HTML inert to a RENDERER. This + // pipeline's own readers scan the raw body: `stripLedgerMarker` takes + // the FIRST ` .sh'; + const out = mdField(hostile); + expect(out).not.toContain(''); + expect(out).toBe('`a qwen-review-ledger {"v":1,"round":9} .sh`'); + // The whole body still holds exactly one marker: the real one. + const body = `Some review prose.\n\n${out}\n\n`; + expect(body.split('`, deleting that round's prose AND + // its marker. A complete forged pair additionally parses as the + // recovered ledger on any round the real marker is missing. No + // legitimate value needs raw comment grammar inside a code span. + .replace(/[`\r\n]+|/g, ' ') .trim(); // A value that strips to nothing would emit a bare pair of backticks, which // is not a code span at all: two such runs in one paragraph pair up as diff --git a/packages/cli/src/commands/review/pr-context-persist.test.ts b/packages/cli/src/commands/review/pr-context-persist.test.ts index 9caa6cf9e5..07d752a41a 100644 --- a/packages/cli/src/commands/review/pr-context-persist.test.ts +++ b/packages/cli/src/commands/review/pr-context-persist.test.ts @@ -596,7 +596,14 @@ describe('persistRecoveredLedger', () => { persistRecoveredLedger( side, { - ledger: { ...ledger, round: 4 }, + ledger: { + ...ledger, + round: 4, + posted: 7, + prevPosted: 3, + fresh: 4, + floor: 'c', + }, commitId: null, reviewId: 40, // What recovery actually hands this branch anonymously. @@ -613,6 +620,14 @@ describe('persistRecoveredLedger', () => { // may well have posted. expect(written.foreign).toBe(false); expect(written.merged).toBe(false); + // ...but it cannot VOUCH for the volume either. Without a `me` every + // marker walks as foreign, so the upstream strip never fires and any + // marker inside the headroom wins — kept, a stranger's counts become + // this loop's baseline and are stamped into the next own marker. + expect(written.posted).toBeUndefined(); + expect(written.prevPosted).toBeUndefined(); + expect(written.fresh).toBeUndefined(); + expect(written.floor).toBeUndefined(); } finally { rmSync(dir, { recursive: true, force: true }); } diff --git a/packages/cli/src/commands/review/pr-context.ts b/packages/cli/src/commands/review/pr-context.ts index 11a0f6d207..a83c1acc74 100644 --- a/packages/cli/src/commands/review/pr-context.ts +++ b/packages/cli/src/commands/review/pr-context.ts @@ -1239,10 +1239,24 @@ export function persistRecoveredLedger( return; } mkdirSync(dirname(sideFilePath), { recursive: true }); + // An ANONYMOUS recovery cannot vouch for the volume it adopts. Without + // a `me` every marker walks as foreign, so the upstream strip + // (`if (me && best.foreign)`) never fires and `ownMax` is 0 — any + // marker inside the headroom wins. Kept, a stranger's counts become + // this loop's baseline: the trend evaluates against them, the + // paragraph cites them as own history, and they are stamped into this + // account's own next marker as `prevPosted`, which later recovery + // trusts. The counter-advance branch already sheds the group for this + // exact reason; this seam takes the same "not recorded" degradation. + const recoveredOut = identityKnown + ? recovered.ledger + : (withoutVolume( + recovered.ledger as unknown as Record, + ) as unknown as Ledger); writeAtomic( JSON.stringify( { - ...recovered.ledger, + ...recoveredOut, ...(recovered.commitId ? { commitId: recovered.commitId } : {}), reviewId: recovered.reviewId, // Provenance travels WITH the list it describes. Written even