[Lxr-max]

Reject new Windows drive-root storage locations, including bare drive letters and paths that normalize to a root. Preserve existing users' ability to start: saved root locations use default folders for the session with an explicit startup warning directing the user to Settings > Storage. Original files and storage-settings.json remain untouched until the user saves new locations.

Resolve a bare XDG_CONFIG_HOME drive letter before appending open-codesign. A root XDG base is permitted because the actual config directory is its application subfolder. Keep a defensive config-write guard and add a desktop changeset. Remove the unrelated CLI timeout edits; retain main's existing CLI configuration.

Validation: Windows storage recovery, unchanged persisted settings, saving a replacement location, root rejection and XDG path tests passed (11 tests); platform-parameterized root parsing also runs on Linux/macOS. Repository typecheck and lint passed. Full pre-push/remote checks are tracked on the current head. No real user files were used or migrated during tests.

Fixes #375. The native startup notice has not been manually exercised in a visible desktop session; the boot recovery behavior is covered by Windows unit tests.

Final local validation on b00fee4: all 11 pre-push tasks passed, including desktop 3,052 tests passed / 12 skipped. The full suite exposed a whole-module storage mock in web-search-settings.test.ts; it now retains the real path helpers while overriding only the storage location. Related settings and storage tests passed (32 tests), then the full desktop suite passed. No production search-settings behavior changed.

Review follow-up: GitHub had retained base e94c3c1 and displayed already-merged changes as this PR. Closing/reopening refreshed the base to current main 014d409 and the PR now shows only 7 storage-related files; there are no release, pi, CLI timeout or changelog changes in this PR. initStorageSettings runs synchronously at index.ts line 66, before the app.whenReady handler reads recovery warnings. The native recovery notice now includes English and Chinese, matching existing main-process bilingual messages.

Final head 5d40eae: full local checks passed again after the bilingual notice update (desktop 3,052 passed / 12 skipped), and all required remote checks passed. Linux and native Intel packaging checks passed on the preceding b00fee4 head; the final follow-up only changes notice text. Latest review confirms the earlier scope and recovery findings are resolved, with no blockers. The remaining non-blocking suggestion is to guard the native warning dialog against an Electron dialog-service exception; that failure has not been reproduced.

[github-actions]

Findings

