Skip to content

feat: unify agents & BrowserOS AI (one picker, merged settings)#1065

Merged
Nikhil (shadowfax92) merged 10 commits into
mainfrom
fix/no-agent-ui
May 28, 2026
Merged

feat: unify agents & BrowserOS AI (one picker, merged settings)#1065
Nikhil (shadowfax92) merged 10 commits into
mainfrom
fix/no-agent-ui

Conversation

@shadowfax92
Copy link
Copy Markdown
Contributor

Summary

  • BrowserOS AI is just another target. The home/new-tab composer now uses the same merged "AI Providers + Agents" picker the sidepanel already had, with BrowserOS pre-selected — so it works with zero agents (the "No agents yet / Create agent" dead-end is gone).
  • AI Settings → "AI & Agents" master-detail. A ?section=-driven page: BrowserOS AI (the LLM-providers UI) plus one pane per visible adapter (Claude/Codex) that owns instance create/delete. Hermes is hidden in the UI (kept in the backend/types).
  • Standalone Agents page removed. The /agents route, its page, and the "Agents" sidebar item are gone; agent management lives in settings, conversations under /home/agents. Old links redirect (/agents/settings/ai?section=claude, /agents/:id/home/agents/:id).

Design

Lift the sidepanel's proven pattern (Provider { kind: 'llm' | 'acp' }, kind-based routing) up to the home composer and reorganize settings — no new runtime. LLM picks route to the existing /home/chat; agent picks to /home/agents/:id. A single lib/chat/adapter-visibility helper hides Hermes at the buildSidepanelChatTargets chokepoint, so every picker/list inherits it. The home→/home/chat handoff persists the shared chat-target selection (the value that surface actually resolves from, preferred over the global default). entrypoints/app/agents/* shared libs (data hooks, NewAgentDialog, AgentList) are reused by the new settings panes; only the page-level files were deleted.

Test plan

  • bun run typecheck — passes
  • bun test apps/agent — 81 unit tests pass (new: adapter-visibility, ai-settings-sections, home-compose routing)
  • /settings/ai lists BrowserOS AI / Claude / Codex (no Hermes); ?section=claude deep-links; create + delete an instance from a pane
  • /home with zero agents shows the composer with BrowserOS selected (no empty-state card)
  • Pick an LLM provider on /home + send → lands in /home/chat on that provider; pick an agent → lands in /home/agents/:id
  • /agents and /agents/:id redirect correctly; no "Agents" sidebar item

@shadowfax92
Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

@shadowfax92
Copy link
Copy Markdown
Contributor Author

Claude (@claude) review

@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Claude finished Nikhil (@shadowfax92)'s task in 5m 10s —— View job


Code Review

Tasks

  • Gather context and understand the PR
  • Read changed files and analyze code
  • Post review feedback

Overview

This is a well-executed refactor that delivers on its goals: the dead-end "no agents" empty state is gone, agent management moves into settings, and adapter visibility is now a single-source-of-truth. The new utility modules are small, well-named, and properly tested. Overall the PR is in good shape — the findings below are mostly minor.


Findings

1. Dead ternary branches in ConversationInput.tsx

Two places produce identical class names in both branches, making the ternary dead code:

ConversationInput.tsx:556-558

className={cn(
  'flex gap-3',
  variant === 'home' ? 'px-4 py-3' : 'px-4 py-3',  // both branches identical
  isExpandedDraft ? 'items-end' : 'items-center',
)}

ConversationInput.tsx:717-724 (BotInputIcon)

className={cn(
  'flex items-center justify-center text-[var(--accent-orange)]',
  variant === 'home'
    ? 'h-8 w-8 rounded-lg bg-[var(--accent-orange)]/10'
    : 'h-8 w-8 rounded-lg bg-[var(--accent-orange)]/10', // both branches identical
)}

Both can be simplified to a static string. Fix this →


2. deletingAgentKey cleanup sits outside finally (AdapterAgentsPane.tsx:121-133)

const handleDelete = async (item: AgentListItem) => {
  setDeletingAgentKey(item.key)
  try {
    await deleteHarnessAgent.mutateAsync(item.agentId)
    track(AGENT_DELETED_EVENT, ...)
  } catch (err) {
    setPageError(...)
  }
  setDeletingAgentKey(null)  // only reached if nothing throws past the catch
}

