Deduplicate cold-load network requests - #2672
Conversation
…th-dev-browser-thr_2fun4nxmap
## What was wrong PR #2616 removed the `host.list_branches` command from the host daemon contract and its dispatch table, but one call to it stayed behind in `host-branches-dispatch.test.ts`. `main` is red: `tsc` reports TS2820 for the unknown command type, and the test itself throws `onlineRpcHandlers[command.type] is not a function`. ## What changed - `apps/host-daemon/test/command/host-branches-dispatch.test.ts` dispatches `host.inspect_git_source` with `remoteRefresh: "blocking"`, like the other tests in the same describe block. A blocking refresh keeps the promise pending until the fetch ends, which is what this test waits for. ## How you verified - `pnpm exec turbo run typecheck test --filter=@bb/host-daemon` passes, 45 test files. The repaired test runs instead of throwing. > AGENT GENERATED Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![bb-slop-cop[bot]](https://avatars.githubusercontent.com/in/4577281?s=60&v=4){kind=small}
| const options = systemConfigQueryOptions(); | ||
| const config = await appQueryClient.fetchQuery({ | ||
| ...options, | ||
| staleTime: refresh ? 0 : options.staleTime, |
There was a problem hiding this comment.
🚨 slopcop/review — One config event can start two requests.
The atom starts this fetch when its WebSocket listener updates the refresh tick.
The realtime cache owner handles the same event and invalidates this query.
If the atom listener runs first, the invalidation cancels this request and starts another request.
My QueryClient reproduction recorded three calls and one abort. The initial load used one call. One refresh used two calls.
Please use one refresh owner, or prevent the later invalidation from canceling an active request.
Add a test with both consumers active. Test both listener orders. Expect one request after config-changed.
| queryClient: QueryClient = appQueryClient, | ||
| ): Promise<PluginFrontendCandidate[]> { | ||
| const plugins = await queryClient.fetchQuery( | ||
| pluginListQueryOptions({ enabled: true }), |
There was a problem hiding this comment.
🚨 slopcop/review — A restored page can use an old plugin list.
The persisted pageshow path calls this function without cache invalidation.
This query stays fresh for 30 seconds. A short page freeze can hide plugin installation, removal, or state changes.
Please force a fresh inventory request after a persisted pageshow event.
Add a test with recent cached data and changed server data. The restore must request and apply the new list.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
This PR gives early app startup and React one shared QueryClient.
It joins matching system config and plugin inventory requests. It also fixes the pending-interaction freshness policy.
I found two event paths that still need work.
- A
config-changedevent can start two config requests. The second refresh cancels the first request. - A persisted page restore can reuse the plugin inventory for 30 seconds. The page can miss plugin changes during the freeze.
I found no confirmed security defect. The product has no supported same-page server or user transition.
The focused app suite passed all 88 tests. All GitHub CI checks passed.
I could not complete browser QA. The checkout-specific dev server stayed in the plugin SDK type build for 20 minutes.
Please add one test for each event sequence. The inline comments give the exact cases.
2 participants
Human comments
What was wrong
Cold app loads had multiple owners for the same system-config and plugin-list resources, while pending-interaction query options accidentally overwrote the shared QueryClient freshness default with
undefined. The plugin frontend loader also interpreted a failed shared query as an authoritative empty installation set, and overlapping initial-connection atom refreshes repeated local daemon discovery.What changed
The app now has one shared QueryClient for React and non-React owners, with stable shared query options for system config and the installed-plugin list. Plugin frontend reconciliation reuses that query and preserves active frontends when inventory loading fails. Pending interactions inherit the app freshness default unless callers provide an override, while stale remounts still revalidate. The redundant local-daemon connection refresh was removed, cache writers use the raw installed-plugin array shape, and narrow regressions cover each ownership and failure boundary.
There are no wire, daemon protocol, CLI, guide, or public Plugin SDK changes.
Review requested from @slopcop.
How you verified
The regressions failed before the fixes and pass afterward. Browser verification used the copied production repro database and confirmed one cold request each for system config, plugins, thread include, interactions, daemon status, and workspace targets. Hosts and sidebar retain their intentional initial-connect race-closing revalidation.
pnpm exec turbo run test --filter=@bb/app --force: 443 files passed; 3,510 tests passed and 3 skippedpnpm exec turbo run typecheck --filter=@bb/app --force: passedpnpm exec turbo run build --filter=@bb/app --force: passedpnpm exec turbo run lint --filter=@bb/app --force: 0 errorsoxfmt --checkandgit diff --check: passedFixes: none