feat(review): enforce the submit-only write contract with a cleanup tripwire

packages/cli/src/commands/review/cleanup.test.ts

@@ -7,6 +7,9 @@ const mocks = vi.hoisted(() => ({
   execFileSync: vi.fn(),
   existsSync: vi.fn(() => false),
   readdirSync: vi.fn(() => []),
+  readFileSync: vi.fn((): string => {
+    throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+  }),
   rmSync: vi.fn(),
   writeStdoutLine: vi.fn(),
   writeStderrLine: vi.fn(),
@@ -17,6 +20,9 @@ const mocks = vi.hoisted(() => ({
     freed: false,
     reason: undefined,
   })),
+  ghApiAll: vi.fn((): unknown[] => []),
+  currentUser: vi.fn(() => 'reviewer'),
+  setGhHost: vi.fn(),
 }));
 
 vi.mock('node:child_process', async (importOriginal) => {
@@ -36,10 +42,12 @@ vi.mock('node:fs', async (importOriginal) => {
       ...actual,
       existsSync: mocks.existsSync,
       readdirSync: mocks.readdirSync,
+      readFileSync: mocks.readFileSync,
       rmSync: mocks.rmSync,
     },
     existsSync: mocks.existsSync,
     readdirSync: mocks.readdirSync,
+    readFileSync: mocks.readFileSync,
     rmSync: mocks.rmSync,
   };
 });
@@ -58,6 +66,12 @@ vi.mock('./lib/git.js', () => ({
   releaseWorktree: mocks.releaseWorktree,
 }));
 