If the mutation rejects with something the catch re-throws (unlikely now, but defensive), or if the component unmounts mid-flight, the key stays set and the UI shows a ghost "deleting" spinner indefinitely. Move the reset into finally. Fix this →


3. as HarnessAgentAdapter cast in AISettingsPage.tsx:89

<AdapterAgentsPane adapterId={activeSection as HarnessAgentAdapter} />

resolveAiSettingsSection returns string (AiSettingsSection). The cast silences the type error rather than proving correctness. If an unknown section string ever reaches this branch (e.g., a future adapter is added to visibleAdapterIds without being added to HarnessAgentAdapter), the pane gets an invalid adapter ID at runtime. A simple narrow — validate activeSection against the loaded adapters list before rendering — would make this type-safe without the cast.


4. MANAGE_AGENTS_PATH hardcodes the Claude section (AgentCommandHome.tsx:33)

const MANAGE_AGENTS_PATH = '/settings/ai?section=claude'

"Manage agents" deep-links only into Claude. A user who has only Codex agents configured lands in the Claude pane (which would be empty). Consider pointing to /settings/ai and letting resolveAiSettingsSection do its job, or picking the first visible adapter dynamically.


5. ACP target selection is not persisted on home send (AgentCommandHome.tsx:132-146)

LLM sends call persistSidepanelChatTargetSelection(target) before navigating; ACP sends navigate immediately without persisting. This means returning to /home after an agent conversation always resets the picker back to the default LLM provider rather than the agent the user just used. The PR description notes this is intentional for LLM picks, but it may feel inconsistent to users who repeatedly switch to the same agent. Worth a follow-up ticket if not by design.


6. LegacyAgentRedirect lands at a non-alpha route for non-alpha builds (App.tsx:42-46)

const LegacyAgentRedirect: FC = () => {
  const params = useParams()
  return <Navigate to={`/home/agents/${params.agentId ?? ''}`} replace />
}

/home/agents/:agentId only renders inside the alphaEnabled conditional. On a non-alpha build, the redirect sends the user to /home/agents/:id, which isn't a registered route and falls through to the * catch-all (/home). The agent ID is silently lost. If old bookmarked links to /agents/:id are expected to work across build types, the redirect should check alphaEnabled and fall back to /settings/ai?section=claude or /home for non-alpha.


7. Fragile relative-import workaround in tests

Both sidepanel-chat-targets.ts and adapter-visibility.test.ts use relative imports to work around bun test's incomplete @/ alias resolution for value imports. The comments explain the reason, which is helpful. It's an existing limitation rather than a new problem, but now that two separate files have this pattern it's worth raising as a known gap — if someone moves a file without updating these paths, the tests silently break with a module-not-found error rather than a TypeScript error.


Minor Nits

  • AdapterAgentsPane.tsx:197-220: createRuntime and harnessAdapterId are both set to adapterId, and hermesProviders/hermesSelectedProviderId/onHermesProviderChange are all dead no-ops. If NewAgentDialog will never have a Hermes path again, these props could be cleaned up.
  • agents-page-hooks.ts is now only one function (useDefaultAgentName). Could be inlined or the file renamed to match its single purpose (e.g., use-default-agent-name.ts per the kebab-case convention).

What's Well Done

  • adapter-visibility.ts is a clean, tiny module. The one-line re-enable comment is great future-proofing.
  • Test coverage for all three new utility modules is solid and covers the edge cases.
  • routeHomeSend as a pure function with its own test suite is the right call.
  • The resolveAiSettingsSection fallback chain (null → hidden adapter → stale link → before-load) is explicit and well-tested.
  • Redirect chain for old /agents links is comprehensive.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

✅ Tests passed — 1056/1061

Suite Passed Failed Skipped
agent 89/89 0 0
build 10/10 0 0
eval 95/95 0 0
server-agent 246/246 0 0
server-api 63/63 0 0
server-browser 4/4 0 0
server-integration 9/10 0 1
server-lib 252/253 0 1
server-root 59/62 0 3
server-tools 229/229 0 0

