fix(cli): date the legacy usage-statistics key and coerce falsy opt-o…

…uts in bare mode Bare mode read a scope's top-level `usageStatisticsEnabled` regardless of that file's `$version`, so a stray legacy key in a current-version file — which a normal load only warns about, because `config.ts` reads `privacy.*` alone — could outvote a lower scope's explicit opt-out and re-enable the beacon under `--bare`. Gate the fallback at `$version >= 2`, mirroring the v1->v2 migration's own short-circuit, so only genuinely pre-v2 files contribute it. Also stop dropping falsy non-boolean values. The normal path coerces with `?? true`, which keeps `0` and `""` as opt-outs; the strict boolean guard turned them into "unset" and let `?? true` fire, so `--bare` re-enabled statistics for a file that suppressed it. Truthy junk (`'no'`) is still ignored rather than read as an opt-in. The privacy tests now cross-check `loadSettings()` against `createMinimalSettings()` on every expectation, and pin the `??` operand order with a same-file fixture holding both key shapes. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmufdy4iv1n
yiliang114 · Sep 24, 2026 · Sep 24, 2026 · Sep 24, 2026 · Sep 24, 2026 · Sep 24, 2026
commit 0cde996f2696542bf48c4c9c009b19056d7b1a85
diff --git a/packages/cli/src/config/execution-sandbox-settings.test.ts b/packages/cli/src/config/execution-sandbox-settings.test.ts
@@ -29,6 +29,11 @@ const write = (file: string, value: unknown) => {
   fs.mkdirSync(path.dirname(file), { recursive: true });
   fs.writeFileSync(file, JSON.stringify({ $version: 4, ...(value as object) }));
 };
+/** The two readers that carry the privacy choice: a normal load and `--bare`. */
+const normalAndBareMerged = () => [
+  loadSettings(workspace, { skipLoadEnvironment: true }).merged,
+  createMinimalSettings().merged,
+];
 beforeEach(() => {
   fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'public-sandbox-policy-'));
   workspace = path.join(fixture, 'workspace');
@@ -134,8 +139,8 @@ describe('operator execution sandbox policy', () => {
       { privacy: { usageStatisticsEnabled: false } },
     ],
     [
-      'legacy top-level usageStatisticsEnabled',
-      { usageStatisticsEnabled: false },
+      'a pre-v2 top-level usageStatisticsEnabled',
+      { $version: 1, usageStatisticsEnabled: false },
     ],
   ])('bare mode keeps a usage-statistics opt-out from %s', (_shape, value) => {
     write(user, value);
@@ -144,17 +149,44 @@ describe('operator execution sandbox policy', () => {
     // Only the privacy choice survives; confinement stays unset.
     expect(minimal.tools?.executionSandbox).toBeUndefined();
   });
+  it('bare mode prefers privacy.* over a stale pre-v2 key in the same file', () => {
+    write(user, {
+      $version: 1,
+      privacy: { usageStatisticsEnabled: false },
+      usageStatisticsEnabled: true,
+    });
+    expect(createMinimalSettings().merged.privacy?.usageStatisticsEnabled).toBe(
+      false,
+    );
+  });
+  it('bare mode ignores a stray legacy key in a current-version file', () => {
+    write(defaults, { privacy: { usageStatisticsEnabled: false } });
+    write(user, { usageStatisticsEnabled: true });
+    for (const merged of normalAndBareMerged()) {
+      // `?? true` is the coercion config.ts applies before sending a beacon.
+      expect(merged.privacy?.usageStatisticsEnabled ?? true).toBe(false);
+    }
+  });
+  it.each([0, ''])(
+    'bare mode keeps a falsy %j usage-statistics value as an opt-out',
+    (value) => {
+      write(user, { privacy: { usageStatisticsEnabled: value } });
+      for (const merged of normalAndBareMerged()) {
+        expect(merged.privacy?.usageStatisticsEnabled ?? true).toBeFalsy();
+      }
+    },
+  );
   it('bare mode resolves usage statistics with normal scope precedence', () => {
     write(defaults, { privacy: { usageStatisticsEnabled: false } });
     write(user, { privacy: { usageStatisticsEnabled: true } });
     write(system, { privacy: { usageStatisticsEnabled: false } });
-    expect(createMinimalSettings().merged.privacy?.usageStatisticsEnabled).toBe(
-      false,
-    );
+    for (const merged of normalAndBareMerged()) {
+      expect(merged.privacy?.usageStatisticsEnabled).toBe(false);
+    }
     write(system, {});
-    expect(createMinimalSettings().merged.privacy?.usageStatisticsEnabled).toBe(
-      true,
-    );
+    for (const merged of normalAndBareMerged()) {
+      expect(merged.privacy?.usageStatisticsEnabled).toBe(true);
+    }
   });
   it('bare mode leaves usage statistics unset when no scope configures it', () => {
     write(user, { privacy: { usageStatisticsEnabled: 'no' } });

diff --git a/packages/cli/src/config/execution-sandbox-settings.ts b/packages/cli/src/config/execution-sandbox-settings.ts
@@ -61,11 +61,29 @@ type SandboxSettingsInput = {
 };
 
 type OperatorSettingsScope = SandboxSettingsInput & {
+  /** Settings format version, used to date the legacy usage-statistics key. */
+  $version?: unknown;
   privacy?: { usageStatisticsEnabled?: unknown };
   /** Pre-v2 location, migrated to `privacy.*` by a normal settings load. */
   usageStatisticsEnabled?: unknown;
 };
 
+/**
+ * First version whose files a normal load no longer migrates from v1, so a
+ * top-level `usageStatisticsEnabled` at or above it is a stray key that a
+ * normal load only warns about. Mirrors the `$version >= 2` short-circuit in
+ * `migration/versions/v1-to-v2.ts`; kept as a literal because `settings.ts`
+ * already value-imports this module.
+ */
+const FIRST_NON_V1_SETTINGS_VERSION = 2;
+
+function legacyUsageStatisticsEnabled(scope: OperatorSettingsScope): unknown {
+  const version = scope.$version;
+  return typeof version === 'number' && version >= FIRST_NON_V1_SETTINGS_VERSION
+    ? undefined
+    : scope.usageStatisticsEnabled;
+}
+
 export function selectOperatorExecutionSandbox(
   ...scopes: SandboxSettingsInput[]
 ): ExecutionSandboxSettings | undefined {
@@ -94,8 +112,14 @@ export function readBareModeOperatorSettings(): SandboxSettingsInput & {
   const usageStatisticsEnabled = scopes.reduce<boolean | undefined>(
     (current, scope) => {
       const value =
-        scope.privacy?.usageStatisticsEnabled ?? scope.usageStatisticsEnabled;
-      return typeof value === 'boolean' ? value : current;
+        scope.privacy?.usageStatisticsEnabled ??
+        legacyUsageStatisticsEnabled(scope);
+      if (value === undefined || value === null) return current;
+      if (typeof value === 'boolean') return value;
+      // A normal load coerces with `?? true` (`config.ts`), which keeps any
+      // falsy value as an opt-out; truthy junk is ignored rather than read as
+      // an opt-in, so `--bare` decides the same way.
+      return value ? current : false;
     },
     undefined,
   );