Skip to content

fix(workbench): stop an uninstalled harness from looping catalog errors in the composer - #527

Merged
lucas77778 merged 1 commit into
masterfrom
ruocheng/code-655
Sep 9, 2026
Merged

fix(workbench): stop an uninstalled harness from looping catalog errors in the composer#527
lucas77778 merged 1 commit into
masterfrom
ruocheng/code-655

Conversation

@Zerlight

@Zerlight Zerlight commented Sep 8, 2026

Copy link
Copy Markdown
Member

Summary

The empty "What should we build?" composer logged [LinkCode data error] Error: Failed to load agent catalog about 9–10 times per page load whenever a selectable harness has no runtime installed on the host — here opencode, whose binary is absent on this machine (tracked separately).

useAgentStartCatalogs requested a catalog for every harness unconditionally, gating only Pi on the agent-runtime.list probe (added for CODE-579). For a missing runtime, the engine's agent.catalog handler wraps the adapter's startCatalog() throw in an OperationError with publicMessage: 'Failed to load agent catalog' — and one such failure amplifies into ~10 reports through handleFetchError:

  • SWR 2.4.2 defaults retry every error with exponential backoff (errorRetryInterval 5s, no errorRetryCount cap) and revalidate on focus and reconnect.
  • Each connection generation remounts the tayori hooks, and ReadyRevalidator forces mutate(trueFn) across all keys once the transport is ready.
  • Catalog keys are cwd-scoped, so each workspace resolution is a new key with its own failing fetch and retry chain.

The fix consults the agent-runtime probe for all five harnesses instead of gating Pi alone — the same source the onboarding Download card already reads:

  • runtimes still loading → every request paused, so the picker never flashes a state that isn't true yet;
  • status: 'missing' → never requested; the harness submenu already badges it "Not installed" through the existing runtime cues, so no UI change was needed;
  • out-of-range and unevaluated (absent) kinds stay requested, matching deriveAgentRuntimeCues' fail-open reading.

No wire-protocol or engine change. The dev mock host needs none either: it never answers agent.catalog at all, and its runtime fixture already covers every status the gate reads.

Linear: CODE-655

Verification

Ran the webview against the development daemon on 127.0.0.1:19533, where opencode is the uninstalled harness:

  • The empty composer logged zero [LinkCode data error] lines. The same page served from a pre-fix checkout still logs Failed to load agent catalog, so the difference is the change and not the environment.
  • OpenCode still appears in the Harness submenu, badged "Not installed".

Gates, both run under devenv shell:

  • pnpm check:ci — format clean, lint 0 errors, typecheck clean.
  • pnpm test — 353 files / 3039 tests passed (3 files / 5 tests skipped).

use-agent-catalogs.test.ts now covers runtimes-loading, all-available, the mixed fixture (missing / out-of-range / available / absent), and a workspace switch.

Checklist

  • pnpm check:ci and pnpm test both pass (no Rust changes, so no cargo gates)
  • I ran the affected surface and observed the change working
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped — n/a, no wire change
  • New code and assets are my own work, or their origin and license compatibility are noted above
  • Docs and comments are updated where behavior changed

Copilot AI lite review requested due to automatic review settings September 8, 2026 20:07
@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown

CODE-655

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change aligns with the stated goal (preventing repeated catalog errors for missing runtimes) and includes targeted test coverage for the new gating behavior.

Pull request overview

This PR prevents repeated agent-catalog fetch failures (and resulting error spam/retries) in the composer by gating catalog requests based on the host’s probed runtime availability, skipping harnesses that are definitively missing on the host.

Changes:

  • Gate all getAgentCatalog requests on useAgentRuntimes() being loaded, and skip requesting catalogs for runtimes with status: 'missing' (fail-open for absent/unevaluated kinds once loaded).
  • Update/expand useAgentStartCatalogs tests to cover loading, all-available, mixed runtime states (missing/out-of-range/absent), and workspace switching.
