fix: resolve 11 audit issues + repair audit-fix pipeline#972
Merged
Conversation
The audit-fix matrix installed pnpm via an unpinned `npm install -g pnpm`, which now resolves to pnpm 11. pnpm 11 no longer reads `pnpm.overrides` from package.json, so `pnpm install --frozen-lockfile` aborts with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH against the committed lockfile — every job died before Claude ran. Switch to `pnpm/action-setup@v6` with `version: 10` plus `cache: pnpm`, matching ci.yml / release.yml / deploy-website.yml.
…loses #962) Replace the naive `!content.endsWith("\\|")` check with the shared `endsWithDelimiterPipe` helper (backslash parity), consistent with the tableTabNav (#934) and structuralCharProtection (#838) fixes. Drops the now-unnecessary v8-ignore directive and adds tests covering both the real-delimiter (strip) and escaped-pipe (no-strip) branches. Note: this is a correctness/consistency hardening — splitTableCells already splits even-parity `\\|` as a delimiter and the plugin guards zero-width ranges, so the prior naive check produced only an invisible phantom trailing cell, not a visible highlight misalignment.
FENCE_OPEN_PATTERN only matched backtick fences, so Cmd+D / Cmd+Shift+L treated text inside `~~~` blocks as plain document text. Match both `+ and ~+ openers, capture the delimiter char, and require the closing fence to use the same character (a tilde fence can't be closed by a backtick line).
Shift+Tab pushed one delete change per cursor, so two cursors on the same line produced overlapping changes at the same line.from. Collapse to one change per line (removing the widest warranted amount) so correctness no longer depends on CodeMirror silently merging overlapping ranges. Note: the current @codemirror/state tolerates the overlap (merges identical deletes) and normalizes same-offset cursors, so the reported crash does not reproduce in this version — this hardens against that reliance and the version-dependent behavior behind #762/#763. Tab needs no change: same-offset cursors are normalized away and different offsets are non-overlapping.
The upward scan classified any language-less fence as "close-only", which also matches a plain ``` opener — and the skip counter ignored the fence character, so ``` content lines inside a ~~~mermaid block were counted as closes and the opener was paired off, suppressing the preview. Replace it with a forward pass that pairs fences by document order and only closes a block on a same-character, >=-length, info-less fence (CommonMark). This also yields the correct interpretation for sibling/nested blocks. The issue's stated example (plain block then mermaid) already worked; the real failure was the cross-delimiter case (``` lines inside ~~~mermaid), now covered by tests.
queueRequest's timeout called reject() unconditionally. Once flushRequestQueue removed the entry and was awaiting sendImmediate, a firing timer rejected the promise with a spurious "timed out" error even though the in-flight request later succeeded (the wrapped resolve was then a no-op). Move reject() inside the `if (idx !== -1)` guard so the queue-wait timeout only fires while the request is still queued; once flushed, sendImmediate's own timeout governs the wait.
The Advanced Settings "Test Restore" button always invoked hot_exit_restore, which restores only the main window — secondary windows were silently dropped while a green success toast still showed. Extract the single-vs-multi decision into a shared restoreDispatch helper (restoreCommandFor / hasSecondaryWindows) and route both the automatic restart flow and the manual button through it so they cannot diverge again.
The coordinator emits hot-exit:partial-capture when a window misses the capture timeout, but no frontend listener consumed it — the user saw the normal success path while a timed-out window's unsaved edits were silently dropped. Add useHotExitCaptureWarning (registered in MainWindowRunners) that listens for partial-capture / capture-timeout and surfaces a warning toast naming the missing windows, plus the PARTIAL_CAPTURE event constant and i18n strings.
…loses #968) When create_document_window_with_label failed for a secondary window, the coordinator dropped it from expected/window state and let the rest finish; RESTORE_COMPLETE then fired and the frontend cleared session.json, so the failed window's tabs (including dirty documents) were lost with only a log::error as evidence. Option A: on any creation failure, abort the whole restore — clear the pending state (advance_and_clear) and close already-created windows, then return Err. The frontend invoke throws, session.json is preserved, and the full session is retried on the next launch. No unit test: injecting a create_document_window_with_label failure needs a trait seam this issue's scope guard forbids; the cleanup mirrors the existing RESTORE_START emit-failure path and is covered by the issue's E2E step.
Two mcp-server tests asserted superseded behavior and had been failing on main (the mcp-server suite is not run by the root check:all CI gate, so it went unnoticed): - websocket "queue full": the overflow policy was deliberately changed to drop-oldest (3551372) — oldest request evicted + rejected, newest accepted — but the test still expected the removed reject-new ("Request queue full") path. Rewritten to assert drop-oldest. - parentProcess win32: the PowerShell PID lookup was hardened (#280) to bind the PID to $args[0] and pass it positionally after `--` instead of interpolating it into the -Command string; the tests still expected the old interpolated command. Updated to assert the stronger argument-isolation. No production code changed — both implementations are correct; the tests lagged.
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.
Resolves all 11 open
[audit]issues, fixes the broken audit-fix automation that should have handled them, and repairs two pre-existing stale tests in the mcp-server suite.Why the auto-fix failed (today's run)
The
audit-fixmatrix inclaude.ymlinstalled pnpm via an unpinnednpm install -g pnpm. The npmlatesttag is now pnpm 11, which no longer readspnpm.overridesfrompackage.json— sopnpm install --frozen-lockfileaborts withERR_PNPM_LOCKFILE_CONFIG_MISMATCHagainst the committed lockfile. Every job died at "Install dependencies" before Claude ran. CI was unaffected because it pinspnpm/action-setup@v6→version: 10. Fixed by aligningclaude.ymlwith the other workflows (first commit).Audit fixes (one commit per issue)
~~~tilde code fences in select-occurrence~~~mermaidcontainingAlso: two stale mcp-server tests repaired
The
vmark-mcp-serversuite is not run by the rootcheck:allCI gate, so two tests had been red onmainunnoticed. Both implementations are correct; the tests lagged behind hardening changes:3551372f); test still expected the removed reject-new path. Rewritten to assert drop-oldest.$args[0]and pass it positionally after--(no command-string interpolation); tests still expected the old interpolated command. Updated to assert the stronger argument-isolation.Findings worth a reviewer's attention (I challenged the audit, didn't rubber-stamp)
~~~mermaidblock), now fixed and tested.session.json). No unit test: injecting a window-creation failure needs a trait seam the issue's scope guard forbids; covered by the issue's E2E step and mirrors the existing emit-failure cleanup.Verification
Everything green: root
pnpm check:all(all lints, 19017 frontend tests, coverage thresholds, build, size), the full mcp-server suite (185 tests), and Rust (cargo check+ 558 lib tests).Closes #959
Closes #961
Closes #962
Closes #963
Closes #964
Closes #965
Closes #966
Closes #967
Closes #968
Closes #969
Closes #970