You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b4e87be
Browse filesBrowse the repository at this point in the historyBrowse files
fix(desktop): retain in-flight detached agent starts across community switches
PR #7154 review point 4 (P2): resetCommunityState cleared the in-flight
detached-start map on every community switch, and the justification —
"those starts belong to the community being left and are about to fail
closed at the backend's scope assertion anyway" — is wrong for a round
trip. The scope assertion is a current-state check, so a deploy still
held from community A becomes valid again the moment A is re-applied.
Nothing settles the held start across the switch (the mutation lives on
the QueryClient above the remount boundary; apply_workspace aborts no
in-flight deploys), the record still reads not_deployed for the whole
window, and the map entry was the only duplicate guard on this path —
so an A→B→A round trip plus a second mention deployed a provider agent
twice, handing the provider the second message's replay floor (past the
first message: the exact failure class this branch exists to prevent).
Local spawns were never exposed; the backend coalesces those under the
process lock.
Implement the reviewer's fix (option (a) of the analysis): retain the
entries across switches.
- resetCommunityState no longer clears the map; an explicit exemption
comment at the call site (the canonical singleton-reset inventory)
records why, so a future contributor does not "fix" the omission.
Retention is safe by construction: the key IS the tenant scope
(relay URL + pubkey), so a retained A entry can never suppress or
affect a wake in B, and the identity-guarded finally delete
self-cleans at settlement, which the deploy op's timeout bounds. The
backend's own provider_deploy_locks map is likewise never cleared on
workspace apply.
- resetDetachedAgentStarts stays exported as a test-only isolation seam
and its doc now says so; the map header and finally-guard comments
that encoded the refuted "fail closed anyway" rationale are rewritten
to record the retention contract instead.
- Deliberately NOT scope-filtered clearing ("only entries for the
community being left"): the A-scoped entry is exactly the one that
must survive the A→B switch, so that variant recreates the bug.
- The detachedToastScope mirror reset stays: on-screen warning delivery
is genuinely per-community state.
Out of scope, unchanged: backend deploy-epoch coalescing (option (b))
remains the deferred durable fix for the wake paths that do not funnel
through useDetachedAgentStart (Agents-panel Start, restore,
inbound-persona deploys, reconcile_on_workspace_apply). Residual
accepted: on a same-relay identity switch, a retained old-identity
entry suppresses the new identity's wake until the old start fails
closed and settles — the same accepted within-community residual,
self-resolving on the user's next send.
Tests:
- New E2E regression, the load-bearing one — it drives the real
rail-switch → remount → resetCommunityState path that did the
clearing: a not_deployed provider agent's deploy is held open via
startManagedAgentDelayMs; the mention publishes and deploys once,
scoped to A; a real A→B→A rail round-trip runs under the hold; a
second mention back in A publishes while start_managed_agent stays at
exactly 1; the held deploy is then settled on demand and rejects, its
failure warning delivers back in A, and a third mention re-fires the
wake (2 calls) — retention ends at settlement rather than latching
the agent.
- New mock-bridge seam for that spec:
__BUZZ_E2E_RELEASE_MANAGED_AGENT_STARTS__() settles starts held
behind startManagedAgentDelayMs on demand, following the bridge's
defer/release idiom — the hold must deterministically outlast the
round-trip AND settle before the retry leg. The first settlement is
armed to reject via startManagedAgentErrors because a successful mock
settle writes `deployed` into the record, which would hide the retry
leg behind the status check rather than the map — the seam under
test.
- Two unit tests pin the complementary hook contracts in the existing
real-CommunitiesProvider harness: an A→B→A switch hands back a
callback whose key matches the retained entry, so the wake stays
suppressed; and a rejection across the round-trip re-permits. The
existing cross-community test keeps pinning that a B wake during the
interlude is unaffected.
Verified: desktop tsc --noEmit, biome check over the touched files,
desktop unit tests (5856 passed; the 5 failures are the pre-existing
inboxReopenNavigation/useRetainedProjectGitViews baseline, present on
origin/main), the full mentions (87) and community-rail (25) Playwright
smoke suites against a fresh pnpm build:e2e bundle, a 3x stress rerun
of the new spec, and the repository file-size gate. The new E2E spec
was confirmed red on the pre-fix build — 2 deploys vs 1 at the
second-send snapshot — with the map clearing temporarily restored.
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
0 commit comments