Fix timer operation ownership and stabilize timing regressions - #317
Conversation
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review — via Wes’s account
No actionable findings identified in the pinned change: head e9cf2a57de0cd64064050200ba82b7b4dca84a92, base f5c49be04b0f4cdabc926580caf73e6035ea39d8.
Reviewed the full 22-file diff, relevant caller/lifecycle paths, repository guidance and product/design documentation. Mantis independently reviewed plugin-operation ownership and directory recovery; I checked those conclusions against source and integrated them with the profile/browser review.
- Plugin ownership:
src/plugins/manager.ts:91–167retains the catalog-read guard and management busy state until actual settlement, fences stale reads, and ignores late timed-out results. Disposal stops publication/polling, not an already-running native call. The native-lock fixture and resolve/reject tests cover the intended ownership boundaries. - Directory recovery:
src/features/relay/session.ts:1359–1393retries only one access-epoch cancellation through the verified scheduler. Caller cancellation, cache clear, connection retirement, disposal and repeated invalidation terminate recovery. Directory results remain outside shared profile admission; exhausted failures reach explicit UI Retry. - Agent panels: the refresh hook is separated from subscription-only controls, with exact-instance panels suppressing the nested timer. Read-only ready/error recovery, explicit Retry, controller coalescing and app-owned process lifetime remain intact. Staggered child-mount tests avoid hiding duplicate intervals behind coalescing.
- Test boundaries: notification replay uses a fresh ordered-stream receipt; unread cancellation waits for reconciliation and a durable queue barrier. The clocked visible-arrival case establishes policy/wiring, not native frame ordering. Native exact-row/reflow journeys remain. The removed 50-ms send assertion is now explicitly diagnostic; deterministic work/identity assertions remain, but no latency guarantee or measured speedup is established.
Validation limits: source-only review; neither reviewer ran tests, PR code, installs or an app, or edited source. Extract hashes were checked against pinned Git objects; no dirty checkout inputs were used. Remote CI for this head is observed evidence, not our execution: run 36329685626 and CI required succeeded, as did DCO/security checks; Windows native validation was skipped. The hosted timing artifact reports 4,408/4,408 Vitest tests, 310.50 s wrapper wall time and 505.61 s summed test execution; slowest file was unread-startup.test.ts (27.85 s). These are not an equivalent before/after benchmark.
Human app acceptance, native desktop/OS-banner behavior, cross-platform release validation and the acknowledged native presentation-order audit remain unverified here. This COMMENT is not an approval or merge authorization.
…ad-on-send * origin/main: (58 commits) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) fix(status): reopen a Today status as Today near 16:00 (#275) test: use current navigation for GIF send roundtrip (#309) Fix composer focus when selecting channels and DMs (#307) fix: retire mention searches after chips and refuted prose (#303) ... # Conflicts: # src/features/messages/MessageComposer.test.tsx # src/features/messages/MessageComposer.tsx
* origin/main: (45 commits) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentCard.tsx # src/bundled/agents/AgentsPage.tsx
* origin/main: (36 commits) Delay message timestamp tooltips by 500 ms (#321) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentEditor.tsx # src/bundled/profiles/ProfileAgentIdentity.test.tsx
Opened by Brain on behalf of Wes.
Result
Bound timer ownership to actual operation lifetimes instead of treating elapsed time as completion. Three bounded production fixes; no blanket timer removal.
Production scope is seven frontend files, 94 additions / 31 deletions after formatting. The Rust change is test-only fixture support. Remaining changes are regression tests and contract documentation.
Test stability
Browser inventory: zero cases added/removed; the same 13 scenarios run in Chromium and WebKit (26 executions). Browser coverage remains necessary for real relay-stream/app wiring, IndexedDB durability, focus, virtualized geometry and navigation. No replacement with jsdom, engine removal, retry increase or longer sleep.
Evidence
Final commit:
e9cf2a57de0cd64064050200ba82b7b4dca84a92; base:f5c49be04b0f4cdabc926580caf73e6035ea39d8. Local macOS with pinned Hermit tools; working tree clean and remote head verified.unread-startup.test.ts, 15.104 s. Full CI suites remain separate.node --test tests/integration/plugin-cli.test.mjs tests/integration/plugin-manager-lock.test.mjs— 4/4, 13.23 s. Includes a real held native registry lock across 44 simulated seconds, actual release/recovery and a separate native writer.pnpm exec playwright test --config tests/browser/playwright.config.mjs tests/browser/notifications.spec.mjs tests/browser/unread.spec.mjs --project chromium --project webkit --no-deps— 26/26, 1.3 min, two workers, no retries. Measurement dependencies deliberately excluded from this focused run, not from CI.The pre-format restored browser run and final formatted run both report 1.3 min. This is not an unmodified-main benchmark or a demonstrated speedup; separate before/after setup timings were not collected. No shared test infrastructure was redesigned.
Remaining gates / non-goals
Human walkthrough
From the agreed timer-fixes worktree, launch
bin/just desktopwhen ready (coordinate rather than replacing an existing dev app):Please report explicit success/failure before this is marked ready.
Originating conversation: buzz://message?channel=3428ec3f-a58b-429b-afbc-dc6a77918ce8&id=40212d7bc1d3d424623e73643c995d9e6d36a2927b58f0ab9e7752c0fd710f98