View workflow run

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR unifies the standalone Agents page and BrowserOS AI settings into a single "AI & Agents" master-detail settings pane, and replaces the home composer's agent-only picker with a merged LLM provider + agent picker backed by the sidepanel's existing Provider model.

  • Navigation restructure: /agents route removed; /agents redirects to /settings/ai?section=claude and /agents/:id redirects to /home/agents/:id for backward compatibility. A new ?section=-driven AISettingsPage renders either the BrowserOS AI pane or a per-adapter AdapterAgentsPane (Claude/Codex; Hermes hidden via adapter-visibility.ts).
  • Home composer refactor: AgentCommandHome now uses buildSidepanelChatTargets + toProviderOption to populate the same picker the sidepanel uses; LLM picks persist the shared chat-target selection before navigating to /home/chat, agent picks stash a pendingInitialMessage and navigate to /home/agents/:id.
  • Dead code removed: AgentsPage, AgentsHeader, AgentSelector, hermes-supported-providers, agents-page-actions and several hook/util functions are deleted; adapter-visibility.ts is the new single point of control for Hermes visibility.

Confidence Score: 3/5

Safe to merge for navigation and UI changes; agent creation in the new settings pane can produce incorrect data when the user switches between adapter sections without completing a create flow.

The home composer refactor and redirect wiring are clean and well-tested. The main concern is in AISettingsPage: AdapterAgentsPane shares a React instance across adapter sections (no key prop), so form state from a prior adapter can bleed into a new agent creation on a different adapter — a user who manually changed a model string and then switched panes would silently submit the wrong model to the backend.

packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx — the AdapterAgentsPane rendering needs a key prop to prevent stale state across adapter switches.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx Refactored into a master-detail shell; stale adapter state bug when switching panes (no key prop on AdapterAgentsPane)
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AdapterAgentsPane.tsx New pane for per-adapter agent CRUD; model/reasoning seeding effect uses or-guard which can carry stale values from a prior adapter instance
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx Replaced agent-only picker with unified LLM+agent provider picker; MANAGE_AGENTS_PATH hardcoded to claude section
packages/browseros-agent/apps/agent/entrypoints/app/App.tsx Removed standalone /agents routes, added LegacyAgentRedirect for /agents/:agentId, /agents now redirects to AI settings
packages/browseros-agent/apps/agent/lib/chat/adapter-visibility.ts New single source of truth for hidden adapters (Hermes); clean and testable with good test coverage
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/home-compose.helpers.ts New pure routing helper; well-tested and cleanly separates routing decisions from navigation side effects
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/ai-settings-sections.ts New section resolver with solid fallback logic; correctly handles hidden adapters, missing params, and stale deep links
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/ConversationInput.tsx Swapped agent-only selector for unified ChatProviderSelector; removed status prop and onCreateAgent callback cleanly

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Home["/home (AgentCommandHome)"] --> Picker["Unified Provider Picker\n(LLM providers + Agents)"]
    Picker -->|"LLM selected"| PersistTarget["persistSidepanelChatTargetSelection\nsetDefaultProvider"]
    PersistTarget --> HomeChat["/home/chat"]
    Picker -->|"Agent selected"| PendingMsg["setPendingInitialMessage"]
    PendingMsg --> AgentConvo["/home/agents/:id"]

    LegacyAgents["/agents"] -->|redirect| AISettings["/settings/ai?section=claude"]
    LegacyAgentId["/agents/:agentId"] -->|redirect| AgentConvo

    AISettings --> MasterDetail["AI & Agents Master-Detail"]
    MasterDetail -->|"section=browseros"| BrowserOsPane["BrowserOsAiPane\n(LLM providers)"]
    MasterDetail -->|"section=claude"| ClaudePane["AdapterAgentsPane(claude)"]
    MasterDetail -->|"section=codex"| CodexPane["AdapterAgentsPane(codex)"]
    MasterDetail -.->|"hidden"| Hermes["Hermes (filtered by adapter-visibility.ts)"]

    style Hermes stroke-dasharray: 5 5
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx:33
**Hardcoded `?section=claude` may not link to the right pane**

`MANAGE_AGENTS_PATH` always points at the Claude adapter pane. If the user has no Claude adapter (or if Claude is not listed as a visible adapter), `resolveAiSettingsSection` will fall back to the BrowserOS AI pane — hiding all agents rather than surfacing the active adapter's management UI. Consider deriving the first visible adapter's ID at runtime (e.g., from `adapters`) rather than hardcoding `'claude'`.

