fix: harden desktop AI integrations - #336
Merged
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds OpenAI-compatible local backends, controlled preload retries, Gemini key validation, selectable Python runtimes for LoRA, cancellation tracking, localized diagnostics, and related tests. ChangesLocal AI behavior
LoRA Python runtime
Gemini API-key validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ProviderSettings
participant aiProviderService
participant LocalBackend
ProviderSettings->>aiProviderService: test connection or list models
aiProviderService->>LocalBackend: request /v1/models
LocalBackend-->>aiProviderService: models or diagnostic error
ProviderSettings->>aiProviderService: generate with selected preset
aiProviderService->>LocalBackend: stream /v1/chat/completions
LocalBackend-->>ProviderSettings: SSE response chunks
sequenceDiagram
participant LoraOnboarding
participant loraTrainingService
participant TauriDialog
participant NativeLoraCommands
LoraOnboarding->>loraTrainingService: select Python executable
loraTrainingService->>TauriDialog: open file picker
TauriDialog-->>loraTrainingService: selected path
loraTrainingService->>NativeLoraCommands: persist and validate path
NativeLoraCommands-->>LoraOnboarding: environment report
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideDesktop AI integrations are hardened by routing LM Studio/vLLM/custom local backends through OpenAI-compatible endpoints, tightening Gemini key syntax checks, making Local AI preload/cancel/retry operate on the actual underlying work, and centralizing Python interpreter discovery and training environment reporting with explicit locked-storage behavior for encrypted IndexedDB libraries. Sequence diagram for local backend routing via OpenAI-compatible endpointssequenceDiagram
actor User
participant AiProviderCard
participant aiProviderService
participant LocalServer as Local_server
User->>AiProviderCard: click Test Connection
AiProviderCard->>aiProviderService: testAIConnection(provider='ollama', localBackendPreset)
alt LM Studio / vLLM / custom
aiProviderService->>aiProviderService: isOpenAiCompatibleLocalPreset(preset)
aiProviderService->>LocalServer: testOpenAiCompatibleLocalConnection(ollamaBaseUrl)
LocalServer-->>aiProviderService: /models (OpenAI-compatible)
aiProviderService-->>AiProviderCard: TestConnectionResult{localServer.modelNames}
else Native Ollama
aiProviderService->>LocalServer: testOllamaConnection(ollamaBaseUrl)
LocalServer-->>aiProviderService: Ollama status/models
aiProviderService-->>AiProviderCard: TestConnectionResult (Ollama)
end
User->>AiProviderCard: start local generation
AiProviderCard->>aiProviderService: streamProvider(provider='ollama', localBackendPreset)
alt OpenAI-compatible preset
aiProviderService->>LocalServer: streamOpenAiCompatibleLocal(prompt, /chat/completions)
else Native Ollama
aiProviderService->>LocalServer: streamOllama(prompt)
end
LocalServer-->>aiProviderService: streamed chunks
aiProviderService-->>AiProviderCard: onChunk(text)
AiProviderCard-->>User: show local AI response
Sequence diagram for encrypted IndexedDB writes with locked-storage handlingsequenceDiagram
participant Component as SettingsView/ProjectStore
participant EncSvc as StorageEncryptionService
participant IDB as IndexedDBStore
Component->>EncSvc: resolveProtectedWriteKey()
alt Runtime key present
EncSvc-->>Component: CryptoKey
Component->>EncSvc: idbEncryptWithKey(key, data)
EncSvc-->>Component: Uint8Array ciphertext
Component->>IDB: put(ciphertext)
IDB-->>Component: success
else No runtime key
Component->>EncSvc: hasPassphraseSentinel()
alt Sentinel exists (ENABLED_LOCKED)
EncSvc-->>Component: throw IdbStorageLockedError
Component-->>Component: surface locked-storage error
else No sentinel (DISABLED)
EncSvc-->>Component: null
Component->>IDB: put(compressData(data))
IDB-->>Component: success (plaintext legacy write)
end
end
Sequence diagram for LoRA training start and cancellation across Tauri and front-endsequenceDiagram
actor User
participant LoraUI as LoraTrainingUI
participant Thunks as loraThunks
participant Service as loraTrainingService
participant Tauri as tauri::lora
User->>LoraUI: start training
LoraUI->>Thunks: startTrainingThunk(config)
Thunks->>Tauri: invoke train_lora(payload)
Tauri->>Tauri: reserve_training_slot()
Tauri->>Tauri: resolve_python(app)
Tauri->>Tauri: spawn Python child (train_writer_lora.py)
Tauri-->>Thunks: training_completed / error
Thunks-->>LoraUI: trainingCompleted or trainingFailed
User->>LoraUI: abort training
LoraUI->>Thunks: abortTrainingThunk()
Thunks->>Thunks: trainingCancellationRequested()
Thunks->>Service: abortTraining()
Service->>Tauri: invoke abort_lora_training()
Tauri->>Tauri: request_training_abort()
alt Child running
Tauri->>Tauri: terminate_training_process(pid)
Tauri-->>Service: true (confirmedActiveProcess)
Service-->>Thunks: true
Thunks->>Thunks: trainingAborted()
Thunks-->>LoraUI: show cancelled
else Nothing running / pending start
Tauri-->>Service: false
Service-->>Thunks: false
Thunks->>Thunks: trainingCancellationNotConfirmed()
Thunks-->>LoraUI: no-op (no run cancelled)
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
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. |
Uh oh!
There was an error while loading. Please reload this page.
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
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. |
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.
CodeAnt AI finished running the review. |
qnbs
marked this pull request as ready for review
August 11, 2026 08:55
PR Summary by QodoHarden desktop AI integrations and fail-closed IDB encryption lifecycle
AI Description
Diagram
High-Level Assessment
Files changed (143) |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (9)
services/aiProviderService.ts (1)
844-865: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog a sanitized local-server failure diagnostic.
This mapper returns a safe error classification but discards the transport cause. Log a sanitized error kind and message through
services/logger.ts. Do not log URL credentials. Keep the returned localization parameters limited to safe endpoint context.Based on learnings: “log the full diagnostic details via services/logger.ts, and return only safe localization context.” As per coding guidelines: “technical details may only be emitted through
services/logger.tsusing Biome-compliantwarnorerror.”🤖 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/aiProviderService.ts` around lines 844 - 865, Update localServerFailure to emit a sanitized diagnostic through services/logger.ts using a Biome-compliant warn or error call, including the failure kind and message without URL credentials. Preserve the existing safe classifications and keep returned params limited to the endpoint context.Sources: Coding guidelines, Learnings
tests/unit/ApiKeySection.test.tsx (3)
101-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required QNBS-v3 rationale for this test change.
This block adds behavior-relevant TypeScript tests but contains no one-line explanation of the change. Add the comment near Line 101.
As per coding guidelines, every content-relevant TypeScript or JavaScript change must include a one-line
QNBS-v3comment explaining the reason and impact.🤖 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 `@tests/unit/ApiKeySection.test.tsx` around lines 101 - 125, Add a one-line QNBS-v3 comment immediately before the new tests beginning with “accepts an unfamiliar non-empty key format,” explaining that unfamiliar non-empty keys are trimmed and accepted while unsafe or overlong keys are rejected before provider validation.Source: Coding guidelines
115-119: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the validator boundary and the component save path.
Add an assertion that exactly 4096 characters are accepted. Also exercise invalid values through
ApiKeySectionand assert thatmockSaveGeminiApiKeyis not called. The current test only calls the helper directly.Suggested boundary assertion
expect(isSyntacticallySafeGeminiApiKey('a'.repeat(4097))).toBe(false); +expect(isSyntacticallySafeGeminiApiKey('a'.repeat(4096))).toBe(true);🤖 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 `@tests/unit/ApiKeySection.test.tsx` around lines 115 - 119, The test covering isSyntacticallySafeGeminiApiKey should assert that exactly 4096 characters are accepted, while retaining the rejection for 4097 characters. Extend the ApiKeySection component tests to submit invalid keys, including control-character and overlong values, and verify mockSaveGeminiApiKey is not called.
106-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
userEventfor normal input and click actions.Replace
fireEvent.changeandfireEvent.clickwithuserEvent.setup(),user.type(...), anduser.click(...). KeepfireEventonly for low-level input thatuserEventcannot model, such as injecting a control character.As per coding guidelines, tests must use
@testing-library/user-eventfor click, typing, and change interactions.🤖 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 `@tests/unit/ApiKeySection.test.tsx` around lines 106 - 109, Update the test around the API key input and save action to create a userEvent instance with userEvent.setup(), then replace fireEvent.change with user.type and fireEvent.click with user.click. Keep fireEvent only for interactions userEvent cannot model, such as control-character injection.Source: Coding guidelines
services/lora/loraTrainingService.ts (1)
144-175: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the required
QNBS-v3why-comment for the new environment contract.The new
TrainingEnvironmenttype, theNativeTrainingEnvironmentshape, andfromNativeEnvironmentare a non-trivial change. The repository requires one single-line// QNBS-v3: ...comment for such changes. The Rust counterpart insrc-tauri/src/lora.rsfollows this convention, this block does not.As per coding guidelines: "Bei jeder inhaltlich relevanten Änderung in TypeScript oder JavaScript einen einzeiligen Kommentar im Format
// QNBS-v3: [Grund / Impact / Kreativer Mehrwert]ergänzen".🤖 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/lora/loraTrainingService.ts` around lines 144 - 175, Add exactly one single-line `// QNBS-v3: ...` why-comment adjacent to the changed environment contract, covering the purpose, impact, or benefit of `TrainingEnvironment`, `NativeTrainingEnvironment`, and `fromNativeEnvironment`; do not add additional comments or alter the mapping behavior.Source: Coding guidelines
src-tauri/src/lora.rs (3)
472-488: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a boundary test for the minimum version check.
The tests cover parsing only. The
(major, minor) < (3, 10)comparison at Line 108 is the rule most likely to regress, for example if someone reintroduces string comparison. Add cases forPython 3.9.18andPython 3.10.0against the parsed tuple.💚 Proposed additional test
#[test] fn rejects_malformed_python_version_output() { assert_eq!(parse_python_version("not-python"), None); } ++ #[test]+ fn orders_minor_versions_numerically() {+ let old = parse_python_version("Python 3.9.18").expect("parses");+ let new = parse_python_version("Python 3.10.0").expect("parses");+ assert!((old.0, old.1) < (super::MIN_PYTHON_MAJOR, super::MIN_PYTHON_MINOR));+ assert!((new.0, new.1) >= (super::MIN_PYTHON_MAJOR, super::MIN_PYTHON_MINOR));+ }🤖 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 `@src-tauri/src/lora.rs` around lines 472 - 488, Add tests in the existing `tests` module covering the minimum-version boundary through `parse_python_version`: assert `Python 3.9.18` parses to `(3, 9, "3.9.18")` and is below the supported minimum, while `Python 3.10.0` parses to `(3, 10, "3.10.0")` and is accepted. Keep the assertions tuple-based to guard numeric major/minor comparison.
382-441: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated degraded-report construction.
Four
LoraEnvReportliterals differ only in thelast_errorvalue. A small helper reduces the duplication and prevents field drift whenLoraEnvReportgains fields.♻️ Proposed helper
+fn python_only_report(python: &ResolvedPython, error: &str) -> LoraEnvReport {+ LoraEnvReport {+ python_available: true,+ unsloth_available: false,+ cuda_available: false,+ vram_gb: 0.0,+ python_version: python.version.clone(),+ python_path: Some(python.path.clone()),+ last_error: Some(error.to_string()),+ }+}🤖 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 `@src-tauri/src/lora.rs` around lines 382 - 441, Extract the repeated degraded LoraEnvReport construction from the helper-report flow into a small local helper, parameterized by the last_error value. Replace the missing-script, spawn-failure, nonzero-exit, and JSON-parse-failure literals with calls to that helper while preserving their existing error strings and report field values.
341-348: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftUnix cancellation stops only the parent process, unlike the Windows path.
The Windows branch uses
taskkill /T, which terminates the process tree. The Unix branch sendsSIGTERMto the single PID. Python training commonly spawns child processes, for example dataloader workers. Those children can survive cancellation and keep holding GPU memory.Spawn the trainer in its own process group and signal the group, for example with
kill -TERM -<pgid>after setting the process group at spawn time.🤖 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 `@src-tauri/src/lora.rs` around lines 341 - 348, Update the Unix trainer launch and cancellation flow to use a dedicated process group: configure the spawned trainer’s process group at creation, then change the cancellation command in the Unix branch of the relevant function to target the group ID (for example, a negative PGID) so SIGTERM reaches the trainer and all descendants. Preserve the existing error mapping and Windows behavior.components/lora/LoraOnboarding.tsx (1)
16-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the exported
TrainingEnvironmenttype instead of redeclaring its fields.
services/lora/loraTrainingService.tsnow exportsTrainingEnvironmentwith exactly these fields.EnvStatusduplicates them. The two definitions will drift when the native report gains a field.♻️ Proposed refactor
+import type { TrainingEnvironment } from '../../services/lora/loraTrainingService';++interface EnvStatus extends TrainingEnvironment {+ loaded: boolean;+}const [api, setApi] = useState<{ - checkTrainingEnvironment: () => Promise<Omit<EnvStatus, 'loaded'>>;- selectPythonExecutable: () => Promise<Omit<EnvStatus, 'loaded'> | null>;+ checkTrainingEnvironment: () => Promise<TrainingEnvironment>;+ selectPythonExecutable: () => Promise<TrainingEnvironment | null>; } | null>(null);A type-only import does not defeat the code-splitting intent of the dynamic import.
🤖 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 `@components/lora/LoraOnboarding.tsx` around lines 16 - 27, Update the type declarations in useLoraTrainingService to reuse the exported TrainingEnvironment type from loraTrainingService via a type-only import, deriving the existing loaded-excluded status shape instead of duplicating EnvStatus fields. Preserve the current dynamic runtime import and API behavior.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/lora/LoraOnboarding.tsx`:
- Around line 126-128: Update components/lora/LoraOnboarding.tsx at lines
126-128 to include env.lastError when Python is available and fall back to
env.message when unavailable. At lines 80-86, map native diagnostic categories
such as configured_path_not_absolute, incompatible_version, and
permission_denied to translation keys, replacing the raw python_selection_failed
fallback with a translated generic message. Add the required translation keys to
locales/en/lora.json first, then propagate them to every other locale tree.
- Around line 150-157: The Python selection button in LoraOnboarding must use
the required --sc-ring-focus token, expose aria-busy while handleSelectPython is
running, and provide a brief aria-live status update for the async state. Update
the loading label to use the dedicated lora.onboarding.selectingPython
translation key so it describes the file-picker action.
In `@components/settings/AiSections.tsx`:
- Around line 72-73: Remove the provider-based key from the provider card so
switching providers preserves unsaved inputs and local state. In AiProviderCard,
add an effect depending on provider and browserOllamaEnabled that resets
testStatus and testError and increments testRequestIdRef.current to invalidate
pending tests. Add coverage that enters an unsaved key, switches providers, and
verifies the input remains.
In `@services/aiProviderService.ts`:
- Line 257: Update the endpoint resolution used by the generation/streaming path
around normalizeOpenAiCompatibleBaseUrl so a blank or whitespace-only
ollamaBaseUrl resolves to the same preset-aware default used by diagnostics at
lines 874-876, rather than producing an app-relative URL. Reuse a shared
resolver for both paths and add coverage for an empty ollamaBaseUrl.
- Around line 283-304: Update the streaming loop in the response-handling method
around reader.read() so an aborted opts.signal throws an abort error instead of
breaking and reaching callbacks.onDone. Preserve normal completion behavior for
non-aborted streams, and add a test covering cancellation after response headers
that verifies callbacks.onDone is not invoked.
- Around line 251-305: Add one single-line QNBS-v3 why-comment for each affected
site: services/aiProviderService.ts lines 251-305 should explain the
OpenAI-compatible local streaming behavior;
features/project/thunks/thunkUtils.ts line 21 should explain preset propagation
into runtime AI options; components/settings/AiProviderCard.tsx lines 148-199
should explain explicit preset-aware model loading and testing;
tests/unit/aiProviderService.test.ts lines 334-405 should explain LM Studio
contract coverage; and tests/unit/settings/AiProviderCard.test.tsx lines 251-278
should explain explicit preset-aware UI coverage. Place each comment adjacent to
the relevant behavior without adding more than one comment per site.
In `@services/localAiFacade.ts`:
- Around line 50-54: Add exactly one physical-line QNBS-v3 why-comment for each
substantive changed block at services/localAiFacade.ts lines 50-54, 138-145,
190-192, 223, 306-317, and 343, plus tests/unit/LocalAiDownloadProgress.test.tsx
lines 33, 43-52, and 119-136, and tests/unit/localAiFacade.test.ts lines
295-306. Each comment must explain the block’s specific rationale: centralized
abort/cancellation classification, progress normalization before UI emission,
ready-state emission for successful WebLLM results, bypassing heuristic fallback
on caller cancellation, retaining the last preload model for retry, updating the
retry target on preload entry, reset behavior as part of the mock contract,
facade-boundary retry/abort mocking, invoking underlying preload operations for
cancellation and retry, and publishing a terminal error for fallback preload.
- Around line 395-401: Update the preload error handling around the catch block
in the local model preload method to reset progress through reportWebLlmError
when the caller-provided signal is aborted, ensuring loading and isLocalAiBusy()
clear. Preserve the existing component-owned cancellation behavior used by
abortActivePreload(), and add a regression test covering caller-signal abortion
and the cleared loading state.
In `@services/lora/loraTrainingService.ts`:
- Around line 214-216: Update the tauriInvoke arguments in the LoRA training
flow to use camelCase keys: replace python_path with pythonPath, and apply the
equivalent baseModel and adapterPath key changes to the merge_lora invocation.
Preserve the existing argument values and command behavior.
In `@src-tauri/src/lora.rs`:
- Around line 252-262: Update the child startup error paths around child.id(),
set_active_training_pid(), and stdout.take() to kill the spawned child before
returning when PID retrieval or active-PID bookkeeping fails, as well as when
stdout is unavailable. Preserve the existing error propagation and cleanup
behavior after invoking child.kill().
- Around line 134-159: Move the synchronous resolve_python work off Tokio worker
threads at each async command call site: check_lora_environment, train_lora,
merge_lora, and set_lora_python_path. Wrap the resolution in
tokio::task::spawn_blocking and propagate its result and errors, preserving
explicit-path behavior and candidate probing.
- Around line 285-289: Update the stderr truncation in the training failure path
to limit the lossy-decoded string by characters rather than slicing at a raw
byte offset. Preserve the 500-character maximum and existing “Training failed”
error format while ensuring truncation never splits a multibyte character.
- Around line 200-202: Update train_lora around current_training_pid and the
later active-PID write to reserve the training slot atomically under one lock
before spawning, preventing concurrent invocations from both proceeding. After
the Python process starts, replace the reservation with its real PID; on every
early return between reservation and spawn, including the paths around the
existing lines 210-215, 243, 250, and 254, call set_active_training_pid(None) to
release it.
- Around line 43-45: Add #[serde(default)] to the optional python_path and
last_error fields in the relevant LoRA report struct so deserialization succeeds
when scripts/check_lora_env.py omits them, preserving check_lora_environment’s
normal parsing behavior.
---
Nitpick comments:
In `@components/lora/LoraOnboarding.tsx`:
- Around line 16-27: Update the type declarations in useLoraTrainingService to
reuse the exported TrainingEnvironment type from loraTrainingService via a
type-only import, deriving the existing loaded-excluded status shape instead of
duplicating EnvStatus fields. Preserve the current dynamic runtime import and
API behavior.
In `@services/aiProviderService.ts`:
- Around line 844-865: Update localServerFailure to emit a sanitized diagnostic
through services/logger.ts using a Biome-compliant warn or error call, including
the failure kind and message without URL credentials. Preserve the existing safe
classifications and keep returned params limited to the endpoint context.
In `@services/lora/loraTrainingService.ts`:
- Around line 144-175: Add exactly one single-line `// QNBS-v3: ...` why-comment
adjacent to the changed environment contract, covering the purpose, impact, or
benefit of `TrainingEnvironment`, `NativeTrainingEnvironment`, and
`fromNativeEnvironment`; do not add additional comments or alter the mapping
behavior.
In `@src-tauri/src/lora.rs`:
- Around line 472-488: Add tests in the existing `tests` module covering the
minimum-version boundary through `parse_python_version`: assert `Python 3.9.18`
parses to `(3, 9, "3.9.18")` and is below the supported minimum, while `Python
3.10.0` parses to `(3, 10, "3.10.0")` and is accepted. Keep the assertions
tuple-based to guard numeric major/minor comparison.
- Around line 382-441: Extract the repeated degraded LoraEnvReport construction
from the helper-report flow into a small local helper, parameterized by the
last_error value. Replace the missing-script, spawn-failure, nonzero-exit, and
JSON-parse-failure literals with calls to that helper while preserving their
existing error strings and report field values.
- Around line 341-348: Update the Unix trainer launch and cancellation flow to
use a dedicated process group: configure the spawned trainer’s process group at
creation, then change the cancellation command in the Unix branch of the
relevant function to target the group ID (for example, a negative PGID) so
SIGTERM reaches the trainer and all descendants. Preserve the existing error
mapping and Windows behavior.
In `@tests/unit/ApiKeySection.test.tsx`:
- Around line 101-125: Add a one-line QNBS-v3 comment immediately before the new
tests beginning with “accepts an unfamiliar non-empty key format,” explaining
that unfamiliar non-empty keys are trimmed and accepted while unsafe or overlong
keys are rejected before provider validation.
- Around line 115-119: The test covering isSyntacticallySafeGeminiApiKey should
assert that exactly 4096 characters are accepted, while retaining the rejection
for 4097 characters. Extend the ApiKeySection component tests to submit invalid
keys, including control-character and overlong values, and verify
mockSaveGeminiApiKey is not called.
- Around line 106-109: Update the test around the API key input and save action
to create a userEvent instance with userEvent.setup(), then replace
fireEvent.change with user.type and fireEvent.click with user.click. Keep
fireEvent only for interactions userEvent cannot model, such as
control-character injection.
🪄 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: 0e6cb05e-a4b1-4785-8aa3-3e3fa1ee87b9
📒 Files selected for processing (36)
components/ApiKeySection.tsxcomponents/lora/LoraOnboarding.tsxcomponents/settings/AiProviderCard.tsxcomponents/settings/AiSections.tsxcomponents/settings/LocalAiDownloadProgress.tsxfeatures/project/thunks/thunkUtils.tslocales/ar/lora.jsonlocales/de/lora.jsonlocales/el/lora.jsonlocales/en/lora.jsonlocales/es/lora.jsonlocales/eu/lora.jsonlocales/fa/lora.jsonlocales/fi/lora.jsonlocales/fr/lora.jsonlocales/he/lora.jsonlocales/hu/lora.jsonlocales/is/lora.jsonlocales/it/lora.jsonlocales/ja/lora.jsonlocales/ko/lora.jsonlocales/pt/lora.jsonlocales/ru/lora.jsonlocales/sv/lora.jsonlocales/zh/lora.jsonservices/aiProviderService.tsservices/localAiFacade.tsservices/lora/loraTrainingService.tssrc-tauri/src/lib.rssrc-tauri/src/lora.rstests/unit/ApiKeySection.test.tsxtests/unit/LocalAiDownloadProgress.test.tsxtests/unit/aiProviderService.test.tstests/unit/localAiFacade.test.tstests/unit/lora/loraTrainingService.test.tstests/unit/settings/AiProviderCard.test.tsx
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.
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="services/aiProviderService.ts"line_range="252-261" />
<code_context>
+async function streamOpenAiCompatibleLocal(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Passing `null` as `signal` to `localServerFetch` can be inconsistent with Fetch semantics.
Elsewhere you pass `signal: opts.signal` directly, but here you use `opts.signal ?? null`. If `localServerFetch` forwards this to `fetch`, `null` is not a valid `AbortSignal` and introduces an unnecessary extra case. Please drop the `?? null` and either pass `opts.signal` or omit `signal` when it’s `undefined` so behavior stays consistent with the Fetch API.
Suggested implementation:
```typescriptconst endpoint =normalizeOpenAiCompatibleBaseUrl(opts.ollamaBaseUrl??'http://localhost:1234');
const messages =opts.systemPrompt? [
{ role: 'system', content: sanitizePromptValue(opts.systemPrompt) },
{ role: 'user', content: sanitizePromptValue(prompt) },
]
``````typescriptawaitlocalServerFetch(endpoint+'/v1/chat/completions', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
...(opts.apiKey? { Authorization: `Bearer ${opts.apiKey}` } : {}),
},
body: JSON.stringify({
model: opts.model,
messages,
stream: true,
}),
// Pass the AbortSignal directly; undefined will be ignored by fetch,// matching standard Fetch semantics and avoiding a `null` special case.
signal: opts.signal,
});
```If there are other call sites in this file (or elsewhere) that use `signal:someSignal??null` when calling `localServerFetch`, they should be updated similarly to `signal: someSignal` to keep behavior consistent with the Fetch API.</issue_to_address>### Comment 2<location path="tests/unit/aiProviderService.test.ts" line_range="334-332" /><code_context> expect(result.error).toContain('timed out'); });++ it('uses the OpenAI-compatible models endpoint for the LM Studio preset', async () => {+ (window as { __TAURI_INTERNALS__?: unknown }).__TAURI_INTERNALS__ = {};+ mockPluginHttpFetch.mockResolvedValueOnce(+ new Response(JSON.stringify({ data: [{ id: 'local-model' }] }), { status: 200 }),+ );++ const result = await testAIConnection('ollama', {+ ollamaBaseUrl: 'http://127.0.0.1:1234/',+ localBackendPreset: 'lm_studio',+ });++ expect(result).toMatchObject({</code_context><issue_to_address>**suggestion (testing):** Add coverage for vLLM preset and invalid /models payloads in OpenAI-compatible local diagnosticsTo fully exercise the new behavior:- Add a matching test for the `vllm` preset to confirm it uses the OpenAI-compatible `/v1` endpoints and does not fall back to the Ollama-specific path.- Add a unit test for `testOpenAiCompatibleLocalConnection` that covers an invalid models response (e.g., missing or non-array `data`, or a non-JSON body) and asserts `kind: 'invalidResponse'` is returned.These tests will help catch regressions if local server responses or presets change.Suggested implementation:```typescriptit('uses the OpenAI-compatible models endpoint for the LM Studio preset', async () => {
(windowas { __TAURI_INTERNALS__?:unknown }).__TAURI_INTERNALS__= {};
mockPluginHttpFetch.mockResolvedValueOnce(
newResponse(JSON.stringify({ data: [{ id: 'local-model' }] }), { status: 200 }),
);
const result =awaittestAIConnection('ollama', {
ollamaBaseUrl: 'http://127.0.0.1:1234/',
localBackendPreset: 'lm_studio',
});
expect(result).toMatchObject({
ok: true,
localServer: {
normalizedEndpoint: 'http://127.0.0.1:1234/v1',
transport: 'tauri-http',
modelNames: ['local-model'],
},
});
expect(mockPluginHttpFetch).toHaveBeenCalledWith(
'http://127.0.0.1:1234/v1/models',
expect.any(Object),
);
});
it('uses the OpenAI-compatible models endpoint for the vLLM preset', async () => {
(windowas { __TAURI_INTERNALS__?:unknown }).__TAURI_INTERNALS__= {};
mockPluginHttpFetch.mockResolvedValueOnce(
newResponse(JSON.stringify({ data: [{ id: 'vllm-local-model' }] }), { status: 200 }),
);
const result =awaittestAIConnection('ollama', {
ollamaBaseUrl: 'http://127.0.0.1:5678/',
localBackendPreset: 'vllm',
});
expect(result).toMatchObject({
ok: true,
localServer: {
normalizedEndpoint: 'http://127.0.0.1:5678/v1',
transport: 'tauri-http',
modelNames: ['vllm-local-model'],
},
});
expect(mockPluginHttpFetch).toHaveBeenCalledWith(
'http://127.0.0.1:5678/v1/models',
expect.any(Object),
);
});
it('returns invalidResponse when OpenAI-compatible models response is malformed', async () => {
(windowas { __TAURI_INTERNALS__?:unknown }).__TAURI_INTERNALS__= {};
// data is not an array -> should be treated as an invalid responsemockPluginHttpFetch.mockResolvedValueOnce(
newResponse(JSON.stringify({ data: { id: 'not-an-array' } }), { status: 200 }),
);
const result =awaittestOpenAiCompatibleLocalConnection({
baseUrl: 'http://127.0.0.1:9999/',
transport: 'tauri-http',
});
expect(result).toMatchObject({
kind: 'invalidResponse',
});
```The new `testOpenAiCompatibleLocalConnection` test assumes:1. `testOpenAiCompatibleLocalConnection` is exported from the module under test and already imported at the top of this file. If it is not imported yet, add an import such as:`import { testOpenAiCompatibleLocalConnection } from'@/services/aiProviderService';`2. The function accepts a single options object with at least `{ baseUrl: string; transport: 'tauri-http'|'fetch'|... }`. If the actual signature differs (e.g., positional arguments or different option names), adjust the call accordingly while keeping the semantics: - It should target an OpenAI-compatible `/v1/models` endpoint. - It should receive a malformed `models` response (non-array or missing `data`) and return `{ kind: 'invalidResponse' }`.3. If your implementation differentiates transports differently, ensure `'tauri-http'` matches your existing enum/string literal used elsewhere in these tests.</issue_to_address>### Comment 3<location path="tests/unit/LocalAiDownloadProgress.test.tsx" line_range="122-131" /><code_context>+ expect(mockAbortActivePreload).toHaveBeenCalledTimes(1);+ });++ it('retries the actual prior preload after an error', async () => {+ const { inferenceProgressEmitter } = await import('../../services/ai/inferenceProgressEmitter');+ const user = userEvent.setup();+ mockSnapshot = {+ state: 'error',+ progress: 0.2,+ estimatedSecondsRemaining: null,+ text: 'Download failed',+ };+ mockRetryLastPreload.mockResolvedValue(undefined);+ render(<LocalAiDownloadProgress />);++ await user.click(screen.getByText('settings.ai.localAi.retryButton'));+ expect(inferenceProgressEmitter.reset).toHaveBeenCalledTimes(1);+ expect(mockRetryLastPreload).toHaveBeenCalledTimes(1); });</code_context><issue_to_address>**suggestion (testing):** Add a test for retry failure to ensure LocalAiDownloadProgress surfaces errors correctlyRight now we only test the successful retry path. Since `handleRetry` now catches failures from `retryLastPreload` and forwards them to `reportWebLlmError`, please add a test where `retryLastPreload` rejects (e.g., with `newError('No local model download is available to retry')` or a generic error) and assert that `reportWebLlmError` is called with the expected message. This will ensure retry failures remain visible to the user and aren’t silently swallowed in future changes.Suggested implementation:```typescriptconst { mockAbortActivePreload, mockRetryLastPreload, mockReportWebLlmError } =vi.hoisted(() => ({
mockAbortActivePreload: vi.fn(),
mockRetryLastPreload: vi.fn(),
mockReportWebLlmError: vi.fn(),
}));
``````typescriptvi.mock('../../services/localAiFacade', () => ({
abortActivePreload: mockAbortActivePreload,
retryLastPreload: mockRetryLastPreload,
}));
vi.mock('../../services/ai/webllmErrorReporter', () => ({
reportWebLlmError: mockReportWebLlmError,
``````typescript
it('retries the actual prior preload after an error', async () => {
const { inferenceProgressEmitter } = await import('../../services/ai/inferenceProgressEmitter');
const user =userEvent.setup();
mockSnapshot = {
state: 'error',
progress: 0.2,
estimatedSecondsRemaining: null,
text: 'Download failed',
};
mockRetryLastPreload.mockResolvedValue(undefined);
render(<LocalAiDownloadProgress />);
awaituser.click(screen.getByText('settings.ai.localAi.retryButton'));
expect(inferenceProgressEmitter.reset).toHaveBeenCalledTimes(1);
expect(mockRetryLastPreload).toHaveBeenCalledTimes(1);
});
it('surfaces retry failures via reportWebLlmError when retryLastPreload rejects', async () => {
const user =userEvent.setup();
mockSnapshot = {
state: 'error',
progress: 0.2,
estimatedSecondsRemaining: null,
text: 'Download failed',
};
const retryError =newError('No local model download is available to retry');
mockRetryLastPreload.mockRejectedValueOnce(retryError);
render(<LocalAiDownloadProgress />);
awaituser.click(screen.getByText('settings.ai.localAi.retryButton'));
expect(mockRetryLastPreload).toHaveBeenCalledTimes(1);
expect(mockReportWebLlmError).toHaveBeenCalledTimes(1);
expect(mockReportWebLlmError).toHaveBeenCalledWith(retryError.message);
});
```Depending on the existing implementation of `reportWebLlmError`, you may need to adjust:1. The import path used in the `vi.mock('../../services/ai/webllmErrorReporter', ...)` call to match your actual module location.2. The `expect(mockReportWebLlmError).toHaveBeenCalledWith(...)` assertion if `handleRetry` passes a richer error object or includes additional metadata instead of just `error.message`. In that case, update the expectation to match the actual signature (e.g., `toHaveBeenCalledWith(retryError)` or with a structured payload).3. If the file already contains a `vi.mock` for the web LLM error reporter, merge the `mockReportWebLlmError` hoisted declaration into the existing mock rather than adding a duplicate.</issue_to_address>### Comment 4<location path="tests/unit/localAiFacade.test.ts" line_range="295-304" /><code_context> await pB; });++ it('publishes a terminal error when a main-thread fallback cannot warm the requested WebLLM model', async () => {+ mockRunLocalTextGeneration.mockResolvedValue({ layer: 'onnx', text: 'fallback' });+ const { inferenceProgressEmitter } = await import('../../services/ai/inferenceProgressEmitter');+ const progressSpy = vi.spyOn(inferenceProgressEmitter, 'reportWebLlmProgress');+ const errorSpy = vi.spyOn(inferenceProgressEmitter, 'reportWebLlmError');+ const { preloadLocalModel } = await import('../../services/localAiFacade');++ await expect(preloadLocalModel('Qwen2.5-0.5B')).resolves.toMatchObject({ downloaded: false });++ expect(progressSpy).toHaveBeenCalledWith(0, 'Preparing local model');+ expect(errorSpy).toHaveBeenCalledWith('Local model preload did not complete');+ }); });</code_context><issue_to_address>**suggestion (testing):** Consider adding direct tests for retryLastPreload behavior and abort-induced error suppressionThe new test properly covers terminal error reporting when the main-thread fallback cannot warm WebLLM. However, two behaviors remain only indirectly tested:- `retryLastPreload` throwing when `lastPreloadModelId` is unset (e.g., user clicks Retry with no prior preload) and how callers handle that error.- The error path in `preloadLocalModel` when a failure occurs after the internal `AbortController` has been aborted, where `reportWebLlmError` should not be invoked.Adding focused unit tests for these scenarios would solidify coverage of cancel/retry semantics and guard against both noisy errors on legitimate cancels and silent issues from misconfigured retries.Suggested implementation:```typescriptconst { preloadLocalModel } =awaitimport('../../services/localAiFacade');
awaitexpect(preloadLocalModel('Qwen2.5-0.5B')).resolves.toMatchObject({ downloaded: false });
expect(progressSpy).toHaveBeenCalledWith(0, 'Preparing local model');
expect(errorSpy).toHaveBeenCalledWith('Local model preload did not complete');
});
it('throws when retryLastPreload is called without a prior preload', async () => {
const { retryLastPreload } =awaitimport('../../services/localAiFacade');
awaitexpect(retryLastPreload()).rejects.toThrow(/lastPreloadModelId/i);
});
it('does not publish WebLLM error when preloadLocalModel fails after abort', async () => {
const { inferenceProgressEmitter } =awaitimport('../../services/ai/inferenceProgressEmitter');
const progressSpy =vi.spyOn(inferenceProgressEmitter, 'reportWebLlmProgress');
const errorSpy =vi.spyOn(inferenceProgressEmitter, 'reportWebLlmError');
const { preloadLocalModel, abortLocalModelPreload } =awaitimport('../../services/localAiFacade');
mockRunLocalTextGeneration.mockImplementation(async () => {
// Simulate an abort occurring before the failure is surfacedabortLocalModelPreload?.();
thrownewError('simulated failure after abort');
});
awaitexpect(preloadLocalModel('Qwen2.5-0.5B')).resolves.toMatchObject({ downloaded: false });
expect(progressSpy).toHaveBeenCalledWith(0, 'Preparing local model');
expect(errorSpy).not.toHaveBeenCalled();
});
```To fully implement the abort-suppression test, you may need to:1. Expose an `abortLocalModelPreload` (or equivalent) function from `../../services/localAiFacade` that triggers the same internal `AbortController` used by `preloadLocalModel`. If the existing API uses a different name or mechanism (e.g., `cancelLocalModelPreload`, passing in an `AbortSignal`, etc.), update the test import and call site accordingly.2. Adjust the `expect(retryLastPreload()).rejects.toThrow(/lastPreloadModelId/i);` assertion to match the actual error message or error type thrown when `lastPreloadModelId` is unset (for example, a custom error class or a different message string).3. Ensure any global or shared mocks (like `mockRunLocalTextGeneration`) are reset between tests if the surrounding file does not already do this in a `beforeEach`/`afterEach`, so that the new mock implementation in the abort test does not leak into other tests.</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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:f978606224
ℹ️ 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.
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.
Hey - I've found 1 issue
Fixed security issues:
- nanoid (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="services/aiProviderService.ts"line_range="316-317" />
<code_context>
+ let buffer = '';
+ // QNBS-v3: parses one accumulated SSE line into an onChunk call — shared by the loop below and
+ // the final buffer flush after `done`, so the last frame (no trailing newline) isn't dropped.
+ const parseLine = (line: string) => {
+ if (!line.startsWith('data: ') || line === 'data: [DONE]') return;
+ try {
+ const json: unknown = JSON.parse(line.slice(6));
</code_context>
<issue_to_address>
**suggestion (bug_risk):** SSE parsing may miss valid frames with Windows-style CRLF line endings
Because `parseLine` uses `line.startsWith('data: ')` and `JSON.parse(line.slice(6))` on the raw line split by `\n`, any SSE stream using Windows-style `\r\n` will leave a trailing `\r`. A line like `"data: {...}\r"` won’t match `startsWith('data: ')`, so valid frames are skipped. Consider trimming the line ending first, e.g.:
```tsconst trimmed =line.trimEnd();
if (!trimmed.startsWith('data: ') ||trimmed==='data: [DONE]') return;
const json:unknown=JSON.parse(trimmed.slice(6));
```
This makes the parser robust to both `\n` and `\r\n` line endings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Uh oh!
There was an error while loading. Please reload this page.
…ty-hardening # Conflicts: # README.md
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Code review by qodo was updated up to the latest commit 369d064 |
…t 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.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/lora/loraTrainingService.ts (1)
106-118: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not classify cancellation from the request flag.
startTrainingThunkcheckscurrentRun.cancellationRequestedwhileabortTrainingThunkawaitsabortTraining(). Atrain_lorarejection can be classified as aborted before native confirmation for thatTrainingRun. Track the native outcome per run and classify cancellation only forconfirmedorpending_start_cancelled.🤖 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/lora/loraTrainingService.ts` around lines 106 - 118, Update startTrainingThunk and abortTrainingThunk to track AbortTrainingOutcome per TrainingRun instead of relying on currentRun.cancellationRequested. Record the native outcome returned by abortTraining() for the specific run, and classify a train_lora rejection as cancellation only when that outcome is confirmed or pending_start_cancelled; treat nothing_to_cancel and missing outcomes as ordinary failures.Source: Learnings
🤖 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 `@tests/unit/settings/AiProviderCard.test.tsx`:
- Around line 814-815: Condense the QNBS-v3 comment in the handleTest area to a
single physical line while preserving its explanation that the unguarded async
GPU probe could let stale out-of-order results overwrite newer results.
- Around line 842-845: Update the stale-probe test around resolveFirstProbe so
the promise resolution and microtask flush occur inside await act(...),
importing act from the existing `@testing-library/react` import. Replace the
immediate negative query assertion with waitFor, preserving the expectation that
the stale GPU-unavailable message is absent.
---
Outside diff comments:
In `@services/lora/loraTrainingService.ts`:
- Around line 106-118: Update startTrainingThunk and abortTrainingThunk to track
AbortTrainingOutcome per TrainingRun instead of relying on
currentRun.cancellationRequested. Record the native outcome returned by
abortTraining() for the specific run, and classify a train_lora rejection as
cancellation only when that outcome is confirmed or pending_start_cancelled;
treat nothing_to_cancel and missing outcomes as ordinary failures.
🪄 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: e2938963-8c2d-41f1-b81d-0d183b1f5481
📒 Files selected for processing (11)
components/ApiKeySection.tsxcomponents/settings/AiProviderCard.tsxcomponents/settings/PassphraseModal.tsxfeatures/lora/loraThunks.tsservices/aiProviderService.tsservices/lora/loraTrainingService.tssrc-tauri/src/lora.rstests/unit/aiProviderService.test.tstests/unit/lora/loraThunks.test.tstests/unit/lora/loraTrainingService.test.tstests/unit/settings/AiProviderCard.test.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- components/ApiKeySection.tsx
- tests/unit/lora/loraThunks.test.ts
- components/settings/AiProviderCard.tsx
- tests/unit/aiProviderService.test.ts
- features/lora/loraThunks.ts
- services/aiProviderService.ts
- src-tauri/src/lora.rs
Comment on lines
+814
to
+815
| // QNBS-v3 regression: the GPU probe chain was the only async result in handleTest not guarded | ||
| // by testRequestIdRef — an out-of-order stale resolution could overwrite the newer probe's result. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the QNBS-v3 comment on one physical line.
The comment spans Lines 814-815. Shorten it so the complete reason is on one physical line. As per coding guidelines, “add one single-line QNBS-v3 why-comment ... never wrap the comment across physical lines.”
🤖 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 `@tests/unit/settings/AiProviderCard.test.tsx` around lines 814 - 815, Condense
the QNBS-v3 comment in the handleTest area to a single physical line while
preserving its explanation that the unguarded async GPU probe could let stale
out-of-order results overwrite newer results.
Source: Coding guidelines
Comment on lines
+842
to
+845
| // The stale (first) probe settles after the newer (second) one has already started. | ||
| resolveFirstProbe({ status: 'unavailable' }); | ||
| await Promise.resolve(); | ||
| expect(screen.queryByText('settings.ai.webllm.gpuUnavailable')).toBeNull(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -eu
test_file="tests/unit/settings/AiProviderCard.test.tsx"printf'%s\n''--- outline ---'
ast-grep outline "$test_file" --lang tsx ||trueprintf'%s\n''--- imports and test region ---'
sed -n '1,90p'"$test_file"
sed -n '780,870p'"$test_file"printf'%s\n''--- related symbols ---'
rg -n -C 3 "resolveFirstProbe|gpuUnavailable|probe|webllm|act\\(|waitFor|findBy""$test_file"printf'%s\n''--- package/config references ---'
rg -n -C 2 '"`@testing-library/react`"|"react-testing-library"|testEnvironment|setupFiles|globals' package.json pnpm-lock.yaml vitest.config.* vite.config.* tests 2>/dev/null | head -250Repository: qnbs/WorldScript-Studio
Length of output: 21399
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- component symbols ---'
rg -n -C 5 "handleTest|testRequestIdRef|detectWebGpuDetails|gpuUnavailable|gpuAvailable|set.*Gpu|webllm" components tests/unit/settings
printf'%s\n''--- component outline ---'
component_file="$(fd -t f -i 'AiProviderCard'.| head -1)"if [ -n"${component_file:-}" ];then
ast-grep outline "$component_file" --lang tsx ||true
wc -l "$component_file"fiprintf'%s\n''--- implementation candidate ---'if [ -n"${component_file:-}" ];then
rg -n "handleTest|testRequestIdRef|detectWebGpuDetails""$component_file"| cut -d: -f1 |whileread -r line;do
start=$((line-35)); [ "$start"-lt 1 ] && start=1
end=$((line+75))
sed -n "${start},${end}p""$component_file"donefiprintf'%s\n''--- installed testing-library files, if available ---'if [ -d node_modules/@testing-library/react ];then
rg -n -C 3 "act|asyncWrapper|eventWrapper" node_modules/@testing-library/react node_modules/@testing-library/dom | head -250
elseecho'node_modules unavailable'fiRepository: qnbs/WorldScript-Studio
Length of output: 50380
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- exact source paths ---'
git ls-files | rg '(^|/)AiProviderCard\.(tsx|ts)$|AiProviderCard\.test\.tsx$'printf'%s\n''--- exact implementation references ---'
source_file="$(git ls-files | rg '(^|/)AiProviderCard\.tsx$'| head -1)"if [ -z"$source_file" ];thenecho'AiProviderCard source not found'exit 0
fiprintf'source=%s\n'"$source_file"
rg -n "handleTest|testRequestIdRef|detectWebGpuDetails|gpuUnavailable|gpuAvailable|set.*Status|testButton""$source_file"printf'%s\n''--- focused implementation slices ---'forlinein$(rg -n "handleTest|testRequestIdRef|detectWebGpuDetails""$source_file"| cut -d: -f1 | sort -n | uniq);do
start=$((line-25)); [ "$start"-lt 1 ] && start=1
end=$((line+65))
sed -n "${start},${end}p""$source_file"doneprintf'%s\n''--- test scripts and dependency availability ---'
node --version 2>/dev/null ||truetest -d node_modules &&echo'node_modules present'||echo'node_modules unavailable'
node -e "try { console.log(require.resolve('`@testing-library/react`')) } catch (e) { console.log('testing-library-react unavailable') }"2>/dev/null ||true
rg -n -C 2 '"test(:unit)?":|"vitest"|test:unit' package.jsonRepository: qnbs/WorldScript-Studio
Length of output: 50379
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- dependency availability and versions ---'if [ -d node_modules ];thenecho'node_modules present'
node -e "for (const p of ['react','react-dom','`@testing-library/react`','`@testing-library/user-event`']) { try { console.log(p, require(p+'/package.json').version) } catch (e) { console.log(p, 'unavailable') } }"elseecho'node_modules unavailable'fi
node --version 2>/dev/null ||true
rg -n -m 3 '^[[:space:]]+test:|^[[:space:]]+"test|react@|react-dom@|`@testing-library/react`@' package.json pnpm-lock.yaml
printf'%s\n''--- target test command ---'
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.scripts, null, 2))"2>/dev/null | rg -n -C 2 'test|vitest'||trueprintf'%s\n''--- promise scheduling probe ---'
node - <<'JS'const events = [];let resolveProbe;const probe = new Promise((resolve) => { resolveProbe = resolve; });probe.then(() => events.push('probe-reaction'));resolveProbe();Promise.resolve().then(() => events.push('await-continuation'));queueMicrotask(() => events.push('queueMicrotask'));setImmediate(() => console.log(events));JSRepository: qnbs/WorldScript-Studio
Length of output: 2122
🌐 Web query:
React 19 act asynchronous state updates external promise resolution Testing Library waitFor official documentation
💡 Result:
In React 19, the act testing utility has moved from react-dom/test-utils to the react package [1][2][3]. You should now import it as import { act } from 'react' [4][1]. Handling asynchronous state updates and external promise resolution in tests requires careful coordination between React's scheduler and the test environment [5][6]. 1. Using act with Async Functions The official documentation recommends using await act(async () => { ... }) [5][7]. When you provide an asynchronous function to act, React will wait for the code inside the function to execute and will also process any state updates scheduled during that time, including those that occur after async boundaries (such as promise resolutions) [5][7]. 2. Testing Library and waitFor React Testing Library's waitFor utility is designed to wait for expectations to pass by repeatedly executing a callback until it succeeds or times out [8]. While Testing Library utilities often automatically wrap interactions in act, complex scenarios—particularly those involving Suspense, new concurrent features in React 19, or external promises—may still require explicit act wrapping to ensure the DOM reflects the final state before assertions are made [9][4][10]. If you find that waitFor is not catching a state update, consider these strategies: - Explicit Wrapping: Wrap the trigger (e.g., button click) or the assertion block in await act(async () => { ... }) [5][10]. - Async/Await Chain: Ensure your test is correctly awaiting the promise that triggers the update before asserting [4]. - Environment Setup: Ensure global.IS_REACT_ACT_ENVIRONMENT=true is set in your test environment to enable proper act behavior [5][4]. Note on React 19 Behavior: React 19 introduces more concurrent rendering and optimized transition handling [11]. Because React 19 may batch updates differently or delay commits until transitions are finished, tests that rely on synchronous execution order or brittle timing may fail [12][13][6]. When dealing with external promises, ensure that your test waits for the result that drives the UI update rather than assuming a specific sequence of renders [6]. If you are working with Suspense or complex async flows, some developers find success by creating custom async render helpers that explicitly manage the promise lifecycle [10].
Citations:
- 1: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
- 2: Deprecate
actfromreact-dom/test-utilsin favor of act fromreactreact/react#28597 - 3: Add stable React.act export react/react#28160
- 4: https://chrisboakes.com/fixing-act-error-react-testing-library/
- 5: https://react.dev/reference/react/act
- 6: https://testingradar.com/why-react-19-apps-expose-hidden-timing-bugs-in-browser-test-suites/
- 7: https://react.dev/reference/react/act.md
- 8: https://testing-library.com/docs/dom-testing-library/api-async/
- 9:
waitForutils are not wrapped inactso some state updates are not applied to the DOM testing-library/react-testing-library#1416 - 10: @testing-library/react behaves differently when dealing with Suspense in React 18 and React 19 testing-library/react-testing-library#1375
- 11: https://react.dev/blog/2024/12/05/react-19
- 12: [React 19] State dispatch get stuck during an async form action (bug?) react/react#29590
- 13: Bug: SetState Calls not being batched inside Promise calls/micro Task react/react#30605
🌐 Web query:
site:testing-library.com/docs/react-testing-library/api act asyncWrapper waitFor official
💡 Result:
There is no official API or function named asyncWrapper in React or React Testing Library. The confusion likely stems from the fact that React Testing Library automatically wraps its helper functions with the React act utility to reduce boilerplate [1][2]. React's act function is a test helper used to ensure that all pending state updates, effects, and rendering tasks are processed and applied to the DOM before making assertions [1][3]. It is recommended to use the version of act exported by @testing-library/react rather than the one from react to ensure consistency [4]. For handling asynchronous code in tests, React Testing Library provides various findBy queries and the waitFor utility. These utilities are designed to wait for elements to appear in the DOM, effectively handling the asynchronous nature of React updates and data fetching without requiring manual calls to act [1][2]. When you use these asynchronous utilities, they internally handle the necessary scheduling to ensure your tests are deterministic and reflect how React works in the browser [1][3].
Citations:
- 1: https://react.dev/reference/react/act.md
- 2: https://zh-hans.react.dev/reference/react/act
- 3: https://ru.react.dev/reference/react/act
- 4: https://testing-library.com/docs/react-testing-library/api/
🏁 Script executed:
#!/bin/bashset -eu
printf'%s\n''--- repository act import conventions ---'
rg -n -C 2 "import .*\\bact\\b.*`@testing-library/react`|import .*\\bact\\b.*from ['\"]react['\"]|await act\\(|act\\(" tests components | head -240
printf'%s\n''--- test environment setup ---'if [ -f tests/setup.ts ];then
sed -n '1,220p' tests/setup.ts
fiprintf'%s\n''--- exact source/test lines ---'
sed -n '238,280p' components/settings/AiProviderCard.tsx
sed -n '808,850p' tests/unit/settings/AiProviderCard.test.tsxRepository: qnbs/WorldScript-Studio
Length of output: 26244
Flush the stale probe update inside act.
React may commit the promise-driven setGpuInfo update after await Promise.resolve(). Wrap the resolution in await act(...), then use waitFor for the negative assertion.
Proposed test change
- resolveFirstProbe({ status: 'unavailable' });- await Promise.resolve();- expect(screen.queryByText('settings.ai.webllm.gpuUnavailable')).toBeNull();+ await act(async () => {+ resolveFirstProbe({ status: 'unavailable' });+ });+ await waitFor(() => {+ expect(screen.queryByText('settings.ai.webllm.gpuUnavailable')).toBeNull();+ });Import act from the existing @testing-library/react import.
🤖 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 `@tests/unit/settings/AiProviderCard.test.tsx` around lines 842 - 845, Update
the stale-probe test around resolveFirstProbe so the promise resolution and
microtask flush occur inside await act(...), importing act from the existing
`@testing-library/react` import. Replace the immediate negative query assertion
with waitFor, preserving the expectation that the stale GPU-unavailable message
is absent.
Source: Coding guidelines
…o-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.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Uh oh!
There was an error while loading. Please reload this page.
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
…ncile 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.
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
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>
5 tasks
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
Stacked on #335; focuses on user-reported desktop stability and provider/runtime reliability.
AIza-prefix rejection in favor of minimal safe syntax checks plus provider validation/v1/modelsand exposes safe diagnosticsWhy
Issue reports #332/#333 identify release-quality failures in the v1.26.0 desktop path. These changes address code-supported causes without claiming packaged
.debvalidation before the native cloud build completes.Validation
Focused TypeScript tests passed locally;
rustfmtcompleted for the modified Rust module. Full native bundle validation, full coverage, and packaged desktop smoke remain cloud gates.Follow-up
This PR is intentionally stacked on #335. It does not close#332 or #333 until the packaged desktop test matrix and reporter-visible acceptance criteria are verified.
Summary by Sourcery
Harden desktop AI integrations, local model workflows, and encrypted storage behavior, while refining LoRA training orchestration and developer/build tooling.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Bug Fixes
CodeAnt-AI Description
Harden desktop AI providers, local model workflows, and LoRA training reliability
What Changed
AIzaprefix and are verified immediately after saving.Impact
✅ Reliable LM Studio and vLLM completions✅ Fewer stuck local model downloads✅ Clearer LoRA setup and cancellation results💡 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.