fix(review): recover bilingual register from the live PR when the plan omits the Han flag

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

@@ -16,13 +16,15 @@ import {
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { promptRecordDir, briefPath } from './lib/prompt-record.js';
+import { getGhHost, setGhHost } from './lib/gh.js';
 import {
   composeReview,
   composeReviewCommand,
   describeChunkGap,
   verdictLine,
   type ComposeReviewInput,
   type ComposeReviewResult,
+  type PrBodyFetcher,
 } from './compose-review.js';
 
 vi.mock('../../utils/stdioHelpers.js', () => ({
@@ -817,6 +819,29 @@ describe('composeReviewCommand handler (the CLI glue)', () => {
     expect(written.body.endsWith(FOOTER)).toBe(true);
   });
 
+  it('routes its gh calls via the PR host — --host reaches setGhHost', () => {
+    // The bilingual body-language recovery calls `gh pr view`; on GitHub Enterprise
+    // that call must hit the PR's host, or the composed body's language disagrees
+    // with what `submit` (which routes by host) posts. Drop the `setGhHost(host)`
+    // and this reddens.
+    const dir = mkdtempSync(join(tmpdir(), 'compose-host-'));
+    const inputPath = join(dir, 'compose.json');
+    const commentsPath = join(dir, 'comments.json');
+    writeFileSync(inputPath, JSON.stringify({ modelId: MODEL }), 'utf8');
+    writeFileSync(commentsPath, '[]', 'utf8');
+    setGhHost(undefined);
+    try {
+      (composeReviewCommand.handler as (argv: unknown) => void)({
+        input: inputPath,
+        comments: commentsPath,
+        host: 'github.example.com',
+      });
+      expect(getGhHost()).toBe('github.example.com');
+    } finally {
+      setGhHost(undefined);
+    }
+  });
+
   it('a drafted inline Critical reaches the verdict line — the report-only hole', () => {
     // The dogfooded failure this boundary exists for: a report-only run (no
     // submit, so nothing downstream recounts) moved its one Critical from
@@ -2120,3 +2145,125 @@ describe('bilingual body — the PR author writes Chinese (prDescriptionHasHan)'
     ).toHaveLength(2);
   });
 });
+
+/**
+ * The plan flag is the deterministic path; this is the recovery for when it is
+ * missing. `fetch-pr` always writes `prDescriptionHasHan`, but a `plan-diff`
+ * plan never does, and an orchestrator that improvises the pipeline can hand
+ * `compose-review` a plan that is not `fetch-pr`'s report — which is how a
+ * Chinese-authored PR (#7686) shipped an English-only review while the four
+ * bot reviews before it, off a proper plan, were bilingual. When the flag is
+ * absent but the plan still names the PR, the register is recovered from the
+ * live description, which the caller cannot forge.
+ */
+describe('bilingual body — recovered from the live PR when the plan omits the flag', () => {
+  /** A covered plan with a PR identity but no `prDescriptionHasHan`, its mtime
+   *  kept old so its transcripts still read as newer than it. */
+  function namedPlanWithoutFlag(): string {
+    const p = coveredPlan();
+    const parsed = JSON.parse(readFileSync(p, 'utf8'));
+    delete parsed.prDescriptionHasHan;
+    parsed.ownerRepo = 'QwenLM/qwen-code';
+    parsed.prNumber = '7686';
+    writeFileSync(p, JSON.stringify(parsed));
+    const old = new Date(2020, 0, 1);
+    utimesSync(p, old, old);
+    return p;
+  }
+
+  /** A fetcher that records its calls, so a test can prove it was NOT reached. */
+  function recordingFetcher(body: string): PrBodyFetcher & { calls: number } {
+    const fn = ((_ownerRepo: string, _prNumber: string) => {
+      fn.calls++;
+      return body;
+    }) as PrBodyFetcher & { calls: number };
+    fn.calls = 0;
+    return fn;
+  }
+
+  it('folds in Chinese when the recovered description contains Han', () => {
+    const fetch = recordingFetcher('这个 PR 懒加载首次使用的依赖。');
+    const r = composeReview({
+      suggestionsInline: 1,
+      planPath: namedPlanWithoutFlag(),
+      prBodyFetcher: fetch,
+      env: ENV,
+      modelId: MODEL,
+    });
+    expect(fetch.calls).toBe(1);
+    // Both halves: the English rides above the fold, the Chinese inside it.
+    expect(r.body).toContain('<details>\n<summary>中文说明</summary>');
+    expect(r.body).toContain('Suggestions are inline.');
+    expect(r.body).toContain('建议见行内评论。');
+  });
+
+  it('stays English when the recovered description has no Han', () => {
+    const fetch = recordingFetcher(
+      'This PR lazy-loads first-use dependencies.',
+    );
+    const r = composeReview({
+      suggestionsInline: 1,
+      planPath: namedPlanWithoutFlag(),
+      prBodyFetcher: fetch,
+      env: ENV,
+      modelId: MODEL,
+    });
+    expect(fetch.calls).toBe(1);
+    expect(r.body).not.toContain('<details>');
+    expect(r.body).not.toContain('中文');
+  });
+
+  it('honours a recorded false without fetching — the English author is settled', () => {
+    // A real fetch-pr report that fetched the body and found no Han. Re-reading
+    // the live PR on every English review would be waste, and the recorded
+    // snapshot is the answer.
+    const p = coveredPlan();
+    const parsed = JSON.parse(readFileSync(p, 'utf8'));
+    parsed.prDescriptionHasHan = false;
+    parsed.ownerRepo = 'QwenLM/qwen-code';
+    parsed.prNumber = '7686';
+    writeFileSync(p, JSON.stringify(parsed));
+    const old = new Date(2020, 0, 1);
+    utimesSync(p, old, old);
+    const fetch = recordingFetcher('这段中文绝不该被读到。');
+    const r = composeReview({
+      suggestionsInline: 1,
+      planPath: p,
+      prBodyFetcher: fetch,
+      env: ENV,
+      modelId: MODEL,
+    });
+    expect(fetch.calls).toBe(0);
+    expect(r.body).not.toContain('<details>');
+  });
+
+  it('does not fetch when the plan carries no PR identity', () => {
+    const fetch = recordingFetcher('这段中文绝不该被读到。');
+    const r = composeReview({
+      suggestionsInline: 1,
+      planPath: coveredPlan(), // no ownerRepo/prNumber, no flag
+      prBodyFetcher: fetch,
+      env: ENV,
+      modelId: MODEL,
+    });
+    expect(fetch.calls).toBe(0);
+    expect(r.body).not.toContain('<details>');
+  });
+
+  it('falls back to English when the fetch throws — language never takes the review down', () => {
+    const boom: PrBodyFetcher = () => {
+      throw new Error('gh unreachable');
+    };
+    const r = composeReview({
+      suggestionsInline: 1,
+      planPath: namedPlanWithoutFlag(),
+      prBodyFetcher: boom,
+      env: ENV,
+      modelId: MODEL,
+    });
+    expect(r.event).toBe('COMMENT');
+    expect(r.body).not.toContain('<details>');
+    expect(r.body).not.toContain('中文');
+    expect(r.body).toContain('Suggestions are inline.');
+  });
+});

packages/cli/src/commands/review/compose-review.ts

@@ -30,6 +30,8 @@ import {
   TranscriptsUnavailableError,
 } from './lib/coverage.js';
 import { shellQuotePath } from './lib/shell-quote.js';
+import { gh, setGhHost } from './lib/gh.js';
+import { isPositivePrNumber } from './lib/roster.js';
 import {
   CRITICAL_PREFIX,
   SUGGESTION_PREFIX,
@@ -40,6 +42,13 @@ import {
 
 export type ReviewEvent = 'APPROVE' | 'REQUEST_CHANGES' | 'COMMENT';
 
+/**
+ * Reads a PR's description body, given its `owner/repo` and number. The one
+ * production implementation calls `gh pr view`; the bilingual fallback uses it
+ * to recover the Han signal from the live PR when the plan does not carry it.
+ */
+export type PrBodyFetcher = (ownerRepo: string, prNumber: string) => string;
+
 export interface ComposeReviewInput {
   /**
    * Critical findings anchored as inline `comments` entries.
@@ -95,6 +104,18 @@ export interface ComposeReviewInput {
    * anything that would change where the transcripts are found on a real run.
    */
   env?: NodeJS.ProcessEnv;
+  /**
+   * How the bilingual fallback reads the live PR body when the plan carries a
+   * PR identity but no `prDescriptionHasHan` (a `plan-diff` plan, or one an
+   * improvising orchestrator wired in place of `fetch-pr`'s report). A test
+   * seam ONLY: production leaves it undefined and the CLI reads the PR with
+   * `gh pr view`. The handler **strips it from the input JSON** before use (the
+   * same way it strips `env`), so a model cannot supply one — not even a
+   * non-function value that would throw past the default and drop the fold. It
+   * can neither force nor suppress the Chinese fold, which is the whole point of
+   * keeping the signal the CLI's own.
+   */
+  prBodyFetcher?: PrBodyFetcher;
   /** Step 1's lightweight `pr-context` fetch failed. */
   contextUnavailable?: boolean;
   presubmit?: {
@@ -589,11 +610,13 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult {
   // description contains Han characters, the posted body carries the complete
   // Chinese version collapsed under the English one — the shape this repo's
   // own PR descriptions use, decided by the plan the CLI wrote, never by the
-  // caller. Fragments with no deterministic translation (model-written
-  // findings, caller echoes, error interpolations) ride verbatim in both
-  // halves. The footer stays outside the fold, once. A `zh === en` body has
-  // nothing translated, so no empty fold is published.
-  const bilingual = bilingualFromPlan(input.planPath);
+  // caller. When the plan does not record the signal but still names the PR,
+  // the switch recovers it from the live description (see `bilingualFromPlan`).
+  // Fragments with no deterministic translation (model-written findings, caller
+  // echoes, error interpolations) ride verbatim in both halves. The footer
+  // stays outside the fold, once. A `zh === en` body has nothing translated, so
+  // no empty fold is published.
+  const bilingual = bilingualFromPlan(input.planPath, input.prBodyFetcher);
   const render = (parts: Bi[], sep: string): string => {
     const en = parts.map((p) => p.en).join(sep);
     if (en === '') return '';
@@ -1041,21 +1064,70 @@ interface Bi {
   zh: string;
 }
 
+/** The production reader: one `gh pr view` for the description body. */
+const fetchPrBodyViaGh: PrBodyFetcher = (ownerRepo, prNumber) => {
+  const json = gh(
+    'pr',
+    'view',
+    prNumber,
+    '--repo',
+    ownerRepo,
+    '--json',
+    'body',
+  );
+  return (JSON.parse(json) as { body?: string }).body ?? '';
+};
+
 /**
  * Whether the posted body carries the collapsed Chinese version: the plan
  * (fetch-pr's report) recorded Han characters in the PR description. The
  * signal is the CLI's own — never the caller's, who could otherwise toggle
  * the register of a certified body. A local plan has no such field, and a
  * plan that cannot be read defaults to English-only: the language must never
  * take the review down.
+ *
+ * A recorded `false` is authoritative: `fetch-pr` fetched the body and found
+ * no Han, so English-only is the answer and no network is spent — every
+ * English-authored PR review takes this path.
+ *
+ * The field being *absent* is a different state, and the one that shipped an
+ * English-only review over a Chinese-authored PR (#7686): `fetch-pr` always
+ * writes it, but a `plan-diff` plan never does, and an orchestrator that
+ * improvises the pipeline can wire `compose-review` at a plan that is not
+ * `fetch-pr`'s report at all. So when the flag is missing yet the plan still
+ * carries the PR's identity, recover the signal from the live PR — the real
+ * description, which the caller cannot fake, so this hardens the "signal is
+ * the CLI's own" property rather than loosening it. Any failure of that fetch
+ * falls back to English: the language must never take the review down.
  */
-function bilingualFromPlan(planPath: string | undefined): boolean {
+function bilingualFromPlan(
+  planPath: string | undefined,
+  fetchPrBody: PrBodyFetcher = fetchPrBodyViaGh,
+): boolean {
   if (!planPath) return false;
+  let plan: {
+    prDescriptionHasHan?: unknown;
+    ownerRepo?: unknown;
+    prNumber?: unknown;
+  };
   try {
-    const plan = JSON.parse(readFileSync(planPath, 'utf8')) as {
-      prDescriptionHasHan?: unknown;
-    };
-    return plan?.prDescriptionHasHan === true;
+    plan = JSON.parse(readFileSync(planPath, 'utf8'));
+  } catch {
+    return false;
+  }
+  if (typeof plan?.prDescriptionHasHan === 'boolean') {
+    return plan.prDescriptionHasHan;
+  }
+  const ownerRepo =
+    typeof plan?.ownerRepo === 'string' && plan.ownerRepo
+      ? plan.ownerRepo
+      : undefined;
+  const prNumber = isPositivePrNumber(plan?.prNumber)
+    ? String(plan.prNumber)
+    : undefined;
+  if (!ownerRepo || !prNumber) return false;
+  try {
+    return /\p{Script=Han}/u.test(fetchPrBody(ownerRepo, prNumber));
   } catch {
     return false;
   }
@@ -1065,6 +1137,8 @@ interface ComposeReviewCliArgs {
   input: string | undefined;
   comments: string;
   out: string | undefined;
+  /** GitHub Enterprise host — routes this command's `gh` calls via GH_HOST. */
+  host?: string;
 }
 
 /**
@@ -1140,9 +1214,22 @@ export const composeReviewCommand: CommandModule = {
       .option('out', {
         type: 'string',
         describe: 'Also write the {event, body} JSON to this path',
+      })
+      .option('host', {
+        type: 'string',
+        describe:
+          'GitHub Enterprise host (routes gh via GH_HOST) — needed only when ' +
+          'the bilingual body-language recovery has to fetch the PR description',
       }),
   handler: (argv) => {
-    const { input, comments, out } = argv as unknown as ComposeReviewCliArgs;
+    const { input, comments, out, host } =
+      argv as unknown as ComposeReviewCliArgs;
+    // Route this command's own `gh` call — the bilingual recovery's `gh pr view`
+    // (see `fetchPrBodyViaGh`) — via the PR's host, exactly as fetch-pr and submit
+    // do. Without it a GHE review whose plan lacks the Han flag fetches the body
+    // from github.com, fails, and composes an English-only body that disagrees
+    // with what `submit` (which routes by host) posts.
+    setGhHost(host);
     // yargs enforces --comments on the real command line; this covers every
     // other way in (tests, programmatic calls) with the same sentence instead
     // of an ENOENT on `undefined`.
@@ -1162,6 +1249,13 @@ export const composeReviewCommand: CommandModule = {
     // always resolves the transcripts from the environment the CLI exported.
     const parsed = JSON.parse(raw) as ComposeReviewInput;
     delete parsed.env;
+    // Same reasoning for the bilingual body-language fetcher: it is a unit-test
+    // seam (production reads the PR with `gh pr view`). A state JSON carrying it —
+    // even a non-function value like `"suppress"` — would otherwise reach
+    // `bilingualFromPlan`, be called, throw, and drop the Chinese fold through the
+    // fail-safe. Stripping it here keeps the register the CLI's own, not the
+    // caller's, which is the whole point of the seam.
+    delete parsed.prBodyFetcher;
     // The inline counts are counted, not accepted — `submit` has refused them
     // since the count-beside-the-comments bug, and this boundary refusing them
     // too is what makes the Step 6 line and the posted verdict the same

packages/cli/src/commands/review/lib/roster.ts

@@ -106,7 +106,7 @@ function hasDeletions(plan: RosterPlan): boolean {
 
 /** A PR number the plan actually resolved: a positive integer, as a number or the
  *  string `fetch-pr` writes. `null`, `0`, `''` and non-numeric junk are 'no PR'. */
-function isPositivePrNumber(value: unknown): boolean {
+export function isPositivePrNumber(value: unknown): boolean {
   if (typeof value === 'number') return Number.isInteger(value) && value > 0;
   if (typeof value === 'string')
     return /^\d+$/.test(value) && Number(value) > 0;