### Issue 2 of 2
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx:86-90
**Stale form fields after switching adapter panes**

`AdapterAgentsPane` lacks a reconciliation identity prop, so React reuses the same instance when the user navigates between adapters. The `modelId` and `reasoningEffort` states carry over, and the seeding effect — which guards with `(current) => current || default` — will not overwrite truthy leftover values. A user who customised a model in one adapter pane and then switched would silently create an agent in the second pane with an incompatible model string. Adding a `key` prop set to `activeSection` forces a fresh mount on every section transition.

Reviews (1): Last reviewed commit: "chore: self-review fixes — persist chat-..." | Re-trigger Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR unifies BrowserOS AI and agent management into a single settings page (/settings/ai?section=) and replaces the home composer's agent-only picker with a merged LLM + agent picker, removing the standalone /agents route in favour of redirects.

  • New adapter-visibility helper (lib/chat/adapter-visibility.ts) provides a single, well-tested chokepoint for hiding Hermes across every picker and settings list; all three affected surfaces (sidepanel, home composer, AI settings) inherit the filter correctly.
  • AISettingsPage refactored into a master-detail shell (BrowserOsAiPane + AdapterAgentsPane), with section state driven by ?section= query param; the resolveAiSettingsSection guard handles stale/hidden section values robustly.
  • AgentCommandHome now builds SidepanelChatTarget options via the shared buildSidepanelChatTargets helper and persists the selection to the shared storage before navigating to /home/chat, keeping the sidepanel's selection in sync.

Confidence Score: 3/5

Safe to merge after fixing the adapter state leakage — creating an agent from the wrong adapter pane would silently use a stale model ID from a previously visited adapter.

The master-detail settings shell has no key on AdapterAgentsPane, so React preserves the component's modelId and reasoningEffort state across adapter section switches. A user who visits Claude's pane, partially fills the dialog, then switches to the Codex pane will see Claude's model pre-filled for a new Codex agent. Because the seeding effect guards with current || default, the stale value is never overridden, and agent creation would attach a potentially unrecognised model ID to the new agent.

packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx — missing key on AdapterAgentsPane

Important Files Changed

Filename Overview
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx Refactored to a master-detail shell; all provider management moved to BrowserOsAiPane and agent CRUD to AdapterAgentsPane. Missing key prop on AdapterAgentsPane causes state leakage between adapter sections.
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AdapterAgentsPane.tsx New pane for per-adapter agent CRUD. Local state (modelId, reasoningEffort) not keyed to adapterId, so switching sections without a key prop lets stale values from a previous adapter persist into the next adapter's create dialog.
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx Replaced agent-only picker with unified LLM+agent picker. handleSend is async but onSend type is void, so storage/provider errors silently abort navigation without user feedback.
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/home-compose.helpers.ts New pure routing helper. ACP provider with no agentId silently falls through to the LLM path, but this is an unreachable edge case in the current data model.
packages/browseros-agent/apps/agent/lib/chat/adapter-visibility.ts New single-source-of-truth visibility helper. Clean, well-tested, and correctly hides Hermes while exposing Claude and Codex.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/sidepanel-chat-targets.ts Added isAdapterHidden filter to agent list; Hermes agents are now excluded from the picker. Clean and well-tested change.
packages/browseros-agent/apps/agent/entrypoints/app/App.tsx Removed old /agents route and replaced with two redirect routes. LegacyAgentRedirect correctly maps /agents/:agentId to /home/agents/:agentId.
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/BrowserOsAiPane.tsx Straightforward extraction of the old AISettingsPage provider-management content into its own pane component with no logic changes.
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/ConversationInput.tsx Replaced agent-specific selector props with generic Provider picker props. Clean refactor with no logic changes.
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/ai-settings-sections.ts New section resolver with solid fallback logic. Well-tested covering hidden adapters, unknown values, and pre-load state.

