Repository navigation
fix/refactor: 2026-09-11 audit execution, successor to #2409 #2531
Copy link
Copy link
Closed
Labels
Description
Purpose
Execute the vetted findings of the 2026-09-11 codebase audit (26 rows, four parallel static audits, every row re-verified at eefe37b51e by read-only verifiers, then the execution shape adversarially reviewed). Successor to #2409 (vertical cost) and #2335 (decoupling wave 3). This umbrella is reliability and repo truth, not features: after wave 2 the primitives under every command (spawn, deadline, kill, lock, runner recovery) are honest.
Rules for every child:
- Fix-shaped, not detector-shaped. No new ratchet, validator, guard script or helper module unless the child says so. The 2026-09-02 review ranked enforcement machinery as the top waste.
- Behavior changes land with characterization tests observed on unmodified code first.
- One local full gate per branch; stacked PRs share the tip's gate.
- No collocation file moves (measured zero effect in refactor: cut the layers and files one feature change must touch (successor to #2278 and #2335) #2409); no refactor inside the lock child.
Children (waves = independent branches; ← = stack order)
Wave 1
- fix(daemon): a discarded promise rejection must not shut the daemon down #2519 daemon death on discarded rejection — PR fix(daemon): keep a discarded rejection from shutting the daemon down #2532, merged 2026-09-13
- docs: AGENTS.md package specifiers, duplicate ADR 0019, Node floor and web Node 24 requirement #2520 docs and manifests truth — PR docs: fix phantom specifiers, the duplicate ADR 0019, and the Node floor #2533, merged 2026-09-13
- chore(config): scope dependency overrides and delete dead fallow, worktree and prettier entries #2521 config drift: overrides, fallow roots, worktree, prettier — PR chore(config): scope the qs override and drop dead fallow, worktree and prettier entries #2535, merged 2026-09-13
Wave 2
- fix(host-kit): a killed command settles on exit, background exec cannot take a timeout #2522 host-kit exec: settle-on-exit ← then background type narrowing and opt-in cap (M, MED risk)
- fix(host-kit): process lock proves ownership before release and never evicts a live owner #2523 host-kit process lock ownership (M, MED risk)
- fix(apple-runner): recovery decisions read typed rules; a connection deadline gets zero recovery today #2524 apple-runner typed recovery; deadline gets zero recovery today (M, MED risk)
- refactor: one shellQuote implementation instead of three #2525 shellQuote dedupe (S)
Wave 3
- fix(cli): open the device-auth URL on Windows without cmd /c #2526 Windows auth URL launch without
cmd /c— PR fix(cli): hand device-auth URLs to the browser without a shell #2567, merged 2026-09-14 - fix(android-helper): build with the pinned build-tools version, fail on CI when unset #2527 Android helper build-tools pin, hard-fail on CI — PR fix(android-helper): build with the pinned build-tools version, fail on CI when unset #2568, merged 2026-09-14
- perf(scroll-until): build the snapshot visibility index once per pass #2528 scroll-until visibility index once per pass (S)
- decision: enforce or delete the implementation-file size tiers in AGENTS.md #2529 decision: size tiers in AGENTS.md (delete or generalize the existing ratchet)
Last
- chore(worktrees): stop copying .env into agent worktrees #2530 stop copying
.envinto worktrees (touches.worktreeincludeafter chore(config): scope dependency overrides and delete dead fallow, worktree and prettier entries #2521)
Evaluated and not scheduled
- Android helper tests "never run": refuted. They run in CI via the
android-helpersgate fromandroid.yml; they are never in the APK. - Torture lane asserts its own model: documented scope; a real SIGKILL lane exists (
test/integration/daemon-replace-exit-flush.test.ts). - Maestro
runScripttrust boundary: flows are operator-authored; env visibility mirrors upstream. - Bearer token accepted in the JSON-RPC body: shape since refactor: split daemon server runtime into daemon/server/ — Phase 5 #985, proxy rewrite is intentional, loopback plus 192-bit token, no attacker model. Unpinned by tests.
- Mobile snapshot presentation computed twice (daemon keeps counts, CLI recomputes nodes): real, cross-platform, distinct from refactor(ios): prune converged snapshot paths #2383; deferred to the capture-kit move (H2) because the fix changes the
visibilitywire field. runCmdtimeout wedge frequency: mechanism confirmed and fixed in fix(host-kit): a killed command settles on exit, background exec cannot take a timeout #2522; no separate diagnostic.
Needs a maintainer decision (no issue filed)
.addevice-replay corpus has no automatic lane; iOS parked since 2026-07-24 by Prune the test lanes that don't pay for themselves, and add the checks we're missing #1781 A1 with no open tracking issue and no kill criterion. Reactivate or delete.packages/managed-allocationhas one production consumer; ADR 0021 never names the package boundary. Keep while the Simlock plan is alive.undicibundled behind a caret override: aligned to the exact pin in chore(config): scope dependency overrides and delete dead fallow, worktree and prettier entries #2521; vendoring policy stays open.
Reconciliation 2026-09-14 (main 730b20149e)
- fix(daemon): a discarded promise rejection must not shut the daemon down #2519 site 2 (the
Promise.raceloser inrollbackAcquiredSimulatorProcess) was a false finding:Promise.racesubscribes to every member, so a losing rejection is handled. PR fix(daemon): keep a discarded rejection from shutting the daemon down #2532 correctly left it; three sites fixed. - docs: AGENTS.md package specifiers, duplicate ADR 0019, Node floor and web Node 24 requirement #2520:
engines.nodestays>=22.12on purpose. CI verifies the published tarball on 22.12; 22.13 is the contributor requirement for the pinned pnpm and is now documented in CONTRIBUTING. Better than the issue asked. - Wave 1 complete. Wave 2 (fix(host-kit): a killed command settles on exit, background exec cannot take a timeout #2522, fix(host-kit): process lock proves ownership before release and never evicts a live owner #2523, fix(apple-runner): recovery decisions read typed rules; a connection deadline gets zero recovery today #2524, refactor: one shellQuote implementation instead of three #2525) not started; no branch or PR exists. Wave 3: two of four done; perf(scroll-until): build the snapshot visibility index once per pass #2528 and decision: enforce or delete the implementation-file size tiers in AGENTS.md #2529 open. chore(worktrees): stop copying .env into agent worktrees #2530 waits on decision: enforce or delete the implementation-file size tiers in AGENTS.md #2529 only by convention, not by file overlap now that chore(config): scope dependency overrides and delete dead fallow, worktree and prettier entries #2521 landed.
Reactions are currently unavailable
Activity
thymikee commentedon Sep 14, 2026
Status 2026-09-14, main
730b20149e(38 commits past the audit's verification point).Done (5 of 12): #2519 → #2532, #2520 → #2533, #2521 → #2535, #2526 → #2567, #2527 → #2568. Wave 1 is complete.
Corrections recorded in the body: the
Promise.race"fourth site" in #2519 was not a defect (race subscribes to every member).engines.nodekeeps 22.12 by design; 22.13 is documented as the contributor floor.Open (7): wave 2 #2522 (exec settle-on-exit, then background type), #2523 (process lock), #2524 (apple-runner typed recovery), #2525 (shellQuote); wave 3 #2528 (scroll-until hoist), #2529 (size-tier decision); last #2530 (
.envin worktrees). None has a branch or PR. No other commit on main touched their files.Next: #2522 first among wave 2. It carries the characterization-test list that #2524 also depends on for the deadline path.
thymikee commentedon Sep 14, 2026
Status 2026-09-14, main
730b20149e.Wave 3 is complete: #2528 → #2596, #2529 → #2597. Both branch off main and share no files with wave 2, so #2522/#2523/#2524/#2525 stay independent of them.
evaluateIsPredicatenow takes the capture'sSnapshotVisibilityinstead ofnodes, so a--untilpass indexes its tree once for all candidates. The issue's premise about a second, discarding index in the pipeline was wrong for this path —waitisfirst-matchand never builds one; the corrections and four adjacent findings are recorded on perf(scroll-until): build the snapshot visibility index once per pass #2528.post-gesture-*comments that credited AGENTS.md with a 500-line test-file tripwire now state what their fix(daemon): distrust post-gesture stability that matches the pre-gesture baseline #1563 split was instead.Gates:
pnpm check:affected --rungreen at3dbf9dcacf(#2596) andb30cf24e42(#2597). One first-run failure to record for the flake ledger:test/integration/provider-scenarios/android-lifecycle.test.tsframe-action correlation failed once at load average 40+ on 12 cores and passed both alone and on the rerun.Left under this umbrella: #2530 in the Last lane.
thymikee commentedon Sep 21, 2026
Reconciliation at
main45e4c594a1(0.21.8).The wave 2 and wave 3 checkboxes are stale: those children are closed on GitHub and their implementations are on
main. Checked each rather than trusting the boxes.Wave 1 — #2519
#2532, #2520#2533, #2521#2535, all merged 2026-09-13.Wave 2 — #2522
354d3516ec(#2599, killed command settles on exit, background exec refuses a timeout), #2523ef62c7f845(#2598, lock proves ownership before release), #2524a901528351(#2600, typed recovery rules and a verdict on a deadline), #252569667b1a3c(#2595, oneshellQuotebehind the runner host port).Wave 3 — #2526
#2567, #2527#2568, #2528ad9b906140(#2596, one visibility index per pass), #2529751520d49c(#2597, size tiers deleted rather than generalized).That is eleven of twelve children. The last one, #2530, is shipped as PR #2732 (
chore/2530-worktree-dotenv,af177f40ff): the two.worktreeincludelines are gone and the operator recipe is recorded inAGENTS.md's worktree traps.pnpm check:affected --runpassed all runnable checks at that head; the PR isMERGEABLEwith no conflicts. So the umbrella's implementation queue is empty and #2531 can close the moment #2732 merges.What this umbrella still owns is the block that never became an issue, and merging #2732 does not resolve any of it:
.addevice-replay corpus has no automatic lane. iOS has been parked since 2026-07-24 by Prune the test lanes that don't pay for themselves, and add the checks we're missing #1781 A1 with no tracking issue and no kill criterion. Reactivate or delete — "parked indefinitely with no owner" is the worst of the three options.packages/managed-allocationhas one production consumer and ADR 0021 never names the package boundary. Keep while the Simlock plan is alive, per the note here; that expiry condition is worth a date.undicibehind a caret override: the pin was aligned to exact in chore(config): scope dependency overrides and delete dead fallow, worktree and prettier entries #2521, but the vendoring policy question is still open.I did not file these — publishing issues is a maintainer call. Say the word and I'll open three, one per decision, each with the option set above as its opening state. My recommendation: close #2531 when #2732 lands and carry these three as their own issues, since none of them is a finding awaiting execution and leaving them in a closed umbrella is how they get lost again.
thymikee commentedon Oct 2, 2026
Closing: all twelve children are closed and each implementation is verified on main (no reverts; every landing SHA is an ancestor of
cc5e356595): #2519b8d70aa8a1(#2532), #2520b2b084d2e1(#2533), #2521973b74cc14(#2535), #2522354d3516ec(#2599), #2523ef62c7f845(#2598), #2524a901528351(#2600), #252569667b1a3c(#2595), #2526ec712dd741(#2567), #2527e3880f10c8(#2568), #2528ad9b906140(#2596), #2529751520d49c(#2597), #253029ac8e6197(#2732). Body checkboxes are stale in several rows; children were checked against main, not the boxes.The three 'needs a maintainer decision (no issue filed)' rows (replay corpus, managed-allocation, undici vendoring) have no tracker and are not implemented by these children. Per the 2026-09-21 reconciliation note, they should be carried as their own issues rather than kept alive through this umbrella.