Repository navigation
[CI] Simplify reveal and test fixture cleanup - #25978
Conversation
1c32ec5 to
de24ac5
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between 51062e6 and de24ac5a8a64faf8a17f9eb26fc4d9e10e54d9b8. 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe transcript watcher tests now track and clean up subscriptions locally, and the shared test helper was removed. A runtime test configuration now resolves launch arguments to an empty list. Pinned worktree reveal grouping now adds only the section-scoped pinned group key, with related tests updated for visible and filtered pinned parents. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The supplied evidence identifies no actionable issue blocking this cleanup from merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
de24ac5 to
a98b0dc
Compare
nwparker
changed the title
nwparker
changed the title
ELI5
Keep the root-cause fixes and remove two unnecessary layers. Sidebar disclosure and transcript delivery behave the same.
What Changed
Pinned reveal now adds its key to the shared ancestor list, which already filters collapsed keys. The old helper filtered that same key a second time. Its isolated assertions are replaced by picker tests with a visible or filtered pinned ancestor, including restoration on close.
Watcher tests own their subscriptions and close them before deleting temporary files. This removes a module used by just one test file while preserving failure-safe teardown.
A separate one-line commit removes WAL mode from a closed legacy-journal fixture. Bun defers closing its writer until prepared statements are collected, so later cleanup could change the file's bytes or race restore. The fixture now writes a settled database. The byte-preservation and read-only assertions remain intact; production SQLite behavior is unchanged.
Why
The original fixes remain: one host-aware ancestor resolver for normal Reveal and the picker, and a transcript fingerprint captured from the bytes actually read. Removing redundant filtering and a single-use test module makes those mechanisms easier to see. No new guards, retries, caches or state.
The earlier picker PR added only four net production lines, and the transcript PR added 21; most additions were regression and rendered-proof code. This follow-up removes 76 net lines, including 34 production lines.
Linked Issue
Elegance follow-up to #25950 and #25965.
Visual Proof
N/A. This cleanup preserves visible behavior. The original before/after proof remains in #25950 and #25965. All four existing local/SSH picker flows passed again in hidden Electron on macOS.
Testing
84 focused tests passed across nine files: picker disclosure/pinned membership, normal Reveal, transcript publication races, watcher teardown, startup restore and both runtime fixtures.
All 15 restore tests passed in ten consecutive runs. An isolated Bun probe reproduced bytes changing after a closed WAL writer was collected (19/20 runs); the settled fixture stayed unchanged (20/20).
All 25 existing per-session journal-import tests passed, retaining coverage of WAL-backed legacy files.
Four rendered picker checks passed after an Electron rebuild: collapsed local/SSH hosts and parents reveal the target and restore preferences on close. Remote catalogs and agent status are seeded; no agent message is sent.
Full typecheck, full oxlint, changed-code quality gate, reliability manifest and Electron build passed.
After merge, full typechecking and all 109 focused tests passed on main commit
deb737478a3b; the working tree is clean.Review
The removed pinned wrapper always received confirmed membership; adding the same key before the existing final collapse filter produces the same result. Watcher cleanup still runs before file deletion, including failed tests. The consumed-byte reader logic, transport shapes and production launch behavior are unchanged.
Agent skill upstream boundary
Checklist
a98b0dc47b58: all five unit shards, relay integration, Linux/Windows packaging, compatibility, static analysis, mobile checks and verification. An interrupted runner job was rerun on the same commit.