• [Minor] Missing changeset for a user-visible behavior change — this PR adds new hard error paths (drive-root rejection in normalizeDir, XDG_CONFIG_HOME drive-root rejection in defaultConfigDir, and the writeConfig guard) that change how storage configuration behaves for users. No .changeset/*.md is included in the diff (changedFiles: 3: config.ts, storage-settings.ts, storage-settings.test.ts), and the project requires changesets for user-visible changes (see .github/PULL_REQUEST_TEMPLATE.md "Added a changeset" and CLAUDE.md Versioning).
  Suggested fix: run pnpm changeset, select @open-codesign/desktop (or the affected package), and commit the generated file.

• [Nit] The new rejection logic has no coverage on non-Windows CI — both new tests short-circuit before any meaningful assertion on non-win32 runners (apps/desktop/src/main/storage-settings.test.ts, the if (process.platform !== 'win32') return; gate), and the helpers themselves are process.platform-gated (apps/desktop/src/main/storage-settings.ts:221 isWindowsDriveRoot, coerceWindowsDriveBase). If there is no Windows CI job, the actual reject/coerce behavior is never exercised in CI.
  Suggested fix: extract the drive-letter parsing into a pure predicate that takes the platform as an argument (e.g. isWindowsDriveRootOn('win32', value)), and let the exported isWindowsDriveRoot/coerceWindowsDriveBase delegate to it, so the regex/coercion can be unit-tested on Linux/macOS runners too.

Questions

• How are already-persisted drive-root paths handled on upgrade? Issue #375 implies users already had D:\ saved in storage-settings.json, so a fix that only rejects new writes may still leave them broken. initStorageSettings → readStorageLocationsSync → parseStorageSettingsFile reads the persisted configDir/dataDir at boot. Does that read path run normalizeDir on the persisted values? If yes, an existing user with D:\ persisted will now throw STORAGE_SETTINGS_INVALID inside initStorageSettings at boot (the app may fail to start), and the Fixes #375 claim would regress those users from "EPERM on add-provider" to "cannot boot." If no, the writeConfig guard covers config writes but other mkdir call sites for the data/logs directories (outside writeConfig) may still hit EPERM at boot. Either way, please confirm the upgrade behavior for the exact users in #375, and consider a one-time user-visible migration at read time (reset an already-persisted drive-root dir to the default with a clear notice) rather than a hard boot-time throw.

Summary

• Review mode: initial
• The core direction is sound: rejecting Windows drive roots in normalizeDir and coercing bare D: to D:\ is the right shape, and adding defense-in-depth guards in defaultConfigDir/writeConfig matches the reported failure path. The helpers are small and focused.
• Main residual risk is upgrade behavior for users who already persisted D:\ (see Questions): confirm whether the boot read path rejects/resets those values, since that determines whether Fixes #375 fully resolves the issue or merely prevents new occurrences. Per project policy, avoid silent fallbacks for persisted state — a user-visible reset/notice is preferable to either a hard boot throw or a quiet default.
• Not flagged: the CONFIG_READ_FAILED error code used in the write-time guard is slightly off semantically, but it is not material. No direct-SDK imports, dependency additions, license, or hardcoded-UI-token concerns apply to this diff.

Testing

• Not run (automation). The PR's own Windows manual test-plan items (choose D:\ in Settings → Storage; XDG_CONFIG_HOME=D:\codesign\.config) remain unchecked — worth completing before merge, especially the pre-existing-D:\-config upgrade scenario above.

Open-CoDesign Bot

[github-actions]

Findings

• [Major] Verify the upgrade path — existing persisted drive-root configs may now fail at boot for the exact users in #375. normalizeDir now throws STORAGE_SETTINGS_INVALID for D:\/D: (apps/desktop/src/main/storage-settings.ts:245, new isWindowsDriveRoot guard at :222). If parseStorageSettingsFile normalizes persisted dirs through the adjacent normalizeOptionalDir/normalizeDir helpers, then readStorageLocationsSync rethrows any CodesignError, so a Windows user whose storage-settings.json already contains "configDir": "D:\\" would hit a throw inside initStorageSettings during boot — trading "EPERM when adding a provider" for "app will not start". The previous review raised this and the new commits do not address it.
  Suggested fix: add a regression test that seeds {"schemaVersion":1,"configDir":"D:\\"} at storageSettingsPath(root) and asserts initStorageSettings behavior on Windows. If it throws, reset a persisted drive-root location to the OS default on read with a user-visible notice (per AGENTS.md hard constraints, do not silently fall back for persisted state) or catch it in the main boot path and surface the reset, instead of letting it abort startup.

• [Minor] Missing changeset for a user-visible behavior change — this PR adds new hard error paths (drive-root rejection in normalizeDir, XDG_CONFIG_HOME drive-root rejection in defaultConfigDir, and the writeConfig guard) but still includes no .changeset/*.md, while .github/PULL_REQUEST_TEMPLATE.md asks for one for user-visible changes.
  Suggested fix: run pnpm changeset, select the affected workspace package, and commit the generated file.

• [Minor] Unrelated CI-flake changes are bundled into this Windows storage fix and are not mentioned in the PR description — apps/cli/vitest.config.ts:5 adds testTimeout: 30_000 and apps/cli/src/cli.test.ts:189 adds a per-test 30_000. The two settings are also redundant, since the new package config already applies to that test.
  Suggested fix: move the CLI timeout change to its own PR (or describe it in this PR's Summary) and drop the redundant per-test timeout so the config file is the single source of the ceiling.

Questions

• Does parseStorageSettingsFile run normalizeDir on persisted configDir/logsDir/dataDir? This determines whether the Major finding above is a real boot regression or is limited to newly written values.
• Where is initStorageSettings invoked from in the main process, and does that caller catch CodesignError and show a user-visible recovery message, or does the throw escape to an unhandled boot error?

Summary

• Review mode: follow-up after new commits
• The new commits since the previous review are Biome formatting in apps/desktop/src/main/storage-settings.test.ts plus the CLI test-timeout changes; the storage-guard logic itself is unchanged, so the open upgrade-behavior question still stands and is the main thing to settle before merge.
• The direction remains sound: rejecting Windows drive roots in normalizeDir and coercing bare D: to D:\ before joins matches the reported failure path, and the defaultConfigDir/writeConfig guards add defense in depth. No dependency, license, pi-ai, or packages/ui token concerns apply to this diff.
• Residual observations (carried over, non-blocking): the new guards remain process.platform-gated and the second test returns early on non-win32 runners (apps/desktop/src/main/storage-settings.test.ts:107), so on Linux/macOS CI the regex/coercion behavior is never exercised; a platform-parameterized pure predicate would let those assertions run everywhere. Also noted previously and still not material: the writeConfig guard uses ERROR_CODES.CONFIG_READ_FAILED for a write refusal.

Testing

• Not run (automation). The PR's Windows manual test-plan items (choose D:\ in Settings → Storage; XDG_CONFIG_HOME=D:\codesign\.config after restart) remain unchecked, and the pre-existing-D:\ upgrade scenario still needs coverage.
• Suggested: a storage-settings test that writes a persisted configDir of D:\ and asserts boot behavior, plus a config.ts test that defaultConfigDir() with XDG_CONFIG_HOME=D: produces an actionable error rather than an unhandled startup throw.

Open-CoDesign Bot

[github-actions]

Findings

• [Major] This PR now carries ~250 files of unrelated changes, so the storage fix can no longer be reviewed or reverted on its own — the stated change is storage-settings* + config.ts + a startup notice + tests + one changeset, but the diff is 255 files / +11,342 / −2,206 and also contains: the pi namespace migration (apps/desktop/package.json:32-34, new .changeset/align-pi-packages.md, apps/desktop/electron.vite.config.ts:22-24), an undici pin (apps/desktop/package.json:29), a new ~1,000-line design-system library (apps/desktop/src/main/design-system-library.ts, design-system-library-ipc.ts, design-system-sample.ts), web-research plumbing (apps/desktop/src/main/ipc/generate.ts), keychain fail-closed behavior (apps/desktop/src/main/keychain.ts), a generation-timeout semantic change (apps/desktop/src/main/preferences-ipc.ts:124-130), a release/CI rewrite (.github/workflows/release.yml, new .github/scripts/sync-release-manifests.sh, .github/dependabot.yml), and edits to the automated review prompt (.github/prompts/codex-pr-review.md:29). It also contains a changeset version run: ~30 .changeset/*.md files are deleted, apps/desktop/CHANGELOG.md gains 73 lines, apps/cli/CHANGELOG.md is added, and versions are bumped (apps/desktop/package.json:4 0.2.2 → 0.3.0, apps/cli/package.json:4 0.1.0 → 0.2.0). Merging this lands unreviewed release-prep, provider-stack, and bot-instruction changes under a Windows storage bugfix title.
  Suggested fix: rebase onto current main and drop the release/version commit plus the unrelated branches so the diff is limited to the storage settings module, config.ts, the index.ts startup notice, .changeset/windows-storage-roots.md, and their tests. Land the pi migration, design-system library, keychain, CI and prompt changes as separate PRs.

• [Major] The release path is now gated on a reusable workflow and a hard-required secret that the PR description's validation does not cover — publish gained needs: [..., native-intel] (.github/workflows/release.yml:436), where native-intel calls release-macos-x64-native-check.yml (.github/workflows/release.yml:82-90), and that workflow was moved to runs-on: macos-15-intel (.github/workflows/release-macos-x64-native-check.yml:39). If that runner label is not available to this repository, publish never starts and every v*.*.* tag stalls the release. In the same job family, bump-manifests now exits 1 when RELEASE_BOT_TOKEN is unset (.github/workflows/release.yml:498-505), and .github/scripts/sync-release-manifests.sh:41 runs gh pr merge --auto --squash under set -euo pipefail; a missing secret or a repo without auto-merge enabled turns the post-publish manifest sync into a red release run and leaves packaging/ at its current in-repo versions (cask 0.2.2, scoop 0.2.2, winget only 0.2.0) while this PR bumps the desktop package to 0.3.0.
  Suggested fix: move the release-path changes to their own PR, and there confirm the runner label and that RELEASE_BOT_TOKEN plus auto-merge are configured before making the sync job fail closed.

• [Minor] The new startup notice is hardcoded English in the main process — dialog.showMessageBoxSync({ title: 'Storage location needs attention', message: 'Open CoDesign is using default storage folders for this session.', detail: storageWarnings.join(...) }) at apps/desktop/src/main/index.ts:202-211. This app ships zh-CN, and main-process messages already follow a bilingual pattern (e.g. apps/desktop/src/main/design-system-library-ipc.ts:40-42), so the one message a user with a broken persisted storage location will see should not be English-only.
  Suggested fix: add zh-CN/en strings for the title, body and warning lines and resolve them before showing the dialog (or surface the notice through the localized renderer).

Questions

• Does initStorageSettings() complete before getStorageRecoveryWarnings() is read at the top of the app.whenReady() handler (apps/desktop/src/main/index.ts:200)? If the recovery state is populated later in the boot sequence, the warnings array is always empty and the intended startup notice never appears.
• Is macos-15-intel a generally available GitHub-hosted runner for this repository, and is RELEASE_BOT_TOKEN configured with contents: write / pull-requests: write plus auto-merge enabled on main?

Summary

• Review mode: follow-up after new commits
• The earlier Major (existing users with a persisted drive-root location could fail to boot) appears addressed: saved root locations fall back to default folders for the session, the boot path reads getStorageRecoveryWarnings() and shows an explicit notice (apps/desktop/src/main/index.ts:199-211), and .changeset/windows-storage-roots.md was added, which also resolves the earlier missing-changeset Minor.
• The blocker is no longer the storage logic but scope: at 255 files this is no longer the fix that was reviewed, and it bundles a full version bump with provider-stack, release, CI and bot-prompt changes.
• Residual observations (non-blocking, carried over): the writeConfig guard still reports a write refusal as ERROR_CODES.CONFIG_READ_FAILED (apps/desktop/src/main/config.ts:97-103); the new XDG/root assertions in apps/desktop/src/main/config.storage-root.test.ts:9-27 are win32-only, so Linux/macOS CI never exercises the coercion; the CLI test-timeout addition (apps/cli/vitest.config.ts, apps/cli/src/cli.test.ts:189) remains unrelated and redundant with the package-level config.

Testing

• Not run (automation).
• apps/desktop/src/main/storage-settings.ts was outside this run's visible diff, so the persisted-drive-root recovery path and its tests are taken from the PR description rather than verified here.
• Suggested: a Windows test that seeds storage-settings.json with a drive-root configDir and asserts boot continues, getStorageRecoveryWarnings() is non-empty, and the persisted file and user files are unchanged; plus a platform-parameterized test for the root predicate so it also runs on Linux/macOS.

Open-CoDesign Bot

[github-actions]

Findings

• [Minor] The boot recovery notice is shown with a raw synchronous dialog before the boot-fallback guard — apps/desktop/src/main/index.ts:202-212. The call sits at the very top of the app.whenReady() callback, ahead of the extracted try/catch that routes failures through the boot-fallback path. If dialog.showMessageBoxSync throws (no usable dialog service, Electron API error), the rest of the callback does not run, so a non-essential notice can stop the app from creating its window. showBootDialog already exists for guarded boot-time messaging (apps/desktop/src/main/boot-fallback.ts, imported at apps/desktop/src/main/index.ts:10).
  Suggested fix: wrap the notice so a dialog failure degrades to a logged warning instead of a stalled boot, for example:

  try {
    dialog.showMessageBoxSync({ /* ... */ });
  } catch (err) {
    getLogger().warn('Failed to show storage recovery notice', err);
  }

  or route it through showBootDialog if that helper already provides the guarded path.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Prior findings are resolved. The PR now contains only the storage-related files (no release, pi, CLI-timeout, or changelog changes), and the startup notice plus recovery warning are bilingual (apps/desktop/src/main/index.ts:205-210, apps/desktop/src/main/storage-settings.ts:226). initStorageSettings runs at module scope before app.whenReady() (apps/desktop/src/main/index.ts:66-67), so getStorageRecoveryWarnings() is populated before the handler reads it. Platform-parameterized root parsing now runs on Linux/macOS as well (apps/desktop/src/main/storage-settings.test.ts:92-101).
• The core fix is sound: bare drive letters are coerced to D:\ before joining (apps/desktop/src/main/config.ts:22-25, apps/desktop/src/main/storage-settings.ts:252-257), reads with allowRoot recover persisted roots to defaults with a warning while leaving storage-settings.json and user files untouched (apps/desktop/src/main/storage-settings.ts:222-231), and writes still reject roots (apps/desktop/src/main/storage-settings.ts:266-271).
• No blockers. Mergeable after or without the Minor above; that one is defensive hardening, not a demonstrated failure.
• Residual (known, non-blocking): the writeConfig drive-root refusal still reports ERROR_CODES.CONFIG_READ_FAILED for a write (apps/desktop/src/main/config.ts:97-102); the full recovery test is win32-only (apps/desktop/src/main/storage-settings.test.ts:103), so non-Windows CI exercises only the helper predicates; the defensive writeConfig guard has no direct test. Linked issue #375 was not independently re-fetched in this run, but the diff matches the described drive-root failure and preserves existing settings.

Testing

• Not run (automation).
• Suggested: a Windows assertion that writeConfig rejects with the intended error code when configDir() resolves to a drive root, and, if practical, a check that a throwing warning dialog does not prevent window creation.

Open-CoDesign Bot