feat(review): add comment-status helper for existing-thread triage

docs/users/features/code-review.md

@@ -273,7 +273,7 @@ Reports include: timestamp, diff stats, build/test results, all findings with ve
 
 The deterministic halves of the pipeline — argument parsing (`qwen review parse-args`) and the event/body decision (`qwen review compose-review`) — are tested subcommands rather than prompt text, so `--effort` grammar, `--comment` forcing, verdict caps, and downgrade behavior are pinned by unit tests and cannot drift with the model.
 
-**GitHub Enterprise:** reviewing a PR URL on a non-`github.com` host routes every GitHub call at that host — the review subcommands (`fetch-pr`, `pr-context`, `presubmit`) accept `--host` and set it in code, so a forgotten host cannot silently retarget the review at `github.com`.
+**GitHub Enterprise:** reviewing a PR URL on a non-`github.com` host routes every GitHub call at that host — the review subcommands (`fetch-pr`, `pr-context`, `comment-status`, `presubmit`) accept `--host` and set it in code, so a forgotten host cannot silently retarget the review at `github.com`.
 
 Every run ends with one machine-readable line (`Review complete: <target> — <disposition>`), so scripts and CI wrappers can detect completion and outcome with a single `^Review complete: ` match.
 

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

@@ -44,6 +44,7 @@ describe('reviewCommand', () => {
       'capture-local',
       'plan-diff',
       'pr-context',
+      'comment-status',
       'load-rules',
       'agent-prompt',
       'build-test',

packages/cli/src/commands/review.ts

@@ -15,6 +15,7 @@ import { fetchPrCommand } from './review/fetch-pr.js';
 import { captureLocalCommand } from './review/capture-local.js';
 import { planDiffCommand } from './review/plan-diff.js';
 import { prContextCommand } from './review/pr-context.js';
+import { commentStatusCommand } from './review/comment-status.js';
 import { loadRulesCommand } from './review/load-rules.js';
 import { presubmitCommand } from './review/presubmit.js';
 import { resolveAnchorsCommand } from './review/resolve-anchors.js';
@@ -36,6 +37,7 @@ export const reviewCommand: CommandModule = {
       .command(captureLocalCommand)
       .command(planDiffCommand)
       .command(prContextCommand)
+      .command(commentStatusCommand)
       .command(loadRulesCommand)
       .command(agentPromptCommand)
       .command(buildTestCommand)
@@ -48,7 +50,7 @@ export const reviewCommand: CommandModule = {
       .command(cleanupCommand)
       .demandCommand(
         1,
-        'Specify a subcommand: parse-args, fetch-pr, capture-local, plan-diff, pr-context, load-rules, agent-prompt, build-test, resolve-anchors, check-coverage, presubmit, test-efficacy, compose-review, submit, or cleanup.',
+        'Specify a subcommand: parse-args, fetch-pr, capture-local, plan-diff, pr-context, comment-status, load-rules, agent-prompt, build-test, resolve-anchors, check-coverage, presubmit, test-efficacy, compose-review, submit, or cleanup.',
       )
       .version(false),
   handler: () => {

packages/cli/src/commands/review/comment-status.handler.test.ts

@@ -0,0 +1,264 @@
+/**
+ * @license
+ * Copyright 2026 Qwen Team
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// Command-level tests: the pure core (comment-status.test.ts) and the real
+// git probe (comment-status.integration.test.ts) are covered elsewhere; this
+// file pins the handler wiring — the two head samples, the drift field and
+// its warning, and that the report is written — with gh/git/fs mocked. A
+// regression that reversed the drift comparison or dropped the warning would
+// otherwise leave every other test green while live anchors were silently
+// paired with stale worktree facts.
+
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mocks = vi.hoisted(() => ({
+  gh: vi.fn(),
+  ghApiAll: vi.fn((_p: string): unknown[] => []),
+  ensureAuthenticated: vi.fn(),
+  setGhHost: vi.fn(),
+  gitOpt: vi.fn((..._a: string[]): string | null => null),
+  writeFileSync: vi.fn(),
+  mkdirSync: vi.fn(),
+  writeStdoutLine: vi.fn(),
+}));
+
+vi.mock('./lib/gh.js', () => ({
+  gh: mocks.gh,
+  ghApiAll: mocks.ghApiAll,
+  ensureAuthenticated: mocks.ensureAuthenticated,
+  setGhHost: mocks.setGhHost,
+}));
+
+vi.mock('./lib/git.js', () => ({
+  gitOpt: mocks.gitOpt,
+}));
+
+vi.mock('./lib/paths.js', () => ({
+  worktreePath: (n: string | number) => `/repo/.qwen/tmp/review-pr-${n}`,
+}));
+
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs')>();
+  return {
+    ...actual,
+    default: {
+      ...actual,
+      writeFileSync: mocks.writeFileSync,
+      mkdirSync: mocks.mkdirSync,
+    },
+    writeFileSync: mocks.writeFileSync,
+    mkdirSync: mocks.mkdirSync,
+  };
+});
+
+vi.mock('../../utils/stdioHelpers.js', () => ({
+  writeStdoutLine: mocks.writeStdoutLine,
+}));
+
+const { commentStatusCommand } = await import('./comment-status.js');
+
+async function run(ownerRepo = 'o/r') {
+  const handler = commentStatusCommand.handler;
+  if (!handler) throw new Error('handler missing');
+  await handler({
+    _: [],
+    $0: 'qwen',
+    pr_number: '7632',
+    owner_repo: ownerRepo,
+    out: '/repo/.qwen/tmp/qwen-review-pr-7632-comment-status.json',
+  } as unknown as Parameters<typeof handler>[0]);
+}
+
+function reportWritten() {
+  const call = mocks.writeFileSync.mock.calls.find(([p]) =>
+    String(p).endsWith('comment-status.json'),
+  );
+  if (!call) throw new Error('report not written');
+  return JSON.parse(String(call[1]));
+}
+
+function warnings() {
+  return mocks.writeStdoutLine.mock.calls
+    .map((c) => String(c[0]))
+    .filter((l) => l.startsWith('warning:'));
+}
+
+// gh('pr','view',…) is called for author+head, then again for the second
+// head sample. This drives both from one queue.
+function queueHeads(before: string, after: string, author = 'octocat') {
+  let n = 0;
+  mocks.gh.mockImplementation((..._args: string[]) => {
+    n += 1;
+    return n === 1
+      ? JSON.stringify({ author: { login: author }, headRefOid: before })
+      : JSON.stringify({ headRefOid: after });
+  });
+}
+
+describe('comment-status handler', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.ghApiAll.mockReturnValue([]);
+    // Worktree HEAD matches the (stable) live head by default: no drift.
+    mocks.gitOpt.mockImplementation((...args: string[]) =>
+      args.includes('rev-parse') ? 'headA' : null,
+    );
+  });
+
+  it('reports no drift and writes the report when heads all agree', async () => {
+    queueHeads('headA', 'headA');
+    await run();
+    const report = reportWritten();
+    expect(report.headDrift).toBe(false);
+    expect(report.headMovedDuringFetch).toBe(false);
+    expect(report.liveHeadSha).toBe('headA');
+    expect(warnings()).toEqual([]);
+  });
+
+  it('flags drift, warns, and denormalizes staleWorktree onto each thread when the worktree lags', async () => {
+    // A thread must be present so the denormalization loop is actually
+    // exercised: with zero threads it iterates over nothing and would pass
+    // even if the block were removed.
+    mocks.ghApiAll.mockReturnValue([
+      {
+        id: 1,
+        user: { login: 'r' },
+        path: 'a.ts',
+        line: 1,
+        original_commit_id: 's',
+      },
+    ]);
+    queueHeads('headB', 'headB'); // live head B, worktree still at headA
+    await run();
+    const report = reportWritten();
+    expect(report.headDrift).toBe(true);
+    expect(report.headMovedDuringFetch).toBe(false);
+    expect(report.threads[0].code.staleWorktree).toBe(true);
+    expect(warnings().join('\n')).toContain('worktree HEAD');
+  });
+
+  it('threads --host to setGhHost so a GHE review targets the right host', async () => {
+    queueHeads('headA', 'headA');
+    const handler = commentStatusCommand.handler;
+    if (!handler) throw new Error('handler missing');
+    await handler({
+      _: [],
+      $0: 'qwen',
+      pr_number: '7632',
+      owner_repo: 'o/r',
+      out: '/repo/.qwen/tmp/qwen-review-pr-7632-comment-status.json',
+      host: 'github.example.com',
+    } as unknown as Parameters<typeof handler>[0]);
+    expect(mocks.setGhHost).toHaveBeenCalledWith('github.example.com');
+  });
+
+  it('flags a push that raced the comments fetch, recording both samples', async () => {
+    // Worktree happens to match the post-fetch head, but the head MOVED
+    // during the fetch — anchor facts may be mixed across commits.
+    mocks.gitOpt.mockImplementation((...args: string[]) =>
+      args.includes('rev-parse') ? 'headB' : null,
+    );
+    queueHeads('headA', 'headB');
+    await run();
+    const report = reportWritten();
+    expect(report.headMovedDuringFetch).toBe(true);
+    expect(report.headDrift).toBe(true);
+    expect(report.liveHeadBefore).toBe('headA');
+    expect(report.liveHeadSha).toBe('headB');
+    expect(warnings().join('\n')).toContain('moved during the comments fetch');
+  });
+
+  it('does not mark threads staleWorktree when the head raced but the worktree matches the final head', async () => {
+    // Worktree is at headB (the final live head); the head merely moved
+    // mid-fetch. The checkout is NOT superseded, so staleWorktree must stay
+    // false even though headDrift/headMovedDuringFetch are true.
+    mocks.gitOpt.mockImplementation((...args: string[]) =>
+      args.includes('rev-parse') ? 'headB' : null,
+    );
+    // One thread so we can inspect its code facts.
+    mocks.ghApiAll.mockReturnValue([
+      {
+        id: 1,
+        user: { login: 'r' },
+        path: 'a.ts',
+        line: 1,
+        original_commit_id: 's',
+      },
+    ]);
+    queueHeads('headA', 'headB');
+    await run();
+    const report = reportWritten();
+    expect(report.headMovedDuringFetch).toBe(true);
+    expect(report.threads[0].code.staleWorktree).toBe(false);
+  });
+
+  it('warns and flags worktreeMissing when the worktree is absent', async () => {
+    // gitOpt returns null for everything (no worktree): every thread's code
+    // facts are unknown, which must not pass silently.
+    mocks.gitOpt.mockReturnValue(null);
+    mocks.ghApiAll.mockReturnValue([
+      {
+        id: 1,
+        user: { login: 'r' },
+        path: 'a.ts',
+        line: 1,
+        original_commit_id: 's',
+      },
+    ]);
+    queueHeads('headA', 'headA');
+    await run();
+    const report = reportWritten();
+    expect(report.worktreeMissing).toBe(true);
+    expect(report.threads[0].code.changedSinceComment).toBe('unknown');
+    expect(warnings().join('\n')).toContain('no worktree at');
+  });
+
+  it('rejects an owner_repo with no slash (a caller error, not runtime degradation)', async () => {
+    await expect(run('ownerrepo')).rejects.toThrow(/owner\/repo/);
+  });
+
+  it('warns when the report exceeds the read_file truncation threshold', async () => {
+    // A big PR pushes the JSON past ~25k chars; the size warning is the only
+    // signal a consumer gets before a silent isTruncated on cut, unparseable
+    // JSON. Generate enough threads to cross the threshold.
+    mocks.ghApiAll.mockReturnValue(
+      Array.from({ length: 200 }, (_, i) => ({
+        id: i + 1,
+        user: { login: `reviewer-with-a-longish-name-${i}` },
+        path: `packages/cli/src/commands/review/some/deep/path/file-${i}.ts`,
+        line: i + 1,
+        original_line: i + 1,
+        original_commit_id: `commit-sha-placeholder-${i}`,
+        subject_type: 'line',
+        body: `A finding body number ${i} with enough text to add weight.`,
+      })),
+    );
+    queueHeads('headA', 'headA');
+    await run();
+    expect(warnings().join('\n')).toMatch(/chars; read_file returns the first/);
+  });
+
+  it('degrades gracefully when gh auth fails: no throw, empty report, warning', async () => {
+    // SKILL.md promises this command cannot kill the pipeline on an auth or
+    // network failure. A throw must become a minimal empty report + warning,
+    // not an unhandled rejection.
+    mocks.ensureAuthenticated.mockImplementation(() => {
+      throw new Error('gh CLI is not authenticated. Run `gh auth login`.');
+    });
+
+    await expect(run()).resolves.toBeUndefined();
+    const report = reportWritten();
+    expect(report.threads).toEqual([]);
+    expect(report.error).toContain('not authenticated');
+    expect(warnings().join('\n')).toContain('comment-status failed');
+    // The degraded report carries the SAME shape as success so a consumer
+    // reading headDrift/summary gets a neutral value, not a misleading
+    // `undefined` (which reads as "no drift").
+    expect(report.headDrift).toBe(false);
+    expect(report.summary).toMatchObject({ threads: 0, blockers: 0 });
+    expect(report.inlineComments).toBe(0);
+  });
+});