fix(review): the sandbox default named an image that does not exist

packages/cli/src/commands/review/lib/sandboxed-exec.test.ts

@@ -11,9 +11,10 @@
 // happens when there is no runtime at all.
 
 import { describe, it, expect, afterEach, vi } from 'vitest';
-import { join, sep } from 'node:path';
+import { dirname, join, sep } from 'node:path';
 import {
   existsSync,
+  readFileSync,
   mkdirSync,
   writeFileSync,
   mkdtempSync,
@@ -22,6 +23,8 @@ import {
   symlinkSync,
 } from 'node:fs';
 import { tmpdir } from 'node:os';
+import { CLI_VERSION } from '../../../generated/git-commit.js';
+import { fileURLToPath } from 'node:url';
 import type { spawnSync } from 'node:child_process';
 import { isolateOperatorReviewSettings } from './test-utils.js';
 import * as environment from '../../../config/environment.js';
@@ -917,6 +920,79 @@ describe('reviewSandboxImage', () => {
     expect(reviewSandboxImage({ QWEN_REVIEW_SANDBOX_IMAGE: 'mine:1' })).toBe(
       'mine:1',
     );
-    expect(reviewSandboxImage({})).toContain('sandbox');
+    // The operator's own sandbox image, if they configured one for
+    // `qwen --sandbox`, rather than ignoring it and pulling a second one.
+    expect(reviewSandboxImage({ QWEN_SANDBOX_IMAGE: 'theirs:2' })).toBe(
+      'theirs:2',
+    );
+    // ORDER, with both set at once — the only way a reordering of the chain
+    // can be seen. Reviewed-repository-specific beats the operator's general
+    // one, because the review override exists for a toolchain the repository
+    // declared it needs; the other way round runs the build in an image
+    // missing it.
+    expect(
+      reviewSandboxImage({
+        QWEN_REVIEW_SANDBOX_IMAGE: 'mine:1',
+        QWEN_SANDBOX_IMAGE: 'theirs:2',
+      }),
+    ).toBe('mine:1');
+    expect(
+      reviewSandboxImage({
+        QWEN_CODE_CUSTOM_SANDBOX_IMAGE: 'custom:3',
+        QWEN_SANDBOX_IMAGE: 'theirs:2',
+      }),
+    ).toBe('custom:3');
+    expect(
+      reviewSandboxImage({
+        QWEN_REVIEW_SANDBOX_IMAGE: 'mine:1',
+        QWEN_CODE_CUSTOM_SANDBOX_IMAGE: 'custom:3',
+      }),
+    ).toBe('mine:1');
+  });
+
+  it('consults the manifest, and falls back to a name that exists', () => {
+    // These two cannot be told apart by their VALUE: `DEFAULT_IMAGE`'s tag is
+    // `CLI_VERSION`, generated from the same manifest version, so today the
+    // fallback string-equals the manifest field. Deleting the manifest lookup
+    // entirely therefore leaves the pin below green. Injecting the reader is
+    // what makes the mechanism visible at all.
+    expect(reviewSandboxImage({}, () => 'manifest-image:test')).toBe(
+      'manifest-image:test',
+    );
+    // ...and when the manifest cannot be found — the unusual install layout
+    // the literal exists for — the argv still names something that resolves.
+    // This is the branch the defect this PR fixes lived in: with no coverage,
+    // reverting it to an unpullable name ships green.
+    const fallback = reviewSandboxImage({}, () => undefined);
+    expect(fallback).toBe(`ghcr.io/qwenlm/qwen-code:${CLI_VERSION}`);
+    expect(fallback).not.toContain('/sandbox');
+    expect(fallback).not.toContain(':latest');
+  });
+
+  it('defaults to the image this CLI actually ships with', () => {
+    // Pinned against the MANIFEST, not against a spelling. The assertion this
+    // replaces was `toContain('sandbox')`, which is how a default of
+    // `ghcr.io/qwenlm/qwen-code/sandbox:latest` shipped: it contains the word,
+    // it is not the CLI's image, and it does not resolve at all — an anonymous
+    // manifest request answers 403 where the real one answers 200, so every
+    // command of an opted-in review failed at image pull. Nineteen rounds of
+    // argv-level review could not see it because no container was ever
+    // started. The real image happens NOT to contain "sandbox", so the old
+    // assertion would fail on the correct value and pass on the broken one.
+    const manifest = JSON.parse(
+      readFileSync(
+        join(
+          dirname(fileURLToPath(import.meta.url)),
+          '..',
+          '..',
+          '..',
+          '..',
+          'package.json',
+        ),
+        'utf8',
+      ),
+    ) as { config?: { sandboxImageUri?: string } };
+    expect(manifest.config?.sandboxImageUri).toBeTruthy();
+    expect(reviewSandboxImage({})).toBe(manifest.config?.sandboxImageUri);
   });
 });

packages/cli/src/commands/review/lib/sandboxed-exec.ts

@@ -48,19 +48,26 @@
 import { spawnSync } from 'node:child_process';
 import { realpathSync } from 'node:fs';
 import { basename, dirname, join, resolve, sep } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { operatorReviewSettings } from './review-settings.js';
 import { REVIEW_TMP_DIR } from './paths.js';
 import { redirectedAncestor } from './worktree.js';
 import { CUSTOM_SANDBOX_IMAGE_ENV_VAR } from '../../../utils/processUtils.js';
 import { isFileSourcedEnvKey } from '../../../config/environment.js';