Comments Outside Diff (1)

  1. packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx, line 312-362 (link)

    P2 Async errors silently abort navigation

    handleSend is async, but the onSend prop type is (input: ConversationInputSendInput) => void, so the returned Promise is never awaited by the caller. If persistSidepanelChatTargetSelection or setDefaultProvider rejects (e.g., storage failure), the exception is swallowed as an unhandled rejection and navigate(route.path) is never called — the user clicks send and nothing happens with no error shown.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx
    Line: 312-362
    
    Comment:
    **Async errors silently abort navigation**
    
    `handleSend` is `async`, but the `onSend` prop type is `(input: ConversationInputSendInput) => void`, so the returned `Promise` is never awaited by the caller. If `persistSidepanelChatTargetSelection` or `setDefaultProvider` rejects (e.g., storage failure), the exception is swallowed as an unhandled rejection and `navigate(route.path)` is never called — the user clicks send and nothing happens with no error shown.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/browseros-agent/apps/agent/entrypoints/app/ai-settings/AISettingsPage.tsx:85-90
Stale adapter state leaks between sections. `AdapterAgentsPane` is a single component instance that receives a new `adapterId` prop when the user switches from (e.g.) `claude` to `codex`. Without a `key`, React keeps the same instance, so local `modelId` and `reasoningEffort` state from the previous adapter stays live. The seeding effect uses `current || adapter.defaultModelId`, so a non-empty `modelId` from Claude will never be overwritten by Codex's default — the "New Agent" dialog for Codex will be pre-filled with a Claude model ID.

```suggestion
      <div className="min-w-0 flex-1">
        {activeSection === BROWSEROS_SECTION ? (
          <BrowserOsAiPane />
        ) : (
          <AdapterAgentsPane key={activeSection} adapterId={activeSection as HarnessAgentAdapter} />
        )}
```

### Issue 2 of 3
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/AgentCommandHome.tsx:312-362
**Async errors silently abort navigation**

`handleSend` is `async`, but the `onSend` prop type is `(input: ConversationInputSendInput) => void`, so the returned `Promise` is never awaited by the caller. If `persistSidepanelChatTargetSelection` or `setDefaultProvider` rejects (e.g., storage failure), the exception is swallowed as an unhandled rejection and `navigate(route.path)` is never called — the user clicks send and nothing happens with no error shown.

### Issue 3 of 3
packages/browseros-agent/apps/agent/entrypoints/app/agent-command/home-compose.helpers.ts:14-21
**ACP provider without `agentId` silently falls to LLM path**

If a `Provider` with `kind === 'acp'` has an absent `agentId`, the condition `provider.kind === 'acp' && provider.agentId` is falsy, and the function falls through to the `llm` branch — returning a route to `/home/chat` with the agent's `id` used as a provider ID. The intent is likely to return `null` (or throw) for this malformed case rather than silently misroute. While `agentId` should always be present for ACP providers in practice, an explicit guard would make the failure visible.

Reviews (1): Last reviewed commit: "chore: self-review fixes — persist chat-..." | Re-trigger Greptile

@shadowfax92
Copy link
Copy Markdown
Contributor Author

Thanks Greptile (@greptileai) / Claude (@claude) — addressed in 0fb0577.

Accepted

  • [P1] Stale model/reasoning state between adapter panesAdapterAgentsPane is now keyed (key={activeAdapter.id}) so it remounts on section switch and create-form state can't leak Claude↔Codex. Also dropped the as HarnessAgentAdapter cast in favor of a typed lookup of the active adapter descriptor.
  • deletingAgentKey not in finally — moved the reset into finally so the row spinner always clears.
  • routeHomeSend acp-without-agentId misroute — now returns null instead of falling through to the LLM path; added a unit test for the malformed case.
  • MANAGE_AGENTS_PATH hardcoded to claude — "Manage agents" now derives the section from the user's most-recent visible agent's adapter (falls back to /settings/ai), so a Codex-only user no longer lands on an empty Claude pane.

Declined

  • Dead ternaries in ConversationInput (px-4 py-3, BotInputIcon) — these predate this PR and are unrelated to the agents/AI merge; leaving them out to keep the diff focused. Can do a separate cleanup if you'd like.

Gates: typecheck ✅ · Biome ✅ · bun test apps/agent ✅ (added the acp-guard test).

@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Claude finished Nikhil (@shadowfax92)'s task in 1m 15s —— View job


Claude finished reviewing Nikhil (@shadowfax92)'s follow-up fixes


