Skip to content

fix(workbench): hide disabled harnesses from new tasks - #460

Merged
lucas77778 merged 1 commit into
masterfrom
chenyu/code-597
Aug 20, 2026
Merged

fix(workbench): hide disabled harnesses from new tasks#460
lucas77778 merged 1 commit into
masterfrom
chenyu/code-597

Conversation

@lucas77778

Copy link
Copy Markdown
Member

Summary

Fixes CODE-597.

  • Filter the new-task Harness picker using the enablement state from Agents settings, so a disabled OpenCode Harness is no longer offered.
  • Wait for provider configuration before exposing choices, fall back when the remembered Harness is disabled, and prevent submission when no Harness is enabled.
  • Pass the filtered Harness list through both the shared and desktop shell paths, with regression coverage for filtering and fallback behavior.

Verification

  • pnpm check:ci
  • pnpm test — 2973 passed, 1 skipped
  • Ran the Electron desktop surface, disabled OpenCode under Agents, and confirmed OpenCode disappeared from the new-task Harness picker; restored the setting afterward.

Checklist

  • pnpm check:ci and pnpm test both pass (plus cargo fmt / clippy / test for Rust changes)
  • I ran the affected surface and observed the change working
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped
  • 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 August 20, 2026 08:30
@linear-code

linear-code Bot commented Aug 20, 2026

Copy link
Copy Markdown

CODE-597

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778
lucas77778 requested a review from PeronGH August 20, 2026 08:32

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The filtering itself is correct and well-tested, but "config still loading" and "every harness disabled" collapse into the same silent state: the composer's harness/model selector unmounts entirely and Enter stops working, with nothing on screen to explain either.

Reviewed changes — full diff of 96f69f2 (7 files, 1 commit), plus the surrounding composer/settings/daemon code the change depends on.

  • Enablement filterselectableHarnessKinds(providers) filters AgentKindSchema.options by providers[kind]?.enabled ?? true. This matches ProviderConfigSchema.enabled's documented intent ("whether the agent is offered in the client's agent picker"), a flag the new-thread page simply never read before.
  • Harness becomes derived, not storedNewSessionSurface splits state into preferredHarness plus an effective harness that falls back to availableHarnesses.at(0) when the remembered one is disabled, so the surface's harness is now AgentKind | undefined and ~15 call sites gain guards.
  • Prop plumbed as requiredShellFrameProps.selectableHarnesses is non-optional, so desktop-shell and webview (which spreads {...props}) are both covered by typecheck; no call site was missed.
  • Send gating widenedsendBlocked now also fires when no harness resolves, and submit() throws rather than posting kind: undefined.
  • Coverage — two new UI tests (fallback from a disabled remembered harness, blocked submit with none enabled) plus a unit test for the filter. Both UI tests genuinely fail with the bug present.

Two things I checked and cleared, so they don't need re-deriving: Object.keys(AGENT_LABELS) and AgentKindSchema.options are the same kinds in the same order, so swapping the static list for the schema-derived one changes neither picker order nor the at(0) default; and the rewritten effort ternary is behavior-preserving for every defined-harness case, including the explicit-null reset.

ℹ️ Mobile's new-thread sheet still offers every harness

apps/mobile/src/components/host/new-thread-sheet.tsx:83 builds its segmented picker straight from AgentKindSchema.options and never reads getProviderConfig, so a harness disabled in Settings → Agents stays on offer there. The daemon doesn't enforce enabled at session start either — it is a client-picker flag only — so mobile will happily start a thread on an agent the user switched off. Not a defect in this diff, but the issue title says "new tasks" rather than "desktop new tasks", so it's worth deciding explicitly whether mobile is in scope or gets a follow-up.

Technical details
# Mobile new-thread sheet ignores per-agent enablement

## Affected sites
- `apps/mobile/src/components/host/new-thread-sheet.tsx:49,83``useState<AgentKind>(AgentKindSchema.options[0])` and the `Picker` body map the full schema enum; no `getProviderConfig` read.
- `packages/foundation/schema/src/model/provider-config.ts:8-9``enabled` is documented as a client-picker flag, and no host adapter gates on it, so nothing downstream catches a disabled pick.

## Required outcome
- A decision (not necessarily code in this PR): either mobile applies the same filter, or the enablement flag is documented as desktop/web-only and a follow-up issue tracks mobile.

## Open questions for the human
- Does CODE-597 cover mobile, or is that a separate ticket?

ℹ️ Nitpicks

  • selectableHarnessKinds is a pure ProvidersConfig → AgentKind[] derivation, which is exactly what settings/providers/view.ts is for ("Pure view helpers … no hooks, unit-testable"). Its sibling withEnabled writes the very flag this reads, AGENT_KINDS there is already AgentKindSchema.options, and that module has its own __tests__/view.test.ts. model-options.ts is otherwise about models and the hook that builds them.
  • surface/workbench.tsx:249 repeats the useData(getProviderConfig, {}) call that useAccountModelOptions already makes internally. SWR dedupes so there's no extra request, but a useSelectableHarnesses() beside useAccountModelOptions would keep the "wait for daemon config, return null until then" rule in one place instead of two.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/presentation/ui/src/shell/new-session-surface.tsx
Comment thread packages/presentation/ui/src/shell/new-session-surface.tsx

@PeronGH PeronGH left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed: filtering, fallback, and blocked-submission behavior are correct and well-scoped. Wired through all three shell render sites; empty/undefined harness handled safely in Composer and ModelSelectorMenu. Approving.

@lucas77778
lucas77778 merged commit 03a6b01 into master Aug 20, 2026
12 checks passed
@lucas77778
lucas77778 deleted the chenyu/code-597 branch August 20, 2026 13:33
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