fix: fail closed encrypted storage lifecycle - #335
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (67)
📝 WalkthroughWalkthroughThe change makes IndexedDB encryption fail closed while locked, removes passphrase reset, rotation, and disable controls, and adds persisted textured or plain writing-surface settings. Documentation, translations, workspace policy, and tests reflect the updated behavior. ChangesEncryption lifecycle and protected storage
Writing-surface preference
Workspace policy
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideThis PR hardens the IndexedDB encryption lifecycle by failing closed when encrypted storage is configured but locked, blocking disable/rekey paths until a durable migration exists, fixing salt persistence, adding a plain writing-surface preference, and updating UI, docs, tests, and deps to reflect the more conservative behavior. Sequence diagram for fail-closed protected IDB writessequenceDiagram
actor User
participant App
participant IdbProjectStore
participant StorageEncryptionService
participant Idb
User->>App: trigger save
App->>IdbProjectStore: saveSlice(sliceName, data)
IdbProjectStore->>StorageEncryptionService: assertIdbProtectedWriteAllowed()
alt [key available]
StorageEncryptionService-->>IdbProjectStore: ok
IdbProjectStore->>StorageEncryptionService: idbEncrypt(data)
StorageEncryptionService-->>IdbProjectStore: encryptedPayload
IdbProjectStore->>Idb: put(encryptedPayload, sliceName)
Idb-->>IdbProjectStore: success
else [key missing and hasPassphraseSentinel]
StorageEncryptionService-->>IdbProjectStore: IdbStorageLockedError
IdbProjectStore-->>App: propagate IdbStorageLockedError
App-->>User: show locked storage error
else [encryption never configured]
StorageEncryptionService-->>IdbProjectStore: ok
IdbProjectStore->>Idb: put(compressData(data), sliceName)
Idb-->>IdbProjectStore: success
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
qnbs
commented
Aug 11, 2026
OwnerAuthor
@CodeAnt-AI review |
CodeAnt AI is running the review. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
CodeAnt AI finished running the review. |
qnbs
commented
Aug 11, 2026
OwnerAuthor
@CodeAnt-AI review |
CodeAnt AI is running the review. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. |
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
CodeAnt AI finished running the review. |
qnbs
marked this pull request as ready for review
August 11, 2026 08:55
PR Summary by QodoFail closed: lock-aware IDB encryption and block unsafe disable/rekey paths
AI Description
Diagram
High-Level Assessment
Files changed (86) |
Code Review by Qodo
1. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:366fcfe9b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/storage/idbProjectStore.ts (1)
45-63: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd the required
QNBS-v3rationale.
writingSurfaceStyleadds persisted-setting migration and validation. Add one physical-line comment that explains the default and invalid-value fallback.Suggested comment
+ // QNBS-v3: Persisted writing-surface preference — validate legacy values for deterministic rendering. writingSurfaceStyle: 'textured',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/storage/idbProjectStore.ts` around lines 45 - 63, Add a single physical-line QNBS-v3 comment immediately before the writingSurfaceStyle default or validation logic, documenting that the setting defaults to 'textured' for older persisted settings and invalid stored values fall back to 'textured'; leave the existing migration behavior unchanged.Source: Coding guidelines
🧹 Nitpick comments (1)
services/storage/idbCodexStore.ts (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required QNBS-v3 rationale.
These assertions change the storage contract, but the changed lines have no single-line comment explaining the fail-closed check.
Proposed comment
+// QNBS-v3: Fail closed before opening IDB while protected storage is locked. await assertIdbProtectedWriteAllowed();As per coding guidelines, “For every non-trivial code change, add one single-line
QNBS-v3comment explaining why.”Also applies to: 47-47, 97-97, 149-149
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/storage/idbCodexStore.ts` at line 21, Add a single-line QNBS-v3 comment immediately before each assertIdbProtectedWriteAllowed() call in the affected storage methods, including the occurrences at the referenced locations, explaining that the fail-closed assertion enforces the protected-write storage contract.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/settings/GeneralSections.tsx`:
- Around line 209-234: Add a single physical-line QNBS-v3 why-comment
immediately above the writing-surface selector container in GeneralSections,
explaining why users can choose between textured and plain writing surfaces. Do
not add additional comments or alter the selector behavior.
- Around line 217-229: Add aria-pressed to both writing-surface option Buttons
in the GeneralSections component, setting it true when
settings.writingSurfaceStyle matches that button’s textured or plain value and
false otherwise, while preserving the existing variant and click behavior.
In `@docs/IDB-ENCRYPTION.md`:
- Around line 57-58: Update getOrCreateSalt() and the initialization flows in
initIdbEncryption(), verifyAndInitIdbEncryption(), and setupIdbEncryption() so a
missing or invalid persisted worldscript-idb-kdf-salt-v1 rejects when the IDB
sentinel already exists, rather than generating a replacement salt. Preserve
salt creation only for first-time setup, and ensure repeated setup calls also
fail closed for this state.
In `@locales/ar/settings.json`:
- Line 233: Regenerate the locale bundles from the source locale definitions so
all eight settings.* keys remain synchronized across the 19 locales and the
obsolete lora.onboarding.selectPython key is removed from every generated
bundle, including the Arabic settings bundle.
In `@locales/en/help.json`:
- Line 6: Update the encryption coverage wording in
help.settingsGuide.flags.content to refer only to supported IndexedDB stores,
matching the narrower scope used by the primary encryption description and
documentation; remove the “all IndexedDB stores” guarantee without changing the
surrounding guidance.
In `@locales/fr/settings.json`:
- Around line 341-344: Update the settings.appearance.writingSurfaceHint
translation to use wording applicable to both Texturée and Unie, then regenerate
the runtime bundles. Apply neutral wording in
locales/fr/settings.json#L341-L344, locales/fi/settings.json#L341-L344,
locales/he/settings.json#L341-L344, locales/hu/settings.json#L341-L344,
locales/is/settings.json#L341-L344, locales/it/settings.json#L341-L344,
locales/ja/settings.json#L341-L344, locales/ko/settings.json#L341-L344,
locales/pt/settings.json#L341-L344, locales/ru/settings.json#L341-L344,
locales/sv/settings.json#L341-L344, and locales/zh/settings.json#L341-L344;
leave the writingSurfaceTextured and writingSurfacePlain values unchanged.
In `@locales/sv/help.json`:
- Line 6: Update the stale IndexedDB encryption-scope statement around the help
entry at line 124 to match the supported-store boundary described by
help.advanced.encryption.content, removing any claim that all IndexedDB stores
are encrypted. Regenerate the affected locale bundles after updating the source
translation.
In `@pnpm-workspace.yaml`:
- Around line 58-68: Update the uuid override in the workspace configuration to
use major-line-scoped ranges that exclude vulnerable uuid 12.x and 13.x while
pinning each supported major line to a safe patched minimum. Preserve the
existing safe uuid@14.0.1 resolution and ensure future dependency resolution
cannot select vulnerable versions.
- Around line 18-19: Correct the advisory comment metadata in
pnpm-workspace.yaml: update fast-uri and ip-address impacts while retaining
fixed versions 3.1.5 and 10.3.1, add brace-expansion’s unbounded-array DoS
advisory fixed in 5.0.9, and replace esbuild’s invalid advisory with
GHSA-67mh-4wv8-2f99 plus the correct binary-integrity and Windows-file-read
advisories, all fixed in 0.28.1. Update joi to the recursive link() RangeError
issue fixed in 18.2.1 and undici to the cross-user disclosure/parse crash issue
fixed in 7.29.0. Replace the uuid and qs placeholders with
CVE-2026-41907/GHSA-w5hq-g745-h8pq and CVE-2026-8723/GHSA-q8mj-m7cp-5q26,
including their exact impacts and fixed versions.
In `@public/locales/eu/bundle.json`:
- Around line 1882-1883: Translate the affected AI settings strings in the
source file locales/eu/settings.json, including the keys corresponding to
settings.ai.testError.invalidResponse and settings.ai.testError.noModels plus
those at the referenced later lines. Then regenerate
public/locales/eu/bundle.json from the source; do not edit the generated bundle
directly, and report the beta-locale gaps for these changed keys.
In `@services/storage/idbAssetStore.ts`:
- Line 24: Apply assertIdbProtectedWriteAllowed() at the start of deleteImage(),
deleteBinderAsset(), and listBinderAssetIds() before any protected-store access,
preserving fail-closed behavior for locked sessions. Add tests covering
locked-state rejection for both delete paths and the list path.
In `@services/storage/idbCodexStore.ts`:
- Line 21: Extend fail-closed protection by invoking
assertIdbProtectedWriteAllowed() before every protected read and write in
deleteStoryCodex, the snapshot methods listSnapshots, getSnapshotData, and
deleteSnapshot, and the protected listing/deletion methods in idbAssetStore. Add
locked-state tests covering each guarded path, while preserving existing
behavior for unlocked sessions.
In `@services/storage/idbProjectStore.ts`:
- Around line 219-221: Guard every protected IndexedDB mutation: update
deleteProject and deleteBinderAsset to call assertIdbProtectedWriteAllowed
before opening their write object stores or deleting binder assets, reusing a
shared guarded helper where appropriate. Remove reEncryptAllAppData, or
explicitly document and authorize it as a lock-state exception.
- Around line 219-221: Make saveSlice’s encryption validation and write
operation atomic across assertIdbProtectedWriteAllowed, getObjectStore,
isIdbEncryptionReady, compressData, and store.put. Prevent clearIdbEncryptionKey
from changing encryption state between the guard and the write by using a stable
write capability or holding the existing key lock through payload preparation
and store.put, while preserving plaintext writes only when encryption was never
configured.
In `@services/storage/idbSnapshotStore.ts`:
- Around line 32-33: The saveProject snapshot flow must handle rejections from
createSnapshot, including the expected locked-write failure from
assertIdbProtectedWriteAllowed, using try/catch without silently swallowing
undocumented errors. Move the lastAutoSnapshotTime update until after
createSnapshot completes successfully so a failed snapshot does not suppress
subsequent automatic attempts.
In `@services/storage/storageEncryptionService.ts`:
- Around line 207-212: Update the asynchronous IDB callbacks in
IdbProjectStore.loadState() and IdbAssetStore.getImage() to catch failures from
idbReadSecure() and reject their enclosing Promises. Preserve successful read
handling while ensuring IdbStorageLockedError and other read failures propagate
to the storage callers instead of leaving hydration or image loading pending.
- Around line 32-51: services/storage/storageEncryptionService.ts lines 32-51:
add one physical-line QNBS-v3 rationale comment covering why
IdbStorageLockedError and IdbEncryptionMigrationRequiredError explicitly protect
locked operations and incomplete migrations. Add separate single-line QNBS-v3
rationale comments at services/storage/storageEncryptionService.ts lines
173-181, services/storage/idbAssetStore.ts lines 24 and 37, 76 and 98, and
hooks/useSettingsView.ts lines 115-117, explaining respectively the session-key
check before protected IDB access, locked checks for image and binder
writes/reads, and why the writing-surface setting must dispatch through the
settings slice.
In `@tests/unit/storage/idbStoreEncryption.test.ts`:
- Around line 181-205: Extend the locked-read regression test to instantiate and
use IdbProjectStore alongside the existing stores, save legacy plaintext project
settings and snapshot records before setupIdbEncryption, then clear the
encryption key and assert that loadSettings() and the project snapshot read
method reject with IdbStorageLockedError. Keep the existing asset and codex
assertions unchanged.
In `@tests/unit/storage/storageEncryptionService.test.ts`:
- Around line 227-236: Wrap the `initIdbEncryption` rejection and readiness
assertions in a `try` block and restore `localStorageMock.setItem` in a
`finally` block, ensuring cleanup occurs even when either assertion fails.
Preserve the existing throwing mock behavior and expectations in the test.
In `@types.ts`:
- Line 646: Backfill the required writingSurfaceStyle field in every Settings
producer: update tests/unit/languageToolClient.test.ts baseSettings(), and
normalize persisted results returned by FsSettingsStore.loadSettings() and
CloudSyncBackend.loadSettings() with the established default fallback for older
data.
---
Outside diff comments:
In `@services/storage/idbProjectStore.ts`:
- Around line 45-63: Add a single physical-line QNBS-v3 comment immediately
before the writingSurfaceStyle default or validation logic, documenting that the
setting defaults to 'textured' for older persisted settings and invalid stored
values fall back to 'textured'; leave the existing migration behavior unchanged.
---
Nitpick comments:
In `@services/storage/idbCodexStore.ts`:
- Line 21: Add a single-line QNBS-v3 comment immediately before each
assertIdbProtectedWriteAllowed() call in the affected storage methods, including
the occurrences at the referenced locations, explaining that the fail-closed
assertion enforces the protected-write storage contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2954fa05-2f5e-42d2-8d3e-b0ae43526d32
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (85)
App.tsxREADME.mdTODO.mdcomponents/settings/GeneralSections.tsxcomponents/settings/IdbUnlockModal.tsxcomponents/settings/PassphraseModal.tsxcomponents/settings/PrivacySection.tsxdocs/IDB-ENCRYPTION.mddocs/adr/0018-idb-encryption-lifecycle-and-recovery.mdfeatures/settings/settingsSlice.tshooks/useSettingsView.tsindex.csslocales/ar/help.jsonlocales/ar/settings.jsonlocales/de/help.jsonlocales/de/settings.jsonlocales/el/help.jsonlocales/el/settings.jsonlocales/en/help.jsonlocales/en/settings.jsonlocales/es/help.jsonlocales/es/settings.jsonlocales/eu/help.jsonlocales/eu/settings.jsonlocales/fa/help.jsonlocales/fa/settings.jsonlocales/fi/help.jsonlocales/fi/settings.jsonlocales/fr/help.jsonlocales/fr/settings.jsonlocales/he/help.jsonlocales/he/settings.jsonlocales/hu/help.jsonlocales/hu/settings.jsonlocales/is/help.jsonlocales/is/settings.jsonlocales/it/help.jsonlocales/it/settings.jsonlocales/ja/help.jsonlocales/ja/settings.jsonlocales/ko/help.jsonlocales/ko/settings.jsonlocales/pt/help.jsonlocales/pt/settings.jsonlocales/ru/help.jsonlocales/ru/settings.jsonlocales/sv/help.jsonlocales/sv/settings.jsonlocales/zh/help.jsonlocales/zh/settings.jsonpnpm-workspace.yamlpublic/locales/ar/bundle.jsonpublic/locales/de/bundle.jsonpublic/locales/el/bundle.jsonpublic/locales/en/bundle.jsonpublic/locales/es/bundle.jsonpublic/locales/eu/bundle.jsonpublic/locales/fa/bundle.jsonpublic/locales/fi/bundle.jsonpublic/locales/fr/bundle.jsonpublic/locales/he/bundle.jsonpublic/locales/hu/bundle.jsonpublic/locales/is/bundle.jsonpublic/locales/it/bundle.jsonpublic/locales/ja/bundle.jsonpublic/locales/ko/bundle.jsonpublic/locales/pt/bundle.jsonpublic/locales/ru/bundle.jsonpublic/locales/sv/bundle.jsonpublic/locales/zh/bundle.jsonservices/storage/idbAssetStore.tsservices/storage/idbCodexStore.tsservices/storage/idbProjectStore.tsservices/storage/idbSnapshotStore.tsservices/storage/storageEncryptionService.tstests/unit/services/storage/idbProjectStore.test.tstests/unit/settings/GeneralSections.test.tsxtests/unit/settings/IdbUnlockModal.test.tsxtests/unit/settings/PassphraseModal.test.tsxtests/unit/settings/PrivacySection.test.tsxtests/unit/settingsSlice.test.tstests/unit/storage/idbStoreEncryption.test.tstests/unit/storage/storageEncryptionService.test.tstests/unit/storageEncryptionService.test.tstypes.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
qnbs added a commit
that referenced
this pull request
Aug 11, 2026
…-recovery-journal Cascades the #335 storage-layer fixes forward into #337 (recovery journal). This branch had independently built its own, more architecturally complete fail-closed salt system (readStoredSalt/createAndPersistSalt/getOrCreateSalt/ getExistingSalt, wired correctly at every call site) plus the SecureRecordEnvelope journal-integration layer — both preserved. Conflicts resolved by: - Keeping #337's assertSecureStorageReadable() (superset: also checks assertNoActiveEncryptionMigration) in place of assertIdbProtectedWriteAllowed() wherever #337 had already applied it to a read path. - Keeping #337's typed-error salt architecture, upgraded getExistingSalt() to throw the new IdbEncryptionSaltLostError (covering both missing AND corrupted salt, catching readStoredSalt's thrown case too) instead of a generic Error. - Keeping #337's getSnapshotData missing-snapshot check (PR310-R013) alongside the promise-propagation fix. - Keeping my resolveProtectedWriteKey/idbEncryptWithKey atomic write-key capture (TOCTOU fix) and hasPassphraseSentinel's cache-only-true fix — orthogonal to #337's journal work, both merged in cleanly. - Removed a redundant double assertIdbProtectedWriteAllowed() call in deleteSnapshot introduced by the auto-merge. - Updated mocks in idbAssetStore.test.ts/idbSnapshotStore.test.ts for the new assertSecureStorageReadable export; fixed two pre-existing Biome formatting issues in protectedStoreMigration.ts/secondaryProtectedStoreAdapters.ts found while verifying the merge. - Regenerated i18n bundles and corrected README's key-count badges to the true post-merge total (2884, reflecting both branches' independent key additions). Verified: 60/60 storageEncryptionService tests, 19/19 idbStoreEncryption tests, 17/17 asset/snapshot store tests, 46/46 IdbUnlockModal/protectedStoreMigration/ normalizePersistedSettings tests all pass post-merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
qnbs added a commit
that referenced
this pull request
Aug 11, 2026
…009 to a valid disposition The performance ledger's baseline table still had #335/#336/#337 pinned to SHAs from before this session's fix passes (fa3cd98/fd7ed7c1/dda48b33) — all three have since moved (5e80aaa/2438f991/dc0b5262, the last now pushed with 0 unresolved review threads instead of "local merge pending push"). PR310-R009 and two related rows carried the interim disposition REWRITE, which the reconciliation doc's own taxonomy treats as impermissible as a final state — every other row resolves to an ADOPTED_WITH_MODIFICATIONS / SUPERSEDED_BY_BETTER_IMPLEMENTATION / NO_LONGER_APPLICABLE / RETAIN category. R009 asked for missing-store coverage plus interruption, legacy-shape, resume, and verification test cases; all five are now present as passing tests (protectedStoreMigration.test.ts's missing-adapter/checkpoint, interruption+resume, and verification-shortfall tests, plus secondaryPayloadStoreAdapter.test.ts's plaintext-to-encrypted conversion test). New PR310-R016 row consolidates the evidence with concrete test-name citations; R009 and the two related REWRITE rows now point to it instead of carrying an open-ended disposition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…unlock modal, not a dead end Two confirmed paths left a user with configured at-rest encryption unable to reach the passphrase prompt at all: 1. Cold start: when the passphrase sentinel exists but no key is active yet, dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this before mounting <App>, so App.tsx's own unlock-detection effect (which shows IdbUnlockModal) never got a chance to run — the user landed on the generic StorageErrorScreen, whose only action is "Reset Database & Reload" (destroys all local data). The bootstrap IIFE is now a named, re-invocable bootApp() function; a locked-storage catch renders a standalone IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency) and retries the full boot in place on success. No page reload is used, since the freshly-unlocked in-memory key would be lost on one. 2. Mid-session: handleLockSession() cleared the key but never opened the unlock modal and didn't block editing, so a user could keep typing while every subsequent autosave silently failed closed (generic "Auto-Save Failed" toast, no path back to unlocking short of manually reopening Settings). It now also opens the same global unlock modal (transientUiStore's isIdbUnlockOpen) that the cold-start path uses. index.tsx has no existing test harness (side-effecting module-level bootstrap, no exported units) — this fix is verified by static tracing of every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap, useTranslation, I18nProvider all confirmed Redux-free) rather than an automated test; useSettingsView.test.ts covers the Lock Session path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
settingsSlice.ts's defaultSettings.appearancePreset had drifted to 'default' on this branch, with a comment claiming first-run and legacy-rehydration defaults "must agree." That invariant doesn't hold on main: main deliberately keeps them different — defaultSettings.appearancePreset is 'sepia' (the first-run showcase theme) while normalizePersistedSettings's fallback for a missing/legacy field stays 'default' (so an existing user's old saved settings, missing the field entirely, isn't retroactively theme-shifted into a preset they never chose). This branch's rehydration path already correctly uses 'default' (services/storage/idbProjectStore.ts, unchanged); only the first-run default had regressed. This directly caused tests/e2e/a11y.spec.ts's "dark sepia theme has no serious axe violations" test to fail on #335's current SHA — the test emulates a dark color scheme on a brand-new (no persisted data) project and waits for both `.dark-theme` and `.appearance-sepia` to be applied, which now never happened since the first-run default silently stopped being sepia. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
qnbs added a commit
that referenced
this pull request
Aug 11, 2026
The prior CURRENT-HANDOFF.md (captured 2026-08-11T11:14:45Z) is now stale relative to this session's work: #335/#336/#337 all reached review-thread quiescence (except one deliberately-deferred #335 thread), two real data-loss/lockout defects were found and fixed on #335 (cold-start unlock routing, Lock Session routing), a genuine sepia-default regression was found and fixed, PR310-R009's disposition was corrected, and the performance ledger's stack SHAs were refreshed twice. Archived the prior capture alongside the existing codex handoff rather than discarding it, and wrote a new CURRENT-HANDOFF.md reflecting live state, including what's still genuinely open (the deferred pnpm thread, #310's remaining threads, #332/#333 packaged evidence, and whether #335's final E2E run landed green — a background poll was still in flight when this was written). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…range uuid: ">=11.1.1" was a bare floor that still permitted the two known- vulnerable exact releases 12.0.0 and 13.0.0 (GHSA-w5hq-g745-h8pq / CVE-2026-41907 — missing buffer bounds check in v3/v5/v6 API methods, fixed per major line at 11.1.1/12.0.1/13.0.1) if a future resolution ever landed on them; the currently-locked uuid@14.0.1 was unaffected today, but the override itself didn't structurally rule out regressing to a vulnerable version. Tightened to ">=11.1.1 <12.0.0 || >=12.0.1 <13.0.0 || >=13.0.1", excluding exactly those two releases per major line while still allowing everything else. Ran via a real `pnpm install` (not a hand-edit) so verifyDepsBeforeRun's installed-state check stays consistent; resolved version is unchanged (still 14.0.1, already satisfies the tighter range). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
qnbs
enabled auto-merge (squash)
August 12, 2026 04:44
Uh oh!
There was an error while loading. Please reload this page.
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
* fix: fail closed encrypted storage lifecycle * fix: harden desktop AI integrations * fix: resolve audited dependency vulnerabilities * fix(tauri): bound Python probes and LoRA process lifecycle * chore(deps): align pnpm v11 security policy * docs: correct README i18n key count (2869 → 2876) CI's docs:check drift gate flagged 4 stale README badges/lines still citing the pre-expansion key count; bring them in line with the current locale bundle so the Node quality gate and its downstream Build/E2E/Storybook/Lighthouse jobs stop skipping. * fix(storage): close TOCTOU/fail-closed gaps in the IDB encryption guard Addresses the #335 review-thread cluster (CodeAnt/CodeRabbit/Codex/Qodo) converging on services/storage/*Store.ts: - Atomic write-key resolution (resolveProtectedWriteKey/idbEncryptWithKey): saveSlice/createSnapshot/saveImage/saveBinderAsset/saveStoryCodex/ saveRagVectors previously checked assertIdbProtectedWriteAllowed() then re-read isIdbEncryptionReady() after an intervening await (opening the IDB transaction) — Lock Session firing in that gap silently downgraded the write to plaintext. The two checks now collapse into one snapshot. - Fail-closed guards added to every previously-unguarded destructive/listing path: deleteImage, deleteBinderAsset, listBinderAssetIds, deleteStoryCodex, listSnapshots, deleteSnapshot, deleteProject (before its binder-asset cascade); getSnapshotData gets an explicit guard for clarity. - IDBRequest.onsuccess handlers that call idbReadSecure() now catch/reject instead of producing an unhandled rejection that left loadState()/ getImage()/getSnapshotData() pending forever on a locked read. - hasPassphraseSentinel() caches its result (invalidated by clearIdbEncryptionKey()) — was previously an unconditional IDB round trip on every protected read/write, including for users who never configured encryption. - Removed reEncryptAllAppData/reEncryptAllSnapshots: confirmed dead code (zero callers on this branch or #337) per CodeRabbit's own suggested disposition; rotateIdbPassphrase already unconditionally blocks rekeying pending the journal-based implementation. - Fixed two test files whose storageEncryptionService mocks were already missing assertIdbProtectedWriteAllowed (pre-existing gap, now widened by the new guards) and added coverage for every fix above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage,i18n): correct encryption-scope claims and salt-loss fail-closed gap Addresses the remaining #335 review-thread cluster: - storageEncryptionService.ts: getOrCreateSalt() now fails closed with a new IdbEncryptionSaltLostError when the persisted PBKDF2 salt is missing/invalid but a passphrase sentinel already exists — previously it silently derived a new salt, producing a different key and permanently orphaning existing encrypted data instead of surfacing the loss. Applies to initIdbEncryption, setupIdbEncryption, verifyAndInitIdbEncryption, and the (currently unreachable) rotateKey. - README + docs/IDB-ENCRYPTION.md: corrected the false claim that Tauri desktop gets the same IndexedDB-backed at-rest encryption as the web build. Tauri's filesystem-backed store (services/fs/*) writes compressed-but- unencrypted data regardless of the passphrase/unlock state; the unlock screen appearing on desktop was misleading users about actual protection. - locales/*/help.json (en/sv/fi/hu/is/eu/ru/fa): corrected the same overclaim ("all IndexedDB stores") in the in-app feature-flag help article to match the narrower, accurate scope already used elsewhere in the file. - locales/*/settings.json (all 19): writingSurfaceHint previously described the backdrop as "texture-free," contradicting the selector's own Textured option. Reworded to apply to both options; regenerated all 19 runtime bundles (also drops a stale lora.onboarding.selectPython key that had no source definition). - components/settings/GeneralSections.tsx: added aria-pressed to the writing- surface toggle buttons so assistive technology can identify the active option, plus the required QNBS-v3 rationale comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: backfill writingSurfaceStyle in all Settings producers; correct advisory metadata; document hooks:install - services/fs/settingsFsStore.ts, services/cloudSync/cloudSyncBackend.ts: loadSettings() used an unchecked `as Settings`/generic-decrypt cast, so settings persisted before writingSurfaceStyle (or any newer required field) existed would violate the Settings contract at runtime. Both now route through the same normalizePersistedSettings() already used by the IDB path. - tests/unit/languageToolClient.test.ts: baseSettings() was missing the required writingSurfaceStyle field. - pnpm-workspace.yaml: corrected several advisory comments that misdescribed their GHSA (wrong summary text for fast-uri/ip-address/joi/undici, an invalid esbuild GHSA id, and two long-standing CVE-2024-XXXX placeholders for uuid/qs). Left override values unchanged — tightening the uuid floor needs a lockfile-only pnpm update this host can't safely run right now; documented as a follow-up rather than silently dropped. - CONTRIBUTING.md: documented `pnpm run hooks:install` as an explicit step after `pnpm install` — pnpm v11's allowBuilds policy denies simple-git-hooks' own install script, so a `prepare` lifecycle script would silently no-op; contributors who skip this step bypass lint-staged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): correct transaction-ordering regression and cross-tab sentinel-cache staleness Fresh CodeRabbit review on the previous commits caught two real bugs in my own changes: - idbProjectStore.ts saveSlice: opened the IDB transaction BEFORE encrypting the payload, reintroducing the exact TransactionInactiveError risk the sibling stores' own QNBS-v3 comments warn about (encrypt yields the event loop, which can auto-commit an already-open transaction before store.put runs). Reordered to resolve the key and encrypt first, matching idbAssetStore/idbCodexStore/idbSnapshotStore's established safe pattern. - storageEncryptionService.ts hasPassphraseSentinel(): cached `false` as well as `true`. If a passphrase is set up in another tab (or by any other runtime instance) after this one cached a negative result, the cache never invalidates — resolveProtectedWriteKey()/assertIdbProtectedWriteAllowed() would keep treating a now-configured library as never-configured and allow a plaintext write. Only `true` is cached now; a negative lookup always re-reads IDB (the fail-closed direction). Updated the two tests that asserted the old negative-caching behavior and added regression coverage for the cross-tab scenario. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(i18n): synchronize stale feature-flag counts and add missing Basque translations - locales/{en,eu,fi,hu,is}/help.json: the feature-flag help paragraph cited conflicting, stale facts — en said "22 flags, 6 opt-in" (missing enableBrowserOllama), eu/fi/hu/is said "21 flags, only RTL off" (a v1.21-era description never updated). Synchronized all five to the current 23 flags / 16 default-on / 7 opt-in-off state. - locales/eu/settings.json: the four writing-surface keys this PR introduced were left in English fallback; added Basque translations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): distinguish setup-failed from wrong-passphrase errors; add missing QNBS-v3 comment - components/settings/PassphraseModal.tsx: 'set' mode (first-time setup) has no prior passphrase to be "wrong" — any rejection there is a storage/salt failure (e.g. localStorage blocked, or the new IdbEncryptionSaltLostError). Showing "wrong passphrase" left users unable to diagnose the real problem. Added settings.privacy.encryptionSetupFailed (all 19 locales) and route 'set'-mode failures to it; 'unlock' mode keeps the existing message since it's the only mode that verifies an existing passphrase. - hooks/useSettingsView.ts: added the QNBS-v3 rationale comment CodeRabbit flagged as missing for the writingSurfaceStyle dispatch case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(desktop): fix LoRA env-report deserialization, panic risk, Python identity check, and stuck cancellation Addresses the src-tauri/src/lora.rs review cluster on #336 (5 distinct, current-code-valid findings from CodeAnt/CodeRabbit/Codex): - LoraEnvReport: scripts/check_lora_env.py never emits python_path/last_error, so every successful helper run failed to deserialize without #[serde(default)] on those two fields — every environment check reported helper_report_parse_failed, permanently hiding real Unsloth/CUDA status. - train_lora: stderr truncation sliced by raw byte index (&stderr[..stderr.len().min(500)]), which panics if byte 500 lands inside a multibyte UTF-8 character (e.g. non-ASCII paths in a Python traceback). Truncates by char now. - probe_python: validated a manually-configured interpreter path only by parsing `--version` output for a numeric string — `bash --version` parses as "5.1", which is >= the minimum and would be accepted as "Python" and later invoked with Python script arguments. Now probes actual identity via `-c "import sys; print(f'{major}.{minor}.{micro}')"`. - abort_lora_training: a confirmed StopProcess termination never cleared ACTIVE_LORA_TRAINING itself (only train_lora's own reap did, later), so an immediate retry right after a successful cancel could still see training_already_running. Now clears the slot as soon as termination is confirmed (PID-guarded, so the later reap in train_lora is a safe no-op). AlreadyStopping (retry) only polled and never re-signalled — a process that ignored the first SIGTERM stayed uncancellable forever. Now escalates to SIGKILL on Unix retries. The concurrent-training-slot race and blocking-probe findings on this same cluster are already fixed by existing code (reserve_training_slot's single-lock check-and-set, and probe_python's spawn_blocking/tokio::process usage) — replied with evidence citing current line numbers rather than re-fixing; both threads' own isOutdated flags corroborate this. Verified: rustfmt --check --edition 2021 passes (syntax-valid, correctly formatted); full cargo build not run locally per this session's resource constraints — relies on CI's Tauri build job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct Tauri arg casing and surface native diagnostics in LoRA onboarding merge_lora, generate_ollama_modelfile, and set_lora_python_path were all invoked with snake_case argument keys. Tauri's #[tauri::command] macro requires camelCase for top-level JS invoke keys by default, and none of these three commands declare rename_all = "snake_case" — so every one of these desktop LoRA actions failed argument binding before reaching Rust (confirmed against src-tauri/src/lora.rs; train_lora's nested payload fields are unaffected since struct deserialization follows its own un-renamed serde impl, not Tauri's command-arg binding). LoraOnboarding.tsx: add a request-generation guard so a slow initial checkTrainingEnvironment() can no longer overwrite a newer manual selectPythonExecutable() result. Surface env.lastError regardless of pythonAvailable — check_lora_environment can return python_available: true alongside a helper diagnostic failure (helper_spawn_failed etc.), which was previously hidden whenever Python itself was found. Map raw Rust category identifiers through an i18n lookup table instead of leaking snake_case tokens to users. Fix the focus-ring token (--sc-border-focus -> --sc-ring-focus) and add aria-busy + a dedicated loading label to the select-Python button. Adds 18 lora.onboarding.* keys across all 19 locales (real translations for de/es/fr/it, English fallback for the rest per existing i18n policy); updates README key-count badges to 2897. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct AiCreativity casing typo blocking typecheck 'balanced' isn't a valid AiCreativity value (only 'Focused' | 'Balanced' | 'Imaginative' are) — every other call site in this file already used 'Balanced'. This one typo failed tsgo --noEmit for the whole project, pre-existing and unrelated to the LoRA fix in the prior commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: classify a killed training process as aborted, not failed abort_lora_training waits for the killed child to exit before resolving, so the concurrent train_lora invocation it just killed can reject first. startTrainingThunk's catch unconditionally dispatched trainingFailed, which archives and clears currentRun before abortTrainingThunk's own trainingAborted dispatch runs — that reducer then no-ops (guarded on `if (!state.currentRun) return`), so a successful user cancellation was recorded and surfaced as a training failure. Adds TrainingRun.cancellationRequested, set synchronously by abortTrainingThunk before it awaits confirmation. startTrainingThunk's catch checks this flag and dispatches trainingAborted instead of trainingFailed when set. Correct regardless of which thunk's dispatch wins the race, since both trainingFailed/trainingAborted reducers are no-ops once currentRun is already cleared. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: sync Local AI panel when a preload finishes outside handleDownload preloadLocalModel can be triggered from outside LocalAiSection — the global download-progress modal's Retry button calls it directly via retryLastPreload. Only LocalAiSection.handleDownload updated readyIds, throughput, storage, and the ready announcement, so a successful retry left a mounted panel showing the model as not-ready with stale stats until the section unmounted and remounted. Adds subscribeLocalModelReady to localAiFacade, notified at the end of preloadLocalModel's success branch (after its own readyLocalModelIds/ lastLocalThroughput bookkeeping, so listeners never observe stale data). LocalAiSection subscribes and re-syncs on notification, guarded by a downloadingIdRef so its own handleDownload-initiated downloads (which already update this state directly) don't get double-announced — the notification fires synchronously inside preloadLocalModel, before handleDownload's own await resumes. Also fixes two unrelated pre-existing test issues found while working in these files: a stale assertion in localAiFacade.test.ts expecting generateLocalText's runLocalTextGeneration call to receive `undefined` for onProgress (it always wraps onProgress in an internal reportProgress closure so inferenceProgressEmitter gets progress unconditionally), and adds the sourcery-suggested retry-failure-surfaces-via-reportWebLlmError coverage plus direct tests for retryLastPreload-with-no-prior-preload and abort-suppresses-error-reporting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: scope the global download modal to explicit preloads only generateLocalText unconditionally drove the singleton inferenceProgressEmitter (used by the global LocalAiDownloadProgress modal) for every call, including ordinary Writer/Copilot/ProForge generation through aiProviderService.ts. A user generating text with an already-warm WebLLM model could see a "downloading a model" dialog appear, and isLocalAiBusy() report busy, for a call that wasn't downloading anything. Adds an opt-in generateLocalText option, reportToGlobalProgress, defaulting to unset/false for every existing call site except preloadLocalModel (the only function that should legitimately drive that modal). This also fixes two related "stuck in loading" gaps found in the same review pass, now that generateLocalText owns its own terminal-state responsibility for any opted-in caller: - a preload that falls back off WebLLM (ONNX/Transformers/heuristic) now resets the emitter instead of leaving it in 'loading' forever when reached from a caller other than preloadLocalModel's own outer handling (which already covered its own case via reportWebLlmError). - a caller-provided AbortSignal aborting preloadLocalModel (distinct from the modal's own Cancel button, which already reports its own terminal state) now resets the emitter instead of only suppressing the error report, which previously left the modal and isLocalAiBusy() stuck indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: guard connection-context races and the WebGPU spinner in AiProviderCard Model loading (handleLoadOllamaModels) had no staleness guard at all, and the manual-test guard (testRequestIdRef) was only bumped by an explicit second handleTest() call — editing the endpoint URL, preset, or browser-Ollama toggle while a request was in flight let its result land under the new context. AiSections.tsx worked around a related symptom (a provider switch mid-flight) with `key={provider}`, which fixed that one case by remounting the whole card — but also discarded any unsaved openaiKey/grokKey/anthropicKey input and restarted the stored-key reads. Removes the key prop. Adds useConnectionContextReset (extracted to a standalone hook to keep AiProviderCard's cognitive complexity under the CI threshold): a single effect that bumps testRequestIdRef and resets testStatus/testError/isLoadingModels whenever provider, ollamaBaseUrl, localBackendPreset, openAiCompatibleBaseUrl, or browserOllamaEnabled change — covering every case the key-based remount did, plus the ones it didn't (editing fields without switching providers), without discarding component state. handleLoadOllamaModels now captures the request id and checks it before applying results, via a small applyIfCurrent helper (also extracted for the same complexity reason). Separately: the WebGPU status badge rendered gpuInfo === null as a permanent spinner, since null was its only "loading" signal — opening the WebLLM tab without ever clicking Test Connection showed an indicator that never settles. Adds isProbingGpu to distinguish "actively probing" from "not yet tested", and changes gpuInfo's default from null to {status: 'unknown'} so both states render through the existing status-badge branches with no new conditional. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct LM Studio/vLLM/custom local-backend streaming and routing streamOpenAiCompatibleLocal (LM Studio/vLLM OpenAI-compatible local streaming, added in this PR): - Never flushed the final SSE buffer after reader.read() returned done — a server closing the stream without a trailing newline after the last data: frame silently dropped that final delta. - Broke the read loop and called onDone on an aborted signal instead of throwing, so a user-cancelled generation was recorded as a successfully completed request with partial output. - Resolved a blank ollamaBaseUrl ('') via `?? 'http://localhost:1234'` — nullish coalescing doesn't catch empty string, so an explicitly cleared URL produced an app-relative request instead of falling back to the same default testOpenAiCompatibleLocalConnection uses. - Discarded the server's JSON error body on a non-ok response, losing the actionable detail (invalid model, bad request) LM Studio/vLLM return, down to just the HTTP status code. Custom-preset protocol mismatch: listLocalBackendModels already treated the 'custom' preset as OpenAI-compatible for model discovery (the common case — hand-editing the base URL to a non-default LM Studio/vLLM port selects 'custom'), but streaming, non-streaming generation, and connection testing all routed 'custom' through the native Ollama protocol instead. A customized server could list its models successfully and then fail every completion. Extracts isOpenAiCompatibleLocalPreset as the single source of truth and uses it at all four call sites. Also threads localBackendPreset through useCriticView and useConsistencyCheckerView's AIRequestOptions builders — they already passed ollamaBaseUrl but omitted the preset, so the Critic and Consistency Checker surfaces silently fell back to Ollama-native even for the named LM Studio/vLLM presets while the main Writer surface worked correctly. sourcery flagged `signal: opts.signal ?? null` as an inconsistency; verified against LocalServerFetchInit's actual type (AbortSignal | null, not | undefined — this codebase's own wrapper, not a raw Fetch passthrough) and confirmed `?? null` is the required, correct form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: cover vLLM preset and invalid OpenAI-compatible models responses Sourcery flagged that testAIConnection's LM Studio-preset coverage had no vLLM equivalent, and testOpenAiCompatibleLocalConnection's invalid-response branch (non-array data, non-JSON body) had no direct test at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): route locked encrypted startup and Lock Session to the unlock modal, not a dead end Two confirmed paths left a user with configured at-rest encryption unable to reach the passphrase prompt at all: 1. Cold start: when the passphrase sentinel exists but no key is active yet, dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this before mounting <App>, so App.tsx's own unlock-detection effect (which shows IdbUnlockModal) never got a chance to run — the user landed on the generic StorageErrorScreen, whose only action is "Reset Database & Reload" (destroys all local data). The bootstrap IIFE is now a named, re-invocable bootApp() function; a locked-storage catch renders a standalone IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency) and retries the full boot in place on success. No page reload is used, since the freshly-unlocked in-memory key would be lost on one. 2. Mid-session: handleLockSession() cleared the key but never opened the unlock modal and didn't block editing, so a user could keep typing while every subsequent autosave silently failed closed (generic "Auto-Save Failed" toast, no path back to unlocking short of manually reopening Settings). It now also opens the same global unlock modal (transientUiStore's isIdbUnlockOpen) that the cold-start path uses. index.tsx has no existing test harness (side-effecting module-level bootstrap, no exported units) — this fix is verified by static tracing of every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap, useTranslation, I18nProvider all confirmed Redux-free) rather than an automated test; useSettingsView.test.ts covers the Lock Session path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): restore sepia as the first-run appearance default settingsSlice.ts's defaultSettings.appearancePreset had drifted to 'default' on this branch, with a comment claiming first-run and legacy-rehydration defaults "must agree." That invariant doesn't hold on main: main deliberately keeps them different — defaultSettings.appearancePreset is 'sepia' (the first-run showcase theme) while normalizePersistedSettings's fallback for a missing/legacy field stays 'default' (so an existing user's old saved settings, missing the field entirely, isn't retroactively theme-shifted into a preset they never chose). This branch's rehydration path already correctly uses 'default' (services/storage/idbProjectStore.ts, unchanged); only the first-run default had regressed. This directly caused tests/e2e/a11y.spec.ts's "dark sepia theme has no serious axe violations" test to fail on #335's current SHA — the test emulates a dark color scheme on a brand-new (no persisted data) project and waits for both `.dark-theme` and `.appearance-sepia` to be applied, which now never happened since the first-run default silently stopped being sepia. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lora,settings): stale-run training race, stuck onboarding on cancel, stale model list on context switch Three confirmed race conditions from a fresh CodeAnt review wave: - loraThunks.ts: startTrainingThunk's catch classified a killed process's rejection using the CURRENT Redux currentRun.cancellationRequested, not the run that actually produced it. abort_lora_training awaits child-process exit before resolving, so a new training run can start before the killed run's own train_lora promise finally rejects — the catch would then wrongly archive the NEWER run as failed/aborted using the OLDER run's outcome. Now guards on currentRun.id matching the runId this invocation generated; a mismatch is a stale rejection and a no-op. - LoraOnboarding.tsx: cancelling the native Python file picker resolves with null, but handleSelectPython unconditionally bumped the request-generation guard before checking the result — invalidating the still-pending initial environment check without ever applying a replacement value, since the code only calls setEnv on a truthy result. The onboarding could get stuck showing "checking environment..." forever. The guard now only advances once there's an actual new result (success or error) to apply. - AiProviderCard.tsx: useConnectionContextReset invalidated in-flight tests/model-loads on a provider/endpoint/preset change but never cleared the already-rendered ollamaModels list from the previous context, so a user could select a model id that doesn't exist on the newly selected server until a fresh load happened to complete. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings,ai): decouple URL edits from protocol preset; guard preload progress against superseded attempts AiProviderCard.tsx: editing the "Ollama Server URL" field unconditionally reassigned localBackendPreset to 'custom', which isOpenAiCompatibleLocalPreset always routes through the OpenAI-compatible /v1/chat/completions protocol. A native-Ollama user just changing host/port (e.g. pointing at a LAN server) had their protocol silently switched and every completion started failing. The URL input now only updates ollamaBaseUrl; protocol selection stays explicit via the preset dropdown. localAiFacade.ts: preloadLocalModel's own inferenceProgressEmitter calls, and the ones inside generateLocalText gated by reportToGlobalProgress, ran unconditionally. retryLastPreload() starting a new attempt while an older, cancelled/superseded attempt's generateLocalText call is still settling could let that stale attempt's reportWebLlmReady/reportWebLlmError/reset overwrite the newer attempt's modal state after the fact. Extended the existing activePreloadAbort identity-guard pattern (already used to protect the cancel hook) to every progress-report call site via a new isCurrentAttempt option. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(desktop): revalidate the cached Python interpreter before trusting it resolve_python() trusted a cached ResolvedPython for the rest of the process's lifetime once found, whether from an explicit configured path match or the auto-discovery scan — never re-checking that the executable still exists, still has its executable bit, or still resolves. If the interpreter was removed, replaced, or lost permissions while the desktop app stayed open (a long-running session), environment checks would keep reporting Python as available and training would only fail later at spawn with a confusing error. cached_python_still_valid() reuses probe_python's own lightweight, synchronous filesystem-only check (validate_python_candidate_path — no subprocess spawn, so the cache's performance benefit is preserved) before trusting either cache hit; a stale entry now falls through to a full re-probe instead of being silently believed. Added unit coverage for validate_python_candidate_path's three outcomes (relative candidate, valid absolute path, missing absolute path) since the async caching path itself isn't practically unit-testable without tokio::test infra this crate doesn't carry yet. Verified via `cargo fmt --check` (this host cannot safely run a full `cargo check`/build — no cached target/ artifacts, would be a from-scratch compile of the whole dependency tree); CI's Tauri build job is the real gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): auto-validate a newly saved Gemini key instead of deferring to the next generation call handleSaveKey only ran syntactic checks (length + control characters) before persisting and marking hasKey active — a misleading QNBS-v3 comment claimed "provider validation... determines credential validity" when no such validation actually ran at save time. A malformed or unrelated credential appeared saved successfully, and the Settings status disagreed with the actual provider authentication state until a later generation request happened to fail. Saving now automatically runs the same test-connection flow Test Connection already uses (which already flips hasKey back to false on an INVALID_API_KEY response); "Saved" no longer implies "verified." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deps): exclude the two unpatched uuid releases from the override range uuid: ">=11.1.1" was a bare floor that still permitted the two known- vulnerable exact releases 12.0.0 and 13.0.0 (GHSA-w5hq-g745-h8pq / CVE-2026-41907 — missing buffer bounds check in v3/v5/v6 API methods, fixed per major line at 11.1.1/12.0.1/13.0.1) if a future resolution ever landed on them; the currently-locked uuid@14.0.1 was unaffected today, but the override itself didn't structurally rule out regressing to a vulnerable version. Tightened to ">=11.1.1 <12.0.0 || >=12.0.1 <13.0.0 || >=13.0.1", excluding exactly those two releases per major line while still allowing everything else. Ran via a real `pnpm install` (not a hand-edit) so verifyDepsBeforeRun's installed-state check stays consistent; resolved version is unchanged (still 14.0.1, already satisfies the tighter range). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lora): clear cancellationRequested on a failed native abort; collapse wrapped QNBS-v3 comments If abortTraining() rejects, cancellationRequested stayed true forever — training may still be genuinely running, and its own later failure (dispatched by startTrainingThunk's catch) would then be misclassified as a user-requested abort instead of a real error. New trainingCancellationFailed reducer resets the marker; abortTrainingThunk dispatches it in a catch around the native call before rethrowing. Also: a fresh CodeRabbit pass (previously rate-limited/skipped on this PR) flagged 20 QNBS-v3 comments across services/lora/, features/lora/, components/settings/LocalAiSection.tsx, and their tests that wrapped across multiple physical lines, violating this repo's one-line QNBS-v3 convention — collapsed to single lines without changing their content. Regression tests: a deferred-promise version of the existing "dispatches trainingCancellationRequested before awaiting" test now actually proves the dispatch happens before the native abort settles (an immediately-resolving mock couldn't distinguish that from "after it resolved but before the next dispatch"); new test for the abort-failure path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(i18n): translate LoRA onboarding strings and fix broken shell commands in 9 locales lora.onboarding.installCmd (a literal shell command, "pip install unsloth trl peft") had been translated word-for-word in pt/sv/ko/is, producing non-functional commands (e.g. Portuguese "pip instalar sem preguiça trl peft" — "unsloth" mistranslated as "without laziness"). Restored to the exact original command in all four. Separately, all 18 lora.onboarding.error.* / selectingPython keys (added alongside the native Python environment-check work) were still raw English in the he/hu/is/ja/ko/pt/ru/sv/zh source locale trees — never translated at all, not just a stale-bundle issue. Translated all 18 keys across all 9 locales; also fixed two pre-existing partial translations noticed in the same section (ja and zh's lora.onboarding.description mixed English mid-sentence). Regenerated all 19 runtime bundles via i18n:bundle; key parity confirmed at 2903 keys per locale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lora): only classify training as aborted on a confirmed native cancellation abort_lora_training resolved Ok(()) for NothingRunning/CancelPendingStart just like a confirmed process kill, so a coincidental unrelated training failure racing with a no-op abort request could be misclassified as user-cancelled. The Rust command now returns whether it actually confirmed stopping a process; abortTrainingThunk only dispatches trainingAborted() when true, and resets cancellationRequested (via the renamed trainingCancellationNotConfirmed) on both a native error and a no-op success. Also fixes an unnatural Swedish timeout translation to match the established wording pattern. * fix(lora,ai): distinguish pending-start cancellation from a true abort no-op abort_lora_training's wave-3 boolean return conflated CancelPendingStart (a genuine recorded cancellation for a not-yet-spawned run) with NothingRunning (a true no-op) — cancelling during Python discovery cleared cancellationRequested before the pending train_lora invocation's own training_cancelled rejection arrived, misclassifying a successful startup cancel as a training failure. The command now returns a three-way AbortOutcome; the thunk only clears the flag for NothingToCancel and leaves it set for PendingStartCancelled. Also: cached_python_still_valid now re-runs the full identity probe instead of a filesystem-only check, so an interpreter replaced in place at the same path is no longer trusted with a stale cached version. Fixes a stale-response race in AiProviderCard's WebGPU probe (missing the same testRequestIdRef guard used by every other async result in handleTest) and a CRLF edge case in the local OpenAI-compatible SSE parser. Collapses two wrapped QNBS-v3 comments. * fix(settings): restore the WebGPU auto-probe lost when the ollama auto-test was removed A prior commit removed a combined useEffect that auto-tested ollama AND auto-probed WebGPU capability whenever the provider was selected. The ollama removal was intentional (CORS/privacy — avoid auto-fetching a local server from the PWA), but WebGPU detection is a local hardware check with no such concern, so dropping it as collateral damage left the GPU badge stuck on "untested" until the user clicked Test Connection. This was never caught because #336 never ran its full test suite under CI while stacked — tests/unit/settings/ WebLlmPanel.test.tsx already asserted the auto-probe behavior and was silently broken. Restores a guarded auto-probe effect (reusing the same applyIfCurrent staleness guard as the manual Test Connection click) and updates the one AiProviderCard.test.tsx test whose premise no longer holds. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
…ll-CI wave Records: two admin-authorized merges (#335, #336) with two stacked-PR auto-close recoveries; the second CodeRabbit review wave on #337 (now based on main) that found and fixed real bugs plus the major discovery that the entire encryption migration journal system has zero production callers.
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
* fix: fail closed encrypted storage lifecycle
* fix: harden desktop AI integrations
* feat: add durable encryption migration journal
* docs: clarify encryption journal rollout
* fix: expose local provider diagnostics
* fix: resolve audited dependency vulnerabilities
* fix: resolve audited dependency vulnerabilities
* fix: make encryption journal updates atomic
* fix: enforce pnpm v11 build-script policy
* docs: reconcile PR 310 migration work
* feat(storage): add secure record envelope primitives
* feat(storage): checkpoint resumable migration batches
* chore: harden dependency execution policy
* feat(storage): add resumable secondary store adapters
* fix(storage): fail closed during migration reads
* fix(storage): harden migration recovery protocol
* fix(deps): reconcile release-age lockfile
* fix(storage): harden migration recovery protocol
* fix(tauri): bound Python probes and LoRA process lifecycle
* chore(deps): align pnpm v11 security policy
* docs: add desktop performance evidence ledger
* docs: capture current agent handoff state
* docs: correct README i18n key count (2869 → 2876)
CI's docs:check drift gate flagged 4 stale README badges/lines still
citing the pre-expansion key count; bring them in line with the
current locale bundle so the Node quality gate and its downstream
Build/E2E/Storybook/Lighthouse jobs stop skipping.
* fix(storage): close TOCTOU/fail-closed gaps in the IDB encryption guard
Addresses the #335 review-thread cluster (CodeAnt/CodeRabbit/Codex/Qodo)
converging on services/storage/*Store.ts:
- Atomic write-key resolution (resolveProtectedWriteKey/idbEncryptWithKey):
saveSlice/createSnapshot/saveImage/saveBinderAsset/saveStoryCodex/
saveRagVectors previously checked assertIdbProtectedWriteAllowed() then
re-read isIdbEncryptionReady() after an intervening await (opening the IDB
transaction) — Lock Session firing in that gap silently downgraded the
write to plaintext. The two checks now collapse into one snapshot.
- Fail-closed guards added to every previously-unguarded destructive/listing
path: deleteImage, deleteBinderAsset, listBinderAssetIds, deleteStoryCodex,
listSnapshots, deleteSnapshot, deleteProject (before its binder-asset
cascade); getSnapshotData gets an explicit guard for clarity.
- IDBRequest.onsuccess handlers that call idbReadSecure() now catch/reject
instead of producing an unhandled rejection that left loadState()/
getImage()/getSnapshotData() pending forever on a locked read.
- hasPassphraseSentinel() caches its result (invalidated by
clearIdbEncryptionKey()) — was previously an unconditional IDB round trip
on every protected read/write, including for users who never configured
encryption.
- Removed reEncryptAllAppData/reEncryptAllSnapshots: confirmed dead code
(zero callers on this branch or #337) per CodeRabbit's own suggested
disposition; rotateIdbPassphrase already unconditionally blocks rekeying
pending the journal-based implementation.
- Fixed two test files whose storageEncryptionService mocks were already
missing assertIdbProtectedWriteAllowed (pre-existing gap, now widened by
the new guards) and added coverage for every fix above.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage,i18n): correct encryption-scope claims and salt-loss fail-closed gap
Addresses the remaining #335 review-thread cluster:
- storageEncryptionService.ts: getOrCreateSalt() now fails closed with a new
IdbEncryptionSaltLostError when the persisted PBKDF2 salt is missing/invalid
but a passphrase sentinel already exists — previously it silently derived a
new salt, producing a different key and permanently orphaning existing
encrypted data instead of surfacing the loss. Applies to initIdbEncryption,
setupIdbEncryption, verifyAndInitIdbEncryption, and the (currently
unreachable) rotateKey.
- README + docs/IDB-ENCRYPTION.md: corrected the false claim that Tauri
desktop gets the same IndexedDB-backed at-rest encryption as the web build.
Tauri's filesystem-backed store (services/fs/*) writes compressed-but-
unencrypted data regardless of the passphrase/unlock state; the unlock
screen appearing on desktop was misleading users about actual protection.
- locales/*/help.json (en/sv/fi/hu/is/eu/ru/fa): corrected the same
overclaim ("all IndexedDB stores") in the in-app feature-flag help article
to match the narrower, accurate scope already used elsewhere in the file.
- locales/*/settings.json (all 19): writingSurfaceHint previously described
the backdrop as "texture-free," contradicting the selector's own Textured
option. Reworded to apply to both options; regenerated all 19 runtime
bundles (also drops a stale lora.onboarding.selectPython key that had no
source definition).
- components/settings/GeneralSections.tsx: added aria-pressed to the writing-
surface toggle buttons so assistive technology can identify the active
option, plus the required QNBS-v3 rationale comment.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: backfill writingSurfaceStyle in all Settings producers; correct advisory metadata; document hooks:install
- services/fs/settingsFsStore.ts, services/cloudSync/cloudSyncBackend.ts:
loadSettings() used an unchecked `as Settings`/generic-decrypt cast, so
settings persisted before writingSurfaceStyle (or any newer required field)
existed would violate the Settings contract at runtime. Both now route
through the same normalizePersistedSettings() already used by the IDB path.
- tests/unit/languageToolClient.test.ts: baseSettings() was missing the
required writingSurfaceStyle field.
- pnpm-workspace.yaml: corrected several advisory comments that misdescribed
their GHSA (wrong summary text for fast-uri/ip-address/joi/undici, an
invalid esbuild GHSA id, and two long-standing CVE-2024-XXXX placeholders
for uuid/qs). Left override values unchanged — tightening the uuid floor
needs a lockfile-only pnpm update this host can't safely run right now;
documented as a follow-up rather than silently dropped.
- CONTRIBUTING.md: documented `pnpm run hooks:install` as an explicit step
after `pnpm install` — pnpm v11's allowBuilds policy denies
simple-git-hooks' own install script, so a `prepare` lifecycle script
would silently no-op; contributors who skip this step bypass lint-staged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage): correct transaction-ordering regression and cross-tab sentinel-cache staleness
Fresh CodeRabbit review on the previous commits caught two real bugs in my
own changes:
- idbProjectStore.ts saveSlice: opened the IDB transaction BEFORE encrypting
the payload, reintroducing the exact TransactionInactiveError risk the
sibling stores' own QNBS-v3 comments warn about (encrypt yields the event
loop, which can auto-commit an already-open transaction before store.put
runs). Reordered to resolve the key and encrypt first, matching
idbAssetStore/idbCodexStore/idbSnapshotStore's established safe pattern.
- storageEncryptionService.ts hasPassphraseSentinel(): cached `false` as well
as `true`. If a passphrase is set up in another tab (or by any other
runtime instance) after this one cached a negative result, the cache never
invalidates — resolveProtectedWriteKey()/assertIdbProtectedWriteAllowed()
would keep treating a now-configured library as never-configured and allow
a plaintext write. Only `true` is cached now; a negative lookup always
re-reads IDB (the fail-closed direction). Updated the two tests that
asserted the old negative-caching behavior and added regression coverage
for the cross-tab scenario.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(i18n): synchronize stale feature-flag counts and add missing Basque translations
- locales/{en,eu,fi,hu,is}/help.json: the feature-flag help paragraph cited
conflicting, stale facts — en said "22 flags, 6 opt-in" (missing
enableBrowserOllama), eu/fi/hu/is said "21 flags, only RTL off" (a v1.21-era
description never updated). Synchronized all five to the current 23
flags / 16 default-on / 7 opt-in-off state.
- locales/eu/settings.json: the four writing-surface keys this PR introduced
were left in English fallback; added Basque translations.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(settings): distinguish setup-failed from wrong-passphrase errors; add missing QNBS-v3 comment
- components/settings/PassphraseModal.tsx: 'set' mode (first-time setup) has
no prior passphrase to be "wrong" — any rejection there is a storage/salt
failure (e.g. localStorage blocked, or the new IdbEncryptionSaltLostError).
Showing "wrong passphrase" left users unable to diagnose the real problem.
Added settings.privacy.encryptionSetupFailed (all 19 locales) and route
'set'-mode failures to it; 'unlock' mode keeps the existing message since
it's the only mode that verifies an existing passphrase.
- hooks/useSettingsView.ts: added the QNBS-v3 rationale comment CodeRabbit
flagged as missing for the writingSurfaceStyle dispatch case.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(desktop): fix LoRA env-report deserialization, panic risk, Python identity check, and stuck cancellation
Addresses the src-tauri/src/lora.rs review cluster on #336 (5 distinct,
current-code-valid findings from CodeAnt/CodeRabbit/Codex):
- LoraEnvReport: scripts/check_lora_env.py never emits python_path/last_error,
so every successful helper run failed to deserialize without #[serde(default)]
on those two fields — every environment check reported
helper_report_parse_failed, permanently hiding real Unsloth/CUDA status.
- train_lora: stderr truncation sliced by raw byte index
(&stderr[..stderr.len().min(500)]), which panics if byte 500 lands inside a
multibyte UTF-8 character (e.g. non-ASCII paths in a Python traceback).
Truncates by char now.
- probe_python: validated a manually-configured interpreter path only by
parsing `--version` output for a numeric string — `bash --version` parses
as "5.1", which is >= the minimum and would be accepted as "Python" and
later invoked with Python script arguments. Now probes actual identity via
`-c "import sys; print(f'{major}.{minor}.{micro}')"`.
- abort_lora_training: a confirmed StopProcess termination never cleared
ACTIVE_LORA_TRAINING itself (only train_lora's own reap did, later), so an
immediate retry right after a successful cancel could still see
training_already_running. Now clears the slot as soon as termination is
confirmed (PID-guarded, so the later reap in train_lora is a safe no-op).
AlreadyStopping (retry) only polled and never re-signalled — a process that
ignored the first SIGTERM stayed uncancellable forever. Now escalates to
SIGKILL on Unix retries.
The concurrent-training-slot race and blocking-probe findings on this same
cluster are already fixed by existing code (reserve_training_slot's
single-lock check-and-set, and probe_python's spawn_blocking/tokio::process
usage) — replied with evidence citing current line numbers rather than
re-fixing; both threads' own isOutdated flags corroborate this.
Verified: rustfmt --check --edition 2021 passes (syntax-valid, correctly
formatted); full cargo build not run locally per this session's resource
constraints — relies on CI's Tauri build job.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: correct Tauri arg casing and surface native diagnostics in LoRA onboarding
merge_lora, generate_ollama_modelfile, and set_lora_python_path were all
invoked with snake_case argument keys. Tauri's #[tauri::command] macro
requires camelCase for top-level JS invoke keys by default, and none of
these three commands declare rename_all = "snake_case" — so every one of
these desktop LoRA actions failed argument binding before reaching Rust
(confirmed against src-tauri/src/lora.rs; train_lora's nested payload
fields are unaffected since struct deserialization follows its own
un-renamed serde impl, not Tauri's command-arg binding).
LoraOnboarding.tsx: add a request-generation guard so a slow initial
checkTrainingEnvironment() can no longer overwrite a newer manual
selectPythonExecutable() result. Surface env.lastError regardless of
pythonAvailable — check_lora_environment can return python_available:
true alongside a helper diagnostic failure (helper_spawn_failed etc.),
which was previously hidden whenever Python itself was found. Map raw
Rust category identifiers through an i18n lookup table instead of
leaking snake_case tokens to users. Fix the focus-ring token
(--sc-border-focus -> --sc-ring-focus) and add aria-busy + a dedicated
loading label to the select-Python button.
Adds 18 lora.onboarding.* keys across all 19 locales (real translations
for de/es/fr/it, English fallback for the rest per existing i18n
policy); updates README key-count badges to 2897.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: correct AiCreativity casing typo blocking typecheck
'balanced' isn't a valid AiCreativity value (only 'Focused' | 'Balanced'
| 'Imaginative' are) — every other call site in this file already used
'Balanced'. This one typo failed tsgo --noEmit for the whole project,
pre-existing and unrelated to the LoRA fix in the prior commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: classify a killed training process as aborted, not failed
abort_lora_training waits for the killed child to exit before
resolving, so the concurrent train_lora invocation it just killed can
reject first. startTrainingThunk's catch unconditionally dispatched
trainingFailed, which archives and clears currentRun before
abortTrainingThunk's own trainingAborted dispatch runs — that reducer
then no-ops (guarded on `if (!state.currentRun) return`), so a
successful user cancellation was recorded and surfaced as a training
failure.
Adds TrainingRun.cancellationRequested, set synchronously by
abortTrainingThunk before it awaits confirmation. startTrainingThunk's
catch checks this flag and dispatches trainingAborted instead of
trainingFailed when set. Correct regardless of which thunk's dispatch
wins the race, since both trainingFailed/trainingAborted reducers are
no-ops once currentRun is already cleared.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: sync Local AI panel when a preload finishes outside handleDownload
preloadLocalModel can be triggered from outside LocalAiSection — the
global download-progress modal's Retry button calls it directly via
retryLastPreload. Only LocalAiSection.handleDownload updated readyIds,
throughput, storage, and the ready announcement, so a successful retry
left a mounted panel showing the model as not-ready with stale stats
until the section unmounted and remounted.
Adds subscribeLocalModelReady to localAiFacade, notified at the end of
preloadLocalModel's success branch (after its own readyLocalModelIds/
lastLocalThroughput bookkeeping, so listeners never observe stale
data). LocalAiSection subscribes and re-syncs on notification, guarded
by a downloadingIdRef so its own handleDownload-initiated downloads
(which already update this state directly) don't get double-announced
— the notification fires synchronously inside preloadLocalModel,
before handleDownload's own await resumes.
Also fixes two unrelated pre-existing test issues found while working
in these files: a stale assertion in localAiFacade.test.ts expecting
generateLocalText's runLocalTextGeneration call to receive `undefined`
for onProgress (it always wraps onProgress in an internal reportProgress
closure so inferenceProgressEmitter gets progress unconditionally), and
adds the sourcery-suggested retry-failure-surfaces-via-reportWebLlmError
coverage plus direct tests for retryLastPreload-with-no-prior-preload
and abort-suppresses-error-reporting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: scope the global download modal to explicit preloads only
generateLocalText unconditionally drove the singleton
inferenceProgressEmitter (used by the global LocalAiDownloadProgress
modal) for every call, including ordinary Writer/Copilot/ProForge
generation through aiProviderService.ts. A user generating text with
an already-warm WebLLM model could see a "downloading a model" dialog
appear, and isLocalAiBusy() report busy, for a call that wasn't
downloading anything.
Adds an opt-in generateLocalText option, reportToGlobalProgress,
defaulting to unset/false for every existing call site except
preloadLocalModel (the only function that should legitimately drive
that modal). This also fixes two related "stuck in loading" gaps
found in the same review pass, now that generateLocalText owns its own
terminal-state responsibility for any opted-in caller:
- a preload that falls back off WebLLM (ONNX/Transformers/heuristic)
now resets the emitter instead of leaving it in 'loading' forever
when reached from a caller other than preloadLocalModel's own outer
handling (which already covered its own case via reportWebLlmError).
- a caller-provided AbortSignal aborting preloadLocalModel (distinct
from the modal's own Cancel button, which already reports its own
terminal state) now resets the emitter instead of only suppressing
the error report, which previously left the modal and
isLocalAiBusy() stuck indefinitely.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: guard connection-context races and the WebGPU spinner in AiProviderCard
Model loading (handleLoadOllamaModels) had no staleness guard at all,
and the manual-test guard (testRequestIdRef) was only bumped by an
explicit second handleTest() call — editing the endpoint URL, preset,
or browser-Ollama toggle while a request was in flight let its result
land under the new context. AiSections.tsx worked around a related
symptom (a provider switch mid-flight) with `key={provider}`, which
fixed that one case by remounting the whole card — but also discarded
any unsaved openaiKey/grokKey/anthropicKey input and restarted the
stored-key reads.
Removes the key prop. Adds useConnectionContextReset (extracted to a
standalone hook to keep AiProviderCard's cognitive complexity under
the CI threshold): a single effect that bumps testRequestIdRef and
resets testStatus/testError/isLoadingModels whenever provider,
ollamaBaseUrl, localBackendPreset, openAiCompatibleBaseUrl, or
browserOllamaEnabled change — covering every case the key-based
remount did, plus the ones it didn't (editing fields without switching
providers), without discarding component state. handleLoadOllamaModels
now captures the request id and checks it before applying results,
via a small applyIfCurrent helper (also extracted for the same
complexity reason).
Separately: the WebGPU status badge rendered gpuInfo === null as a
permanent spinner, since null was its only "loading" signal — opening
the WebLLM tab without ever clicking Test Connection showed an
indicator that never settles. Adds isProbingGpu to distinguish
"actively probing" from "not yet tested", and changes gpuInfo's default
from null to {status: 'unknown'} so both states render through the
existing status-badge branches with no new conditional.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: correct LM Studio/vLLM/custom local-backend streaming and routing
streamOpenAiCompatibleLocal (LM Studio/vLLM OpenAI-compatible local
streaming, added in this PR):
- Never flushed the final SSE buffer after reader.read() returned
done — a server closing the stream without a trailing newline after
the last data: frame silently dropped that final delta.
- Broke the read loop and called onDone on an aborted signal instead
of throwing, so a user-cancelled generation was recorded as a
successfully completed request with partial output.
- Resolved a blank ollamaBaseUrl ('') via `?? 'http://localhost:1234'`
— nullish coalescing doesn't catch empty string, so an explicitly
cleared URL produced an app-relative request instead of falling
back to the same default testOpenAiCompatibleLocalConnection uses.
- Discarded the server's JSON error body on a non-ok response, losing
the actionable detail (invalid model, bad request) LM Studio/vLLM
return, down to just the HTTP status code.
Custom-preset protocol mismatch: listLocalBackendModels already
treated the 'custom' preset as OpenAI-compatible for model discovery
(the common case — hand-editing the base URL to a non-default LM
Studio/vLLM port selects 'custom'), but streaming, non-streaming
generation, and connection testing all routed 'custom' through the
native Ollama protocol instead. A customized server could list its
models successfully and then fail every completion. Extracts
isOpenAiCompatibleLocalPreset as the single source of truth and uses
it at all four call sites. Also threads localBackendPreset through
useCriticView and useConsistencyCheckerView's AIRequestOptions
builders — they already passed ollamaBaseUrl but omitted the preset,
so the Critic and Consistency Checker surfaces silently fell back to
Ollama-native even for the named LM Studio/vLLM presets while the
main Writer surface worked correctly.
sourcery flagged `signal: opts.signal ?? null` as an inconsistency;
verified against LocalServerFetchInit's actual type (AbortSignal |
null, not | undefined — this codebase's own wrapper, not a raw Fetch
passthrough) and confirmed `?? null` is the required, correct form.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* test: cover vLLM preset and invalid OpenAI-compatible models responses
Sourcery flagged that testAIConnection's LM Studio-preset coverage had
no vLLM equivalent, and testOpenAiCompatibleLocalConnection's
invalid-response branch (non-array data, non-JSON body) had no direct
test at all.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: resolve tsgo narrowing issue in AiProviderCard's stale-context test
resolveTest (let, reassigned inside a Promise executor closure) was
narrowed to never by tsgo after the awaits earlier in the test,
following the merge of fix/desktop-reliability-hardening into this
branch. An explicit type annotation on the captured const sidesteps
the incorrect inference.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: enforce legal phase transitions and close the checkpoint cursor-stall gap
saveIfCurrent (the shared CAS write path for both
updateEncryptionMigrationJournal and completeEncryptionMigration) only
validated revision/owner match — it never validated that a phase
TRANSITION was a legal state-machine step. Any caller could pass a
journal still 'prepared' with {phase: 'committing'} (or call
completeEncryptionMigration on it directly) and skip conversion and
verification entirely; assertNoActiveEncryptionMigration() would then
treat the unconverted data as safe for ordinary access. Confirmed
exploitable via the codebase's own test helper (markJournalCommitting),
which jumped prepared straight to committing purely for test setup
convenience — exactly the shape of the described attack.
Adds an allowed-transition table enforced inside saveIfCurrent against
the durably-stored phase (not the caller-supplied one), with same-phase
entries permitted for the repeated per-batch/per-store checkpoint
writes within migrating/verifying, and recovery-required reachable
from any active phase but never left except via out-of-band recovery.
Updates the test helper to route through the legal
prepared→migrating→verifying→committing chain instead of skipping it.
Separately, protectedStoreMigration.ts's nextCheckpoint accepted a
nonterminal batch reporting progress without an advanced cursor,
silently retaining the previous cursor — the next iteration would
replay the same records forever, inflating `processed` without ever
completing. Now rejected explicitly.
Also fixes five pre-existing typecheck errors uncovered while working
in these files (unrelated to the above): a narrowing gap in
parseJournal's ownerLeaseExpiresAt validation, four index-signature
property accesses in storageEncryptionService.ts, one noUncheckedIndexedAccess
gap in a test, and four exactOptionalPropertyTypes violations in
secondaryPayloadStoreAdapter.test.ts.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: gate protected-store writes against an active migration
saveSlice, saveImage, saveBinderAsset, saveStoryCodex, saveRagVectors,
and createSnapshot all resolved the write key and wrote directly —
none of them called assertIdbProtectedWriteAllowed() first, unlike
their sibling delete/read methods (deleteImage, deleteBinderAsset,
deleteStoryCodex, getSnapshotData, etc.), which already had it. An
ordinary write could therefore proceed while a migration journal owned
the store's encryption-lifecycle state, racing the migration's own
conversion of that same data. Confirmed via a pre-existing, previously
failing test (encryptionMigrationJournal.test.ts's "rejects a
competing migration owner..." — IdbProjectStore().saveSettings() was
expected to reject during an active migration but silently resolved).
Also fixes two related gaps found while auditing this write path:
- deleteAllBinderAssetsForProject deleted assets one transaction per
asset — a later failure (or the migration guard starting to reject
mid-loop) left earlier assets permanently removed while the project
record and later assets survived, with no rollback. Batches every
delete into one transaction so a failure aborts the whole batch.
- The scene-revision and inference-cache secondary-store adapters
used a record's `id`/`key` field as the migration cursor without
validating it's actually a string, even though IndexedDB key paths
permit numeric keys. An unvalidated numeric key would still
successfully rewrite the payload but then persist a non-string
journal cursor, which parseJournal() rejects on the next read —
pushing the whole migration into recovery-required with a confusing
root cause. Now fails fast with a specific error at the point of use.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage): skip corrupt scene revisions instead of hiding history; narrow write guard to migration-only
listRevisions now logs and skips a single damaged/unparseable revision instead
of rejecting the whole call, so one corrupt record no longer hides an entire
section's readable history (a genuine lock-state change still aborts the call).
The TOCTOU guard reordering from the previous commit re-ran the full
assertIdbProtectedWriteAllowed() (migration + lock check) immediately before
each write's transaction opens. That double-checks the lock state that
resolveProtectedWriteKey() already captured atomically with the encryption
key, so a write that safely encrypted with a validly-resolved key was wrongly
rejected if the session locked in between (caught by an existing regression
test: "saveSlice still encrypts even when the key is cleared right after the
write key is resolved"). The six write methods (saveSlice, saveImage,
saveBinderAsset, saveStoryCodex, saveRagVectors, createSnapshot) now re-check
only assertNoActiveEncryptionMigration() pre-write, newly re-exported from
storageEncryptionService for this narrower use; delete-only methods that never
resolve a write key keep the full guard since they have no earlier lock check.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage): fail migration to recovery-required on a verification shortfall
runProtectedStoreMigration's verifying phase threw a plain
ProtectedStoreMigrationAdapterError when an adapter's verify() found fewer
valid records than this saga already migrated (most likely because an
ordinary write landed on an already-migrated record using a superseded key
after that store's migrating pass finished). The catch block only released
the ownership lease and re-threw, leaving the journal parked at 'verifying'
with nothing to distinguish this from a transient interruption — a caller
that retries keeps re-running the same doomed verify() call forever, since
nothing in the saga can revisit and reconvert the stray record.
Introduces ProtectedStoreVerificationShortfallError, a distinguishable
subtype thrown only for this specific verified-count shortfall (not for a
transient/interrupted verify() exception, which must still support the
existing "resume verification on retry" path). On this specific error the
catch block now transitions the journal to recovery-required before
releasing ownership, converting an invisible infinite retry loop into an
explicit, visible state that requires the same out-of-band recovery
procedure already used elsewhere in this journal.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ai): degrade cache reads to a miss on lock/migration, re-encrypt legacy entries on read
getCachedInference() called assertSecureStorageReadable() unguarded, so a
locked library, an active migration, or the check's own IDB access failing
would reject the call instead of degrading to a miss — breaking this cache's
documented non-authoritative, best-effort contract (setCachedInference
already had this protection via its own try/catch). The lifecycle check is
now wrapped so any failure returns null instead of propagating.
Legacy plaintext cache entries were decoded and returned correctly but never
rewritten, so a cache populated before encryption was enabled stayed
plaintext in IndexedDB for the remainder of its 7-day TTL even after the
library was unlocked with an active key. decodeEntry() now checks
readSecureRecordPayload's `needsMigration` flag (the same signal the
migration adapters already use) and best-effort re-persists the entry
encrypted, preserving its original timestamp so TTL/LRU ordering is
unaffected; a failure (e.g. a concurrent migration) is swallowed since this
cache is explicitly non-authoritative.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ollama): report invalidResponse instead of a false-positive connection success
testOllamaConnection() caught a JSON parse failure by setting payload to null
and then computed an empty model list from it, so a 200 response with an
unparseable body (e.g. a proxy login page) or valid JSON missing the models
array was reported as ok:true with zero models — a false-positive success,
inconsistent with the OpenAI-compatible diagnostic path
(testOpenAiCompatibleLocalConnection), which already returns its existing
'invalidResponse' kind for the same two cases. Ollama's own
TestConnectionErrorKind union gains that same kind (already i18n-mapped via
settings.ai.testError.invalidResponse, shared across providers) and both
failure paths now return it explicitly instead of falling through to a
misleading success. A validly-shaped but genuinely empty models array (a
fresh Ollama install with nothing pulled yet) still reports ok:true, since
that is not an error.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(test): clarify encryptionMigrationJournal.test.ts's ownership-CAS comment
The file-header comment said "independent module-owner CAS behavior," which
reads as claiming cross-tab/reloaded-module coverage. The actual tests
(competing owner rejection, delayed-owner rejection, crashed-lease recovery)
all exercise ownerId-scoped CAS rejection within the same imported module
instance — reworded to say exactly that.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(settings): stop rendering the connection test result twice in AiProviderCard
ProviderConnectionStatus (the status panel above the provider picker) already
renders the status badge, testError text, and diagnostic panel for every
provider. The action row further down duplicated the exact same testError
string and a separate success message next to the Test Connection button, so
one test produced the identical error text twice on screen (and a redundant
success message). The action row now renders only the button; the status
panel is the single source of truth. Existing tests that asserted on the
duplicate via getAllByText(...).length now assert a single instance via
getByText, and the stale "renders in two places" comment is removed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: refresh stack SHAs in the performance ledger; reconcile PR310-R009 to a valid disposition
The performance ledger's baseline table still had #335/#336/#337 pinned to
SHAs from before this session's fix passes (fa3cd983/fd7ed7c1/dda48b33) — all
three have since moved (5e80aaa4/2438f991/dc0b5262, the last now pushed with
0 unresolved review threads instead of "local merge pending push").
PR310-R009 and two related rows carried the interim disposition REWRITE,
which the reconciliation doc's own taxonomy treats as impermissible as a
final state — every other row resolves to an ADOPTED_WITH_MODIFICATIONS /
SUPERSEDED_BY_BETTER_IMPLEMENTATION / NO_LONGER_APPLICABLE / RETAIN category.
R009 asked for missing-store coverage plus interruption, legacy-shape,
resume, and verification test cases; all five are now present as passing
tests (protectedStoreMigration.test.ts's missing-adapter/checkpoint,
interruption+resume, and verification-shortfall tests, plus
secondaryPayloadStoreAdapter.test.ts's plaintext-to-encrypted conversion
test). New PR310-R016 row consolidates the evidence with concrete test-name
citations; R009 and the two related REWRITE rows now point to it instead of
carrying an open-ended disposition.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage): route locked encrypted startup and Lock Session to the unlock modal, not a dead end
Two confirmed paths left a user with configured at-rest encryption unable to
reach the passphrase prompt at all:
1. Cold start: when the passphrase sentinel exists but no key is active yet,
dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this
before mounting <App>, so App.tsx's own unlock-detection effect (which
shows IdbUnlockModal) never got a chance to run — the user landed on the
generic StorageErrorScreen, whose only action is "Reset Database & Reload"
(destroys all local data). The bootstrap IIFE is now a named, re-invocable
bootApp() function; a locked-storage catch renders a standalone
IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency)
and retries the full boot in place on success. No page reload is used,
since the freshly-unlocked in-memory key would be lost on one.
2. Mid-session: handleLockSession() cleared the key but never opened the
unlock modal and didn't block editing, so a user could keep typing while
every subsequent autosave silently failed closed (generic "Auto-Save
Failed" toast, no path back to unlocking short of manually reopening
Settings). It now also opens the same global unlock modal
(transientUiStore's isIdbUnlockOpen) that the cold-start path uses.
index.tsx has no existing test harness (side-effecting module-level
bootstrap, no exported units) — this fix is verified by static tracing of
every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap,
useTranslation, I18nProvider all confirmed Redux-free) rather than an
automated test; useSettingsView.test.ts covers the Lock Session path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(storage): route locked encrypted startup and Lock Session to the unlock modal, not a dead end
Two confirmed paths left a user with configured at-rest encryption unable to
reach the passphrase prompt at all:
1. Cold start: when the passphrase sentinel exists but no key is active yet,
dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this
before mounting <App>, so App.tsx's own unlock-detection effect (which
shows IdbUnlockModal) never got a chance to run — the user landed on the
generic StorageErrorScreen, whose only action is "Reset Database & Reload"
(destroys all local data). The bootstrap IIFE is now a named, re-invocable
bootApp() function; a locked-storage catch renders a standalone
IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency)
and retries the full boot in place on success. No page reload is used,
since the freshly-unlocked in-memory key would be lost on one.
2. Mid-session: handleLockSession() cleared the key but never opened the
unlock modal and didn't block editing, so a user could keep typing while
every subsequent autosave silently failed closed (generic "Auto-Save
Failed" toast, no path back to unlocking short of manually reopening
Settings). It now also opens the same global unlock modal
(transientUiStore's isIdbUnlockOpen) that the cold-start path uses.
index.tsx has no existing test harness (side-effecting module-level
bootstrap, no exported units) — this fix is verified by static tracing of
every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap,
useTranslation, I18nProvider all confirmed Redux-free) rather than an
automated test; useSettingsView.test.ts covers the Lock Session path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(settings): restore sepia as the first-run appearance default
settingsSlice.ts's defaultSettings.appearancePreset had drifted to 'default'
on this branch, with a comment claiming first-run and legacy-rehydration
defaults "must agree." That invariant doesn't hold on main: main deliberately
keeps them different — defaultSettings.appearancePreset is 'sepia' (the
first-run showcase theme) while normalizePersistedSettings's fallback for a
missing/legacy field stays 'default' (so an existing user's old saved
settings, missing the field entirely, isn't retroactively theme-shifted into
a preset they never chose). This branch's rehydration path already correctly
uses 'default' (services/storage/idbProjectStore.ts, unchanged); only the
first-run default had regressed.
This directly caused tests/e2e/a11y.spec.ts's "dark sepia theme has no
serious axe violations" test to fail on #335's current SHA — the test emulates
a dark color scheme on a brand-new (no persisted data) project and waits for
both `.dark-theme` and `.appearance-sepia` to be applied, which now never
happened since the first-run default silently stopped being sepia.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: refresh stack SHAs after the appearancePreset/unlock-routing fix cascade
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: refresh session handoff, archive the prior capture
The prior CURRENT-HANDOFF.md (captured 2026-08-11T11:14:45Z) is now stale
relative to this session's work: #335/#336/#337 all reached review-thread
quiescence (except one deliberately-deferred #335 thread), two real
data-loss/lockout defects were found and fixed on #335 (cold-start unlock
routing, Lock Session routing), a genuine sepia-default regression was found
and fixed, PR310-R009's disposition was corrected, and the performance
ledger's stack SHAs were refreshed twice. Archived the prior capture
alongside the existing codex handoff rather than discarding it, and wrote a
new CURRENT-HANDOFF.md reflecting live state, including what's still
genuinely open (the deferred pnpm thread, #310's remaining threads, #332/#333
packaged evidence, and whether #335's final E2E run landed green — a
background poll was still in flight when this was written).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(lora,settings): stale-run training race, stuck onboarding on cancel, stale model list on context switch
Three confirmed race conditions from a fresh CodeAnt review wave:
- loraThunks.ts: startTrainingThunk's catch classified a killed process's
rejection using the CURRENT Redux currentRun.cancellationRequested, not the
run that actually produced it. abort_lora_training awaits child-process exit
before resolving, so a new training run can start before the killed run's
own train_lora promise finally rejects — the catch would then wrongly
archive the NEWER run as failed/aborted using the OLDER run's outcome. Now
guards on currentRun.id matching the runId this invocation generated; a
mismatch is a stale rejection and a no-op.
- LoraOnboarding.tsx: cancelling the native Python file picker resolves with
null, but handleSelectPython unconditionally bumped the request-generation
guard before checking the result — invalidating the still-pending initial
environment check without ever applying a replacement value, since the
code only calls setEnv on a truthy result. The onboarding could get stuck
showing "checking environment..." forever. The guard now only advances once
there's an actual new result (success or error) to apply.
- AiProviderCard.tsx: useConnectionContextReset invalidated in-flight
tests/model-loads on a provider/endpoint/preset change but never cleared
the already-rendered ollamaModels list from the previous context, so a user
could select a model id that doesn't exist on the newly selected server
until a fresh load happened to complete.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(settings,ai): decouple URL edits from protocol preset; guard preload progress against superseded attempts
AiProviderCard.tsx: editing the "Ollama Server URL" field unconditionally
reassigned localBackendPreset to 'custom', which isOpenAiCompatibleLocalPreset
always routes through the OpenAI-compatible /v1/chat/completions protocol.
A native-Ollama user just changing host/port (e.g. pointing at a LAN server)
had their protocol silently switched and every completion started failing.
The URL input now only updates ollamaBaseUrl; protocol selection stays
explicit via the preset dropdown.
localAiFacade.ts: preloadLocalModel's own inferenceProgressEmitter calls, and
the ones inside generateLocalText gated by reportToGlobalProgress, ran
unconditionally. retryLastPreload() starting a new attempt while an older,
cancelled/superseded attempt's generateLocalText call is still settling could
let that stale attempt's reportWebLlmReady/reportWebLlmError/reset overwrite
the newer attempt's modal state after the fact. Extended the existing
activePreloadAbort identity-guard pattern (already used to protect the cancel
hook) to every progress-report call site via a new isCurrentAttempt option.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(desktop): revalidate the cached Python interpreter before trusting it
resolve_python() trusted a cached ResolvedPython for the rest of the process's
lifetime once found, whether from an explicit configured path match or the
auto-discovery scan — never re-checking that the executable still exists,
still has its executable bit, or still resolves. If the interpreter was
removed, replaced, or lost permissions while the desktop app stayed open
(a long-running session), environment checks would keep reporting Python as
available and training would only fail later at spawn with a confusing error.
cached_python_still_valid() reuses probe_python's own lightweight,
synchronous filesystem-only check (validate_python_candidate_path — no
subprocess spawn, so the cache's performance benefit is preserved) before
trusting either cache hit; a stale entry now falls through to a full re-probe
instead of being silently believed. Added unit coverage for
validate_python_candidate_path's three outcomes (relative candidate, valid
absolute path, missing absolute path) since the async caching path itself
isn't practically unit-testable without tokio::test infra this crate doesn't
carry yet. Verified via `cargo fmt --check` (this host cannot safely run a
full `cargo check`/build — no cached target/ artifacts, would be a from-scratch
compile of the whole dependency tree); CI's Tauri build job is the real gate.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(settings): auto-validate a newly saved Gemini key instead of deferring to the next generation call
handleSaveKey only ran syntactic checks (length + control characters) before
persisting and marking hasKey active — a misleading QNBS-v3 comment claimed
"provider validation... determines credential validity" when no such
validation actually ran at save time. A malformed or unrelated credential
appeared saved successfully, and the Settings status disagreed with the
actual provider authentication state until a later generation request
happened to fail. Saving now automatically runs the same test-connection flow
Test Connection already uses (which already flips hasKey back to false on an
INVALID_API_KEY response); "Saved" no longer implies "verified."
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: refresh session handoff after the #336 fresh-review-wave fix cascade
Records the 7 additional real defects found and fixed on #336 (training-run
race, onboarding stuck-on-checking, stale model list, protocol-preset
mismatch, preload progress race, Python cache staleness, Gemini key
validation gap), the cascade into #337, and flags the two things not yet
independently confirmed: whether CI is green on the newest #336/#337 SHAs,
and whether the unverified-locally Rust change actually compiles (only
cargo fmt --check ran on this host).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(deps): exclude the two unpatched uuid releases from the override range
uuid: ">=11.1.1" was a bare floor that still permitted the two known-
vulnerable exact releases 12.0.0 and 13.0.0 (GHSA-w5hq-g745-h8pq /
CVE-2026-41907 — missing buffer bounds check in v3/v5/v6 API methods, fixed
per major line at 11.1.1/12.0.1/13.0.1) if a future resolution ever landed
on them; the currently-locked uuid@14.0.1 was unaffected today, but the
override itself didn't structurally rule out regressing to a vulnerable
version. Tightened to ">=11.1.1 <12.0.0 || >=12.0.1 <13.0.0 || >=13.0.1",
excluding exactly those two releases per major line while still allowing
everything else. Ran via a real `pnpm install` (not a hand-edit) so
verifyDepsBeforeRun's installed-state check stays consistent; resolved
version is unchanged (still 14.0.1, already satisfies the tighter range).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(lora): clear cancellationRequested on a failed native abort; collapse wrapped QNBS-v3 comments
If abortTraining() rejects, cancellationRequested stayed true forever —
training may still be genuinely running, and its own later failure
(dispatched by startTrainingThunk's catch) would then be misclassified as a
user-requested abort instead of a real error. New trainingCancellationFailed
reducer resets the marker; abortTrainingThunk dispatches it in a catch around
the native call before rethrowing.
Also: a fresh CodeRabbit pass (previously rate-limited/skipped on this PR)
flagged 20 QNBS-v3 comments across services/lora/, features/lora/,
components/settings/LocalAiSection.tsx, and their tests that wrapped across
multiple physical lines, violating this repo's one-line QNBS-v3 convention —
collapsed to single lines without changing their content.
Regression tests: a deferred-promise version of the existing
"dispatches trainingCancellationRequested before awaiting" test now actually
proves the dispatch happens before the native abort settles (an
immediately-resolving mock couldn't distinguish that from "after it resolved
but before the next dispatch"); new test for the abort-failure path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(i18n): translate LoRA onboarding strings and fix broken shell commands in 9 locales
lora.onboarding.installCmd (a literal shell command, "pip install unsloth trl
peft") had been translated word-for-word in pt/sv/ko/is, producing non-functional
commands (e.g. Portuguese "pip instalar sem preguiça trl peft" — "unsloth"
mistranslated as "without laziness"). Restored to the exact original command
in all four.
Separately, all 18 lora.onboarding.error.* / selectingPython keys (added
alongside the native Python environment-check work) were still raw English
in the he/hu/is/ja/ko/pt/ru/sv/zh source locale trees — never translated at
all, not just a stale-bundle issue. Translated all 18 keys across all 9
locales; also fixed two pre-existing partial translations noticed in the same
section (ja and zh's lora.onboarding.description mixed English mid-sentence).
Regenerated all 19 runtime bundles via i18n:bundle; key parity confirmed at
2903 keys per locale.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: refresh performance ledger SHAs after #336 layering-mistake correction
#335/#336/#337 heads and thread counts were stale after this session's cherry-pick
correction and the 12-finding CodeRabbit loop on #336. Also drops the now-resolved
uuid override-range note (applied via a real pnpm install) and marks PERF-333-006's
#336 review reconciliation complete.
* docs: capture session handoff after the second layering-mistake correction
Archives the prior capture (pre-correction, stale c3f00cff/0b1cc2ed SHAs) and
records the full correction: cherry-pick onto #336, reset+merge on #337, all
12 second-wave CodeRabbit threads replied and resolved, plus the still-open
items (CodeRabbit re-review rate-limited, Tauri build dispatched but pending).
* docs(pr310): mark review-thread reconciliation complete (0/317 unresolved)
All 28 previously-unresolved PR #310 review threads verified against current
code and replied to citing the specific replacement file/commit/test, then
resolved via GraphQL resolveReviewThread. Commit/behavior/test reconciliation
tables and packaged verification remain outstanding; #310 stays open (Option C).
* docs: update handoff with Tauri build success and #310 reconciliation completion
Tauri build 31549539018 confirmed success on all 3 platforms against #336's
b01564ed, closing the previously-open Rust-compile NO-GO gap. PR #310's
review-thread queue is now fully reconciled (0/317). CodeRabbit's re-review
on #336 remains the one still-unconfirmed item.
* fix(lora): only classify training as aborted on a confirmed native cancellation
abort_lora_training resolved Ok(()) for NothingRunning/CancelPendingStart just
like a confirmed process kill, so a coincidental unrelated training failure
racing with a no-op abort request could be misclassified as user-cancelled.
The Rust command now returns whether it actually confirmed stopping a process;
abortTrainingThunk only dispatches trainingAborted() when true, and resets
cancellationRequested (via the renamed trainingCancellationNotConfirmed) on
both a native error and a no-op success. Also fixes an unnatural Swedish
timeout translation to match the established wording pattern.
* fix(lora,ai): distinguish pending-start cancellation from a true abort no-op
abort_lora_training's wave-3 boolean return conflated CancelPendingStart (a
genuine recorded cancellation for a not-yet-spawned run) with NothingRunning
(a true no-op) — cancelling during Python discovery cleared cancellationRequested
before the pending train_lora invocation's own training_cancelled rejection
arrived, misclassifying a successful startup cancel as a training failure. The
command now returns a three-way AbortOutcome; the thunk only clears the flag
for NothingToCancel and leaves it set for PendingStartCancelled.
Also: cached_python_still_valid now re-runs the full identity probe instead of
a filesystem-only check, so an interpreter replaced in place at the same path
is no longer trusted with a stale cached version. Fixes a stale-response race
in AiProviderCard's WebGPU probe (missing the same testRequestIdRef guard used
by every other async result in handleTest) and a CRLF edge case in the local
OpenAI-compatible SSE parser. Collapses two wrapped QNBS-v3 comments.
* fix(settings): restore the WebGPU auto-probe lost when the ollama auto-test was removed
A prior commit removed a combined useEffect that auto-tested ollama AND
auto-probed WebGPU capability whenever the provider was selected. The ollama
removal was intentional (CORS/privacy — avoid auto-fetching a local server from
the PWA), but WebGPU detection is a local hardware check with no such concern,
so dropping it as collateral damage left the GPU badge stuck on "untested"
until the user clicked Test Connection. This was never caught because #336
never ran its full test suite under CI while stacked — tests/unit/settings/
WebLlmPanel.test.tsx already asserted the auto-probe behavior and was
silently broken. Restores a guarded auto-probe effect (reusing the same
applyIfCurrent staleness guard as the manual Test Connection click) and
updates the one AiProviderCard.test.tsx test whose premise no longer holds.
* fix(a11y): set aria-busy on the provider connection status region while loading
The status region announced changes via aria-live but never set aria-busy,
unlike every other loading indicator in this codebase, so assistive tech had
no programmatic signal that a connection test was actively running.
* fix(test): give the binder-asset transaction mock a real .transaction back-reference
deleteAllBinderAssetsForProject (services/storage/idbAssetStore.ts) batches its
deletes into one transaction and reads store.transaction to attach oncomplete/
onerror/onabort, matching real IndexedDB. This test file's hand-rolled fake
store predates that pattern and never exposed .transaction, so the mocked
store was undefined at that access — crashing with "Cannot set properties of
undefined (setting 'oncomplete')". Only surfaced now: this PR never ran its
full test suite under CI while its base was a stacked branch instead of main.
The fake store now tracks each queued request's completion promise so the
transaction mock can fire oncomplete once they've all settled, mirroring real
IDB batching semantics.
* fix(storage,ai,a11y): fail-soft cache reads, single-flight IDB open, abort guard
Real bugs found by #337's first full-CI review pass (this PR never ran full
CI while stacked, so these were never caught):
- aiInferenceCacheService.getCachedInference rejected on a malformed/undecodable
cache row, contradicting its own documented non-authoritative cache policy —
now degrades to a miss like setCachedInference already does.
- sceneRevisionService.getDb() let concurrent callers each open a separate
IndexedDB connection before the first resolved — now single-flights the open
promise so concurrent saves share one connection.
- secondaryPayloadStoreAdapter's abortForFailure could call transaction.abort()
twice within one batch (two records failing before the transaction settles),
throwing an uncaught InvalidStateError — now guarded to abort only once.
- AiProviderCard's connection-status region never rendered text or aria-busy
during 'loading', leaving assistive tech with no signal a test was running —
adds a loading label (new settings.ai.providerStatusTesting key, all 19
locales) and aria-busy.
- A test rerendered the wrong prop (openAiCompatibleBaseUrl instead of
ollamaBaseUrl) for an lm_studio-preset scenario, not exercising the endpoint
it claimed to.
Also collapses wrapped QNBS-v3 comments across 9 files and refreshes stale
doc-metric references (README locale count, performance ledger #337 SHA,
an archived handoff's lockfile-editing instruction).
Three deeper findings from the same review (cross-tab write admission across
7 methods, migration-verification-vs-concurrent-deletion races, and secondary
adapters never being wired into production migration startup) were confirmed
real but are deliberately left unresolved, not fixed here: the entire
migration journal system has zero production callers today (only tests invoke
it — the live UI has no disable/rotate-passphrase flow, matching this
project's own Phase-4 tech debt note), so these races aren't yet reachable.
Fixing them properly requires the same dedicated design work as that Phase-4
wiring, not a rushed patch in a review-response pass.
* docs: capture handoff after merging #335/#336 into main and #337's full-CI wave
Records: two admin-authorized merges (#335, #336) with two stacked-PR
auto-close recoveries; the second CodeRabbit review wave on #337 (now based
on main) that found and fixed real bugs plus the major discovery that the
entire encryption migration journal system has zero production callers.
* fix(test,docs): track cursor completion in binder mock; fix doc drift and MD018
deleteAllBinderAssetsForProject's fake transaction mock tracked put/get/delete
completions but not openCursor's own walk, so oncomplete could theoretically
fire before a cursor-driven delete settled — now the whole cursor walk is
tracked as one pending item. Also collapses two of my own newly-wrapped
QNBS-v3 comments from the prior commit (same one-line rule I've been fixing
for others). Docs: reconciles a stale "Tauri build in flight" line against
the same archived handoff's own later "confirmed success" sections, notes
why the archive filename's timestamp intentionally differs from the internal
capture time, fixes 3 markdownlint MD018 violations (bare #NNN at line start
misparsed as a heading), and separates "not currently reachable" from "root
cause" in the current handoff's description of the 3 deferred findings.
* test(storage): cover scene/cache payload-shape branches in the secondary adapters
secondaryProtectedStoreAdapters.ts sat at 26% coverage — only the two
routing-key rejection paths were tested. Adds 9 tests exercising the
legacy-flat-field vs. nested-payload branches, the schemaVersion guard,
and the assertExactKeys rejection for both scene revisions and the
inference cache, closing the codecov/patch shortfall (73.46% -> target).
* test(storage): fix test-fixture Date.now() nondeterminism; docs: reconcile archived handoff's contradictory CodeRabbit status
secondaryProtectedStoreAdapters.test.ts's new fixtures used Date.now() for
createdAt/timestamp fields never asserted on; a fixed literal is simpler and
safer than fake timers around real WebCrypto + fake-indexeddb async work.
The archived handoff's capture-metadata header claimed CodeRabbit's #336
re-review completed with 0 findings while §15/18/23 (drafted earlier in the
same capture) still called it unconfirmed — reconciled to one status.
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>2 tasks
5 tasks
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
…esktop stack CHANGELOG.md: comprehensive [Unreleased] entry for #335 (fail-closed IDB encryption lifecycle), #336 (desktop AI/Python hardening for #332/#333), #337 (durable resumable migration journal + secondary-store adapters), #339 (cross-tab write-admission closing the migration TOCTOU race), plus the #310 ledger closure and #338 tracking issue. AUDIT.md: appended this session's entry to the follow-up chain and a new quality-gate line; corrected the stale "2869 keys" figure in the current- version summary line to the actual 2904 (already true since an earlier commit this session, just never synced here) — node scripts/check-doc- metrics.mjs now genuinely passes rather than coincidentally matching.
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
…ryption stack (#340) * docs: close out PR #310 reconciliation ledger — SUPERSEDED #335/#336/#337 are all now merged into main (4fadbd7), the one condition this ledger's merge decision previously left open. Every commit/behavior reconciliation row already had a final disposition; independently re-verified the deeper PR310-R010-R016 hardening checkpoint's cited test names against main's current tree — all exist and pass. Updates the status banner (was stale, claiming the tables were still outstanding when they were already complete) and the merge-decision section to record closure, with an explicit note that the disable/rotate production wiring B006/B007 describe is separate Phase-4 work (issue #338), not yet done. * docs: housekeeping pass — CHANGELOG + AUDIT sync for the encryption/desktop stack CHANGELOG.md: comprehensive [Unreleased] entry for #335 (fail-closed IDB encryption lifecycle), #336 (desktop AI/Python hardening for #332/#333), #337 (durable resumable migration journal + secondary-store adapters), #339 (cross-tab write-admission closing the migration TOCTOU race), plus the #310 ledger closure and #338 tracking issue. AUDIT.md: appended this session's entry to the follow-up chain and a new quality-gate line; corrected the stale "2869 keys" figure in the current- version summary line to the actual 2904 (already true since an earlier commit this session, just never synced here) — node scripts/check-doc- metrics.mjs now genuinely passes rather than coincidentally matching. * docs: tighten grammar per Sourcery nitpick
qnbs added a commit
that referenced
this pull request
Aug 12, 2026
…rations (#339) * fix: fail closed encrypted storage lifecycle * fix: harden desktop AI integrations * feat: add durable encryption migration journal * docs: clarify encryption journal rollout * fix: expose local provider diagnostics * fix: resolve audited dependency vulnerabilities * fix: resolve audited dependency vulnerabilities * fix: make encryption journal updates atomic * fix: enforce pnpm v11 build-script policy * docs: reconcile PR 310 migration work * feat(storage): add secure record envelope primitives * feat(storage): checkpoint resumable migration batches * chore: harden dependency execution policy * feat(storage): add resumable secondary store adapters * fix(storage): fail closed during migration reads * fix(storage): harden migration recovery protocol * fix(deps): reconcile release-age lockfile * fix(storage): harden migration recovery protocol * fix(tauri): bound Python probes and LoRA process lifecycle * chore(deps): align pnpm v11 security policy * docs: add desktop performance evidence ledger * docs: capture current agent handoff state * docs: correct README i18n key count (2869 → 2876) CI's docs:check drift gate flagged 4 stale README badges/lines still citing the pre-expansion key count; bring them in line with the current locale bundle so the Node quality gate and its downstream Build/E2E/Storybook/Lighthouse jobs stop skipping. * fix(storage): close TOCTOU/fail-closed gaps in the IDB encryption guard Addresses the #335 review-thread cluster (CodeAnt/CodeRabbit/Codex/Qodo) converging on services/storage/*Store.ts: - Atomic write-key resolution (resolveProtectedWriteKey/idbEncryptWithKey): saveSlice/createSnapshot/saveImage/saveBinderAsset/saveStoryCodex/ saveRagVectors previously checked assertIdbProtectedWriteAllowed() then re-read isIdbEncryptionReady() after an intervening await (opening the IDB transaction) — Lock Session firing in that gap silently downgraded the write to plaintext. The two checks now collapse into one snapshot. - Fail-closed guards added to every previously-unguarded destructive/listing path: deleteImage, deleteBinderAsset, listBinderAssetIds, deleteStoryCodex, listSnapshots, deleteSnapshot, deleteProject (before its binder-asset cascade); getSnapshotData gets an explicit guard for clarity. - IDBRequest.onsuccess handlers that call idbReadSecure() now catch/reject instead of producing an unhandled rejection that left loadState()/ getImage()/getSnapshotData() pending forever on a locked read. - hasPassphraseSentinel() caches its result (invalidated by clearIdbEncryptionKey()) — was previously an unconditional IDB round trip on every protected read/write, including for users who never configured encryption. - Removed reEncryptAllAppData/reEncryptAllSnapshots: confirmed dead code (zero callers on this branch or #337) per CodeRabbit's own suggested disposition; rotateIdbPassphrase already unconditionally blocks rekeying pending the journal-based implementation. - Fixed two test files whose storageEncryptionService mocks were already missing assertIdbProtectedWriteAllowed (pre-existing gap, now widened by the new guards) and added coverage for every fix above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage,i18n): correct encryption-scope claims and salt-loss fail-closed gap Addresses the remaining #335 review-thread cluster: - storageEncryptionService.ts: getOrCreateSalt() now fails closed with a new IdbEncryptionSaltLostError when the persisted PBKDF2 salt is missing/invalid but a passphrase sentinel already exists — previously it silently derived a new salt, producing a different key and permanently orphaning existing encrypted data instead of surfacing the loss. Applies to initIdbEncryption, setupIdbEncryption, verifyAndInitIdbEncryption, and the (currently unreachable) rotateKey. - README + docs/IDB-ENCRYPTION.md: corrected the false claim that Tauri desktop gets the same IndexedDB-backed at-rest encryption as the web build. Tauri's filesystem-backed store (services/fs/*) writes compressed-but- unencrypted data regardless of the passphrase/unlock state; the unlock screen appearing on desktop was misleading users about actual protection. - locales/*/help.json (en/sv/fi/hu/is/eu/ru/fa): corrected the same overclaim ("all IndexedDB stores") in the in-app feature-flag help article to match the narrower, accurate scope already used elsewhere in the file. - locales/*/settings.json (all 19): writingSurfaceHint previously described the backdrop as "texture-free," contradicting the selector's own Textured option. Reworded to apply to both options; regenerated all 19 runtime bundles (also drops a stale lora.onboarding.selectPython key that had no source definition). - components/settings/GeneralSections.tsx: added aria-pressed to the writing- surface toggle buttons so assistive technology can identify the active option, plus the required QNBS-v3 rationale comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: backfill writingSurfaceStyle in all Settings producers; correct advisory metadata; document hooks:install - services/fs/settingsFsStore.ts, services/cloudSync/cloudSyncBackend.ts: loadSettings() used an unchecked `as Settings`/generic-decrypt cast, so settings persisted before writingSurfaceStyle (or any newer required field) existed would violate the Settings contract at runtime. Both now route through the same normalizePersistedSettings() already used by the IDB path. - tests/unit/languageToolClient.test.ts: baseSettings() was missing the required writingSurfaceStyle field. - pnpm-workspace.yaml: corrected several advisory comments that misdescribed their GHSA (wrong summary text for fast-uri/ip-address/joi/undici, an invalid esbuild GHSA id, and two long-standing CVE-2024-XXXX placeholders for uuid/qs). Left override values unchanged — tightening the uuid floor needs a lockfile-only pnpm update this host can't safely run right now; documented as a follow-up rather than silently dropped. - CONTRIBUTING.md: documented `pnpm run hooks:install` as an explicit step after `pnpm install` — pnpm v11's allowBuilds policy denies simple-git-hooks' own install script, so a `prepare` lifecycle script would silently no-op; contributors who skip this step bypass lint-staged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): correct transaction-ordering regression and cross-tab sentinel-cache staleness Fresh CodeRabbit review on the previous commits caught two real bugs in my own changes: - idbProjectStore.ts saveSlice: opened the IDB transaction BEFORE encrypting the payload, reintroducing the exact TransactionInactiveError risk the sibling stores' own QNBS-v3 comments warn about (encrypt yields the event loop, which can auto-commit an already-open transaction before store.put runs). Reordered to resolve the key and encrypt first, matching idbAssetStore/idbCodexStore/idbSnapshotStore's established safe pattern. - storageEncryptionService.ts hasPassphraseSentinel(): cached `false` as well as `true`. If a passphrase is set up in another tab (or by any other runtime instance) after this one cached a negative result, the cache never invalidates — resolveProtectedWriteKey()/assertIdbProtectedWriteAllowed() would keep treating a now-configured library as never-configured and allow a plaintext write. Only `true` is cached now; a negative lookup always re-reads IDB (the fail-closed direction). Updated the two tests that asserted the old negative-caching behavior and added regression coverage for the cross-tab scenario. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(i18n): synchronize stale feature-flag counts and add missing Basque translations - locales/{en,eu,fi,hu,is}/help.json: the feature-flag help paragraph cited conflicting, stale facts — en said "22 flags, 6 opt-in" (missing enableBrowserOllama), eu/fi/hu/is said "21 flags, only RTL off" (a v1.21-era description never updated). Synchronized all five to the current 23 flags / 16 default-on / 7 opt-in-off state. - locales/eu/settings.json: the four writing-surface keys this PR introduced were left in English fallback; added Basque translations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): distinguish setup-failed from wrong-passphrase errors; add missing QNBS-v3 comment - components/settings/PassphraseModal.tsx: 'set' mode (first-time setup) has no prior passphrase to be "wrong" — any rejection there is a storage/salt failure (e.g. localStorage blocked, or the new IdbEncryptionSaltLostError). Showing "wrong passphrase" left users unable to diagnose the real problem. Added settings.privacy.encryptionSetupFailed (all 19 locales) and route 'set'-mode failures to it; 'unlock' mode keeps the existing message since it's the only mode that verifies an existing passphrase. - hooks/useSettingsView.ts: added the QNBS-v3 rationale comment CodeRabbit flagged as missing for the writingSurfaceStyle dispatch case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(desktop): fix LoRA env-report deserialization, panic risk, Python identity check, and stuck cancellation Addresses the src-tauri/src/lora.rs review cluster on #336 (5 distinct, current-code-valid findings from CodeAnt/CodeRabbit/Codex): - LoraEnvReport: scripts/check_lora_env.py never emits python_path/last_error, so every successful helper run failed to deserialize without #[serde(default)] on those two fields — every environment check reported helper_report_parse_failed, permanently hiding real Unsloth/CUDA status. - train_lora: stderr truncation sliced by raw byte index (&stderr[..stderr.len().min(500)]), which panics if byte 500 lands inside a multibyte UTF-8 character (e.g. non-ASCII paths in a Python traceback). Truncates by char now. - probe_python: validated a manually-configured interpreter path only by parsing `--version` output for a numeric string — `bash --version` parses as "5.1", which is >= the minimum and would be accepted as "Python" and later invoked with Python script arguments. Now probes actual identity via `-c "import sys; print(f'{major}.{minor}.{micro}')"`. - abort_lora_training: a confirmed StopProcess termination never cleared ACTIVE_LORA_TRAINING itself (only train_lora's own reap did, later), so an immediate retry right after a successful cancel could still see training_already_running. Now clears the slot as soon as termination is confirmed (PID-guarded, so the later reap in train_lora is a safe no-op). AlreadyStopping (retry) only polled and never re-signalled — a process that ignored the first SIGTERM stayed uncancellable forever. Now escalates to SIGKILL on Unix retries. The concurrent-training-slot race and blocking-probe findings on this same cluster are already fixed by existing code (reserve_training_slot's single-lock check-and-set, and probe_python's spawn_blocking/tokio::process usage) — replied with evidence citing current line numbers rather than re-fixing; both threads' own isOutdated flags corroborate this. Verified: rustfmt --check --edition 2021 passes (syntax-valid, correctly formatted); full cargo build not run locally per this session's resource constraints — relies on CI's Tauri build job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct Tauri arg casing and surface native diagnostics in LoRA onboarding merge_lora, generate_ollama_modelfile, and set_lora_python_path were all invoked with snake_case argument keys. Tauri's #[tauri::command] macro requires camelCase for top-level JS invoke keys by default, and none of these three commands declare rename_all = "snake_case" — so every one of these desktop LoRA actions failed argument binding before reaching Rust (confirmed against src-tauri/src/lora.rs; train_lora's nested payload fields are unaffected since struct deserialization follows its own un-renamed serde impl, not Tauri's command-arg binding). LoraOnboarding.tsx: add a request-generation guard so a slow initial checkTrainingEnvironment() can no longer overwrite a newer manual selectPythonExecutable() result. Surface env.lastError regardless of pythonAvailable — check_lora_environment can return python_available: true alongside a helper diagnostic failure (helper_spawn_failed etc.), which was previously hidden whenever Python itself was found. Map raw Rust category identifiers through an i18n lookup table instead of leaking snake_case tokens to users. Fix the focus-ring token (--sc-border-focus -> --sc-ring-focus) and add aria-busy + a dedicated loading label to the select-Python button. Adds 18 lora.onboarding.* keys across all 19 locales (real translations for de/es/fr/it, English fallback for the rest per existing i18n policy); updates README key-count badges to 2897. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct AiCreativity casing typo blocking typecheck 'balanced' isn't a valid AiCreativity value (only 'Focused' | 'Balanced' | 'Imaginative' are) — every other call site in this file already used 'Balanced'. This one typo failed tsgo --noEmit for the whole project, pre-existing and unrelated to the LoRA fix in the prior commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: classify a killed training process as aborted, not failed abort_lora_training waits for the killed child to exit before resolving, so the concurrent train_lora invocation it just killed can reject first. startTrainingThunk's catch unconditionally dispatched trainingFailed, which archives and clears currentRun before abortTrainingThunk's own trainingAborted dispatch runs — that reducer then no-ops (guarded on `if (!state.currentRun) return`), so a successful user cancellation was recorded and surfaced as a training failure. Adds TrainingRun.cancellationRequested, set synchronously by abortTrainingThunk before it awaits confirmation. startTrainingThunk's catch checks this flag and dispatches trainingAborted instead of trainingFailed when set. Correct regardless of which thunk's dispatch wins the race, since both trainingFailed/trainingAborted reducers are no-ops once currentRun is already cleared. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: sync Local AI panel when a preload finishes outside handleDownload preloadLocalModel can be triggered from outside LocalAiSection — the global download-progress modal's Retry button calls it directly via retryLastPreload. Only LocalAiSection.handleDownload updated readyIds, throughput, storage, and the ready announcement, so a successful retry left a mounted panel showing the model as not-ready with stale stats until the section unmounted and remounted. Adds subscribeLocalModelReady to localAiFacade, notified at the end of preloadLocalModel's success branch (after its own readyLocalModelIds/ lastLocalThroughput bookkeeping, so listeners never observe stale data). LocalAiSection subscribes and re-syncs on notification, guarded by a downloadingIdRef so its own handleDownload-initiated downloads (which already update this state directly) don't get double-announced — the notification fires synchronously inside preloadLocalModel, before handleDownload's own await resumes. Also fixes two unrelated pre-existing test issues found while working in these files: a stale assertion in localAiFacade.test.ts expecting generateLocalText's runLocalTextGeneration call to receive `undefined` for onProgress (it always wraps onProgress in an internal reportProgress closure so inferenceProgressEmitter gets progress unconditionally), and adds the sourcery-suggested retry-failure-surfaces-via-reportWebLlmError coverage plus direct tests for retryLastPreload-with-no-prior-preload and abort-suppresses-error-reporting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: scope the global download modal to explicit preloads only generateLocalText unconditionally drove the singleton inferenceProgressEmitter (used by the global LocalAiDownloadProgress modal) for every call, including ordinary Writer/Copilot/ProForge generation through aiProviderService.ts. A user generating text with an already-warm WebLLM model could see a "downloading a model" dialog appear, and isLocalAiBusy() report busy, for a call that wasn't downloading anything. Adds an opt-in generateLocalText option, reportToGlobalProgress, defaulting to unset/false for every existing call site except preloadLocalModel (the only function that should legitimately drive that modal). This also fixes two related "stuck in loading" gaps found in the same review pass, now that generateLocalText owns its own terminal-state responsibility for any opted-in caller: - a preload that falls back off WebLLM (ONNX/Transformers/heuristic) now resets the emitter instead of leaving it in 'loading' forever when reached from a caller other than preloadLocalModel's own outer handling (which already covered its own case via reportWebLlmError). - a caller-provided AbortSignal aborting preloadLocalModel (distinct from the modal's own Cancel button, which already reports its own terminal state) now resets the emitter instead of only suppressing the error report, which previously left the modal and isLocalAiBusy() stuck indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: guard connection-context races and the WebGPU spinner in AiProviderCard Model loading (handleLoadOllamaModels) had no staleness guard at all, and the manual-test guard (testRequestIdRef) was only bumped by an explicit second handleTest() call — editing the endpoint URL, preset, or browser-Ollama toggle while a request was in flight let its result land under the new context. AiSections.tsx worked around a related symptom (a provider switch mid-flight) with `key={provider}`, which fixed that one case by remounting the whole card — but also discarded any unsaved openaiKey/grokKey/anthropicKey input and restarted the stored-key reads. Removes the key prop. Adds useConnectionContextReset (extracted to a standalone hook to keep AiProviderCard's cognitive complexity under the CI threshold): a single effect that bumps testRequestIdRef and resets testStatus/testError/isLoadingModels whenever provider, ollamaBaseUrl, localBackendPreset, openAiCompatibleBaseUrl, or browserOllamaEnabled change — covering every case the key-based remount did, plus the ones it didn't (editing fields without switching providers), without discarding component state. handleLoadOllamaModels now captures the request id and checks it before applying results, via a small applyIfCurrent helper (also extracted for the same complexity reason). Separately: the WebGPU status badge rendered gpuInfo === null as a permanent spinner, since null was its only "loading" signal — opening the WebLLM tab without ever clicking Test Connection showed an indicator that never settles. Adds isProbingGpu to distinguish "actively probing" from "not yet tested", and changes gpuInfo's default from null to {status: 'unknown'} so both states render through the existing status-badge branches with no new conditional. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: correct LM Studio/vLLM/custom local-backend streaming and routing streamOpenAiCompatibleLocal (LM Studio/vLLM OpenAI-compatible local streaming, added in this PR): - Never flushed the final SSE buffer after reader.read() returned done — a server closing the stream without a trailing newline after the last data: frame silently dropped that final delta. - Broke the read loop and called onDone on an aborted signal instead of throwing, so a user-cancelled generation was recorded as a successfully completed request with partial output. - Resolved a blank ollamaBaseUrl ('') via `?? 'http://localhost:1234'` — nullish coalescing doesn't catch empty string, so an explicitly cleared URL produced an app-relative request instead of falling back to the same default testOpenAiCompatibleLocalConnection uses. - Discarded the server's JSON error body on a non-ok response, losing the actionable detail (invalid model, bad request) LM Studio/vLLM return, down to just the HTTP status code. Custom-preset protocol mismatch: listLocalBackendModels already treated the 'custom' preset as OpenAI-compatible for model discovery (the common case — hand-editing the base URL to a non-default LM Studio/vLLM port selects 'custom'), but streaming, non-streaming generation, and connection testing all routed 'custom' through the native Ollama protocol instead. A customized server could list its models successfully and then fail every completion. Extracts isOpenAiCompatibleLocalPreset as the single source of truth and uses it at all four call sites. Also threads localBackendPreset through useCriticView and useConsistencyCheckerView's AIRequestOptions builders — they already passed ollamaBaseUrl but omitted the preset, so the Critic and Consistency Checker surfaces silently fell back to Ollama-native even for the named LM Studio/vLLM presets while the main Writer surface worked correctly. sourcery flagged `signal: opts.signal ?? null` as an inconsistency; verified against LocalServerFetchInit's actual type (AbortSignal | null, not | undefined — this codebase's own wrapper, not a raw Fetch passthrough) and confirmed `?? null` is the required, correct form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: cover vLLM preset and invalid OpenAI-compatible models responses Sourcery flagged that testAIConnection's LM Studio-preset coverage had no vLLM equivalent, and testOpenAiCompatibleLocalConnection's invalid-response branch (non-array data, non-JSON body) had no direct test at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: resolve tsgo narrowing issue in AiProviderCard's stale-context test resolveTest (let, reassigned inside a Promise executor closure) was narrowed to never by tsgo after the awaits earlier in the test, following the merge of fix/desktop-reliability-hardening into this branch. An explicit type annotation on the captured const sidesteps the incorrect inference. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: enforce legal phase transitions and close the checkpoint cursor-stall gap saveIfCurrent (the shared CAS write path for both updateEncryptionMigrationJournal and completeEncryptionMigration) only validated revision/owner match — it never validated that a phase TRANSITION was a legal state-machine step. Any caller could pass a journal still 'prepared' with {phase: 'committing'} (or call completeEncryptionMigration on it directly) and skip conversion and verification entirely; assertNoActiveEncryptionMigration() would then treat the unconverted data as safe for ordinary access. Confirmed exploitable via the codebase's own test helper (markJournalCommitting), which jumped prepared straight to committing purely for test setup convenience — exactly the shape of the described attack. Adds an allowed-transition table enforced inside saveIfCurrent against the durably-stored phase (not the caller-supplied one), with same-phase entries permitted for the repeated per-batch/per-store checkpoint writes within migrating/verifying, and recovery-required reachable from any active phase but never left except via out-of-band recovery. Updates the test helper to route through the legal prepared→migrating→verifying→committing chain instead of skipping it. Separately, protectedStoreMigration.ts's nextCheckpoint accepted a nonterminal batch reporting progress without an advanced cursor, silently retaining the previous cursor — the next iteration would replay the same records forever, inflating `processed` without ever completing. Now rejected explicitly. Also fixes five pre-existing typecheck errors uncovered while working in these files (unrelated to the above): a narrowing gap in parseJournal's ownerLeaseExpiresAt validation, four index-signature property accesses in storageEncryptionService.ts, one noUncheckedIndexedAccess gap in a test, and four exactOptionalPropertyTypes violations in secondaryPayloadStoreAdapter.test.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: gate protected-store writes against an active migration saveSlice, saveImage, saveBinderAsset, saveStoryCodex, saveRagVectors, and createSnapshot all resolved the write key and wrote directly — none of them called assertIdbProtectedWriteAllowed() first, unlike their sibling delete/read methods (deleteImage, deleteBinderAsset, deleteStoryCodex, getSnapshotData, etc.), which already had it. An ordinary write could therefore proceed while a migration journal owned the store's encryption-lifecycle state, racing the migration's own conversion of that same data. Confirmed via a pre-existing, previously failing test (encryptionMigrationJournal.test.ts's "rejects a competing migration owner..." — IdbProjectStore().saveSettings() was expected to reject during an active migration but silently resolved). Also fixes two related gaps found while auditing this write path: - deleteAllBinderAssetsForProject deleted assets one transaction per asset — a later failure (or the migration guard starting to reject mid-loop) left earlier assets permanently removed while the project record and later assets survived, with no rollback. Batches every delete into one transaction so a failure aborts the whole batch. - The scene-revision and inference-cache secondary-store adapters used a record's `id`/`key` field as the migration cursor without validating it's actually a string, even though IndexedDB key paths permit numeric keys. An unvalidated numeric key would still successfully rewrite the payload but then persist a non-string journal cursor, which parseJournal() rejects on the next read — pushing the whole migration into recovery-required with a confusing root cause. Now fails fast with a specific error at the point of use. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): skip corrupt scene revisions instead of hiding history; narrow write guard to migration-only listRevisions now logs and skips a single damaged/unparseable revision instead of rejecting the whole call, so one corrupt record no longer hides an entire section's readable history (a genuine lock-state change still aborts the call). The TOCTOU guard reordering from the previous commit re-ran the full assertIdbProtectedWriteAllowed() (migration + lock check) immediately before each write's transaction opens. That double-checks the lock state that resolveProtectedWriteKey() already captured atomically with the encryption key, so a write that safely encrypted with a validly-resolved key was wrongly rejected if the session locked in between (caught by an existing regression test: "saveSlice still encrypts even when the key is cleared right after the write key is resolved"). The six write methods (saveSlice, saveImage, saveBinderAsset, saveStoryCodex, saveRagVectors, createSnapshot) now re-check only assertNoActiveEncryptionMigration() pre-write, newly re-exported from storageEncryptionService for this narrower use; delete-only methods that never resolve a write key keep the full guard since they have no earlier lock check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): fail migration to recovery-required on a verification shortfall runProtectedStoreMigration's verifying phase threw a plain ProtectedStoreMigrationAdapterError when an adapter's verify() found fewer valid records than this saga already migrated (most likely because an ordinary write landed on an already-migrated record using a superseded key after that store's migrating pass finished). The catch block only released the ownership lease and re-threw, leaving the journal parked at 'verifying' with nothing to distinguish this from a transient interruption — a caller that retries keeps re-running the same doomed verify() call forever, since nothing in the saga can revisit and reconvert the stray record. Introduces ProtectedStoreVerificationShortfallError, a distinguishable subtype thrown only for this specific verified-count shortfall (not for a transient/interrupted verify() exception, which must still support the existing "resume verification on retry" path). On this specific error the catch block now transitions the journal to recovery-required before releasing ownership, converting an invisible infinite retry loop into an explicit, visible state that requires the same out-of-band recovery procedure already used elsewhere in this journal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): degrade cache reads to a miss on lock/migration, re-encrypt legacy entries on read getCachedInference() called assertSecureStorageReadable() unguarded, so a locked library, an active migration, or the check's own IDB access failing would reject the call instead of degrading to a miss — breaking this cache's documented non-authoritative, best-effort contract (setCachedInference already had this protection via its own try/catch). The lifecycle check is now wrapped so any failure returns null instead of propagating. Legacy plaintext cache entries were decoded and returned correctly but never rewritten, so a cache populated before encryption was enabled stayed plaintext in IndexedDB for the remainder of its 7-day TTL even after the library was unlocked with an active key. decodeEntry() now checks readSecureRecordPayload's `needsMigration` flag (the same signal the migration adapters already use) and best-effort re-persists the entry encrypted, preserving its original timestamp so TTL/LRU ordering is unaffected; a failure (e.g. a concurrent migration) is swallowed since this cache is explicitly non-authoritative. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ollama): report invalidResponse instead of a false-positive connection success testOllamaConnection() caught a JSON parse failure by setting payload to null and then computed an empty model list from it, so a 200 response with an unparseable body (e.g. a proxy login page) or valid JSON missing the models array was reported as ok:true with zero models — a false-positive success, inconsistent with the OpenAI-compatible diagnostic path (testOpenAiCompatibleLocalConnection), which already returns its existing 'invalidResponse' kind for the same two cases. Ollama's own TestConnectionErrorKind union gains that same kind (already i18n-mapped via settings.ai.testError.invalidResponse, shared across providers) and both failure paths now return it explicitly instead of falling through to a misleading success. A validly-shaped but genuinely empty models array (a fresh Ollama install with nothing pulled yet) still reports ok:true, since that is not an error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(test): clarify encryptionMigrationJournal.test.ts's ownership-CAS comment The file-header comment said "independent module-owner CAS behavior," which reads as claiming cross-tab/reloaded-module coverage. The actual tests (competing owner rejection, delayed-owner rejection, crashed-lease recovery) all exercise ownerId-scoped CAS rejection within the same imported module instance — reworded to say exactly that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): stop rendering the connection test result twice in AiProviderCard ProviderConnectionStatus (the status panel above the provider picker) already renders the status badge, testError text, and diagnostic panel for every provider. The action row further down duplicated the exact same testError string and a separate success message next to the Test Connection button, so one test produced the identical error text twice on screen (and a redundant success message). The action row now renders only the button; the status panel is the single source of truth. Existing tests that asserted on the duplicate via getAllByText(...).length now assert a single instance via getByText, and the stale "renders in two places" comment is removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: refresh stack SHAs in the performance ledger; reconcile PR310-R009 to a valid disposition The performance ledger's baseline table still had #335/#336/#337 pinned to SHAs from before this session's fix passes (fa3cd983/fd7ed7c1/dda48b33) — all three have since moved (5e80aaa4/2438f991/dc0b5262, the last now pushed with 0 unresolved review threads instead of "local merge pending push"). PR310-R009 and two related rows carried the interim disposition REWRITE, which the reconciliation doc's own taxonomy treats as impermissible as a final state — every other row resolves to an ADOPTED_WITH_MODIFICATIONS / SUPERSEDED_BY_BETTER_IMPLEMENTATION / NO_LONGER_APPLICABLE / RETAIN category. R009 asked for missing-store coverage plus interruption, legacy-shape, resume, and verification test cases; all five are now present as passing tests (protectedStoreMigration.test.ts's missing-adapter/checkpoint, interruption+resume, and verification-shortfall tests, plus secondaryPayloadStoreAdapter.test.ts's plaintext-to-encrypted conversion test). New PR310-R016 row consolidates the evidence with concrete test-name citations; R009 and the two related REWRITE rows now point to it instead of carrying an open-ended disposition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): route locked encrypted startup and Lock Session to the unlock modal, not a dead end Two confirmed paths left a user with configured at-rest encryption unable to reach the passphrase prompt at all: 1. Cold start: when the passphrase sentinel exists but no key is active yet, dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this before mounting <App>, so App.tsx's own unlock-detection effect (which shows IdbUnlockModal) never got a chance to run — the user landed on the generic StorageErrorScreen, whose only action is "Reset Database & Reload" (destroys all local data). The bootstrap IIFE is now a named, re-invocable bootApp() function; a locked-storage catch renders a standalone IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency) and retries the full boot in place on success. No page reload is used, since the freshly-unlocked in-memory key would be lost on one. 2. Mid-session: handleLockSession() cleared the key but never opened the unlock modal and didn't block editing, so a user could keep typing while every subsequent autosave silently failed closed (generic "Auto-Save Failed" toast, no path back to unlocking short of manually reopening Settings). It now also opens the same global unlock modal (transientUiStore's isIdbUnlockOpen) that the cold-start path uses. index.tsx has no existing test harness (side-effecting module-level bootstrap, no exported units) — this fix is verified by static tracing of every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap, useTranslation, I18nProvider all confirmed Redux-free) rather than an automated test; useSettingsView.test.ts covers the Lock Session path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(storage): route locked encrypted startup and Lock Session to the unlock modal, not a dead end Two confirmed paths left a user with configured at-rest encryption unable to reach the passphrase prompt at all: 1. Cold start: when the passphrase sentinel exists but no key is active yet, dbService.loadState() throws IdbStorageLockedError. index.tsx awaited this before mounting <App>, so App.tsx's own unlock-detection effect (which shows IdbUnlockModal) never got a chance to run — the user landed on the generic StorageErrorScreen, whose only action is "Reset Database & Reload" (destroys all local data). The bootstrap IIFE is now a named, re-invocable bootApp() function; a locked-storage catch renders a standalone IdbUnlockModal (wrapped only in I18nProvider — it has no Redux dependency) and retries the full boot in place on success. No page reload is used, since the freshly-unlocked in-memory key would be lost on one. 2. Mid-session: handleLockSession() cleared the key but never opened the unlock modal and didn't block editing, so a user could keep typing while every subsequent autosave silently failed closed (generic "Auto-Save Failed" toast, no path back to unlocking short of manually reopening Settings). It now also opens the same global unlock modal (transientUiStore's isIdbUnlockOpen) that the cold-start path uses. index.tsx has no existing test harness (side-effecting module-level bootstrap, no exported units) — this fix is verified by static tracing of every new dependency (IdbUnlockModal, Modal, Button, useFocusTrap, useTranslation, I18nProvider all confirmed Redux-free) rather than an automated test; useSettingsView.test.ts covers the Lock Session path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): restore sepia as the first-run appearance default settingsSlice.ts's defaultSettings.appearancePreset had drifted to 'default' on this branch, with a comment claiming first-run and legacy-rehydration defaults "must agree." That invariant doesn't hold on main: main deliberately keeps them different — defaultSettings.appearancePreset is 'sepia' (the first-run showcase theme) while normalizePersistedSettings's fallback for a missing/legacy field stays 'default' (so an existing user's old saved settings, missing the field entirely, isn't retroactively theme-shifted into a preset they never chose). This branch's rehydration path already correctly uses 'default' (services/storage/idbProjectStore.ts, unchanged); only the first-run default had regressed. This directly caused tests/e2e/a11y.spec.ts's "dark sepia theme has no serious axe violations" test to fail on #335's current SHA — the test emulates a dark color scheme on a brand-new (no persisted data) project and waits for both `.dark-theme` and `.appearance-sepia` to be applied, which now never happened since the first-run default silently stopped being sepia. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: refresh stack SHAs after the appearancePreset/unlock-routing fix cascade Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: refresh session handoff, archive the prior capture The prior CURRENT-HANDOFF.md (captured 2026-08-11T11:14:45Z) is now stale relative to this session's work: #335/#336/#337 all reached review-thread quiescence (except one deliberately-deferred #335 thread), two real data-loss/lockout defects were found and fixed on #335 (cold-start unlock routing, Lock Session routing), a genuine sepia-default regression was found and fixed, PR310-R009's disposition was corrected, and the performance ledger's stack SHAs were refreshed twice. Archived the prior capture alongside the existing codex handoff rather than discarding it, and wrote a new CURRENT-HANDOFF.md reflecting live state, including what's still genuinely open (the deferred pnpm thread, #310's remaining threads, #332/#333 packaged evidence, and whether #335's final E2E run landed green — a background poll was still in flight when this was written). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lora,settings): stale-run training race, stuck onboarding on cancel, stale model list on context switch Three confirmed race conditions from a fresh CodeAnt review wave: - loraThunks.ts: startTrainingThunk's catch classified a killed process's rejection using the CURRENT Redux currentRun.cancellationRequested, not the run that actually produced it. abort_lora_training awaits child-process exit before resolving, so a new training run can start before the killed run's own train_lora promise finally rejects — the catch would then wrongly archive the NEWER run as failed/aborted using the OLDER run's outcome. Now guards on currentRun.id matching the runId this invocation generated; a mismatch is a stale rejection and a no-op. - LoraOnboarding.tsx: cancelling the native Python file picker resolves with null, but handleSelectPython unconditionally bumped the request-generation guard before checking the result — invalidating the still-pending initial environment check without ever applying a replacement value, since the code only calls setEnv on a truthy result. The onboarding could get stuck showing "checking environment..." forever. The guard now only advances once there's an actual new result (success or error) to apply. - AiProviderCard.tsx: useConnectionContextReset invalidated in-flight tests/model-loads on a provider/endpoint/preset change but never cleared the already-rendered ollamaModels list from the previous context, so a user could select a model id that doesn't exist on the newly selected server until a fresh load happened to complete. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings,ai): decouple URL edits from protocol preset; guard preload progress against superseded attempts AiProviderCard.tsx: editing the "Ollama Server URL" field unconditionally reassigned localBackendPreset to 'custom', which isOpenAiCompatibleLocalPreset always routes through the OpenAI-compatible /v1/chat/completions protocol. A native-Ollama user just changing host/port (e.g. pointing at a LAN server) had their protocol silently switched and every completion started failing. The URL input now only updates ollamaBaseUrl; protocol selection stays explicit via the preset dropdown. localAiFacade.ts: preloadLocalModel's own inferenceProgressEmitter calls, and the ones inside generateLocalText gated by reportToGlobalProgress, ran unconditionally. retryLastPreload() starting a new attempt while an older, cancelled/superseded attempt's generateLocalText call is still settling could let that stale attempt's reportWebLlmReady/reportWebLlmError/reset overwrite the newer attempt's modal state after the fact. Extended the existing activePreloadAbort identity-guard pattern (already used to protect the cancel hook) to every progress-report call site via a new isCurrentAttempt option. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(desktop): revalidate the cached Python interpreter before trusting it resolve_python() trusted a cached ResolvedPython for the rest of the process's lifetime once found, whether from an explicit configured path match or the auto-discovery scan — never re-checking that the executable still exists, still has its executable bit, or still resolves. If the interpreter was removed, replaced, or lost permissions while the desktop app stayed open (a long-running session), environment checks would keep reporting Python as available and training would only fail later at spawn with a confusing error. cached_python_still_valid() reuses probe_python's own lightweight, synchronous filesystem-only check (validate_python_candidate_path — no subprocess spawn, so the cache's performance benefit is preserved) before trusting either cache hit; a stale entry now falls through to a full re-probe instead of being silently believed. Added unit coverage for validate_python_candidate_path's three outcomes (relative candidate, valid absolute path, missing absolute path) since the async caching path itself isn't practically unit-testable without tokio::test infra this crate doesn't carry yet. Verified via `cargo fmt --check` (this host cannot safely run a full `cargo check`/build — no cached target/ artifacts, would be a from-scratch compile of the whole dependency tree); CI's Tauri build job is the real gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(settings): auto-validate a newly saved Gemini key instead of deferring to the next generation call handleSaveKey only ran syntactic checks (length + control characters) before persisting and marking hasKey active — a misleading QNBS-v3 comment claimed "provider validation... determines credential validity" when no such validation actually ran at save time. A malformed or unrelated credential appeared saved successfully, and the Settings status disagreed with the actual provider authentication state until a later generation request happened to fail. Saving now automatically runs the same test-connection flow Test Connection already uses (which already flips hasKey back to false on an INVALID_API_KEY response); "Saved" no longer implies "verified." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: refresh session handoff after the #336 fresh-review-wave fix cascade Records the 7 additional real defects found and fixed on #336 (training-run race, onboarding stuck-on-checking, stale model list, protocol-preset mismatch, preload progress race, Python cache staleness, Gemini key validation gap), the cascade into #337, and flags the two things not yet independently confirmed: whether CI is green on the newest #336/#337 SHAs, and whether the unverified-locally Rust change actually compiles (only cargo fmt --check ran on this host). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deps): exclude the two unpatched uuid releases from the override range uuid: ">=11.1.1" was a bare floor that still permitted the two known- vulnerable exact releases 12.0.0 and 13.0.0 (GHSA-w5hq-g745-h8pq / CVE-2026-41907 — missing buffer bounds check in v3/v5/v6 API methods, fixed per major line at 11.1.1/12.0.1/13.0.1) if a future resolution ever landed on them; the currently-locked uuid@14.0.1 was unaffected today, but the override itself didn't structurally rule out regressing to a vulnerable version. Tightened to ">=11.1.1 <12.0.0 || >=12.0.1 <13.0.0 || >=13.0.1", excluding exactly those two releases per major line while still allowing everything else. Ran via a real `pnpm install` (not a hand-edit) so verifyDepsBeforeRun's installed-state check stays consistent; resolved version is unchanged (still 14.0.1, already satisfies the tighter range). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lora): clear cancellationRequested on a failed native abort; collapse wrapped QNBS-v3 comments If abortTraining() rejects, cancellationRequested stayed true forever — training may still be genuinely running, and its own later failure (dispatched by startTrainingThunk's catch) would then be misclassified as a user-requested abort instead of a real error. New trainingCancellationFailed reducer resets the marker; abortTrainingThunk dispatches it in a catch around the native call before rethrowing. Also: a fresh CodeRabbit pass (previously rate-limited/skipped on this PR) flagged 20 QNBS-v3 comments across services/lora/, features/lora/, components/settings/LocalAiSection.tsx, and their tests that wrapped across multiple physical lines, violating this repo's one-line QNBS-v3 convention — collapsed to single lines without changing their content. Regression tests: a deferred-promise version of the existing "dispatches trainingCancellationRequested before awaiting" test now actually proves the dispatch happens before the native abort settles (an immediately-resolving mock couldn't distinguish that from "after it resolved but before the next dispatch"); new test for the abort-failure path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(i18n): translate LoRA onboarding strings and fix broken shell commands in 9 locales lora.onboarding.installCmd (a literal shell command, "pip install unsloth trl peft") had been translated word-for-word in pt/sv/ko/is, producing non-functional commands (e.g. Portuguese "pip instalar sem preguiça trl peft" — "unsloth" mistranslated as "without laziness"). Restored to the exact original command in all four. Separately, all 18 lora.onboarding.error.* / selectingPython keys (added alongside the native Python environment-check work) were still raw English in the he/hu/is/ja/ko/pt/ru/sv/zh source locale trees — never translated at all, not just a stale-bundle issue. Translated all 18 keys across all 9 locales; also fixed two pre-existing partial translations noticed in the same section (ja and zh's lora.onboarding.description mixed English mid-sentence). Regenerated all 19 runtime bundles via i18n:bundle; key parity confirmed at 2903 keys per locale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: refresh performance ledger SHAs after #336 layering-mistake correction #335/#336/#337 heads and thread counts were stale after this session's cherry-pick correction and the 12-finding CodeRabbit loop on #336. Also drops the now-resolved uuid override-range note (applied via a real pnpm install) and marks PERF-333-006's #336 review reconciliation complete. * docs: capture session handoff after the second layering-mistake correction Archives the prior capture (pre-correction, stale c3f00cff/0b1cc2ed SHAs) and records the full correction: cherry-pick onto #336, reset+merge on #337, all 12 second-wave CodeRabbit threads replied and resolved, plus the still-open items (CodeRabbit re-review rate-limited, Tauri build dispatched but pending). * docs(pr310): mark review-thread reconciliation complete (0/317 unresolved) All 28 previously-unresolved PR #310 review threads verified against current code and replied to citing the specific replacement file/commit/test, then resolved via GraphQL resolveReviewThread. Commit/behavior/test reconciliation tables and packaged verification remain outstanding; #310 stays open (Option C). * docs: update handoff with Tauri build success and #310 reconciliation completion Tauri build 31549539018 confirmed success on all 3 platforms against #336's b01564ed, closing the previously-open Rust-compile NO-GO gap. PR #310's review-thread queue is now fully reconciled (0/317). CodeRabbit's re-review on #336 remains the one still-unconfirmed item. * fix(lora): only classify training as aborted on a confirmed native cancellation abort_lora_training resolved Ok(()) for NothingRunning/CancelPendingStart just like a confirmed process kill, so a coincidental unrelated training failure racing with a no-op abort request could be misclassified as user-cancelled. The Rust command now returns whether it actually confirmed stopping a process; abortTrainingThunk only dispatches trainingAborted() when true, and resets cancellationRequested (via the renamed trainingCancellationNotConfirmed) on both a native error and a no-op success. Also fixes an unnatural Swedish timeout translation to match the established wording pattern. * fix(lora,ai): distinguish pending-start cancellation from a true abort no-op abort_lora_training's wave-3 boolean return conflated CancelPendingStart (a genuine recorded cancellation for a not-yet-spawned run) with NothingRunning (a true no-op) — cancelling during Python discovery cleared cancellationRequested before the pending train_lora invocation's own training_cancelled rejection arrived, misclassifying a successful startup cancel as a training failure. The command now returns a three-way AbortOutcome; the thunk only clears the flag for NothingToCancel and leaves it set for PendingStartCancelled. Also: cached_python_still_valid now re-runs the full identity probe instead of a filesystem-only check, so an interpreter replaced in place at the same path is no longer trusted with a stale cached version. Fixes a stale-response race in AiProviderCard's WebGPU probe (missing the same testRequestIdRef guard used by every other async result in handleTest) and a CRLF edge case in the local OpenAI-compatible SSE parser. Collapses two wrapped QNBS-v3 comments. * fix(settings): restore the WebGPU auto-probe lost when the ollama auto-test was removed A prior commit removed a combined useEffect that auto-tested ollama AND auto-probed WebGPU capability whenever the provider was selected. The ollama removal was intentional (CORS/privacy — avoid auto-fetching a local server from the PWA), but WebGPU detection is a local hardware check with no such concern, so dropping it as collateral damage left the GPU badge stuck on "untested" until the user clicked Test Connection. This was never caught because #336 never ran its full test suite under CI while stacked — tests/unit/settings/ WebLlmPanel.test.tsx already asserted the auto-probe behavior and was silently broken. Restores a guarded auto-probe effect (reusing the same applyIfCurrent staleness guard as the manual Test Connection click) and updates the one AiProviderCard.test.tsx test whose premise no longer holds. * fix(a11y): set aria-busy on the provider connection status region while loading The status region announced changes via aria-live but never set aria-busy, unlike every other loading indicator in this codebase, so assistive tech had no programmatic signal that a connection test was actively running. * fix(test): give the binder-asset transaction mock a real .transaction back-reference deleteAllBinderAssetsForProject (services/storage/idbAssetStore.ts) batches its deletes into one transaction and reads store.transaction to attach oncomplete/ onerror/onabort, matching real IndexedDB. This test file's hand-rolled fake store predates that pattern and never exposed .transaction, so the mocked store was undefined at that access — crashing with "Cannot set properties of undefined (setting 'oncomplete')". Only surfaced now: this PR never ran its full test suite under CI while its base was a stacked branch instead of main. The fake store now tracks each queued request's completion promise so the transaction mock can fire oncomplete once they've all settled, mirroring real IDB batching semantics. * fix(storage,ai,a11y): fail-soft cache reads, single-flight IDB open, abort guard Real bugs found by #337's first full-CI review pass (this PR never ran full CI while stacked, so these were never caught): - aiInferenceCacheService.getCachedInference rejected on a malformed/undecodable cache row, contradicting its own documented non-authoritative cache policy — now degrades to a miss like setCachedInference already does. - sceneRevisionService.getDb() let concurrent callers each open a separate IndexedDB connection before the first resolved — now single-flights the open promise so concurrent saves share one connection. - secondaryPayloadStoreAdapter's abortForFailure could call transaction.abort() twice within one batch (two records failing before the transaction settles), throwing an uncaught InvalidStateError — now guarded to abort only once. - AiProviderCard's connection-status region never rendered text or aria-busy during 'loading', leaving assistive tech with no signal a test was running — adds a loading label (new settings.ai.providerStatusTesting key, all 19 locales) and aria-busy. - A test rerendered the wrong prop (openAiCompatibleBaseUrl instead of ollamaBaseUrl) for an lm_studio-preset scenario, not exercising the endpoint it claimed to. Also collapses wrapped QNBS-v3 comments across 9 files and refreshes stale doc-metric references (README locale count, performance ledger #337 SHA, an archived handoff's lockfile-editing instruction). Three deeper findings from the same review (cross-tab write admission across 7 methods, migration-verification-vs-concurrent-deletion races, and secondary adapters never being wired into production migration startup) were confirmed real but are deliberately left unresolved, not fixed here: the entire migration journal system has zero production callers today (only tests invoke it — the live UI has no disable/rotate-passphrase flow, matching this project's own Phase-4 tech debt note), so these races aren't yet reachable. Fixing them properly requires the same dedicated design work as that Phase-4 wiring, not a rushed patch in a review-response pass. * docs: capture handoff after merging #335/#336 into main and #337's full-CI wave Records: two admin-authorized merges (#335, #336) with two stacked-PR auto-close recoveries; the second CodeRabbit review wave on #337 (now based on main) that found and fixed real bugs plus the major discovery that the entire encryption migration journal system has zero production callers. * fix(test,docs): track cursor completion in binder mock; fix doc drift and MD018 deleteAllBinderAssetsForProject's fake transaction mock tracked put/get/delete completions but not openCursor's own walk, so oncomplete could theoretically fire before a cursor-driven delete settled — now the whole cursor walk is tracked as one pending item. Also collapses two of my own newly-wrapped QNBS-v3 comments from the prior commit (same one-line rule I've been fixing for others). Docs: reconciles a stale "Tauri build in flight" line against the same archived handoff's own later "confirmed success" sections, notes why the archive filename's timestamp intentionally differs from the internal capture time, fixes 3 markdownlint MD018 violations (bare #NNN at line start misparsed as a heading), and separates "not currently reachable" from "root cause" in the current handoff's description of the 3 deferred findings. * test(storage): cover scene/cache payload-shape branches in the secondary adapters secondaryProtectedStoreAdapters.ts sat at 26% coverage — only the two routing-key rejection paths were tested. Adds 9 tests exercising the legacy-flat-field vs. nested-payload branches, the schemaVersion guard, and the assertExactKeys rejection for both scene revisions and the inference cache, closing the codecov/patch shortfall (73.46% -> target). * fix(storage): cross-tab admission for protected writes vs. active migrations assertNoActiveEncryptionMigration() was a standalone read; a migration could claim ownership and commit in the gap between a writer's key resolution and its transaction commit, landing ciphertext under a superseded key/generation. CodeRabbit and Qodo independently flagged this on #337 (issue #338 items 3+4). Adds services/storage/protectedWriteAdmission.ts: a Web Locks API shared/exclusive reader-writer lock. Ordinary protected writers (images, binder assets, codex, RAG vectors, project/settings, snapshots, plus the two non-authoritative cache-eviction/scene-revision-retention deletion paths) hold it in shared mode for their full key-resolution-through-commit span. Migration batches hold it exclusively for one batch (~25 records), not the whole run, bounding writer starvation while closing the race exactly where it matters. This makes the deletion-vs-verification race (item 4) structurally impossible too — eviction/retention cannot run mid-batch. Feature-detects navigator.locks with a direct-call fallback for older runtimes. Adds a minimal fair-mutex navigator.locks mock to tests/setup.ts (jsdom/Node have no native implementation) and fixes an incomplete logger mock in dbServiceRetry.test.ts that broke once the admission wrapper's createLogger() call entered its import chain. * fix(storage,test): close deadlock risk in deleteProject, fix mock fairness Qodo's review of #339 found real issues: - deleteProject() split its admission guard across two separate lock spans (a guard+cascade, then a later re-acquisition for the final delete), leaving a gap where a migration batch could commit between them — undermining the very race this PR closes. Fixed by giving deleteProject() one continuous admission span and having it call a new unadmitted core (deleteAllBinderAssetsForProjectUnadmitted) instead of the public, self-locking method: nesting two withProtectedWriteAdmission calls for the same lock name can deadlock if an exclusive migration request queues between the outer and inner acquisition. - The tests/setup.ts navigator.locks mock only blocked new shared requests when an exclusive lock was currently held, not when one was queued — letting a stream of readers starve a waiting migration indefinitely, the opposite of the fairness this PR documents. Fixed to track queued waiters by mode and block new shared requests behind an already-queued exclusive one; added a regression test for shared-holder -> queued-exclusive -> later-shared ordering. - pruneAutoSnapshots() called the newly-admitted deleteSnapshot() once per stale snapshot in a loop, performing N separate lock acquisitions and transactions instead of the single-lock/single-transaction batching this PR already uses for deleteAllBinderAssetsForProject. Extracted a shared deleteSnapshotsUnadmitted(ids) core used by both deleteSnapshot() and a now-batched pruneAutoSnapshots(). Also fixes 2 more test files with the same incomplete logger mock found on CI's first full run of this branch (store.test.ts, fileSystemService.test.ts — dbServiceRetry.test.ts was already fixed) and gives dbServiceSnapshots.test.ts's fake IDB store the same .transaction back-reference fix applied earlier to dbServiceBinder.test.ts, now that deleteSnapshot's transaction batching needs it. Collapses 2 more of my own QNBS-v3 comments that got wrapped across multiple lines (tests/setup.ts, protectedWriteAdmission.ts header) back to one physical line each. * fix(storage,test): admit reencryptLegacyEntry, in-process fallback lock, more CodeRabbit's fresh review found real findings beyond the earlier qodo pass: - aiInferenceCacheService.ts's reencryptLegacyEntry() (opportunistic re-encrypt on legacy read) called encodeEntry/persistEntry without withProtectedWriteAdmission — the exact race this PR closes for every other write path. Now shares the same admission boundary. - idbProjectStore.ts's deleteProject() re-checks assertIdbProtectedWriteAllowed immediately before the final project-record delete, not just once at the top — the binder-asset cascade under the same admission hold takes real async time, during which a Lock Session (not a migration, which admission already excludes) could still fire. - protectedWriteAdmission.ts: runtimes without navigator.locks now get a real in-process (same-tab) reader/writer mutex instead of running unguarded — weaker than Web Locks (no cross-tab protection) but strictly better than no admission at all. Same fairness contract (queued exclusive blocks new shared) as the Web Locks path and the test mock. - tests/setup.ts's Web Locks mock now defaults the 2-arg request() form to 'exclusive' per the real LockManager.request() spec (was defaulting to 'shared') — dead code today since production call sites always pass an explicit mode, but closes a latent fidelity gap CodeRabbit's spec lookup caught. - idbSnapshotStore.ts: moved createSnapshot's wordCount computation (pure CPU work) outside the admission window. - Collapsed one more of my own QNBS-v3 comments that stayed wrapped across two lines despite the earlier fix pass (aiInferenceCacheService.ts), and added one that was missing entirely (sceneRevisionService.ts's deleteRevision). - dbServiceSnapshots.test.ts's fake transaction now tracks getAllKeys()'s own request into its pending list too, so oncomplete cannot theoretically fire before it settles, even though it's always a standalone transaction in production code. CodeRabbit's deep dive into tests/setup.ts's fairness fix ran a Node simulation against what turned out to be a stale/cached view of the mock (matching the pre-fix version) — verified the actual current code already has the queued-exclusive-blocks-new-shared fix from the prior commit. * fix(storage): claim fallback lock ownership synchronously, not after wake fallbackWakeNext() previously resolved a waiting caller's promise before updating fallbackExclusiveHeld/fallbackActiveShared — the state update happened later, in the woken waiter's own continuation re-checking its while loop. That left a real gap: any other acquireFallback() call landing between release() firing wakeNext() and the woken waiter's continuation actually running would see the lock as free and could be admitted too. Restructured so ownership is always claimed synchronously at the moment a grant decision is made — either immediately in the new fallbackTryClaim() fast path, or inside fallbackWakeNext() itself before it resolves a waiter's promise. A caller can no longer observe a state where the lock looks free but hasn't actually been claimed by anyone. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Merged
8 tasks
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Scope
P0 safety foundation for optional IndexedDB at-rest encryption.
Why
Existing lifecycle paths could conflate disabled storage with an enabled-but-locked library. This draft deliberately blocks destructive lifecycle actions rather than claiming cross-store conversion is complete.
Validation
Focused storage lifecycle, locked-write, legacy-read, salt, and settings rehydration tests passed locally; Biome, documentation, and i18n checks passed. Full coverage/E2E and desktop verification are deferred to cloud CI per the constrained-host policy.
Follow-up
The stacked child PRs provide desktop/provider hardening and the recovery-journal work. This PR must not be presented as completing rekey/disable recovery by itself.
Summary by Sourcery
Fail closed when IndexedDB encryption is configured but locked, and add a persistent plain writing-surface option.
Bug Fixes:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
CodeAnt-AI Description
Fail closed for locked encrypted storage and add a plain writing surface
What Changed
Impact
✅ No plaintext writes while encrypted storage is locked✅ No legacy content exposure before unlock✅ Existing encrypted data protected from unsafe disable or rekey operations💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.