Droid Updates#319
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughAdds a persistent cross-surface model picker (favorites/recents), shared runtime model catalog with refresh/TTL, new picker UIs for CLI/Desktop/iOS, and migrates Droid runtime from ACP to a new Droid SDK (protocol, worker, pool, service refactors). Includes provider auth gating and tests. ChangesCross-surface Model Picker + Droid SDK
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Contributor
Reviewed at 0682bbc without applying fixes. No blocking issues found in the last 14 commits. One non-blocking note: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (2)
6417-6422:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExplicitly clear fast mode when the next model does not support it.
This now drops
codexFastModefrom the update payload entirely. BecauseupdateSessionis patch-like elsewhere in this file, switching from a fast-mode-capable Codex model to one that does not support it can leave the oldcodexFastMode=truelatched on the session while the toggle disappears from the UI.Suggested fix
void window.ade.agentChat.updateSession({ sessionId: selectedSessionId, modelId: nextModelId, reasoningEffort: snapshot.nextReasoningEffort, - ...(modelSupportsFastMode(snapshot.nextDesc) ? { codexFastMode } : {}), + ...(snapshot.nextProvider === "codex" + ? { codexFastMode: modelSupportsFastMode(snapshot.nextDesc) ? codexFastMode : false } + : {}), ...nextNativeControlPayload, }).then((updatedSession) => {Mirror the same explicit reset in the pre-send update at Line 5104 so the stale flag cannot survive that path either.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 6417 - 6422, When calling window.ade.agentChat.updateSession with the new model (the patch that builds the payload using selectedSessionId, nextModelId, snapshot.nextReasoningEffort, modelSupportsFastMode(snapshot.nextDesc) ? { codexFastMode } : {}, and ...nextNativeControlPayload), explicitly set codexFastMode: false when modelSupportsFastMode(snapshot.nextDesc) is false so the old fast-mode flag cannot remain latched; mirror the same explicit reset in the pre-send update path (the other updateSession call used before sending messages) so both paths clear codexFastMode when the new model does not support fast mode.
6627-6650:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the runtime-catalog resolver when parallel slots are actually launched.
This setter now accepts runtime-catalog-only models, but the parallel preview/launch path still resolves
slot.modelIdwith plaingetModelById(Line 2439, Line 2461, Lines 4722-4743, and Lines 4768-4779). A dynamic Cursor/Droid model can therefore look valid in the picker and then be launched with the wrong provider metadata.Suggested direction
- const desc = resolveModelDescriptorWithRuntimeCatalog(nextModelId) ?? getModelById(nextModelId); + const desc = resolveModelDescriptorWithRuntimeCatalog(nextModelId) ?? getModelById(nextModelId); const tiers = desc?.reasoningTiers ?? [];Use the same
resolveModelDescriptorWithRuntimeCatalog(...) ?? getModelById(...)pattern in the slot preview and submit-timeslot.modelIdlookups as well, so the configured model and launched model stay in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 6627 - 6650, The preview/launch code still uses getModelById for slot.modelId lookups which can mis-resolve runtime-catalog-only models; update the slot preview and submit/launch paths to use resolveModelDescriptorWithRuntimeCatalog(slot.modelId) ?? getModelById(slot.modelId) wherever slot.modelId is resolved (e.g., the preview/render code and the submit/launch handlers that currently call getModelById), so that functions like selectReasoningEffort, getExecutionModeOptions, recommendedOpenCodePermissionModeForModel, and shouldResetOpenCodePermissionForModel operate on the descriptor returned from resolveModelDescriptorWithRuntimeCatalog when available and keep configured vs launched provider metadata in sync.apps/ios/ADE/Models/RemoteModels.swift (1)
1900-1924:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsider making
isAvailableoptional for defensive backward compatibility.
AgentChatModelCatalogModeluses synthesizedCodablewith a non-optionalBoolforisAvailable. If older host versions omit this key in payloads, the entire catalog will fail to decode. While all current desktop payloads includeisAvailable, making it optional—following the pattern used elsewhere inRemoteModels.swiftfor defensive decoding—would safeguard against unexpected older host versions.💡 Suggested adjustment
- var isAvailable: Bool + var isAvailable: Bool?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ios/ADE/Models/RemoteModels.swift` around lines 1900 - 1924, The model's synthesized Codable will fail if the JSON omits isAvailable; change the AgentChatModelCatalogModel property isAvailable from Bool to Bool? to allow missing keys during decoding, and update any consumers of AgentChatModelCatalogModel.isAvailable to handle nil (e.g., treat nil as false or provide a safe default) so runtime code doesn't force-unwrap or assume non-optional state; locate the struct AgentChatModelCatalogModel and adjust usages accordingly (comparisons, UI bindings, filtering) to handle the optional value.
🧹 Nitpick comments (2)
apps/ade-cli/src/services/modelPickerStore.ts (1)
5-7: ⚡ Quick winRename new constants to camelCase to match repo naming rules.
The newly introduced constant identifiers use uppercase snake case; please rename them (including the exported one) to camelCase for guideline compliance.
As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.Also applies to: 139-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/services/modelPickerStore.ts` around lines 5 - 7, The three newly added constants use UPPER_SNAKE_CASE and must be renamed to camelCase: change MAX_RECENTS → maxRecents, STORE_VERSION → storeVersion, and PERSIST_DEBOUNCE_MS → persistDebounceMs, update any export that exposes them (including the exported one around the other occurrence noted), and update every reference/usage (reads, tests, imports/exports) to use the new names; keep the same values and types and run a quick grep/IDE refactor to ensure no lingering references remain.apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx (1)
783-787: ⚡ Quick winExtract repeated model-picker interaction steps into a helper.
The same
pointerDown→clicktrigger →clickprovider-tab sequence is duplicated across many tests. A small helper will reduce drift and future flake-prone edits.As per coding guidelines, "apps/desktop/src/**: Electron desktop app — check for IPC security, proper main/renderer process separation, and React best practices."♻️ Proposed refactor
async function clickEnabledModelOption(name: RegExp | string) { const options = await screen.findAllByRole("option", { name }); const enabledOption = options.find((option) => option.getAttribute("aria-disabled") !== "true"); expect(enabledOption).toBeTruthy(); fireEvent.click(enabledOption!); } + +async function openModelPickerAndSelectProviderTab(providerTabName: RegExp | string) { + const trigger = await screen.findByRole("button", { name: /^Select model/ }); + fireEvent.pointerDown(trigger, { button: 0 }); + fireEvent.click(trigger); + fireEvent.click(await screen.findByRole("tab", { name: providerTabName })); +}Then replace repeated blocks with:
-const modelTrigger = await screen.findByRole("button", { name: /^Select model/ }); -fireEvent.pointerDown(modelTrigger, { button: 0 }); -fireEvent.click(modelTrigger); -fireEvent.click(await screen.findByRole("tab", { name: /^Anthropic$/i })); +await openModelPickerAndSelectProviderTab(/^Anthropic$/i);Also applies to: 1796-1805, 1853-1862, 2107-2113, 2148-2154, 2190-2195, 2245-2250, 2293-2299, 2336-2342, 2367-2373, 2414-2420, 2486-2492, 2972-2977, 2982-2987, 3140-3151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx` around lines 783 - 787, Tests duplicate the model-picker interaction (pointerDown → click trigger → click provider tab → click model) across many cases; extract this into a helper like selectModelOption(providerPattern, modelPattern) used by AgentChatPane.submit tests. Implement a new utility function in the test file (or a shared test-utils) that takes a provider regex (e.g. /Anthropic/i) and a model regex (e.g. /Claude Sonnet 4\.6/i), performs await screen.findByRole("button", { name: /^Select model/ }), fireEvent.pointerDown(...), fireEvent.click(...) on the trigger, clicks the provider tab via screen.findByRole("tab", { name: providerPattern }), then calls the existing clickEnabledModelOption(modelPattern); replace each repeated block (instances referenced in the comment) with calls to this helper to reduce duplication and flakiness.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/services/modelPickerStore.ts`:
- Around line 153-155: Call and await any pending flush on the shared store
before nulling the singleton to avoid lost debents across tests: update
resetSharedModelPickerStoreForTests to first detect sharedStoreInstance and
invoke its flush method (e.g. flushPendingWrites(), flushDebouncedWrites(), or
flush()) and await the result if it returns a Promise, then set
sharedStoreInstance = null; you may need to make
resetSharedModelPickerStoreForTests async to accommodate awaiting the flush.
In `@apps/ade-cli/src/tuiClient/app.tsx`:
- Around line 3362-3401: openModelPicker currently opens the model picker via
setRightPane/setRightOpen but does not mark it as user-sticky, so subsequent
context-default effects can immediately replace it; modify openModelPicker to
set the sticky marker (e.g. include a flag like sticky: true or call
markRightPaneSticky) when calling setRightPane, and ensure the corresponding
commit and Escape-close handlers (the commit path and the Escape/close handlers
referenced around the other ranges) clear that sticky marker (e.g. remove the
flag or call unmarkRightPaneSticky) so the pane is only sticky while explicitly
opened by the user; update usages in openModelPicker and the commit/Escape close
flows to toggle this sticky state alongside setRightPane/setRightOpen.
- Around line 5653-5684: The bug is that when getModelById(descriptor) is
undefined for catalog-only models, the code falls back to modelState.provider
and keeps the old runtime; instead, locate the provider that produced
catalogModel (the provider object in the loop that found catalogModel or the
current picker selection) and use that to compute provider =
normalizeProvider(resolveProviderGroupForModel(...)) (or a normalizeProvider
call that accepts the catalog provider info) before calling
modelStatePatchForModel; update the fallback logic so provider is derived from
catalogModel's provider/subsection/provider.id (from modelCatalogRef.current
groups) instead of modelState.provider so session creation targets the correct
runtime.
- Around line 3337-3357: The effect that hydrates model picker favorites/recents
is currently keyed to socketPath and returns early if connectionRef.current is
not set, so it never retries when the RPC connection arrives; update the
useEffect dependency from [socketPath] to depend on the actual connection state
(e.g., [connection] or connectionRef.current wrapped in state) so the effect
re-runs when the connection becomes available, leaving the internal early-return
and cancel logic intact and still calling
getModelPickerFavorites/getModelPickerRecents and then
setModelPickerFavorites/setModelPickerRecents.
In `@apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.ts`:
- Around line 157-160: The code currently filters pool using
input.selection.provider which can be stale; instead first resolve the selected
rail to an existing entry (e.g. find an entry in allEntries whose family or
provider matches input.selection.provider) and if none exists, set the resolved
selection to a valid fallback (such as the favorites rail or the first available
rail) and update railIndex accordingly (referencing railIndex, favorites,
input.selection.provider); only after that derive pool = allEntries.filter(entry
=> entry.family === resolvedProvider) so headings/empty-states and selection
context stay consistent.
In `@apps/desktop/src/main/services/chat/droidSdkEventMapper.ts`:
- Around line 249-258: The done-event builds a final usage object that can
contain cacheCreationTokens while token events use cacheWriteTokens, causing
downstream inconsistency; update the mapping so the final returned usage uses
the same key as token events (cacheWriteTokens) by normalizing the usage
produced by usageFrom (or immediately after computing usageFrom(tokenUsage))—if
usage.cacheCreationTokens exists, set usage.cacheWriteTokens =
usage.cacheCreationTokens and delete usage.cacheCreationTokens before returning
the done event (touch symbols: tokenUsage, usageFrom, usage, and the returned
object in the done-event branch).
- Around line 136-144: The fallback itemId generation in the extractTextBlocks
flatMap uses a hardcoded "droid-message" prefix which doesn't match the deltas'
itemIdFor fallback ("droid-text"/"droid-thinking"), causing dedupe mismatches;
update the itemId creation in the mapping inside extractTextBlocks flatMap to
use the same logic as itemIdFor (or call itemIdFor) so that when
readString(record.messageId) is null the fallback prefix depends on block.kind
("droid-text" for text, "droid-thinking" for thinking) and produces identical
itemId strings to the delta events (ensure references: extractTextBlocks,
block.kind, itemId, meta.state.assistantDeltaItemIds / thinkingDeltaItemIds).
In `@apps/desktop/src/main/services/chat/droidSdkWorker.ts`:
- Around line 102-110: The permission waiter map currently keys entries by
request.id which can collide; update requestPermission to generate a
guaranteed-unique correlation key (e.g., crypto.randomUUID() or an incrementing
uniqueToken) and use that uniqueKey for
permissionWaiters.set/permissionWaiters.delete and include the same uniqueKey on
the IPC payload (the posted request) so the resolver map and incoming responses
correlate by this uniqueKey rather than request.id; apply the same change to the
corresponding response handler code that resolves entries from permissionWaiters
so it looks up by the uniqueKey and still cleans up the entry after resolving.
- Around line 225-257: sendPrompt is vulnerable to concurrent runs because it
uses a single global currentAbort that gets overwritten; either serialize sends
or scope aborts per-call: update sendPrompt to check and block concurrent sends
for the same session (e.g., if currentAbortForSession[session.sessionId] exists
throw an error) or replace the single currentAbort with a per-session map (e.g.,
currentAbortForSession: Record<string, AbortController>) and set
currentAbortForSession[session.sessionId] = controller before streaming from
session.stream and clear that entry in the finally block only if it still equals
the controller; ensure post-stream cleanup and cancel logic use the per-session
entry so earlier streams are not left active or interleaving events.
In `@apps/desktop/src/preload/preload.ts`:
- Around line 3117-3137: The current modelPicker fallbacks passed into
callProjectRuntimeSyncOr (modelPicker.getFavorites, setFavorites,
toggleFavorite, getRecents, pushRecent) return synthetic empty arrays and never
persist state; replace those inline fallback async handlers to delegate to the
real main-process persistent store instead (i.e., call the existing main-store
API or IPC store methods from the preload layer) so getFavorites/getRecents read
from the main store and setFavorites/pushRecent/toggleFavorite update and
persist the same main-store; keep the same return shapes but ensure each
fallback invokes the main-process store methods rather than returning hard-coded
defaults.
- Around line 1192-1194: The fallback gating only considers async action calls;
extend it so sync runtime calls also fall back: update the condition around
canUseFallback (used when deciding to callLocalProjectActionIfBound) to also
allow fallback for sync paths (callProjectRuntimeSyncOr /
callLocalProjectSyncIfBound) by treating the same "not callable" error as
eligible for fallback (or adding a corresponding
isLocalRuntimeSyncActionNotCallableError) and ensure ADE_LOCAL_RUNTIME_FALLBACK
is applied consistently for both async (callLocalProjectActionIfBound) and sync
(callLocalProjectSyncIfBound / callProjectRuntimeSyncOr) code paths, while still
validating with isSafeLocalRuntimeFallbackError.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Line 2261: selectedModelDesc is now resolved via
resolveModelDescriptorWithRuntimeCatalog but other code paths still call
getModelById(modelId) causing runtime-only models to fall back to the wrong
provider; replace those direct getModelById calls with the runtime-aware
resolver (i.e., use resolveModelDescriptorWithRuntimeCatalog(modelId) ??
getModelById(modelId)) wherever sessionProvider, composer, slash-command fetch,
or CLI launch logic references getModelById (examples: the spots around where
sessionProvider is computed and the handlers at lines you noted), and ensure
downstream code reads provider info from the resulting descriptor
(selectedModelDesc or newly resolved descriptor) instead of re-querying by
modelId.
In `@apps/desktop/src/renderer/components/shared/ModelPicker/ModelListRow.tsx`:
- Around line 177-183: The row action buttons in ModelListRow currently set
tabIndex={-1}, preventing keyboard focus; locate the interactive elements wired
to handleToggleFavorite and handleFavoriteKeyDown (and the analogous handlers
for reasoning and sign-in buttons) and remove the tabIndex={-1} (or change it to
tabIndex={0}) so the controls are focusable by keyboard, ensuring the existing
onKeyDown handlers (handleFavoriteKeyDown, the reasoning and sign-in key
handlers) properly handle Enter/Space activation and that
aria-label/aria-pressed attributes are preserved for accessibility.
In
`@apps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerContent.tsx`:
- Around line 436-442: The current handleCopyId uses try/catch around
navigator.clipboard.writeText(modelId) which returns a Promise, so rejected
promises aren't caught; update handleCopyId to handle async rejections by
awaiting the Promise in an async function or attaching a .catch handler to
navigator.clipboard.writeText(modelId) and log or ignore errors there. Locate
the handleCopyId callback in ModelPickerContent.tsx and ensure clipboard
failures are handled via async/await with try/catch or via .catch to avoid
unhandled promise rejections.
In
`@apps/desktop/src/renderer/components/shared/ModelPicker/useModelFavorites.ts`:
- Around line 85-90: The hydration logic in hydrateFromRemote currently sets
hydrated and leaves the module-level hydrationStarted flag preventing retries
when getRpcApi() is temporarily unavailable; change hydrateFromRemote (and the
related block guarded by hydrationStarted) so that if getRpcApi() returns null
you do NOT set hydrated or permanently flip hydrationStarted — instead exit
early without marking hydrated (and ensure hydrationStarted remains false or is
reset), so the function can be retried later once the RPC surface is available;
conversely, only set hydrated and mark hydrationStarted true after a successful
remote reconciliation (use the existing set(...) and hydrationStarted variables
around the successful RPC calls) and apply the same change in the similar block
referenced at lines ~102-123.
In `@apps/desktop/src/renderer/components/shared/ModelPicker/useModelRecents.ts`:
- Around line 86-91: The hydration path in useModelRecents (hydrateFromRemote)
incorrectly sets hydrated = true when getRpcApi() returns null, and leaves
hydrationStarted blocking future attempts; instead, when !api is detected do NOT
set hydrated to true and reset or clear hydrationStarted so the function can be
retried later (or schedule a retry). Update hydrateFromRemote and the related
hydrationStarted/hydrated state updates so that hydrationStarted is only
set/cleared around actual attempts, and hydrated is only set true after a
successful reconcile (reference hydrateFromRemote, hydrationStarted, hydrated,
and set).
In `@apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx`:
- Around line 434-474: The dropdown menu is rendered inline and gets clipped by
parent overflow; change the inline menu render (the JSX block that currently
conditionally renders the <div role="menu"...> when open) to render via a portal
instead (e.g., ReactDOM.createPortal or your app's Portal component) into
document.body and position it using the trigger/button's bounding rect (use
containerRef or the button ref to compute left/top) so the menu escapes parent
overflow; alternatively integrate a collision-aware positioning helper
(floating-ui) for flip/viewport logic — update the conditional render logic that
uses open to create a portaled menu and compute absolute screen coordinates
before rendering.
In `@apps/desktop/src/shared/modelCatalog.ts`:
- Around line 149-153: The classifyProviderGroup function currently only returns
"ollama" or "lmstudio" when providerRoute === "opencode", so direct local
descriptors (ModelDescriptor with family === "ollama" or "lmstudio" but
providerRoute === "openai-compatible") fall through; fix by adding a family
check at the top of classifyProviderGroup (check model.family === "ollama" ||
model.family === "lmstudio") to return model.family before the providerRoute ===
"opencode" branch so both OpenCode-routed and direct local models are classified
into the new local groups.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 3641-3644: The cachedChatModelCatalog() helper currently picks the
newest entry from chatModelCatalogCache without respecting the per-host/project
scoping encoded in chatModelsCacheKey; update cachedChatModelCatalog() (and the
fallback lookups that use it) to first filter chatModelCatalogCache entries to
only those whose cache key matches the current chatModelsCacheKey scope (or
otherwise contains the current host/project identifier) before sorting by
fetchedAt and checking chatModelsCacheTTL; use the same scoping logic wherever
cachedChatModelCatalog() is used so a catalog from a different host/project
isn’t returned (refer to chatModelCatalogCache, chatModelsCacheKey,
cached.fetchedAt and chatModelsCacheTTL to locate and fix the code).
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 6417-6422: When calling window.ade.agentChat.updateSession with
the new model (the patch that builds the payload using selectedSessionId,
nextModelId, snapshot.nextReasoningEffort,
modelSupportsFastMode(snapshot.nextDesc) ? { codexFastMode } : {}, and
...nextNativeControlPayload), explicitly set codexFastMode: false when
modelSupportsFastMode(snapshot.nextDesc) is false so the old fast-mode flag
cannot remain latched; mirror the same explicit reset in the pre-send update
path (the other updateSession call used before sending messages) so both paths
clear codexFastMode when the new model does not support fast mode.
- Around line 6627-6650: The preview/launch code still uses getModelById for
slot.modelId lookups which can mis-resolve runtime-catalog-only models; update
the slot preview and submit/launch paths to use
resolveModelDescriptorWithRuntimeCatalog(slot.modelId) ??
getModelById(slot.modelId) wherever slot.modelId is resolved (e.g., the
preview/render code and the submit/launch handlers that currently call
getModelById), so that functions like selectReasoningEffort,
getExecutionModeOptions, recommendedOpenCodePermissionModeForModel, and
shouldResetOpenCodePermissionForModel operate on the descriptor returned from
resolveModelDescriptorWithRuntimeCatalog when available and keep configured vs
launched provider metadata in sync.
In `@apps/ios/ADE/Models/RemoteModels.swift`:
- Around line 1900-1924: The model's synthesized Codable will fail if the JSON
omits isAvailable; change the AgentChatModelCatalogModel property isAvailable
from Bool to Bool? to allow missing keys during decoding, and update any
consumers of AgentChatModelCatalogModel.isAvailable to handle nil (e.g., treat
nil as false or provide a safe default) so runtime code doesn't force-unwrap or
assume non-optional state; locate the struct AgentChatModelCatalogModel and
adjust usages accordingly (comparisons, UI bindings, filtering) to handle the
optional value.
---
Nitpick comments:
In `@apps/ade-cli/src/services/modelPickerStore.ts`:
- Around line 5-7: The three newly added constants use UPPER_SNAKE_CASE and must
be renamed to camelCase: change MAX_RECENTS → maxRecents, STORE_VERSION →
storeVersion, and PERSIST_DEBOUNCE_MS → persistDebounceMs, update any export
that exposes them (including the exported one around the other occurrence
noted), and update every reference/usage (reads, tests, imports/exports) to use
the new names; keep the same values and types and run a quick grep/IDE refactor
to ensure no lingering references remain.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx`:
- Around line 783-787: Tests duplicate the model-picker interaction (pointerDown
→ click trigger → click provider tab → click model) across many cases; extract
this into a helper like selectModelOption(providerPattern, modelPattern) used by
AgentChatPane.submit tests. Implement a new utility function in the test file
(or a shared test-utils) that takes a provider regex (e.g. /Anthropic/i) and a
model regex (e.g. /Claude Sonnet 4\.6/i), performs await
screen.findByRole("button", { name: /^Select model/ }),
fireEvent.pointerDown(...), fireEvent.click(...) on the trigger, clicks the
provider tab via screen.findByRole("tab", { name: providerPattern }), then calls
the existing clickEnabledModelOption(modelPattern); replace each repeated block
(instances referenced in the comment) with calls to this helper to reduce
duplication and flakiness.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3c454ab-5337-4443-9bd9-f5bd908019e9
⛔ Files ignored due to path filters (7)
apps/desktop/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/ade-code/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/agent-routing.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/sync-and-multi-device/remote-commands.mdis excluded by!docs/**
📒 Files selected for processing (103)
apps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/services/modelPickerStore.test.tsapps/ade-cli/src/services/modelPickerStore.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/components/ModelPicker/ModelPickerPane.tsxapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.test.tsapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.tsapps/ade-cli/src/tuiClient/components/ModelPicker/types.tsapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/ade-cli/src/tuiClient/theme.tsapps/ade-cli/src/tuiClient/types.tsapps/desktop/package.jsonapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/ai/aiIntegrationService.test.tsapps/desktop/src/main/services/ai/aiIntegrationService.tsapps/desktop/src/main/services/ai/tools/systemPrompt.test.tsapps/desktop/src/main/services/ai/tools/systemPrompt.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/cursorModelsDiscovery.test.tsapps/desktop/src/main/services/chat/cursorModelsDiscovery.tsapps/desktop/src/main/services/chat/cursorSdkPool.tsapps/desktop/src/main/services/chat/cursorSdkProtocol.tsapps/desktop/src/main/services/chat/cursorSdkWorker.tsapps/desktop/src/main/services/chat/droidAcpPool.tsapps/desktop/src/main/services/chat/droidModelsDiscovery.test.tsapps/desktop/src/main/services/chat/droidModelsDiscovery.tsapps/desktop/src/main/services/chat/droidSdkEventMapper.tsapps/desktop/src/main/services/chat/droidSdkPool.tsapps/desktop/src/main/services/chat/droidSdkProtocol.tsapps/desktop/src/main/services/chat/droidSdkWorker.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/ipc/runtimeBridge.tsapps/desktop/src/main/services/opencode/openCodeBinaryManager.test.tsapps/desktop/src/main/services/opencode/openCodeBinaryManager.tsapps/desktop/src/main/services/opencode/openCodeInventory.test.tsapps/desktop/src/main/services/opencode/openCodeInventory.tsapps/desktop/src/main/services/sync/syncRemoteCommandService.test.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.test.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/FeedbackReporterModal.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/cto/IdentityEditor.tsxapps/desktop/src/renderer/components/cto/WorkerCreationWizard.tsxapps/desktop/src/renderer/components/missions/ModelSelector.tsxapps/desktop/src/renderer/components/prs/shared/PrResolverLaunchControls.test.tsxapps/desktop/src/renderer/components/prs/shared/PrResolverLaunchControls.tsxapps/desktop/src/renderer/components/settings/AiFeaturesSection.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.test.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.tsxapps/desktop/src/renderer/components/shared/ModelCatalogPanel.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelListRow.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.test.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerContent.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerRail.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ReasoningEffortPicker.test.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ReasoningEffortPicker.tsxapps/desktop/src/renderer/components/shared/ModelPicker/modelCatalog.test.tsapps/desktop/src/renderer/components/shared/ModelPicker/modelCatalog.tsapps/desktop/src/renderer/components/shared/ModelPicker/modelOrdering.test.tsapps/desktop/src/renderer/components/shared/ModelPicker/modelOrdering.tsapps/desktop/src/renderer/components/shared/ModelPicker/modelPickerSearch.test.tsapps/desktop/src/renderer/components/shared/ModelPicker/modelPickerSearch.tsapps/desktop/src/renderer/components/shared/ModelPicker/providerEmptyState.test.tsxapps/desktop/src/renderer/components/shared/ModelPicker/providerEmptyState.tsxapps/desktop/src/renderer/components/shared/ModelPicker/runtimeCatalogCache.tsapps/desktop/src/renderer/components/shared/ModelPicker/useAuthOnlyFilter.tsapps/desktop/src/renderer/components/shared/ModelPicker/useModelFavorites.tsapps/desktop/src/renderer/components/shared/ModelPicker/useModelRecents.tsapps/desktop/src/renderer/components/shared/ModelPicker/usePerSurfaceModelDefaults.tsapps/desktop/src/renderer/components/shared/ModelPicker/useProviderAuthStatus.test.tsapps/desktop/src/renderer/components/shared/ModelPicker/useProviderAuthStatus.tsapps/desktop/src/renderer/components/shared/ModelPicker/useReasoningByFamily.tsapps/desktop/src/renderer/components/shared/ProviderLogos.tsxapps/desktop/src/renderer/components/shared/ProviderModelSelector.tsxapps/desktop/src/renderer/components/shared/ReviewLaunchModelControls.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsxapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/modelCatalog.tsapps/desktop/src/shared/modelRegistry.tsapps/desktop/src/shared/types/chat.tsapps/desktop/src/shared/types/sync.tsapps/desktop/tsup.config.tsapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/Work/WorkModelCatalog.swiftapps/ios/ADE/Views/Work/WorkModelPickerSheet.swiftapps/ios/ADE/Views/Work/WorkPreviews.swiftapps/ios/ADE/Views/Work/WorkSessionSettingsSheet+Actions.swiftapps/ios/ADE/Views/Work/WorkSessionSettingsSheet.swiftapps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swiftapps/ios/ADETests/ADETests.swift
💤 Files with no reviewable changes (3)
- apps/desktop/src/renderer/components/shared/ProviderModelSelector.tsx
- apps/desktop/src/renderer/components/shared/ModelCatalogPanel.tsx
- apps/desktop/src/main/services/chat/droidAcpPool.ts
Comment on lines
+3337
to
+3357
| useEffect(() => { | ||
| const conn = connectionRef.current; | ||
| if (!conn) return; | ||
| let cancelled = false; | ||
| void (async () => { | ||
| try { | ||
| const [favorites, recents] = await Promise.all([ | ||
| getModelPickerFavorites(conn).catch(() => [] as string[]), | ||
| getModelPickerRecents(conn).catch(() => [] as string[]), | ||
| ]); | ||
| if (cancelled) return; | ||
| setModelPickerFavorites(favorites); | ||
| setModelPickerRecents(recents); | ||
| } catch { | ||
| // Best-effort hydration — picker still functions with empty state. | ||
| } | ||
| })(); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [socketPath]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo && wc -l apps/ade-cli/src/tuiClient/app.tsxRepository: arul28/ADE
Length of output: 114
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '3337,3357p' apps/ade-cli/src/tuiClient/app.tsxRepository: arul28/ADE
Length of output: 700
🏁 Script executed:
# Get broader context around this effect to understand connectionRef and socketPath
sed -n '3300,3380p' apps/ade-cli/src/tuiClient/app.tsxRepository: arul28/ADE
Length of output: 3646
🏁 Script executed:
# Search for connectionRef definition and usage
rg "connectionRef\s*=" apps/ade-cli/src/tuiClient/app.tsx -A 2 -B 2Repository: arul28/ADE
Length of output: 321
🏁 Script executed:
# Search for socketPath usage and definition
rg "socketPath" apps/ade-cli/src/tuiClient/app.tsx -B 2 -A 2 | head -50Repository: arul28/ADE
Length of output: 1514
🏁 Script executed:
# Search for "connection" variable definition to see if it exists as suggested in the fix
rg "const connection\s*=" apps/ade-cli/src/tuiClient/app.tsx -B 2 -A 2Repository: arul28/ADE
Length of output: 344
🏁 Script executed:
# Look for any connection-related state or props
rg "\bconnection\b" apps/ade-cli/src/tuiClient/app.tsx | head -20Repository: arul28/ADE
Length of output: 1388
🏁 Script executed:
# Find where connectionRef.current is assigned
rg "connectionRef\.current\s*=" apps/ade-cli/src/tuiClient/app.tsx -B 2 -A 2Repository: arul28/ADE
Length of output: 979
🏁 Script executed:
# Search for setConnection to understand when/how connection state is updated
rg "setConnection" apps/ade-cli/src/tuiClient/app.tsx -B 2 -A 2Repository: arul28/ADE
Length of output: 926
🏁 Script executed:
# Search for useEffect that might set connectionRef.current
rg "useEffect.*connection" apps/ade-cli/src/tuiClient/app.tsx -B 2 -A 5Repository: arul28/ADE
Length of output: 36
🏁 Script executed:
# Check where connection state is defined and used
rg "const \[connection,\s*setConnection\]" apps/ade-cli/src/tuiClient/app.tsx -B 3 -A 10Repository: arul28/ADE
Length of output: 1066
🏁 Script executed:
# Check the detailed flow around connection establishment to confirm the timing
sed -n '2800,2900p' apps/ade-cli/src/tuiClient/app.tsxRepository: arul28/ADE
Length of output: 4378
🏁 Script executed:
# Verify socketPath doesn't change - search for its usage patterns
rg "socketPath" apps/ade-cli/src/tuiClient/app.tsx | grep -E "(props|argument|\{|:)" | head -10Repository: arul28/ADE
Length of output: 495
🏁 Script executed:
# Double-check the useEffect that establishes the connection to understand the race condition
rg -B 5 "connectionRef\.current = conn" apps/ade-cli/src/tuiClient/app.tsx | head -30Repository: arul28/ADE
Length of output: 250
Hydration is keyed off socketPath, not the actual connection.
This effect runs on mount before connectionRef.current is populated, returns early, and never retries once the RPC connection is established. In the common startup path, favorites/recents stay empty instead of hydrating from ade-cli. Depend on the connection state instead of the static socketPath prop to trigger the effect when the connection becomes available.
Suggested fix
- useEffect(() => {
- const conn = connectionRef.current;
- if (!conn) return;
+ useEffect(() => {
+ if (!connection) return;
+ const conn = connection;
let cancelled = false;
void (async () => {
try {
@@
return () => {
cancelled = true;
};
- }, [socketPath]);
+ }, [connection]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/app.tsx` around lines 3337 - 3357, The effect that
hydrates model picker favorites/recents is currently keyed to socketPath and
returns early if connectionRef.current is not set, so it never retries when the
RPC connection arrives; update the useEffect dependency from [socketPath] to
depend on the actual connection state (e.g., [connection] or
connectionRef.current wrapped in state) so the effect re-runs when the
connection becomes available, leaving the internal early-return and cancel logic
intact and still calling getModelPickerFavorites/getModelPickerRecents and then
setModelPickerFavorites/setModelPickerRecents.
Comment on lines
+86
to
+91
| hydrateFromRemote: async () => { | ||
| const api = getRpcApi(); | ||
| if (!api) { | ||
| set({ hydrated: true }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Recents hydration has the same permanent no-retry path.
If RPC is unavailable on first load, hydrated is set to true and hydrationStarted blocks further attempts, so recents may never reconcile with the shared authoritative store.
Also applies to: 103-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/shared/ModelPicker/useModelRecents.ts`
around lines 86 - 91, The hydration path in useModelRecents (hydrateFromRemote)
incorrectly sets hydrated = true when getRpcApi() returns null, and leaves
hydrationStarted blocking future attempts; instead, when !api is detected do NOT
set hydrated to true and reset or clear hydrationStarted so the function can be
retried later (or schedule a retry). Update hydrateFromRemote and the related
hydrationStarted/hydrated state updates so that hydrationStarted is only
set/cleared around actual attempts, and hydrated is only set true after a
successful reconcile (reference hydrateFromRemote, hydrationStarted, hydrated,
and set).
Comment on lines
+434
to
+474
| return ( | ||
| <div ref={containerRef} className="relative min-w-0 shrink-0"> | ||
| <button | ||
| type="button" | ||
| disabled={disabled} | ||
| data-state={open ? "open" : "closed"} | ||
| onClick={() => { | ||
| if (!disabled) setOpen((current) => !current); | ||
| }} | ||
| className={cn( | ||
| "inline-flex min-w-0 max-w-[11rem] items-center gap-1.5 rounded-md border font-sans transition-colors duration-150", | ||
| compact ? "h-7 px-1.5 text-[10px]" : "h-8 px-2 text-[11px] sm:text-[12px]", | ||
| "border-white/[0.06] bg-white/[0.03] text-fg/80", | ||
| "hover:border-violet-400/20 hover:bg-violet-500/[0.06] hover:text-fg", | ||
| open && "border-violet-400/30 bg-violet-500/[0.08] text-fg", | ||
| disabled && "cursor-not-allowed opacity-60 hover:border-white/[0.06] hover:bg-white/[0.03]", | ||
| )} | ||
| aria-label={`${providerLabel} permission mode`} | ||
| aria-haspopup="menu" | ||
| aria-expanded={open} | ||
| title={`Permission mode: ${selected.label}`} | ||
| > | ||
| <span className={cn("h-1.5 w-1.5 shrink-0 rounded-full", permissionSafetyDotClass(selected))} /> | ||
| <span className="min-w-0 truncate font-medium leading-none">{selected.label}</span> | ||
| <CaretDown | ||
| size={compact ? 9 : 10} | ||
| weight="bold" | ||
| className={cn( | ||
| "shrink-0 text-muted-fg/60 transition-transform duration-150", | ||
| open && "rotate-180 text-fg/80", | ||
| )} | ||
| /> | ||
| </button> | ||
| {open ? ( | ||
| <motion.div | ||
| key="cli-permission-picker" | ||
| data-cli-permission-picker-panel="true" | ||
| className="fixed z-[82] overflow-y-auto rounded-xl border border-white/[0.10] bg-popover/95 p-1 shadow-2xl shadow-black/35 backdrop-blur-xl" | ||
| style={panelStyle ?? undefined} | ||
| initial={{ opacity: 0, y: 6, scale: 0.98 }} | ||
| animate={{ opacity: 1, y: 0, scale: 1 }} | ||
| exit={{ opacity: 0, y: 6, scale: 0.98 }} | ||
| transition={{ duration: 0.14 }} | ||
| <div | ||
| role="menu" | ||
| aria-label={`${providerLabel} permission mode`} | ||
| className={cn( | ||
| "absolute right-0 top-full z-[100] mt-1.5 w-[280px] overflow-y-auto rounded-xl border border-white/[0.08] p-1", | ||
| "bg-[#13111A]/95 shadow-[0_18px_48px_rgba(0,0,0,0.55)] backdrop-blur-md", | ||
| )} |
There was a problem hiding this comment.
Keep the permission menu out of the clipped Work surface.
This dropdown now renders inline with absolute top-full, but the continuation composer lives inside multiple overflow-hidden Work containers. In grid tiles and other short panes, the menu can be cut off or rendered off-screen, which blocks changing permission mode. The old portal avoided that failure mode.
Please keep this menu portaled, or add viewport collision/flip handling before shipping.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/terminals/WorkViewArea.tsx` around lines
434 - 474, The dropdown menu is rendered inline and gets clipped by parent
overflow; change the inline menu render (the JSX block that currently
conditionally renders the <div role="menu"...> when open) to render via a portal
instead (e.g., ReactDOM.createPortal or your app's Portal component) into
document.body and position it using the trigger/button's bounding rect (use
containerRef or the button ref to compute left/top) so the menu escapes parent
overflow; alternatively integrate a collision-aware positioning helper
(floating-ui) for flip/viewport logic — update the conditional render logic that
uses open to create a portaled menu and compute absolute screen coordinates
before rendering.
Comment on lines
+3641
to
+3644
| func cachedChatModelCatalog() -> AgentChatModelCatalog? { | ||
| guard let cached = chatModelCatalogCache.values.sorted(by: { $0.fetchedAt > $1.fetchedAt }).first else { return nil } | ||
| guard Date().timeIntervalSince(cached.fetchedAt) < chatModelsCacheTTL else { return nil } | ||
| return cached.catalog |
There was a problem hiding this comment.
Scope the fallback catalog to the current host/project.
This helper ignores the host/project scoping encoded into chatModelsCacheKey, so the fallback paths on Line 3666 and Line 3703 can reuse a catalog from the previous machine or project after reconnect/switch. That will show the wrong model availability/auth state until a live refresh succeeds.
🛠️ Suggested fix
`@MainActor`
func cachedChatModelCatalog() -> AgentChatModelCatalog? {
- guard let cached = chatModelCatalogCache.values.sorted(by: { $0.fetchedAt > $1.fetchedAt }).first else { return nil }
+ let hostKey = activeHostProfile?.hostIdentity
+ ?? activeHostProfile?.lastHostDeviceId
+ ?? activeHostProfile?.lastSuccessfulAddress
+ ?? currentAddress
+ ?? hostName
+ ?? "unpaired-host"
+ let projectKey = activeProjectRootPath ?? activeProjectId ?? "no-project"
+ let scopeSuffix = [hostKey, projectKey].joined(separator: "\u{1f}")
+ guard let cached = chatModelCatalogCache
+ .filter({ $0.key.hasSuffix(scopeSuffix) })
+ .map(\.value)
+ .max(by: { $0.fetchedAt < $1.fetchedAt }) else { return nil }
guard Date().timeIntervalSince(cached.fetchedAt) < chatModelsCacheTTL else { return nil }
return cached.catalog
}Also applies to: 3666-3667, 3703-3704
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ios/ADE/Services/SyncService.swift` around lines 3641 - 3644, The
cachedChatModelCatalog() helper currently picks the newest entry from
chatModelCatalogCache without respecting the per-host/project scoping encoded in
chatModelsCacheKey; update cachedChatModelCatalog() (and the fallback lookups
that use it) to first filter chatModelCatalogCache entries to only those whose
cache key matches the current chatModelsCacheKey scope (or otherwise contains
the current host/project identifier) before sorting by fetchedAt and checking
chatModelsCacheTTL; use the same scoping logic wherever cachedChatModelCatalog()
is used so a catalog from a different host/project isn’t returned (refer to
chatModelCatalogCache, chatModelsCacheKey, cached.fetchedAt and
chatModelsCacheTTL to locate and fix the code).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Pre-merge snapshot to protect uncommitted work before pulling origin/main:
- droidSdk{Pool,Worker,EventMapper,Protocol}.ts replace deleted droidAcpPool.ts
- New apps/desktop/.../shared/ModelPicker/ folder (favorites + recents + providers split)
- Removed ModelCatalogPanel.tsx and ProviderModelSelector.tsx (superseded)
- Wires through agentChatService, AgentChatPane, AgentChatComposer, multiple settings/launch controls
Will be replayed onto merged main; this commit may be squashed later.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… sync
Adds a unified favorites/recents/providers picker in the ADE CLI right pane,
triggered by /model or the model row in new-chat-setup. Authoritative storage
for favorites and recents moves from desktop localStorage into the ade-cli
process so desktop, TUI, and iOS all share one source of truth.
ade-cli
- services/modelPickerStore.ts: process-singleton JSON-backed store at
~/.ade/modelPicker.json, capped at 10 recents, debounced writes, tolerates
malformed files.
- adeRpcServer.ts: modelPicker.{getFavorites,setFavorites,toggleFavorite,
getRecents,pushRecent} JSON-RPC methods.
- tuiClient/components/ModelPicker/: Ink picker pane with mini-rail, search,
windowed list, keyboard handling (tab/shift-tab cycles rail, f favorites,
/ search, arrows move, enter selects, esc closes/clears).
- tuiClient/app.tsx: hydrates favorites/recents on connect, /model and
chat:modelPicker now open the right-pane picker, and Enter on the model
row in new-chat-setup opens the picker with surface=new-chat (escape and
commit return to the setup pane).
Desktop
- useModelFavorites / useModelRecents: optimistic local update + RPC commit
via window.ade.modelPicker.* (localStorage kept as offline cache so first
render stays instant).
- preload.ts + global.d.ts: window.ade.modelPicker namespace wired through
callProjectRuntimeSyncOr.
ModelStatus and the inline model row are intentionally untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- agentChatService.getSlashCommands now returns /clear plus filesystem-discovered Claude + Codex prompt commands for droid sessions, mirroring codex. Previously droid sessions fell through to the OpenCode/Cursor branch and returned an empty list.
- adeApi.ts droid default model now resolves from the model registry via getDefaultModelDescriptor("droid") instead of a hardcoded string.
Tests added for both lane-only and live-session droid slash command paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WorkModelPickerSheet rebuilt to match desktop ModelPicker layout: - Vertical rail (Favorites star, Recents clock, divider, per-provider entries) - Right content pane with section header, search, grouped model rows - Per-row favorite star + active-checkmark + reasoning pills - Optimistic favorites/recents store with rollback on RPC error - Liquid glass styling via ADEColor.glassBorder + recessedBackground SyncService.swift gains 5 RPC methods consuming the cross-surface favorites/recents contract: modelPicker.getFavorites / setFavorites / toggleFavorite / getRecents / pushRecent Chat session structs (AgentChatSession, AgentChatSessionSummary, AgentChatUpdateSessionRequest) now round-trip droidPermissionMode through createChatSession + updateChatSession, matching the desktop schema. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pushRecent debounces persistence by 250ms. If the user changes model and closes the TUI immediately, the write hasn't fired yet and the recent is lost. Wire flush() to a one-shot process.exit handler so the pending write lands before the process terminates. Only registered for the production singleton (filePath unset) — tests provide their own path and handle teardown explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace funnel icon with a visible, labeled toggle ('Show all models')
with a switch indicator. Old funnel button had zero affordance — users
didn't realize a toggle existed.
- When 'Show all models' is on, surface every provider family ADE
supports (anthropic, openai, factory/droid, cursor, opencode, ollama,
lmstudio) in the rail in a stable order — including dynamic-only
providers (opencode, lmstudio) that have no static registry entries.
Previously these families silently disappeared because providersPresent
was derived only from models present in MODEL_REGISTRY.
- Whitelist modelPicker.* methods in runtimeBridge's sync allowlist so
desktop's IPC bridge no longer rejects them as 'Local sync method is
not exposed'.
Known gaps (next PR): Cursor/OpenCode/Droid panes show 'no models match'
empty state because they have no registry entries. Claude row currently
greys out from useProviderAuthStatus mis-classifying anthropic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pull the reasoning effort UI out of the ModelPicker popover and into its own dropdown rendered alongside it in the prompt-box row. The model picker is now purely model selection; reasoning is a sibling control that only appears when the active model has reasoning tiers. - New `ReasoningEffortPicker.tsx` — popover trigger with current tier chip, side='top' (dropup), lists tiers from the descriptor, persists per family via useReasoningByFamily, returns null when no tiers. - Removed showReasoning / reasoningEffort / onReasoningEffortChange props from ModelPicker.tsx and the corresponding render from ModelPickerContent.tsx. Deleted dead ReasoningEffortControl.tsx. - Updated 10 callers across chat composer, chat pane, PR resolver, missions, CTO identity editor, review launch, AI settings, feedback modal, work view, parallel slots. Each renders <ModelPicker /> + <ReasoningEffortPicker /> side by side. - Mobile (apps/ios) and TUI (apps/ade-cli) intentionally untouched; the user only wanted the split on desktop. - Tests: new ReasoningEffortPicker.test.tsx (8 cases), updated ModelPicker.test.tsx + PrResolverLaunchControls.test.tsx for the new surface area. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…up CTA Three fixes for the empty-rail / greyed-out feedback: 1. Canonical model lists for dynamic-only providers (Cursor, Droid, OpenCode, LM Studio). MODEL_REGISTRY only had static entries (4 Anthropic + 6 OpenAI + 1 Ollama); the dynamic providers contributed nothing without runtime discovery, so their rail panes were empty. Now the picker surfaces the saved canonical model lists for those providers always — sourced from DROID_DEFAULT_MODEL_IDS in droidModelsDiscovery.ts and analogous defaults wired into modelRegistry.ts. Rows mark isAvailable=false when the provider isn't connected so they dim with a sign-in affordance. 2. Claude greyed-out auth status bug. useProviderAuthStatus was treating anthropic as "unauthed" whenever availableProviders.claude.runtimeAvailable wasn't a literal true — but the object also exposes auth.ready and binary.present, which can be true while runtimeAvailable is still transitional. Hook now accepts either as the auth signal so Claude models stay clickable when the user is actively chatting with them. 3. New providerEmptyState component with an inline "Set up X" CTA that opens Settings → Providers for any provider the user hasn't connected. Replaces the previous "no models match this view" empty state. Includes 28 new tests across useProviderAuthStatus, modelCatalog, and providerEmptyState. Existing 17 ModelPicker tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause: probeOpenCodeProviderInventory only ran when getStatus() was called with refreshOpenCodeInventory=true. AgentChatPane only forces that flag when sessionProvider === "opencode" — meaning the user has to ALREADY be in an OpenCode chat for the inventory to populate. Chicken-and-egg: model picker shows zero discovered OpenCode models, user can't pick one, user never enters an OpenCode chat, cache stays empty forever. Fix: on the default code path, if peekOpenCodeInventoryCache returns null AND the OpenCode binary is installed, fall through to a probe with force=false. The in-flight dedup map + 60s TTL still apply, so the OpenCode server boots at most once per project+config across the session. Warm reads are unchanged (peek wins). The existing test "does not cold-probe OpenCode inventory on default getStatus" had codified the broken behavior — replaced with one that asserts the cold path now probes once, plus a second test that verifies a populated peek still bypasses the probe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ode on binary
Per user direction, the preview-canonical approach is wrong. Replaced
with a clean separation:
- Anthropic + OpenAI: unchanged. Hardcoded MODEL_REGISTRY entries remain
because they encode first-party CLI/SDK routing metadata that the
vendors don't expose via a list-models API.
- Cursor + Droid: zero hardcoded preview. When the CLI is installed +
authed, dynamic discovery surfaces models. When not, the pane shows
ONLY a "Set up X" empty state — no preview rows.
- OpenCode + Ollama + LM Studio: same pattern, but all three are
gated on OpenCode binary availability. When the OpenCode binary is
missing, all three panes show the SAME "Install OpenCode" empty
state. When present, models flow through probeOpenCodeProviderInventory.
Changes:
- Deleted DROID_CANONICAL_MODEL_IDS, CURSOR_CANONICAL_MODEL_IDS,
OPENCODE_CANONICAL_PROVIDER_MODELS from modelRegistry.ts and the
DROID_DEFAULT_MODEL_IDS re-export in droidModelsDiscovery.ts.
- Removed buildDynamicCanonicalDescriptors + canonical-injection path
from modelCatalog.ts and ModelPickerContent.expandedModels.
- useProviderAuthStatus now tracks opencodeBinaryInstalled via a new
opencodeBinaryInstalledFromStatus mapper; hook returns
{ status, opencodeBinaryInstalled, loaded }.
- ProviderEmptyState gains a discriminated "opencode-required" mode
with family in opencode/ollama/lmstudio. Identical title + body
+ CTAs across all three families.
- ModelPickerContent.filterAvailable hides opencode/ollama/lmstudio
models entirely when binary missing so the pane falls through to
the opencode-required empty state. ProviderSetupBanner suppressed
for those families in that state (single unified empty message).
Tests updated:
- modelCatalog.test.ts: removed canonical-surfacing assertions, added
negative assertions for no-canonical-injection. Rebucket-to-opencode
and discovered-droid-model tests retained and tightened.
- providerEmptyState.test.tsx: new opencode-required describe block
(16 tests total).
- useProviderAuthStatus.test.ts: new describe for
opencodeBinaryInstalledFromStatus boolean coercion.
- ModelPicker.test.tsx: new describe for OpenCode binary gating
(asserts identical empty state copy across all three rails;
ProviderSetupBanner suppressed).
Total: 4 ModelPicker test files, 54 tests, all pass. tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
useProviderAuthStore initializes opencodeBinaryInstalled to false. On first picker open, fetchStatus() takes ~2s (OpenCode server boot + provider.list). During that window the picker rendered the 'Install OpenCode' empty state on the opencode/ollama/lmstudio panes — which is wrong for users who DO have OpenCode installed. Diagnostic confirmed the daemon returns opencodeBinaryInstalled=true with 72 availableModelIds in ~2s; the renderer just hadn't received that response yet. The bug was rendering an authoritative 'not installed' claim during the loading window. Fix: treat opencode as installed until internalAuth.loaded === true. Empty state only fires after a confirmed loaded+missing signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three fixes for the OpenCode-detection UX: 1. Real binary detection, no defaults. Previous fix defaulted opencodeBinaryInstalled to true while the slow getStatus() was in-flight — fast for users who have it, wrong claim for users who don't. Added a cheap dedicated IPC `ai.isOpenCodeInstalled` that only runs the synchronous binary lookup (resolveOpenCodeBinary) without the 2-second server probe. Hook now runs both calls in parallel on mount; binary state is known in ~ms while the full provider/model catalog continues to populate in the background. New `binaryProbed` state field gates the "Install OpenCode" empty state on a CONFIRMED answer, not a guess. 2. Local-provider models stay in their own rail. rebucketOpenCodeFamily no longer moves family=lmstudio or family=ollama into the opencode bucket. LM Studio models discovered via OpenCode now appear in the LM Studio rail (where users expect them), Ollama in the Ollama rail. The OpenCode rail is reserved for cloud-routed models (anthropic, openai, google, etc. via the OpenCode router). 3. Drop "via OpenCode" suffix from sub-provider labels. Rows shown inside the OpenCode rail are already implicitly "via OpenCode" — the underlying provider name alone is enough. "openai via OpenCode" → "Openai". (Title-casing matches existing rail-entry capitalization.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ments CI fix: - aiOrchestratorService.test.ts: bump health-sweep test deadline 180s→280s and vitest timeout 240s→300s; CI shard 8 hit the ceiling, logic is correct (passes in ~4s locally) Review fixes (CodeRabbit): - modelPickerStore: flush before resetSharedModelPickerStoreForTests - tuiClient/app.tsx: sticky model picker pane + catalog-aware provider resolution - modelPickerLayout: stale-provider fallback before deriving railIndex/pool - droidSdkEventMapper: kind-aware itemId fallback for dedupe (text/thinking) - droidSdkWorker: unique waiter IDs + per-run AbortController set - preload: extend not-callable fallback to sync calls (modelPicker.*) - AgentChatPane: thread runtime-catalog descriptor through draft provider paths - ModelPickerContent: handle clipboard rejection via .catch - useModelFavorites / useModelRecents: retry hydration after RPC failure - shared/modelCatalog: classify direct local models into ollama/lmstudio 8 comments deferred with documented reasons (P2 P-fix, iOS Swift, invasive refactor scope) — recorded in shipLane state.
arul28
force-pushed
the
ade/droid-updates-99a3f743
branch
from
0682bbc to
34115ac
Compare
Owner
Author
|
@copilot review but do not make fixes |
Contributor
Reviewed at 34115ac with no fixes applied. Current CI failure is |
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
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Release Notes
Greptile Summary
This PR migrates the Factory Droid runtime from the ACP (Agent Client Protocol) to the Factory Droid SDK, adds a full model picker UI (favorites, recents, per-surface defaults, provider rail), extends reasoning-tier and service-tier metadata to Cursor and Droid model discovery, and introduces Ollama and LM Studio as local provider options.
droidAcpPool.tsis deleted; replaced bydroidSdkPool.ts(ref-counted fork pool) +droidSdkWorker.ts(forked child process wrapping@factory/droid-sdk). Session persistence migrates fromacpSessionIdtodroidSdkSessionIdwith a backward-compat fallback reader.ModelPickercomponent with Zustand stores for favorites/recents, per-provider TTL catalog cache (runtimeCatalogCache.ts), and a runtime catalog that merges the staticMODEL_REGISTRYwith live IPC-fetched data.codexFastModeandreasoningEffortare now applied acrossopencode,cursor, anddroidproviders, not justcodex.Confidence Score: 5/5
Large but well-structured migration; no critical breakage found. The three findings are all non-blocking cleanup or future-proofing items.
The Droid ACP-to-SDK migration, model picker, and provider expansion are all implemented with appropriate guards, backward-compat session ID reading, and test reset hooks. No blocking logic errors were found in the changed paths.
The module-level
runtimeCatalogDescriptorsByIdinmodelCatalog.tsis worth a second look for local providers (Ollama/LM Studio) where the model list can shrink between refreshes.Important Files Changed
sanitizeEnvis already flagged in a prior thread.@factory/droid-sdk; handles permission and ask-user flows via bidirectional IPC with proper abort and cleanup.knownAuthFailureis computed but unused.runtimeCatalogDescriptorsByIdis never pruned across refreshes.arg as nevercast in the catalog handler bypasses type-level validation.Sequence Diagram
sequenceDiagram participant Renderer as Renderer (ModelPicker) participant IPC as IPC Bridge participant AgentChat as AgentChatService participant Pool as DroidSdkPool participant Worker as DroidSdkWorker (fork) participant SDK as @factory/droid-sdk Renderer->>IPC: "agentChatModelCatalog({mode, refreshProvider})" IPC->>AgentChat: getModelCatalog(args) AgentChat->>SDK: createSession (model discovery) SDK-->>AgentChat: initResult (availableModels) AgentChat-->>IPC: AgentChatModelCatalog IPC-->>Renderer: catalog (descriptors, tiers, availability) Renderer->>Renderer: rememberRuntimeCatalog / runtimeCatalogDescriptorsById note over Renderer,SDK: On chat start (ensureDroidRuntime) AgentChat->>Pool: acquireDroidSdkConnection(poolKey, settings) Pool->>Worker: fork(droidSdkWorker.cjs) Worker->>SDK: createSession(execPath, modelId, autonomyLevel) SDK-->>Worker: session ready + sessionId Worker-->>Pool: DroidSdkReady Pool-->>AgentChat: DroidSdkPooled note over AgentChat,SDK: During turn AgentChat->>Worker: sendPrompt(text, attachments) Worker->>SDK: session.prompt(...) SDK-->>Worker: sdk_event stream Worker-->>AgentChat: sdk_event (via IPC) AgentChat->>AgentChat: mapDroidSdkMessageToChatEvents(state, event) note over AgentChat,Worker: On permission request SDK-->>Worker: requestPermission callback Worker-->>AgentChat: permission_request (IPC) AgentChat-->>Renderer: tool approval UI Renderer-->>AgentChat: DroidSdkPermissionDecision AgentChat-->>Worker: permission_response (IPC) Worker-->>SDK: resolve permissionComments Outside Diff (1)
apps/desktop/src/main/services/chat/agentChatService.ts, line 757 (link)eventMapperStateis never reset between turnscreateDroidSdkEventMapperState()is called once whenDroidRuntimeis constructed and the same instance is passed tomapDroidSdkMessageToChatEventson every event for the lifetime of that runtime. TheassistantDeltaItemIdsandthinkingDeltaItemIdssets andtoolNamesByUseIdmap grow across all turns in the session. If the SDK reuses amessageIdor block index across turns (e.g., the samemessageId:text:0key appears in a later turn'screate_message), the deduplication guard will silently suppress valid text events in the new turn. At minimum, resetting the state at the start of each new turn prevents cross-turn ID collisions and bounds memory growth for long sessions.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: iter 1 — fix test-desktop(8) timeo..." | Re-trigger Greptile