File summaries
File Description
packages/client/workbench/src/surface/use-agent-catalogs.ts Pause catalog requests until runtime availability is known; avoid requesting catalogs for missing runtimes to prevent infinite retry/error amplification.
packages/client/workbench/src/surface/tests/use-agent-catalogs.test.ts Adjust and extend tests to validate the new gating behavior across runtime states and workspace changes.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/client/workbench/src/surface/use-agent-catalogs.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the whole diff (2 files, 1 commit): the catalog-request gate in useAgentStartCatalogs and its rewritten unit tests.

  • Uniform runtime gate replaces the Pi-only special caseuse-agent-catalogs.ts:26-27 funnels all five harnesses through one request() helper: paused while runtimes === undefined, never requested at status: 'missing', still requested for out-of-range and for kinds absent from the snapshot.
  • Pi's gate is deliberately loosened from === 'available' to !== 'missing', so an absent-from-snapshot Pi is now requested. Production only ever reports Pi available or missing (piAvailability() has no version check), so the only divergence is the empty-snapshot state the engine serves after a failed boot probe — where every other kind fails open the same way. The PR body states this, so I read it as intended.
  • Tests rewritten around the new rule — loading, all-available, a mixed fixture (missing / out-of-range / available / absent), and a workspace switch. Both reshaped assertions fail under the old code (the loading case previously nulled only index 3, the mixed case previously requested claude-code), so this is real coverage rather than restated behavior.

What I checked and cleared:

  • No capability is lost for a missing harness. claude-code and grok-build are the two kinds whose startCatalog() resolves in-process (native/claude-code.ts:620, native/grok-build/adapter.ts:76), so they previously served a usable catalog with the runtime absent. The only field the composer reads from it in that state is the approval tier, and a missing kind always yields a cue, which sets sendBlocked at new-session-surface.tsx:396 and renders the onboarding card — the harness can't start, so the control is moot. agentCatalogs has one consumer chain (surface/workbench.tsx:599shell-frame.tsxnew-session-surface.tsx:194), and models come from accountModels, not the catalog.
  • No way to strand the catalogs behind the probe. The engine resolves its readiness Deferred even when the boot probe fails (replying {} rather than never replying), the endpoint-scoped SWR cache outlives a connection-generation remount so runtimes stays populated across reconnects, and tayori does fetch on the null→key transition when a managed download flips a kind to available.
  • The premise in the new comment holds. In the installed swr 2.4.2, shouldRetryOnError: true and errorRetryInterval: 5000 are defaults with errorRetryCount unset, so a failing key really does retry indefinitely (backoff capped at 256 × 5s), and the workbench SWRConfig logs each attempt through onError: handleFetchError.

ℹ️ The trigger is fixed; the amplifier is untouched

Worth deciding explicitly rather than changing here: this removes the one request that reliably fails, but any other deterministically failing key still retries forever and console.errors every attempt — including agent.catalog itself for a harness the probe reports available whose server spawn fails anyway (a codex or opencode app-server that starts and then dies). Nothing in the diff needs to change for that; the question is whether CODE-655 closes at "no longer triggered on a clean machine" or wants a retry policy.

Technical details
# Uncapped SWR error retry remains the amplification mechanism

## Affected sites
- `packages/client/workbench/src/runtime/provider.tsx:217-231` — the workbench `SWRConfig` sets
  `onError`, `provider`, and `use`, but no `errorRetryCount` / `shouldRetryOnError`, so swr's
  defaults apply to every key in the data plane.
- `packages/client/workbench/src/runtime/provider.tsx:237-240``handleFetchError` logs on every
  retry attempt, which is what turns one failing key into a stream of console lines.

## Required outcome
- A decision (not necessarily a code change): does CODE-655 close with the trigger removed, or
  should a permanently-failing data-plane request stop retrying and stop re-logging at some point?

## Open questions for the human
- Is an unbounded retry wanted for keys that fail deterministically rather than transiently? A
  global `errorRetryCount` would also change transport-blip recovery, so per-key
  `shouldRetryOnError: false` (the pattern already used in `simulator/panel.tsx:251` and
  `simulator/consent.ts:40`) may be the closer fit if this is worth doing at all.
- Should `handleFetchError` dedupe by key so a retried failure logs once per key per generation
  instead of once per attempt?

Pullfrog  | View workflow run | Using Claude Opus𝕏

@lucas77778
lucas77778 self-requested a review September 9, 2026 05:02
@lucas77778
lucas77778 merged commit 41087da into master Sep 9, 2026
13 checks passed
@lucas77778
lucas77778 deleted the ruocheng/code-655 branch September 9, 2026 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants