Fix stale connection status reads (worker detail, overview, Emily)#2300
Fix stale connection status reads (worker detail, overview, Emily)#2300federicodeponte wants to merge 6 commits into
Conversation
Three surfaces (worker detail's Connect-the-tool banner, Overview's connected-apps read, Emily's capabilities snapshot) could disagree about whether a tool was connected, or show a connected tool as disconnected right after the user connected it. Frontend root cause: the shared TanStack Query cache (30s staleTime, refetchOnMount:false, persisted to localStorage) was never invalidated when a connection completed. Both live completion points now invalidate the connections, worker-detail, and overview query roots before routing the user back: the /connections/redirect poll loop, and the /connections/callback OAuth return leg. Backend root cause (#1209/#1206): a bare try/except in _available_connection_slugs_for_users silently returned an empty set on any DB error, indistinguishable from "genuinely no connections". Now logs the exception so a real DB error is distinguishable in observability from an empty result. Backend root cause (#1205): _build_capabilities_snapshot (baked into Emily's system prompt) enumerated workers via a raw repos.workers.list(user_id=user_id) call, a third worker-enumeration path alongside the grid and Emily's workers__list_all tool (both already route through list_operator_workers, #2270). The raw call used a different (owner-only, no archived filter) visibility rule, so Emily's snapshot could describe workers/state the grid does not show. Now routes through the same list_operator_workers path as the grid and the tool. Fixes floomhq/workeros-cloud#1209, floomhq/workeros-cloud#1205, floomhq/workeros-cloud#1206 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merge-queue FAIL resolved. The OAuth redirect and callback now call a shared connection-cache helper that uses |
|
Freshness recheck completed after Floom main advanced.
The original inactive persisted-query blocker remains resolved on the current-main-containing head. This PR was not merged. |
|
Final current-main refresh completed.
The original blocker is resolved. This PR was not merged. |
Summary
Federico's working theory in floomhq/workeros-cloud#1209 was correct: one shared root cause, not three separate bugs. This fixes the connection-status read path once instead of patching each surface.
connections,worker-detail, andsystem/overviewquery roots before routing the user back:/connections/redirect's poll loop (the browse/connect flow;return_tocan be a worker detail page, which is the exact floomhq/workeros-cloud#1209 repro path)/connections/callback(the OAuth provider's actualredirect_uri, the other live return leg)_available_connection_slugs_for_usershad a baretry/except Exception: return set(). A transient DB failure looked identical to "genuinely no connections" to every caller (worker detail's missing_connections, the grid's missing-connections banner, Overview's connected-apps read), so a DB hiccup on one surface flipped its connection status while a fraction of a second later a different, unaffected surface showed the real state. Now logs the exception so a real DB error is distinguishable from an empty result in observability. Still returns an empty set on failure (no established partial-failure sentinel exists on this response shape yet, and every caller already treats "unknown" and "none" the same way), so this is deliberately not a new error-surfacing contract across three call sites._build_capabilities_snapshot(baked into Emily's system prompt every turn) enumerated workers via a rawrepos.workers.list(user_id=user_id)call: a THIRD worker-enumeration path alongside the grid and Emily'sworkers__list_alltool (both already route throughlist_operator_workers, the documented single source of truth from Worker count disagrees across workers_list (13), system_stats.total_workers (8), and workspace_chat/Emily ("8", missing real workers) #2270). The raw call passed norole, so it used a different, narrower visibility rule (owner-only, and it never excluded archived workers), which is exactly how Emily could describe workers/automations the grid does not show. Now routes through the samelist_operator_workerscall the grid and the tool use, with the same role resolutionworkers__list_allalready uses.Why one root cause explains all three issues
Federico's theory: "the connection-status read path (cache/endpoint) returns empty/stale on first load and different surfaces render different conclusions from it." Confirmed on both sides:
_available_connection_slugs_for_user(s)is the single function all three read surfaces (worker_serialize.py's detailmissing_connections,worker_listing.py's grid banner,overview.py's connected-apps list) call, and it silently degraded to "empty" on any transient failure with zero observability.Test plan
apps/api: newtests/test_emily_capabilities_snapshot_worker_split_brain.py(3 tests: fresh workspace reports 0 workers even with seeded stock workers owned by another identity, snapshot count matcheslist_operator_workersexactly with seeded stock excluded, archived workers excluded). Existingtests/test_emily_worker_split_brain_round09.pystill passes unchanged.apps/web: newtests/connections-invalidate-stale-reads.test.ts(source-inspection, matching the existing pattern for this class of test inrun-page-cache-invalidation.test.ts/emily-create-from-prompt-invalidation.test.ts, since jsdom + fake timers are documented as unreliable for this component's async poll loop). Updated the three existing DOM tests that render/connections/redirectand/connections/callbackto wrap in a realQueryClientProvider(now required since both calluseQueryClient).npm run lint:emdashpasses (0 hits).npx eslinton all changed files: 0 errors.apps/apiconnection/Emily/worker test surface (30 files, ~220 tests) and the widerapps/webconnections/worker-detail test surface (9 files, 42 tests): all pass except 5 pre-existing failures confirmed to also fail identically on a cleanorigin/mainworktree (test-order-dependent flakiness unrelated to this diff:test_connection_reconnect_dedupe.py,test_connection_reliability.py,test_worker_create_connection_preflight.py).Scope note
This does not add a new partial-failure sentinel field to any API response. If Federico wants the frontend to visibly distinguish "we could not check your connections" from "you have no connections" (rather than just having it in logs), that is a small follow-up on top of this, not included here since no existing pattern for it exists in the codebase and the instructions were to fix the observability gap at minimum, not invent a new contract across three call sites.
Fixes floomhq/workeros-cloud#1209, floomhq/workeros-cloud#1205, floomhq/workeros-cloud#1206