Tasks

  • Gather context from trigger comment
  • Verify fix: AdapterAgentsPane key prop + cast removal (AISettingsPage.tsx)
  • Verify fix: deletingAgentKey moved to finally (AdapterAgentsPane.tsx)
  • Verify fix: routeHomeSend returns null for ACP without agentId (home-compose.helpers.ts)
  • Verify fix: MANAGE_AGENTS_PATH derives section dynamically (AgentCommandHome.tsx)

Verification Results

All four accepted fixes are correctly implemented.

[P1] Stale adapter state — AISettingsPage.tsx:93-96
key={activeAdapter.id} is present, and the as HarnessAgentAdapter cast is gone. The active adapter descriptor is now resolved via a typed shownAdapters.find() lookup, so the pane gets a fully-typed adapterId and remounts clean on every section switch. ✅

deletingAgentKey cleanup — AdapterAgentsPane.tsx:132-134
setDeletingAgentKey(null) is inside finally. The spinner will always clear. ✅

ACP misroute guard — home-compose.helpers.ts:23-24
Explicit if (!provider.agentId) return null before the happy path, with a unit test covering the malformed case (routeHomeSend for acp-without-agentId returns null). ✅

Dynamic manageAgentsPathAgentCommandHome.tsx:131-136
The constant is gone. manageAgentsPath is now a useMemo that walks orderedAgents, finds the first agent on a visible adapter, and derives the ?section= from it — falling back to /settings/ai with no section when no agents exist. ✅


One Open Item (not in scope for this PR)

The handleSend function is async, but the onSend prop type is void, so the returned promise is not awaited by ConversationInput. If persistSidepanelChatTargetSelection or setDefaultProvider ever rejects (storage failure), the error is swallowed and navigate is never called — the user clicks send and nothing happens with no feedback. This was flagged by greptile but wasn't in your accepted/declined list. Low probability in practice, but worth a follow-up ticket if you'd like to harden it.


Looks good to merge — the P1 state-leak and all other accepted findings are correctly resolved.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

All four fixes verified in 0fb0577b:

  • key={activeAdapter.id} — confirmed in AISettingsPage.tsx, AdapterAgentsPane remounts on section switch.
  • Typed adapter lookupactiveAdapter is shownAdapters.find(...), no cast.
  • finally blocksetDeletingAgentKey(null) is in finally in AdapterAgentsPane.tsx.
  • routeHomeSend acp-guard — returns null when provider.agentId is missing, confirmed.
  • manageAgentsPath derivation — correctly picks the first non-hidden adapter from orderedAgents, falls back to /settings/ai for a no-agent user.

The declined ConversationInput ternaries are a reasonable deferral — they're pre-existing and unrelated to this PR's scope.

LGTM — good to merge.

@shadowfax92 Nikhil (shadowfax92) merged commit c5d9f26 into main May 28, 2026
24 checks passed
Nikhil (shadowfax92) added a commit that referenced this pull request May 28, 2026
* fix: version-gate Claude Code/Codex agents to BrowserOS 0.46.0.0

The unified picker (#1065) surfaced the agent-harness adapters (Claude
Code, Codex) on every BrowserOS version the auto-updating extension runs
on, but the harness/ACP backend only ships in BrowserOS 0.46.0.0+. On
older builds the agents appeared but could not run.

Add Feature.AGENT_HARNESS_SUPPORT (minBrowserOSVersion 0.46.0.0) and gate
the two data hooks that feed every surface: useAgentAdapters (AI & Agents
settings tabs) and useHarnessAgents (picker ACP targets). When
unsupported, both disable their query and return empty — so the home and
sidepanel pickers, settings tabs, and recent-agents dock all hide the
agents, with no requests to harness endpoints that may not exist.

* fix: address review — guard error field and loading through capabilities init

- loading now folds in useCapabilities().isLoading so consumers don't see a
  premature 'no agents' empty state during the capabilities-init window on
  supported BrowserOS (Greptile P2, both hooks).
- error is now gated by agentsSupported, so urlError from useAgentServerUrl
  can't leak an error banner on older builds where harness endpoints are
  absent — older builds see nothing, per intent (Claude review).
- Note why checkFeatureSupport is exported (tests only).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant