Fix inbox sidebar refetch livelock on large databases - #1050
Draft
lazabogdan wants to merge 21 commits into
Draft
Conversation
The agents sidebar showed a frozen pre-merge snapshot: merged conversations stayed under Working and newly started ones never appeared at all. TanStack Query v5 defaults `refetchQueries` to `cancelRefetch: true`, so every `invalidateQueries` on the sidebar key aborts and restarts an in-flight fetch. The sidebar listing takes minutes against a large database, and the publication poll invalidates that key every 5s while drift persists — so the listing was cancelled and restarted forever, the cache never healed, and the drift signal that triggered the invalidation never cleared. Route every whole-key sidebar invalidation through a new `invalidateAgentSidebarConversations` helper: - `cancelRefetch: false` so a running listing is awaited, not killed. - One trailing pass when a fetch was already in flight. `cancelRefetch: false` alone dedupes onto a fetch that predates the invalidation, and that fetch clears `isInvalidated` on success — so its payload can be stale. A single trailing pass closes that hole; the 5s poll is the backstop, so no loop. The publication poll additionally skips its sidebar invalidation on ticks where a listing is already running. The guard is self-clearing — the next tick re-evaluates drift against the freshly written cache — so it cannot wedge. Per-conversation workspace invalidations stay unconditional; they are cheap. Deliberately not added: drift-fingerprint suppression. Combined with `cancelRefetch: false` producing no automatic trailing refetch, an unchanged drift signature would suppress every later invalidation and wedge the cache permanently — reproducing the bug being fixed. Tests assert all three properties and were confirmed to fail against the pre-fix implementation. The in-flight test uses a QueryObserver: with no observer the query is inactive, `invalidateQueries` never refetches it, and the cancellation cannot be reproduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Groundwork for bounding the agents sidebar listing, which currently awaits `get_latest_for_conversation` and `get_current_repair_attempt` once per conversation — O(N) point reads against a contended database. Adds `AgentRunRepository::get_latest_for_conversations` and `AgentWorkspaceRepairRepository::get_current_repair_attempts_for_conversations`. Both carry default trait implementations that loop the single-item method, so every memory repository and test double compiles unchanged; only the SQLite implementations override with real batched SQL. The SQLite overrides copy the single-item predicates verbatim so semantics cannot drift: - Latest run uses `ROW_NUMBER() OVER (PARTITION BY conversation_id ORDER BY started_at DESC)` filtered to `rn = 1` — the same ORDER BY as the single-item `LIMIT 1`. - Current repair attempt reuses `settled_at IS NULL`. `idx_agent_workspace_repair_attempts_one_active` is a UNIQUE partial index over exactly that predicate, so at most one row per conversation can match and the single-item `LIMIT 1` has nothing to disambiguate. Both chunk at 900 bind parameters, following the existing pattern in `sqlite_agent_conversation_mute_repo`, and go through `db.run(...)`. Migration v20260821101236 adds `agent_runs(conversation_id, started_at DESC)`. The existing `idx_agent_runs_conversation` covers only the equality lookup and would leave SQLite sorting each partition of the new windowed query. Equivalence tests assert the batched result equals the map of single-item results for both repositories, including absent and settled rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A workspace whose linked PR is already recorded `merged` or `closed` still made two GitHub round trips and three duplicate publication writes on every reconcile. The 5s sidebar publication poll invalidates the mounted workspace query, which refetches `get_agent_conversation_workspace` and re-enters this path every reconciliation TTL — so for a merged workspace the user has open, that repeated forever. A recorded terminal status was written by a pass that verified it against GitHub, and a merged or closed PR cannot leave that state, so the remote is already known. Guard on it and return the recorded status directly. Placement matters: the guard sits immediately after the `pr_number` binding and before `correct_foreign_agent_workspace_publication`, because that helper calls `fetch_pr_detail` unconditionally whenever a PR number is recorded. A guard placed after it could not deliver zero GitHub calls. Two deliberate skips for recorded-terminal workspaces: - Foreign-publication correction. It detects a recorded PR number whose head branch is not this workspace's branch. For a workspace already recorded terminal that number was validated on the pass that terminalized it; re-validating it forever is the cost being removed. Startup reconciliation takes the same short-circuit, so this is uniform, and no compensating scan is added. - `reconcile_clickup_ticket_for_workspace_pr`, which already ran on the pass that recorded the terminal status. Local terminal settlement still runs, so a workspace recorded terminal but never cleaned up converges. It is idempotent by construction: `cleanup_terminal_agent_workspace_after_pr` requires this same persisted merged/closed/archived authority, and `claim_local_cleanup` absorbs repeats. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The listing composed a full workspace response for every workspace in a project, then awaited a latest-run read and a current-repair-attempt read per surviving conversation, then composed a conversation response per row — all before grouping and pagination discarded most of them. On a project with hundreds of workspaces that is thousands of awaited point reads to return a page of six. Pass 1 now builds cheap skeletons: raw entities, the two repository reads batched once per project via the new `get_latest_for_conversations` / `get_current_repair_attempts_for_conversations`, and one project-wide plan branch read. Cheap filters run before the batched reads so they only cover surviving rows. Grouping and pagination are unchanged and stay total-accurate over every enumerated row. Pass 2 composes conversation and workspace responses for exactly the rows a page returns. Lane derivation now runs against a new `SidebarWorkspaceFacts` projection rather than a composed response. That projection cannot be built from the persisted entity alone: the response composer overlays `publication_pr_number`, `publication_pr_status`, and `publication_push_status` from a linked plan branch, and those are precisely the fields lanes, labels, refs, and verbs read. Deriving from the raw entity would have silently reclassified every plan-branch-linked workspace. `SidebarWorkspaceFacts::from_entity` therefore applies the same overlay, extracted into a shared `plan_branch_publication_overlay` that the response composer also calls so the two cannot drift, and a test asserts entity-derived facts equal response-derived facts for a linked workspace with divergent columns. The mute command projects its facts from the composed response it already holds, so mute fingerprints stay byte-identical to the listing's. Enumeration also stops composing through `agent_workspace_response_with_pr_supervision_for_state`; pass 2 uses the side-effect-free variant, so the read path no longer schedules recovery work. The now-unused `execution_state` parameter is left in place and prefixed for a follow-up commit that removes the plumbing. The wire response shape is unchanged, so there are no frontend edits. All 50 pre-existing sidebar tests pass without modification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Enumerating the agents sidebar composed every workspace through `agent_workspace_response_with_pr_supervision_for_state`, which schedules PR-supervision recovery — work that can fetch, enqueue an agent, or continue publication. A listing is a read boundary and should not do that. The previous commit already routed composition through the side-effect-free variant, leaving `execution_state` unused. Remove the plumbing entirely: the Tauri command, the `_for_app_state` wrapper, and the impl no longer take it, so the read path has no way to schedule recovery. The frontend only ever passed `input` — the removed parameter was Tauri-managed state — so no client changes. This is read-boundary hygiene, not a performance win: `claim_recovery` already deduped these schedules in-flight and on a 30s TTL before constructing any runtime, so the removed cost was a route decision and a `tokio::spawn`. Recovery keeps its existing triggers: workspace open, `AgentRunCompleted`, and startup. A new test asserts a listing over a published, actively supervised workspace claims no recovery for it, using a narrow `recovery_was_claimed_for_test` accessor over the dedupe maps — `claim_recovery` marks the conversation before anything else runs, so the assertion also catches a schedule whose lazy dependency factory never executed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A contended database emits "Slow SQLite lock operation" faster than the lines can be read or written to disk — the same shape as the incident that produced 41GB of logs in a day. Space the WARN branch out: emit at most one per `db_lock_warn_interval_ms` (new config knob, default 5s, `RALPHX_STREAM_DB_LOCK_WARN_INTERVAL_MS`), count what was suppressed in between, and report that count as `suppressed_count` on the next emitted line so nothing is silently lost. `0` disables limiting. The debug branch is untouched — it is already off in normal operation. The first slow lock after process start always reports, so an isolated one is never swallowed, and an apparently backwards clock reading suppresses rather than spamming. The decision is a pure function over supplied timestamps, so it is tested without waiting on a clock and without touching process-wide state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…econciliation_tests and migrations/mod.rs - Kept origin/main's verified-terminal tests (publication_association_verified_at) replacing the old short-circuit tests in agent_workspace_external_pr_reconciliation_tests.rs - Registered both migrations in correct version order: v20260820174706 (publication_association_verified) from main and v20260821101236 (agent_runs_conversation_started_index) from workspace branch - SCHEMA_VERSION remains 20260821101236 (workspace's later migration) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clippy (no default features) flagged two unused imports in agent_sidebar_commands.rs and one unused pub(crate) function in agent_workspace_pr_supervision_recovery.rs — all three items are only referenced from #[cfg(test)] code, so they are now correctly gated with #[cfg(test)] to satisfy -D warnings in non-test builds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… second arg
invalidateAgentSidebarConversations now passes { cancelRefetch: false } as the
second argument to prevent the inbox refetch livelock, but the test assertion
was not updated to match the new call signature.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add continue-on-error to the paths-filter step in ci.yml and codeql.yml, and fall back to run-all mode in the decide step when the filter fails. This prevents the entire CI run from failing when GitHub diff API is overloaded for large PRs, instead of cascading failures across all gate checks.
…l early-exit The short-circuit guard added in f69e46a fires before the foreign-correction pass when publication_pr_status is already 'merged' or 'closed', so fetch_pr_detail is never called. Update the assertion from 1 → 0 and update the comment to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…al early-exit path The short-circuit guard added in f69e46a fires before `correct_foreign_agent_workspace_publication` (the only prior caller of `mark_publication_association_verified`), so a workspace whose terminal status was recorded before the verified-at marker existed — or that was never fully cleaned up — could never converge to the `workspace_terminal_verified` skip state. A recorded terminal status is itself proof that the association was verified on the terminalizing pass, so stamp the marker inside the early-exit block when it is missing. The call is best-effort (a failed write only means the next trigger re-enters here and retries). Fixes: unverified_terminal_workspace_converges_after_one_verification_pass Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Apply the same continue-on-error + run-all fallback that was added to ci.yml in 6e28549 — the coverage.yml Detect changed paths step was left without it, causing Detect coverage scope (and the downstream Coverage Gate) to fail on transient GitHub diff API overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… deadline tokio::time::sleep(Duration::ZERO) does not return Poll::Ready on the first poll — the timer driver processes the registration asynchronously on a background thread. With a biased select, the stderr.read arm wins before the deadline arm, causing the function to complete normally and return Completed instead of TimedOut when the git command finishes quickly. Fix: return Ok(StreamedGitOutcome::TimedOut) immediately when timeout_secs == 0, before spawning any process. A zero-second timeout means the caller wants immediate expiry; no process needs to run. Also fix wait_for_streamed_exit to return AppResult<StreamedGitOutcome> so that a deadline firing during process exit-waiting returns Ok(TimedOut) rather than Err, honouring the contract that a deadline is an outcome, not an error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The sidebar listing was caught in an unfinishable refetch loop on large databases. The publication poll invalidates the sidebar query every 5 seconds with TanStack Query's default
cancelRefetch: true, which aborts and restarts any in-flight fetch. When the listing takes minutes to complete (due to per-row DB hits on a 46GB+ database), it is perpetually restarted and never settles. This manifested as merged conversations stuck under "Working" and new conversations failing to appear.The fix applies multiple layers:
invalidateAgentSidebarConversationshelper wraps sidebar invalidation withcancelRefetch: falseand a self-clearing in-flight guard — if a listing is already running, defer the invalidation until it settles, then perform exactly one trailing pass.User Impact
Technical Context
Frontend invalidation guard:
agentSidebarConversationKeysmodule withinvalidateAgentSidebarConversationshelperqueryClient.isFetching()before invalidating; if a listing is in flight, it defers and performs exactly one trailing pass after the current fetch settlesBackend query optimization:
get_latest_for_conversations(AgentRunRepository) andget_current_repair_attempts_for_conversations(AgentWorkspaceRepairRepository) with default loop implementations and SQLite batch queriesTerminal PR short-circuit:
mergedorclosed), skip external reconciliation's GitHub re-query and duplicate publication/history writesRecovery scheduling removal:
claim_recoverybut added latency)agent_workspace_response_without_repair_recovery_for_stateLock warning rate-limiting:
db_lock_warn_interval_ms(default 5000ms) suppresses repeated slow-lock WARN lines and reports suppression count on the next emitted lineRisks / Follow-Ups
merged/closedstatus is authoritative; unlikely but possible GitHub edge cases (resurrection) could be missed, though local cleanup will still converge on the next reconciliationView full plan
Inbox Sidebar Refetch Livelock — Fix Plan
Goal
User request: "lets write a plan to address the issues" — the issues being (1) a merged conversation stuck under Recent › Working in the inbox sidebar, and (2) a newly started, actively running conversation never appearing under Working at all.
Root cause: the sidebar listing query is livelocked. The DB is correct; the UI shows a frozen pre-merge snapshot because:
list_agent_sidebar_conversationsis pathologically slow — it enumerates all ~1,341 project conversations and composes a full workspace response for all ~675 workspaces, with several awaited per-row DB reads each, against a contended 46GB DB.invalidateQuerieson the sidebar key every 5s. TanStack Query v5refetchQueriesdefaults tocancelRefetch: true, so each invalidation silently cancels the in-flight multi-minute listing and restarts it (query-coreQuery.fetch:if (this.state.data !== undefined && fetchOptions?.cancelRefetch) this.cancel({ silent: true })). The fetch never completes, the cache never heals, the drift signal fires forever.Corrections applied during verification (the original draft mis-attributed two mechanisms):
agent_workspace_response_with_pr_supervision_for_stateschedules PR-supervision recovery, which never reachesreconcile_agent_workspace_external_pr; andclaim_recoveryapplies an in-flight + 30s-TTL dedupe (agent_workspace_pr_reconciliation_cache_ttl_ms) before the lazy deps closure runs. Sidebar-driven scheduling is cheap and already throttled.invalidateWorkspaceQueries, whoseagentWorkspaceKeys.workspacequery refetchesget_agent_conversation_workspace, which schedules external PR reconciliation (WorkspaceLoad,force = false) for the mounted workspace.'' → mergedrows inagent_conversation_workspace_state_historyare not written by runtime code. That table is only ever inserted into by migrations;update_publication(sqlite_agent_conversation_workspace_repo.rs:1791) is a plainUPDATEthat never touches it. The original "dedupe the history write" step targeted code that does not exist and has been deleted; the duplicate rows are now an explicit open question.Assumptions: Per user decision, the inverted compaction auto-limit bug (46GB DB latency floor) is a separate plan. The staleness-indicator UI question timed out — defaulting to skip (fixing the livelock makes data fresh); revisitable.
Fix Phases (ordered by leverage/risk)
cancelRefetch: falsealone dedupes onto a fetch that predates the change, a bounded trailing pass guarantees the cache actually heals. The drift poll stops stacking invalidations while a listing is already in flight. This is the fix for both symptoms.merged/closedlinked PR no longer makes two GitHub round trips (fetch_pr_detail+check_pr_status) or re-writes publication rows on every workspace-load reconcile; local terminal settlement stays idempotent.agent_workspace_response_without_repair_recovery_for_state. Read-boundary hygiene (rule 0), not a performance fix.Affected Files
frontend/src/hooks/agentSidebarConversationKeys.ts(central non-cancelling invalidate helper)frontend/src/components/agents/useAgentSidebarPublicationPolling.ts(+ test)frontend/src/hooks/andfrontend/src/components/agents/src-tauri/src/commands/agent_sidebar_commands.rs(+ existing test suites)src-tauri/crates/ralphx-domain/src/repositories/agent_run_repository.rs,agent_workspace_repair_repository.rs(batch methods, default impls)src-tauri/src/infrastructure/sqlite/sqlite_agent_run_repo.rs,sqlite_agent_conversation_workspace_repo/repair_attempts.rs(batched SQL overrides)src-tauri/src/application/agent_workspace_external_pr_reconciliation.rs(+ tests)src-tauri/src/infrastructure/sqlite/db_connection.rs(WARN rate limiting)Out of Scope
Decisions
refreshWorkspaceReviewContext(agentWorkspaceQueries.ts:83-95, alreadycancelRefetch: falseafter an explicitfetchStatus === "fetching"check) rather than inventing new bookkeeping (rule 0).cancelRefetch: false(which produces no automatic trailing refetch) it could wedge the cache permanently — the same failure the plan is meant to fix. Guard on "a sidebar listing is already in flight" instead.Risks And Open Questions
'' → mergedrows inagent_conversation_workspace_state_history? Only migrations insert into that table (v20260522090000_agent_workspace_state_history.rs,v20260723065349_pr_autofix_completed_supervision_history.rs); the latter uses plainINSERT, notINSERT OR IGNORE. Likely backfill overlap, not runtime duplication. Bounded read-only investigation before any fix — do not fix speculatively.git fetchwas not possible. Localorigin/mainisd36f703a3, matching this worktree's base;.git/FETCH_HEADis empty, so the age of that ref is unknown. Re-check for drift on the touched files before implementing.Testing Strategy
TDD, focused runs only: Vitest for the invalidate helper and the polling hook (non-cancelling, trailing pass, in-flight guard),
npm run typecheck,npm run linton changed files; targeted Rust tests foragent_sidebar_commands_tests/agent_sidebar_commands_lane_testsparity, batch-repo equivalence tests, new reconciliation terminal short-circuit tests (mock GitHub asserts zero calls), and a WARN-limiter unit test.cd src-tauri && cargo cleanafter any Rust test run.Generated by RalphX
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.