+import { readPackageUpSync } from 'read-package-up';
+import { CLI_VERSION } from '../../../generated/git-commit.js';
 
 /**
- * The fallback when neither override names an image: the published sandbox
- * image for this CLI line. Pinned by tag rather than digest on purpose — a
- * digest would go stale in a file nobody updates, and the override exists for
- * anyone who needs reproducibility.
+ * Last resort only: the real default is the CLI's own `config.sandboxImageUri`
+ * — see `cliSandboxImage`. This literal covers the case where the package
+ * manifest cannot be found at all (an unusual install layout), so the argv
+ * still names something that exists.
+ *
+ * Versioned rather than floating on `:latest`, and by tag rather than digest —
+ * a digest would go stale in a file nobody updates, and the overrides exist
+ * for anyone who needs reproducibility.
  */
-const DEFAULT_IMAGE = 'ghcr.io/qwenlm/qwen-code/sandbox:latest';
+const DEFAULT_IMAGE = `ghcr.io/qwenlm/qwen-code:${CLI_VERSION}`;
 
 /** Container runtimes this module knows how to drive, in preference order. */
 const RUNTIMES = ['docker', 'podman'] as const;
@@ -688,18 +695,76 @@ export function containerCommand(
   return { file: opts.runtime, args };
 }
 
+/**
+ * The image `qwen --sandbox` itself runs, read from the package manifest that
+ * ships with this CLI (`config.sandboxImageUri`) — the same field
+ * `sandboxConfig.ts` reads, so the two cannot drift.
+ *
+ * This is a correction, not a refinement. The first cut hardcoded
+ * `ghcr.io/qwenlm/qwen-code/sandbox:latest` and its comment claimed that was
+ * "the same image `qwen --sandbox` uses". It is not: the CLI's image is
+ * `ghcr.io/qwenlm/qwen-code:<version>`, a different repository path and a
+ * pinned tag, and the hardcoded name does not resolve at all — an anonymous
+ * manifest request answers 403 where the real one answers 200. Every command
+ * of an opted-in review therefore failed at image pull, which nineteen rounds
+ * of argv-level review could not see because no container was ever started
+ * from that argv.
+ *
+ * Read once and cached: this is on the per-command path.
+ */
+function cliSandboxImage(): string | undefined {
+  if (manifestImage !== undefined) return manifestImage ?? undefined;
+  try {
+    const found = readPackageUpSync({
+      cwd: dirname(fileURLToPath(import.meta.url)),
+    });
+    const uri = (
+      found?.packageJson as
+        | { config?: { sandboxImageUri?: string } }
+        | undefined
+    )?.config?.sandboxImageUri;
+    manifestImage = typeof uri === 'string' && uri.trim() ? uri.trim() : null;
+  } catch {
+    manifestImage = null;
+  }
+  return manifestImage ?? undefined;
+}
+
+let manifestImage: string | null | undefined;
+
 /**
  * The image the reviewed repository's commands run in.
  *
  * Defaults to the CLI's own sandbox image, which already carries a Node
- * toolchain — the same image `qwen --sandbox` uses, so a repository that
- * builds under one builds under the other. `QWEN_REVIEW_SANDBOX_IMAGE`
- * overrides it for a repository whose toolchain needs more (a JDK, a Python,
- * a specific Node major), which is the case this default cannot cover and
- * should not pretend to.
+ * toolchain — literally the same image `qwen --sandbox` resolves, read from
+ * the same manifest field, so a repository that builds under one builds under
+ * the other. That sentence was here before `cliSandboxImage` existed, when a
+ * hardcoded name made it false; it is now a description of the code rather
+ * than an intention about it.
+ *
+ * The parity is over the DEFAULT, not over every channel `--sandbox` reads.
+ * An operator who set `QWEN_SANDBOX_IMAGE` in a user-level `~/.qwen/.env` or
+ * in `settings.env` loses it here and falls back to that default: the loader
+ * records every key it applies from a file without distinguishing the user's
+ * own from the reviewed repository's, and this side cannot afford to guess
+ * wrong about which one it is holding. Exporting it in the shell keeps
+ * parity. Widening that would mean teaching the loader to carry the
+ * home-scoped classification it already computes — a change to the loader,
+ * not to this pick.
+ *
+ * `QWEN_REVIEW_SANDBOX_IMAGE` overrides it for a repository whose toolchain
+ * needs more (a JDK, a Python, a specific Node major), which is the case this
+ * default cannot cover and should not pretend to.
  */
 export function reviewSandboxImage(
   env: NodeJS.ProcessEnv = process.env,
+  // Injected so a test can tell the manifest apart from the fallback. It
+  // cannot otherwise: `DEFAULT_IMAGE`'s tag comes from `CLI_VERSION`, which is
+  // generated from the same manifest version, so the two currently produce the
+  // SAME string — deleting the manifest lookup falls through to a literal that
+  // string-equals it and the suite stays green. That is the mechanism this
+  // change exists to add, pinned by nothing until the two can be told apart.
+  manifest: () => string | undefined = cliSandboxImage,
 ): string {
   // File-sourced overrides are ignored for the same reason the policy ignores
   // them, and this one is sharper: the image IS the code the reviewed
@@ -711,6 +776,11 @@ export function reviewSandboxImage(
   return (
     pick('QWEN_REVIEW_SANDBOX_IMAGE') ||
     pick(CUSTOM_SANDBOX_IMAGE_ENV_VAR) ||
+    // The operator's own sandbox image, if they configured one for
+    // `qwen --sandbox`. Through `pick`, so a repository shipping it in its
+    // `.qwen/.env` cannot choose the image its own code runs in.
+    pick('QWEN_SANDBOX_IMAGE') ||
+    manifest() ||
     DEFAULT_IMAGE
   );
 }