+vi.mock('./lib/gh.js', () => ({
+  ghApiAll: mocks.ghApiAll,
+  currentUser: mocks.currentUser,
+  setGhHost: mocks.setGhHost,
+}));
+
 vi.mock('./lib/paths.js', () => ({
   worktreePath: (prNumber: string) => `/repo/.qwen/tmp/review-pr-${prNumber}`,
   probeWorktreePath: (path: string) => `${path}-probe`,
@@ -66,7 +80,11 @@ vi.mock('./lib/paths.js', () => ({
   tmpPrefix: (target: string) => `qwen-review-${target}-`,
 }));
 
-import { runCleanup } from './cleanup.js';
+import {
+  findUnsanctionedIssueComments,
+  runCleanup,
+  type RawIssueComment,
+} from './cleanup.js';
 
 describe('runCleanup', () => {
   beforeEach(() => {
@@ -109,3 +127,132 @@ describe('runCleanup', () => {
     );
   });
 });
+
+describe('findUnsanctionedIssueComments', () => {
+  const since = '2026-07-24T08:00:00Z';
+  const comment = (over: Partial<RawIssueComment> & { id: number }) =>
+    ({
+      user: { login: 'reviewer' },
+      created_at: '2026-07-24T09:00:00Z',
+      ...over,
+    }) as RawIssueComment;
+
+  it('keeps only the reviewing account inside the window, case-insensitively', () => {
+    const got = findUnsanctionedIssueComments(
+      [
+        comment({ id: 1 }),
+        comment({ id: 2, user: { login: 'Reviewer' } }),
+        comment({ id: 3, user: { login: 'someone-else' } }),
+        comment({ id: 4, created_at: '2026-07-24T07:59:59Z' }),
+      ],
+      'reviewer',
+      since,
+    );
+    expect(got.map((c) => c.id)).toEqual([1, 2]);
+  });
+
+  it('drops comments with no author or no timestamp instead of guessing', () => {
+    const got = findUnsanctionedIssueComments(
+      [
+        comment({ id: 1, user: null }),
+        comment({ id: 2, created_at: undefined }),
+      ],
+      'reviewer',
+      since,
+    );
+    expect(got).toEqual([]);
+  });
+});
+
+describe('runCleanup — bypass-write audit', () => {
+  const fetchReport = JSON.stringify({
+    prNumber: '123',
+    ownerRepo: 'acme/widgets',
+    fetchedAt: '2026-07-24T08:00:00Z',
+    host: 'ghe.example.com',
+  });
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.existsSync.mockReturnValue(false);
+    mocks.execFileSync.mockReturnValue(Buffer.from(''));
+    mocks.readFileSync.mockImplementation(() => {
+      throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+    });
+    mocks.currentUser.mockReturnValue('reviewer');
+    mocks.ghApiAll.mockReturnValue([]);
+  });
+
+  it('flags reviewer issue comments posted inside the window', () => {
+    mocks.readFileSync.mockReturnValue(fetchReport);
+    mocks.ghApiAll.mockReturnValue([
+      {
+        id: 42,
+        user: { login: 'reviewer' },
+        created_at: '2026-07-24T09:02:32Z',
+        html_url: 'https://ghe.example.com/acme/widgets/pull/123#c42',
+      },
+      {
+        id: 43,
+        user: { login: 'pr-author' },
+        created_at: '2026-07-24T09:03:00Z',
+      },
+    ]);
+
+    runCleanup('pr-123');
+
+    expect(mocks.readFileSync).toHaveBeenCalledWith(
+      '/repo/.qwen/tmp/qwen-review-pr-123-fetch.json',
+      'utf8',
+    );
+    expect(mocks.setGhHost).toHaveBeenCalledWith('ghe.example.com');
+    expect(mocks.ghApiAll).toHaveBeenCalledWith(
+      expect.stringContaining('repos/acme/widgets/issues/123/comments'),
+    );
+    const warnings = mocks.writeStdoutLine.mock.calls
+      .map((c) => String(c[0]))
+      .filter((l) => l.startsWith('warning:'));
+    expect(warnings.join('\n')).toContain('comment 42');
+    expect(warnings.join('\n')).not.toContain('comment 43');
+    expect(warnings.join('\n')).toContain('qwen review submit');
+  });
+
+  it('stays silent when the window is clean', () => {
+    mocks.readFileSync.mockReturnValue(fetchReport);
+    mocks.ghApiAll.mockReturnValue([
+      {
+        id: 7,
+        user: { login: 'pr-author' },
+        created_at: '2026-07-24T09:00:00Z',
+      },
+    ]);
+
+    runCleanup('pr-123');
+
+    const warnings = mocks.writeStdoutLine.mock.calls
+      .map((c) => String(c[0]))
+      .filter((l) => l.startsWith('warning:'));
+    expect(warnings).toEqual([]);
+  });
+
+  it('skips the audit without gh calls when the fetch report is absent or pre-fetchedAt', () => {
+    runCleanup('pr-123'); // report missing (readFileSync throws)
+    mocks.readFileSync.mockReturnValue(
+      JSON.stringify({ prNumber: '123', ownerRepo: 'acme/widgets' }),
+    );
+    runCleanup('pr-123'); // old report without fetchedAt
+
+    expect(mocks.ghApiAll).not.toHaveBeenCalled();
+    expect(mocks.setGhHost).not.toHaveBeenCalled();
+  });
+
+  it('never fails the cleanup when the audit itself fails', () => {
+    mocks.readFileSync.mockReturnValue(fetchReport);
+    mocks.ghApiAll.mockImplementation(() => {
+      throw new Error('gh: not authenticated');
+    });
+
+    expect(() => runCleanup('pr-123')).not.toThrow();
+    expect(mocks.clearReviewWorktreeLease).toHaveBeenCalled();
+  });
+});

packages/cli/src/commands/review/cleanup.ts

@@ -5,6 +5,7 @@
  */
 
 // Post-review cleanup for /review Step 9.
+//   - Audit the PR for writes that bypassed `qwen review submit` (PR targets).
 //   - Remove the temporary worktree at .qwen/tmp/review-pr-<n>.
 //   - Delete the local branch ref qwen-review/pr-<n>.
 //   - Remove any .qwen/tmp/qwen-review-<target>-* side files.
@@ -13,10 +14,11 @@
 
 import type { CommandModule } from 'yargs';
 import { execFileSync } from 'node:child_process';
-import { existsSync, readdirSync, rmSync } from 'node:fs';
+import { existsSync, readFileSync, readdirSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js';
 import { clearReviewWorktreeLease } from '../../services/review-worktree-lease.js';
+import { currentUser, ghApiAll, setGhHost } from './lib/gh.js';
 import { refExists, releaseWorktree } from './lib/git.js';
 import {
   worktreePath,
@@ -30,6 +32,121 @@ interface CleanupArgs {
   target: string;
 }
 
+/** An issue comment, as listed by `GET /issues/{n}/comments`. */
+export interface RawIssueComment {
+  id: number;
+  user?: { login: string } | null;
+  created_at?: string;
+  html_url?: string;
+}
+
+/**
+ * Issue comments the current user posted inside the review window.
+ *
+ * `qwen review submit` is the ONLY sanctioned write in `/review`, and it
+ * posts a *review* — never an issue comment. So any issue comment by the
+ * reviewing account created after `sinceIso` is either a write that bypassed
+ * the submit gate, or something the user posted by hand from another
+ * terminal; the warning below names both readings and lets the human decide.
+ * Zero overlap with sanctioned output means zero correlation bookkeeping.
+ *
+ * This is a tripwire, not a wall. The gate itself lives in `submit` (it
+ * refuses unauthorised posts), but a model that stops *calling* submit walks
+ * around it — dogfooded: after four context compressions a run hand-posted
+ * its summary with `gh pr comment`, printed no completion line, and nothing
+ * anywhere noticed. Prose bans are exactly what compression loses, so the
+ * detection has to live in the deterministic layer that always runs.
+ */
+export function findUnsanctionedIssueComments(
+  comments: RawIssueComment[],
+  reviewer: string,
+  sinceIso: string,
+): RawIssueComment[] {
+  const reviewerLc = reviewer.toLowerCase();
+  return comments.filter(
+    (c) =>
+      (c.user?.login ?? '').toLowerCase() === reviewerLc &&
+      typeof c.created_at === 'string' &&
+      c.created_at >= sinceIso,
+  );
+}
+
+/**
+ * Fields the audit needs from the fetch report. The report is the carrier
+ * (not the worktree lease) because it is written on every PR run — the lease
+ * only exists when the session env vars are set.
+ */
+interface AuditWindow {
+  prNumber: string;
+  ownerRepo: string;
+  fetchedAt: string;
+  host: string | null;
+}
+
+function readAuditWindow(target: string): AuditWindow | null {
+  try {
+    const raw = readFileSync(
+      join(REVIEW_TMP_DIR, `${tmpPrefix(target)}fetch.json`),
+      'utf8',
+    );
+    const report = JSON.parse(raw) as Partial<AuditWindow>;
+    if (
+      typeof report.prNumber !== 'string' ||
+      typeof report.ownerRepo !== 'string' ||
+      typeof report.fetchedAt !== 'string'
+    ) {
+      return null; // a report from before fetchedAt existed — nothing to audit
+    }
+    return {
+      prNumber: report.prNumber,
+      ownerRepo: report.ownerRepo,
+      fetchedAt: report.fetchedAt,
+      host: typeof report.host === 'string' ? report.host : null,
+    };
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Best-effort by design: cleanup must stay idempotent and offline-safe, so
+ * any failure here (no gh, no auth, no network, report missing) skips the
+ * audit silently rather than failing the cleanup or nagging every offline
+ * run. A skipped audit is the pre-existing state of the world, not an error.
+ */
+function auditPrWrites(target: string): void {
+  try {
+    const window = readAuditWindow(target);
+    if (!window) return;
+    setGhHost(window.host ?? undefined);
+    const comments = ghApiAll(
+      `repos/${window.ownerRepo}/issues/${window.prNumber}/comments?since=${encodeURIComponent(window.fetchedAt)}&per_page=100`,
+    ) as RawIssueComment[];
+    const suspects = findUnsanctionedIssueComments(
+      comments,
+      currentUser(),
+      window.fetchedAt,
+    );
+    if (suspects.length === 0) return;
+    writeStdoutLine(
+      `warning: ${suspects.length} issue comment(s) by the reviewing account were posted to ` +
+        `${window.ownerRepo}#${window.prNumber} during this review window. ` +
+        `The only sanctioned write in /review is \`qwen review submit\`, and it never posts issue comments:`,
+    );
+    for (const c of suspects) {
+      writeStdoutLine(
+        `warning:   comment ${c.id} at ${c.created_at}${c.html_url ? ` — ${c.html_url}` : ''}`,
+      );
+    }
+    writeStdoutLine(
+      `warning: if the user posted these themselves, ignore this; otherwise a write bypassed the ` +
+        `submit gate — relay this warning verbatim in the terminal summary.`,
+    );
+  } catch {
+    /* audit is best-effort — see the doc comment above */
+  }
+}
+
 export function runCleanup(target: string): void {
   let removedAny = false;
   // Tracked separately from `removedAny`, because a failure is neither. Without
@@ -44,6 +161,10 @@ export function runCleanup(target: string): void {
   if (prMatch) {
     const prNumber = prMatch[1];
 
+    // Before the sweep below deletes the fetch report (the audit window's
+    // carrier), check the PR for writes that bypassed `qwen review submit`.
+    auditPrWrites(target);
+
     // Report what actually happened, in both directions. Announcing "Removed …"
     // off a path that is still on disk is a lie; saying nothing at all when we
     // could not remove it leaves a leftover that will wedge the next run's

packages/cli/src/commands/review/fetch-pr.ts

@@ -70,6 +70,7 @@ interface FetchPrArgs {
   owner_repo: string;
   remote: string;
   out: string;
+  host?: string;
   /** yargs camelCases `--max-chunk-lines`; the snake_case form does not exist. */
   maxChunkLines: number;
 }
@@ -80,6 +81,15 @@ type FetchPrResult = PlanReport & {
   remote: string;
   ref: string;
   fetchedSha: string;
+  /**
+   * When this review window opened (ISO-8601). `cleanup` audits the PR for
+   * writes by the current user inside [fetchedAt, cleanup) that did not go
+   * through `qwen review submit` — the submit-only contract's tripwire.
+   */
+  fetchedAt: string;
+  /** GitHub host this PR lives on (Enterprise), null for github.com — so the
+   * cleanup audit queries the same host the review did. */
+  host: string | null;
   worktreePath: string;
   baseRefName: string;
   headRefName: string;
@@ -283,6 +293,8 @@ async function runFetchPr(args: FetchPrArgs): Promise<void> {
     remote,
     ref,
     fetchedSha,
+    fetchedAt: new Date().toISOString(),
+    host: args.host ?? null,
     worktreePath: wt,
     baseRefName: meta.baseRefName,
     headRefName: meta.headRefName,