Repository navigation
docs: record audit remediation in CHANGELOG, fix stale schema claim - #2
Merged
Merged
Conversation
The audit remediation landed on main in PR #1 (d08a692 backend security, 9154540 dashboard a11y/theme) without a CHANGELOG entry, leaving [Unreleased] empty across 87 changed files of user-facing behaviour. CONTRIBUTING.md requires a changelog entry for user-facing changes. - CHANGELOG: fill [Unreleased] with the security, reliability, accessibility and theme-token changes, grouped by theme and tagged with the originating finding IDs for traceability. - AGENTS.md: the schema-version note claimed `v == 7`; the assertion in tests/unit/db/test_db_pool.py is `v == 17` (migrations dir is at 017). Anyone following that line would have edited the wrong assertion. Docs only. Ship bar re-verified on this tree before the commit: make all = format-all/lint/typecheck clean + 3744 passed, 17 skipped; tsc -b clean.
ra-co88
added a commit
that referenced
this pull request
Oct 19, 2026
…hed-node race, and a DOMMatrix collection crash (#23) The reported symptom was ONE intermittently failing test. Root-causing it found FOUR defects across three causes. Full diagnosis: .agents/audit/vitest-intermittency.md ## 1. Worker-pool starvation (the actual cause of the intermittency) `pool: "threads"` opens up to cpus-1 workers - 7 on an 8-core machine. Under that contention, timing-sensitive assertions failed in files that pass BOTH alone and in small groups. Bound the pool and the suite goes green AND gets faster: default pool -> 2 failed files / 10 failed tests / 597s maxWorkers: 3 -> 253 passed / 1331 passed / 277s Faster because the run is dominated by setup/environment time, not parallel throughput: the workers were contending, not scaling. ## 2. A detached-node race in AuthGuard.test.tsx Reported as flaky; it was deterministically broken under parallelism. The test took an instantaneous snapshot - `await screen.findByText(...)` then `.toBeInTheDocument()` - of a node it EXPECTS to churn (NavProbe navigates on every render, so the shell is briefly replaced by the gate spinner). When that landed in the window, findByText resolved, the node detached, and the matcher failed in ~25ms. That duration is what disproved my first hypothesis. findByText cannot resolve early with a failure - @testing-library/dom retries until its deadline and then reports "Timed out in waitFor" - so a 25ms failure meant the element WAS found and then removed. My initial fix (raising the timeout to 5000ms) still flaked 1 in 3, which is what sent me back to the timing evidence. Replaced with a `waitFor` that re-queries, which is also what the test already did two lines below. Mutation-verified: original form fails 8/8, waitFor form passes 8/8. ## 3. DOMMatrix missing, breaking whole files at COLLECTION pdfjs-dist instantiates a transform at module scope (node_modules/pdfjs-dist/build/pdf.mjs:9387, `const SCALE_MATRIX = new DOMMatrix()`). jsdom ships no DOMMatrix, so merely importing react-pdf throws during collection - reported as a whole-file failure with "Tests: no tests", so the real tests never run at all. This reached far past the PDF components: a plain .ts test with no DOM (pages/Agent/Skills/skillMarkdown.test.ts) died because it imports a helper from components/SkillDrawer, which renders react-pdf. Same for AuthGuard.test.tsx and Settings/Models/ModelListEditor.test.tsx. Stub added to src/test/setup.ts beside the existing matchMedia / ResizeObserver / IntersectionObserver shims - that is what the file is for. Deliberately minimal: the module-scope constructor is the no-arg form and only .a/.d are read (both identity 1). A full transform implementation would be untested scaffolding. Mutation-verified: removing it restores the ReferenceError. ## 4. A lazy-import latency vs waitFor's 1000ms default TrajectoryInspector renders Preview through LazyMarkdown (React.lazy -> import("./index")). A cold import of that chunk measures 2086ms; waitFor's default is 1000ms (asyncUtilTimeout, @testing-library/dom config.js:15). The assertion therefore could only pass when another file in the worker had already warmed the chunk. Given an explicit 5000ms budget. Proven causally with an artificial 2.2s import delay: the unpatched assertion fails at 1140ms, the patched one passes. ## Rejected: raising testTimeout to 15000 Tried and reverted. The flagging failures were starved, not slow, and #2 is a race no budget can fix. A generous timeout turns a hard failure into a slow pass without removing the race - worse for diagnosis, not better. ## Verification cd dashboard && npx tsc -b # clean cd dashboard && npx vitest run # 253 files / 1331 tests, 277s Baseline on pristine develop for comparison: 4 failed files / 1 failed test, and three files that could not even collect. Co-authored-by: pt-act <211776491+pt-act@users.noreply.github.com>
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
2 participants
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.
Fork-internal housekeeping. Not for upstream yet — the fork still needs an upstream sync and a branch-consolidation pass before any contribution is proposed.
What
The audit remediation landed on
mainin PR #1 (d08a6928backend security,91545401dashboard a11y/theme) without a CHANGELOG entry, leaving[Unreleased]empty across 87 changed files of user-facing behaviour.CONTRIBUTING.mdrequires a changelog entry for user-facing changes.[Unreleased], grouped as 安全 / 修复 / 可访问性 / 界面. Each bullet is tagged with the originating finding ID (SEC-, A11Y-, UI-, GATE-, FE-, RACE-) for traceability back to the audit.v == 7; the assertion intests/unit/db/test_db_pool.pyisv == 17(migrations dir is at017). Anyone following that line would have edited the wrong assertion.Scope
Documentation only — 2 files, +40/−1. No code, no schema, no dependency changes.
Verification
Pre-commit gate (
make precommit+npm run build) run in full, no bypass:make lint,make typecheck(mypy --strict, 517 files) — cleannpm run build— succeeded (49.35s)tsc -b— cleanFollow-ups tracked separately
developdid not exist on this fork (PR Feature/audit a11y reliability #1 mergedfeature/*straight intomain); it has since been created frommain, so this branch is correctly based.developwithupstream/main(46 commits behind), consolidate 14 stranded fix branches, and fixAgentSelector.tsx:120— areact-hooks/rules-of-hooksviolation introduced by91545401, still unreproduced.