fix(cli): honor the usage-statistics opt-out under --bare - #12604

Standard-tier review at 0cde996f. APPROVE.

Triage: privacy opt-out not honored in --bare / stream-json mode. Standard tier — bug fix touching settings merge logic.

What the change does: introduces readBareModeOperatorSettings() which reads both sandbox confinement settings and the privacy.usageStatisticsEnabled opt-out. createMinimalSettings() (used in --bare) switches from readOperatorSandboxSettings() to this new function so a user's opt-out survives the --bare code path.

Key properties verified:

1. Version gate on legacy key — legacyUsageStatisticsEnabled(scope) returns the top-level usageStatisticsEnabled only when `` is absent or < 2, mirroring the migration short-circuit in `migration/versions/v1-to-v2.ts:75`. A stray top-level key in a ` >= 2` file cannot outvote a `privacy.*` opt-out.

2. Falsy non-boolean opt-out coercion — the reduce uses return value ? current : false, which keeps any falsy value (0, "", false) as an opt-out and ignores truthy junk. This matches the ?? true coercion in config.ts (nullish coalescing keeps 0 and "" as-is, treating them as opted-out).

3. Scope precedence — [systemDefaults, user, system] reduce in that order so system (last) wins, consistent with the normal load. Test confirmed with the new normalAndBareMerged() helper asserting both loadSettings() and createMinimalSettings() together.

4. Bot findings R1-1 through R1-4 — all four were addressed in commit 0cde996f, verified by reading the current-head code.

5. sandboxSettingsFromScopes refactor — clean extraction; readOperatorSandboxSettings() delegates to the new readOperatorSettingsScopes(), semantically unchanged.

No blockers. Approving.

Reviewed by chiga0 (阿奇 / AI00002265)

Critical-only review at 0cde996f, base d0cd622a.

Verdict: APPROVE — no blocking issue was filed here and I found no Critical. For a privacy opt-out the direction of every ambiguous case is what matters, so I traced each one rather than checking only the happy path, and all of them resolve toward not collecting.

The defect being fixed, and why the wiring is real

createMinimalSettings() — the settings object used where a normal load is skipped — previously read only sandbox confinement, so privacy.usageStatisticsEnabled: false was invisible under --bare and the opt-out silently did not apply. The fix adds readBareModeOperatorSettings(), which resolves the same operator scopes and returns the sandbox settings plus the privacy choice, and settings.ts spreads privacy into the resulting Settings.

The shape is the one consumers already read: config.ts coerces with ?? true, and usageStatisticsEnabled is consumed there and in execution-worker.ts and llm-content-generator/index.ts. So this is not a field nothing looks at — populating it changes the decision those readers make. The fourth new test pins the complementary case, that when no scope configures the value the privacy key is omitted entirely rather than written as undefined, so the consumer's own default still applies instead of being shadowed.

Every ambiguous input resolves toward opt-out

The reducer is the privacy-critical part, so I walked it case by case:

if (value === undefined || value === null) return current;
if (typeof value === 'boolean') return value;
return value ? current : false;

• false → opt-out. true → opt-in. Both as written.
• Falsy non-booleans (0, "", NaN) → false, i.e. opt-out. A normal load reaches the same verdict, since 0 ?? true is 0 and reads as disabled.
• Truthy non-booleans → current, i.e. ignored. The comment says this differs from a normal load's mechanism, and it does, but not its outcome: ignoring the value falls through to the consumer's ?? true default, which is the same enabled verdict a normal load reaches by coercing the truthy junk directly. So the two paths agree on every input, and neither can turn a stated opt-out into an opt-in.
• undefined/null in a scope → that scope expresses no opinion and the previously resolved value stands, so a later scope cannot accidentally clear an earlier opt-out.

The legacy-key handling is gated the same way the migration gates it. legacyUsageStatisticsEnabled returns the pre-v2 top-level usageStatisticsEnabled only when $version is not a number at or above FIRST_NON_V1_SETTINGS_VERSION (2), mirroring the $version >= 2 short-circuit in migration/versions/v1-to-v2.ts: from v2 onward a normal load no longer migrates the v1 location and only warns about a stray key there, so --bare must not honour one either. The two paths can diverge on a malformed $version such as the string "2", where typeof version === 'number' is false and bare mode honours the legacy key while the migration's >= comparison would coerce and short-circuit. That divergence makes bare mode more private, not less, so it is the safe side to be wrong on.

Per-scope resolution prefers privacy.usageStatisticsEnabled over the legacy location via ??, and scope order reuses the existing operator convention in this file — system defaults, then user, then system, reduced so later scopes win — rather than inventing a precedence for this one setting. readOperatorSandboxSettings() is refactored onto the shared readOperatorSettingsScopes() and sandboxSettingsFromScopes() helpers, so existing callers get identical behaviour and the file parse now types the scope as the wider OperatorSettingsScope.

Tests

The four new cases are the decision table rather than a sample: privacy preferred over a stale pre-v2 key in the same file, a stray legacy key ignored in a current-version file, normal scope precedence across files, and the value left unset when no scope configures it. Each corresponds to a branch I traced above.

CI and review state

15 checks pass and 26 are skipped at this head, with only review-pr pending and nothing failing. Test (ubuntu-latest, Node 22.x) and Lint & Static are green, so the new suite ran. No thread is unresolved, and a maintainer approval is on this head.

One observation about the precedence, recorded rather than filed: because operator scopes resolve system-over-user, a system settings file asserting usageStatisticsEnabled: true overrides a user's false here exactly as it does for sandbox confinement. That is the pre-existing model for this settings tier and this PR inherits rather than introduces it, but a privacy opt-out is the one setting where that precedence is worth a deliberate confirmation rather than an analogy to confinement — a user who opted out and is then re-opted-in by a machine-level file will not see anything that tells them so.

Approved at head 0cde996.

Required CI complete and green on every substantive lane (Test ubuntu/macos/windows, Lint & Static, both Integration Tests lanes, Desktop Shell x2, web-shell E2E Smoke, OpenTUI no-flicker gate, TUI parity snapshots). All four round-1 findings were Suggestions and all four are verified fixed at this head; no Critical was ever recorded.

Independent verification, end to end:

• The fix reaches the actual decision point. config.ts:2504-2507 resolves usageStatisticsEnabled as the env flag, then settings.privacy?.usageStatisticsEnabled, then true, and createMinimalSettings now puts the resolved privacy block into the operator settings it hands to LoadedSettings, so a bare-mode opt-out really suppresses the beacon.
• The version gate mirrors the migration exactly: v1-to-v2.ts:73-75 short-circuits on typeof version === number and version >= 2, and FIRST_NON_V1_SETTINGS_VERSION expresses the same condition, so a stray top-level key in a current-version file is ignored here just as a normal load ignores it.
• The falsy coercion matches normal-load semantics case by case. Zero and empty string stay an opt-out on both paths, null falls back to the default on both, and truthy junk is ignored rather than read as an opt-in, which lands on the same enabled outcome as the ?? true chain.
• Bare mode does not widen. readOperatorSettingsScopes still returns only the system-defaults, user and system scopes, so a project settings file cannot decide telemetry for the user, and a new test asserts confinement stays unset when only the privacy choice is configured.
• The tests are not tautological: the helper evaluates a real loadSettings alongside createMinimalSettings, the assertions compare the effective ?? true value that config.ts uses rather than the raw setting, and the same-file precedence case deliberately stamps $version 1 because at $version 4 the gate would ignore the legacy key and leave the ?? operand order unpinned.

No new Critical found.