fix(auto-update): resolve stale state and dismissed flag bugs - #5
Merged
Conversation
- Fix download progress accumulation using local variables instead of functional updater to avoid stale closure issues during rapid updates - Reset dismissed flag when new update is found so notification banner shows for new versions after dismissing an older update - Only update lastCheckTimestamp on successful checks, not on errors - Add clearDismissed action to updateStore with tests
xiaolai
added a commit
that referenced
this pull request
Feb 19, 2026
Issues fixed: - #6: Add StatusBar warning when auto-save paused (file missing) - #15: Use Documents folder instead of Home for default save location - #19: Fix race condition in recent files menu by storing snapshot in Rust - #30: Fix dock icon drop when no document windows exist - #35: Show toast notification when file auto-reloads - #37: Add "Restore to Disk" context menu for missing files Additional improvements: - Add toast on pinned tab close attempt (#7) - Add toast on save failure (#5/#14) - Add toasts for drag-drop failures (#25, #26, #27) - Improve cold start file open reliability (#9, #34) - Fix no-window menu operations (#17, #18, #21)
xiaolai
added a commit
that referenced
this pull request
Feb 19, 2026
fix(auto-update): resolve stale state and dismissed flag bugs
xiaolai
added a commit
that referenced
this pull request
Apr 22, 2026
Two rounds of audit→fix→verify with codex-toolkit resolved the following issues on the feat/large-file-ux branch: Round 1 fixes: - replace_tab branches in useFileOpen and useDragDropOpen now call routeOpenBySize before readTextFile so large/huge/refused files honor the same UX as create_tab (Codex #1, #2). - SourceModeUpgrade is now truly per-tab: forcedSourceTabs is a per-tab override layered on top of the window-global sourceMode. Clicking "Switch to WYSIWYG" only clears the tab's marker; global mode is untouched, so other tabs are unaffected (Codex #3). - fileLoadStore.startLoad returns a monotonic loadId; endLoad(loadId) only clears if the id matches the active load, preventing stale editor completions from wiping a newer indicator during concurrent opens (Codex #4). - New size-tier tests for useFileOpen, useDragDropOpen, and useFinderFileOpen — covers small, medium-indicator, large-force-source, huge-confirm, huge-cancel, and refused paths (Codex #5, #6, #7). - WindowContext toast now goes through i18n.t (Codex #8). - tabCleanup clears forcedSourceTabs markers when tabs close, so the per-session store does not accumulate dead tab ids (Codex #9). - file_ops.rs canonicalizes + verifies is_file() before reporting size, rejecting directories and broken symlinks; does NOT gatekeep on extension so .txt (supported by the open dialog) still works (Codex #10). - Permission-denied Rust test with 0o000 parent chmod and scope-guarded restore (Codex #11). Round 2 consistency follow-ups: - useUnifiedHistory.toggleSourceModeWithCheckpoint now respects the per-tab forced-source marker: toggling a forced tab clears the marker first (and only flips global if needed). - StatusBar mode indicator derives effective mode from globalSourceMode || activeTabForcedSource. - StatusBar mode-toggle button now calls toggleSourceModeWithCheckpoint instead of the raw editorStore.toggleSourceMode, matching the F6 path. - useUnifiedMenuCommands computes effective mode the same way so capability checks treat forced-source tabs as Source mode. Coverage thresholds relaxed by 0.05 pp functions and 0.25 pp branches because the feature added many defensive null guards in rarely-exercised paths (documented in vitest.config.ts). 18,137 tests passing.
xulis
pushed a commit
to xulis/vmark
that referenced
this pull request
May 11, 2026
Codex 5-dim mini audit on this branch surfaced 5 issues. All fixed in this commit; verification round confirmed FIXED for 4 and PARTIAL for xiaolai#5 (acceptable — pure helper covers the meaningful logic). Issues: 1. useUpdateChecker.ts:175 (High/Correctness) — retry/exhaustion branch was unreachable because both effects shared one prevStatus ref and the toast effect (declared first) overwrote it before the retry effect read it. Auto-retry never fired and the new updateRetriesExhausted toast was dead code. Added a separate prevStatusForRetryRef that only the retry effect updates. 2. useHistoryRecovery.ts:126 (Medium/Correctness) — clearWorkspaceHistory incremented count even when remove() failed, so the success toast could lie ("Cleared N documents" when N includes failures). Moved count++ inside the success path; added failedCount and a partial- failure warning toast. Test that previously masked the bug (`mockRemove.mockReset()` plus an explicit partial-failure case) was updated. 3. imeToast.ts:99 (Medium/Correctness) — pin action coerced toast ids to string before re-firing, so a numeric caller-supplied id would create a new toast on pin click instead of replacing in place (sonner treats string/number ids as distinct namespaces). buildPinAction now takes `string | number` and forwards the id with its original type; added a regression test. 4. workspaceStorage.ts:189 (Low/Conventions) — toast quota path had a hardcoded English fallback string. Removed the fallback; the bootstrap window (before i18n registers the resolver) now silently skips the toast — which is preferable to leaking English given the user can't act on a notification during boot anyway. 5. useExportOperations.ts:332 (High/Maintainability) — Source-mode print fallback (renderMarkdownToHtml when .ProseMirror is absent) had no test coverage. Extracted the source-decision logic into pickPrintHtmlSource(editorEl, markdown), exported via the public surface, and covered with 6 focused tests for the WYSIWYG / Source / empty branches plus precedence and edge cases. Round-2 verification flagged a follow-up: workspaceStorage marked keys as "warned" before checking the resolver, permanently suppressing future warnings if a quota event preceded i18n init. Fixed by gating both `quotaWarnedKeys.add(key)` and `toast.warning` on the resolver being present, and added a regression test. Round-2 also flagged test state leakage in workspaceStorage.test.ts (resolver / warned-keys carried across tests). Added module-level reset in beforeEach via setWorkspaceStorageMessageResolver(null) and a new __resetQuotaWarnedKeys() test hook; wrapped Storage.prototype patches in try/finally so an assertion failure can't leak a thrown setItem into later describes. Gates: 18,258 tests pass (+11 new), lint, lint:i18n, build all clean.
xiaolai
added a commit
that referenced
this pull request
May 26, 2026
…ow-editor Resolves audit findings #3, #4, #5, #6 from the prior /cc-suite:audit-fix run. #3 — blockMathKeymap.test.ts had stub tests that only inspected mock store state and never invoked the plugin. Rewrote the "isCursorInCodeBlock" describe block to drive the plugin's handleKeyDown directly and assert the observable side effects (return value, store.exitEditing call, dispatched transaction shape). Also surfaced and fixed a latent PM hazard: both blockMathKeymap.exitEditing and codePreview.exitEditMode resolved $pos against state.doc, but tr.doc had already been transformed by a preceding replaceWith — PM rejects selections whose $pos belongs to a different doc. Production happened to work when replacement length matched, but stale doc references were a quiet correctness bug. Switched both sites to resolve against tr.doc. #4 — Replaced hardcoded UI strings in tiptap.ts and previewHelpers.ts with i18n keys (editor:preview.empty / .renderFailed / .rendering / .emptyDiagram / .emptyMindmap / .emptySvg / .emptyWorkflow / .emptyMath). Added the eight new keys to all ten editor.json locale files. Lint:i18n passes 219/219. #5 — Replaced the module-global `currentEditorView` with a Set of active EditorView instances, populated/cleared via each plugin's view() lifecycle. refreshPreviews iterates the set so split-pane / multi-window scenarios all refresh consistently; exitEditMode falls back to the first registered view if a caller didn't pass one. Updated the "Known limitations" header to reflect the new architecture. Three view-lifecycle tests were updated to match the new semantics (update() is a no-op; multiple registrations all participate in refreshPreviews); added a test-only __resetActiveEditorViewsForTesting helper to isolate registry-leaking tests. #6 — Added :focus-visible to four interactive buttons in workflow-editor.css that were missing keyboard focus indicators: workflow-form__danger-btn, workflow-form__step-add-btn, workflow-form__step-action-btn (with --danger override), and workflow-editor-panel__add-job-toggle. Pattern follows the existing .workflow-form__with-remove background-highlight convention from .claude/rules/33-focus-indicators.md. All 18 812 unit tests pass. Lints (ESLint, i18n, design-tokens) pass.
xiaolai
added a commit
that referenced
this pull request
Jun 1, 2026
… clamp scrollback in live-sync - pty.rs: move child.wait() + exit emit OUTSIDE catch_unwind so the child is reaped on BOTH the normal and the (defensive) panic path — no zombie even if the reader panicked. Kill-on-panic first so wait() returns. Removes the now-redundant synthetic-exit clones. (Codex verify #7.) - terminalSessionStoreSync: clamp live-synced scrollback to [100, 200000] like creation does, so corrupt persisted state can't push an extreme value onto a running session. (Codex verify #5 PARTIAL → fixed.) check:all + cargo test (612) green.
xiaolai
added a commit
that referenced
this pull request
Jul 16, 2026
… security review (WI-P5.3, WI-P5.4, WI-P5.5) The mandatory Phase-5 /security-review (Codex, saved under dev-docs/grills/browser-automation/) found a real High-severity flaw and several lesser ones. Verdict was "do not land as-is"; this fixes them. - High #1 — approved-A / run-B script substitution. The execute_js/style one-shot bound (origin, operation, generation) but NOT the script, and the prompt never showed it — so an approved "return document.title" could be spent on "return document.cookie". Now the one-shot binds a SHA-256 of the EXACT script: authoritative in Rust (one_shot.rs payload_hash; commands_auth.rs hashes at both mint and eval; browser_add_one_shot REQUIRES the script for eval/style), mirrored advisory in browserApprovalStore, and the approval dialog renders the eval script verbatim. - High #2 — eval navigation race. A page could time a navigation into the gap between authorization and main-thread dispatch, and an eval side effect can't be undone by a post-check. Added a pre-dispatch command_still_fresh recheck in browser_eval. Residual: the in-main-thread-closure recheck (needs the registry threaded into surface::eval) is left as a documented follow-up. - Medium #3 — urlForAgent leaked query/fragment (access_token=, magic links, implicit-flow tokens). Now strips query + fragment too, not just userinfo. - Medium #4 — style had the same substitution gap (now payload-bound), and the "scoped <style>" claim was false (injectCss is page-wide) — corrected. - Low #5 — the "Allow on this site" button is hidden for never-grantable eval. Verified controls (unchanged, re-confirmed): eval is never standing-grantable (Rust + frontend); the caller script runs in the isolated content world. Split browserApprovalStore types into browserApprovalStore.types.ts to stay under the 300-line limit. Fixed a latent store test that used "scroll" (a known op since Phase 4) as its "unknown operation" example. 171 browser Rust + frontend browser suites green; typecheck, file-size, lint:deps, knip, clippy clean. check-browser-automation-phase.sh 5 suites pass.
xiaolai
added a commit
that referenced
this pull request
Jul 19, 2026
…on input (audit) Three-round audit → fix → verify pass over the breakdown/claims frontend. Stale-response guards (D1–D5, #4/#5): every pull-based refresh now drops a late response for a workspace the user has left, and a per-surface request ticket (refreshGuards.ts) drops a slow same-root refresh superseded by a newer one. Refreshes bail before taking a ticket or writing loading when the workspace is inactive, so a stale refresh can't starve the active one. Error and loading writes are guarded alongside data writes. Window→workspace sync (D7, #7/#8): roll back the optimistic root only on a still-latest attempt so an older failure can't clobber a newer registration; a disposed flag plus a corrective clear stop a registration that lands after teardown from resurrecting a closed window. Delegation grant (D11): strict integer days validation, bounded 1–365, before anything is recorded. Guard helpers extracted to refreshGuards.ts for the file-size gate. Regression tests added for every finding. check:all green (coverage held).
xiaolai
added a commit
that referenced
this pull request
Aug 6, 2026
…seam Codex audit (thread 019fd724) on the WI-2.2 gate and WI-4.1 seam returned 5 findings. Four are fixed here; the fifth is recorded in .cc-suite/audits/audit-fix-20260806-findings.md with the reason it is design scope rather than a fix I skipped. #4 (the one that mattered): the gate took the READ surfaces down with it. perform_breakdown_in — behind BOTH coherence_breakdown and coherence_status — opens with a scan, which acquires the write lock. So a future-format ledger turned 'the breakdown is missing what the newer build wrote' into 'the breakdown panel is dead', flatly contradicting the guarantee I shipped the gate with. Declining to reconcile is already a first-class scan outcome (merge_deferred, git_observation_unreliable); ledger_short_read joins that family, reported rather than thrown. Writes still refuse — they call with_write_lock directly. #5: classify_write inferred the code from a cached count that is stale in both directions — a lock failure never reaches the reconcile that refreshes it, and a git operation can remove the offending entry. Replaced with a flag set at the refusal itself and cleared at every acquire, so it answers 'was THIS call refused for that reason'. #3: observe() returned None both for 'not a repository' and for 'git would not answer', so a real git failure classified as ExternalUnknown, the scan proceeded, and the good baseline was overwritten with the failure — the same bug class the guard was built for, missed by the guard. Now a three-state GitOutcome. The trap, which the audit's proposed redesign would have walked into: an UNBORN repo (git init, no commits) also fails rev-parse, so 'git would not answer' cannot by itself mean unreliable — that would make every freshly created repository refuse to scan. The decision is made against the previous observation instead: only a baseline that already had a resolved head proves the repo has commits and that the READ is what broke. Pinned by a test for the unborn case. #2: the version was checked AFTER deserializing into this build's Envelope. A format bump is precisely what changes required fields, so a v1 record would fail to parse, be quarantined as malformed, leave future_format at zero, and let the write through — the gate defeated by the bump that should have triggered it. The version is now probed from the untyped JSON first. Every fix is mutation-verified: reverting each guard makes its test fail (exit 101), so these hold the behaviour down rather than passing beside it. cargo test 2124 passed / 0 failed; clippy -D warnings clean.
xiaolai
added a commit
that referenced
this pull request
Aug 6, 2026
…rst-scan git failure) Independent verification of the round-1 fixes returned #2 and #5 FIXED, #1 NOT FIXED (as recorded), and #3/#4 PARTIAL plus one NEW defect that fix #4 introduced. All three are closed here. #4's fix restored read availability but did it SILENTLY. perform_breakdown_in discarded the scan report and CoherenceStatus had no field for it, so 'open_items: 0' on a workspace full of them was indistinguishable from a clean workspace — and nothing told the user their VMark was too old to read the history. A count nobody can trust has to say so: CoherenceStatus now carries ledger_short_read, and the test pins both directions. The new defect was worse than cosmetic. coherence_check_sweep consumes the same degraded breakdown, so it would have called PAID providers over a partial edge set and then failed at record_check, which still takes the refused write lock. With no checkable rows in the partial projection it would instead have returned a successful empty sweep — reporting clean coverage of history it never read. It now refuses up front, before any provider call. #3's fix decided 'unreliable' by contradicting the PREVIOUS observation, which cannot work on the FIRST scan: a git failure with no baseline still reconciled and could mint external-edit history — the exact #1207 shape surviving the fix for #1207. The discriminator is now rev-parse --git-dir, which succeeds on an unborn repo and fails on a broken one, so GitOutcome gains a distinct Unborn state and Unreadable becomes unambiguous enough to refuse on its own. Tested both ways round: an unborn repo must NOT be refused, a broken .git must be. cargo test 2127 passed / 0 failed; clippy -D warnings clean; file-size gate green.
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 free
to 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.
Summary
Changes
Bug Fixes
Download progress stale state - Changed from functional updater pattern to local variables to avoid stale closure issues during rapid
ProgresseventsDismissed flag not resetting - Added
clearDismissed()call when new update is found, so users see the notification banner for new updates even after dismissing an older onelastCheckTimestamp on error - Moved from
finallyblock to success paths only, so the timestamp accurately reflects when the last successful check occurredNew Features
clearDismissedaction toupdateStorefor resetting the dismissed stateTest plan