sync: port upstream provider/orchestration foundations (#3740–#4168)#181
Conversation
…ls (pingdotgg#3740) (cherry picked from commit 3795dbb)
(cherry picked from commit 63b6b44)
(cherry picked from commit dfbda84)
(cherry picked from commit d7baa37)
…ingdotgg#3751) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 749baec)
(cherry picked from commit 3235658)
(cherry picked from commit df65a6c)
(cherry picked from commit 5fcfe24)
) Co-authored-by: Shoaib Ansari <shoaibansari@Shoaibs-Mac-mini.local> Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit d266c06)
…otgg#2093) Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit 33f1cb4)
Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: Julius Marminge <jmarminge@gmail.com> Co-authored-by: root <root@localhost.localdomain> (cherry picked from commit 40c0ab0)
…pingdotgg#3159) Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit 501ce27)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 08993a5)
Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 2fae0d0)
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit b6d9ce3)
…tarting an empty one (pingdotgg#3617) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit f4da4f3)
…rver (pingdotgg#4153) (cherry picked from commit 0ca3240)
…gdotgg#3869) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 946b867)
(cherry picked from commit a135f2c)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com> (cherry picked from commit c710167)
(cherry picked from commit fa69f05) Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
…ut provider kind Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
github-actions
Bot
added
the
vouch:trusted
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis pull request updates preview permissions, mobile Liquid Glass handling, provider runtimes, orchestration synchronization, web interactions, shared parsing, and protocol contracts. It adds completion markers, durable OpenCode sessions, Codex launch arguments, Cursor ACP probing, and several UI behavior changes. ChangesDesktop preview permissions
Mobile native glass and synchronization UI
Server provider and orchestration updates
Client synchronization
Web UI updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
size:XXL
Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdc38a7bb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| yield* Effect.forkScoped( | ||
| Stream.fromSubscription(liveSubscription).pipe( | ||
| Stream.mapEffect(toShellStreamEvent), | ||
| Stream.flatMap((event) => | ||
| Option.isSome(event) ? Stream.succeed(event.value) : Stream.empty, | ||
| ), | ||
| Stream.runForEach((item) => Queue.offer(liveBuffer, item)), | ||
| ), | ||
| liveStream.pipe(Stream.runForEach((item) => Queue.offer(liveBuffer, item))), |
There was a problem hiding this comment.
When an event is published after forkScoped schedules this consumer but before Stream.fromPubSub actually starts, the event has no subscriber and is lost; this can leave shell or thread projections permanently stale after snapshot/replay. OrchestrationEngineShape.streamDomainEvents explicitly documents this race and provides subscribeDomainEvents so the subscription can be acquired synchronously before forking, as the previous implementation did.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| const isThisThreadDetailEvent = (event: OrchestrationEvent) => | ||
| event.aggregateKind === "thread" && | ||
| event.aggregateId === input.threadId && | ||
| (isThreadDetailEvent(event) || | ||
| event.type === "thread.deleted" || | ||
| event.type === "thread.archived"); | ||
| isThreadDetailEvent(event); |
There was a problem hiding this comment.
When the subscribed thread is deleted or archived, this predicate now drops the lifecycle event because isThreadDetailEvent excludes both types. Consequently EnvironmentThreadState.applyItem never calls setDeleted or applies the archive timestamp, leaving its detail atom and persisted cache stale; this also invalidates the HTTP loader's explicit 404 fallback assumption that the socket subscription will surface deletion.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| const rawPreview = workEntryPreview(workEntry, workspaceRoot); | ||
| // When showCommandOutput is off, suppress command/detail previews but still show changed-file previews. | ||
| const gatedPreview = showCommandOutput | ||
| ? rawPreview | ||
| : !workEntry.command && !workEntry.detail | ||
| ? rawPreview | ||
| : null; | ||
| // Suppress previews that are effectively the same label as the heading. | ||
| const preview = | ||
| gatedPreview && | ||
| normalizeCompactToolLabel(gatedPreview).toLowerCase() === | ||
| rawPreview && | ||
| normalizeCompactToolLabel(rawPreview).toLowerCase() === | ||
| normalizeCompactToolLabel(heading).toLowerCase() | ||
| ? null | ||
| : gatedPreview; | ||
| : rawPreview; |
There was a problem hiding this comment.
For users with persisted showCommandOutput: false, command and detail previews are now always copied into displayText. The timeline no longer reads this setting anywhere—the only remaining repository reference is its schema default—so a preference that previously suppressed these collapsed previews has no effect.
Useful? React with 👍 / 👎.
| function compactMinimapPreview(text: string | null | undefined) { | ||
| const compact = | ||
| text?.slice(0, TIMELINE_MINIMAP_PREVIEW_MAX_CHARS).replace(/\s+/g, " ").trim() ?? ""; | ||
| const compact = text?.replace(/\s+/g, " ").trim() ?? ""; | ||
| return compact.length > 0 ? compact : null; |
There was a problem hiding this comment.
On threads containing long user prompts or assistant responses, this now normalizes and retains the complete message for every minimap item, even though the preview renders only a few lines. Large generated responses can therefore cause substantial CPU, memory, and DOM-text overhead whenever rows change; retain the prior pre-normalization character cap.
AGENTS.md reference: AGENTS.md:L56-L60
Useful? React with 👍 / 👎.
| const readCursorCliConfigChannel = Effect.fn("readCursorCliConfigChannel")(function* () { | ||
| const fileSystem = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; | ||
| const configPath = path.join(NodeOS.homedir(), ".cursor", "cli-config.json"); | ||
| const raw = yield* fileSystem.readFileString(configPath).pipe(Effect.orElseSucceed(() => "")); |
There was a problem hiding this comment.
When a custom Cursor instance overrides HOME or USERPROFILE, its CLI probe runs with the merged per-instance environment but this channel check still reads the server process user's home. A lab-channel instance can therefore be rejected based on an unrelated host cli-config.json (or a stable-channel instance can bypass the check); resolve the config path using the same environment passed to checkCursorProviderStatus.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| maintainScrollAtEnd={ | ||
| anchoredEndSpace || foldToggleSettling | ||
| anchoredEndSpace | ||
| ? false | ||
| : { |
There was a problem hiding this comment.
When the timeline is currently considered at the end and the user expands or collapses a turn, the row resize now occurs while maintainScrollAtEnd remains enabled, so LegendList follows the new end instead of preserving the disclosure the user clicked. The removed foldToggleSettling guard disabled end following during this layout transition; keep an equivalent guard, particularly for turn folds that lack the work-group anchor compensation.
Useful? React with 👍 / 👎.
| export function resolveClaudeApiModelId(modelSelection: ModelSelection): string { | ||
| const contextWindow = getModelSelectionStringOptionValue(modelSelection, "contextWindow"); | ||
| if (modelSelection.model === "claude-sonnet-5") { | ||
| return contextWindow === "1m" ? "sonnet[1m]" : modelSelection.model; | ||
| } | ||
|
|
||
| switch (contextWindow) { | ||
| switch (getModelSelectionStringOptionValue(modelSelection, "contextWindow")) { | ||
| case "1m": | ||
| return `${modelSelection.model}[1m]`; |
There was a problem hiding this comment.
When a gateway-backed Claude instance selects Sonnet 5 with the 1M context option, this generic branch now sends claude-sonnet-5[1m] to query.setModel. The gateway integration requires the special sonnet[1m] identifier, so starting or switching such a turn fails instead of activating the requested context window; retain the previous Sonnet-specific mapping.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/chat/MessagesTimeline.tsx (1)
702-752: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake a zero-width minimap strip unfocusable.
pointer-events-nonedoes not remove this button from tab order. Focusing it setsactiveItem, expands it to22rem, and permits keyboard selection even though a zero width is documented as disabled/inert.Proposed fix
<button aria-label={`Jump to message: ${activeItem?.userText ?? "User message"}`} + disabled={hitStripWidth <= 0} className={cn(🤖 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/web/src/components/chat/MessagesTimeline.tsx` around lines 702 - 752, Make the minimap button rendered by the visible timeline control unfocusable when hitStripWidth is zero by conditionally setting its tabIndex to -1, while preserving normal keyboard focus when the strip has usable width. Keep the existing pointer-event behavior and keyboard handlers unchanged.apps/web/src/hooks/useThreadActions.ts (1)
91-134: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve archive-success status in the single-thread path.
archiveThreadcallsopts.onArchivedafter a successful mutation but still returns the later navigation failure. OnlyarchiveSelectedThreadEntriestracksdidArchive;attemptArchiveThreadcallsarchiveThread(threadRef)directly and always shows the generic "Failed to archive thread" toast when navigation fails. HaveattemptArchiveThreaddetect this followup case usingonArchivedand show a softer message.Also ensure focused tests cover the new
archiveThreadbehavior (opts.onArchivedtracking and the unconditional refresh ordering).🤖 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/web/src/hooks/useThreadActions.ts` around lines 91 - 134, Update attemptArchiveThread to pass an onArchived callback to archiveThread and track whether archiving succeeded before any follow-up navigation failure; show the softer post-archive message when that callback ran instead of the generic failure toast. Preserve archiveThread’s unconditional refreshArchivedThreadsForEnvironment and callback ordering after mutation success. Add focused tests covering onArchived tracking and refresh occurring unconditionally after a successful archive.Source: Coding guidelines
🧹 Nitpick comments (11)
packages/client-runtime/src/state/shell.ts (1)
110-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
setReadyno longer sets a ready state.Post-change it only ever yields
"synchronizing", making it behaviorally identical tosetSynchronizingexcept for theliveshort-circuit. Consider renaming (e.g.setConnectedAwaitingSync) so the connection-phase mapping at Line 229 reads correctly.🤖 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 `@packages/client-runtime/src/state/shell.ts` around lines 110 - 118, Rename setReady to reflect that it transitions non-live state to "synchronizing" rather than a ready state, using a name such as setConnectedAwaitingSync. Update all references, including the connection-phase mapping around the relevant call site, while preserving the existing live short-circuit and state update behavior.apps/server/src/ws.ts (1)
1110-1118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the completion-marker concat into one helper.
The same four-line marker/buffer composition is repeated in all four subscription paths (shell resume, shell fallback, thread resume, thread fallback). A single helper keeps the ordering contract in one place.
As per coding guidelines, "extract shared logic instead of duplicating local implementations".
♻️ Suggested helper (define near the other stream helpers)
const withCompletionMarker = <A extends { readonly kind: string }, E>( liveBuffer: Queue.Queue<A>, requested: boolean, ): Stream.Stream<A, E> => { const buffered = Stream.fromQueue(liveBuffer); return requested ? Stream.concat( Stream.fromEffect(Queue.offer(liveBuffer, { kind: "synchronized" } as A)).pipe( Stream.drain, ), buffered, ) : buffered; };Then each site becomes:
- const afterCatchUp = - input.requestCompletionMarker === true - ? Stream.concat( - Stream.fromEffect( - Queue.offer(liveBuffer, { kind: "synchronized" as const }), - ).pipe(Stream.drain), - Stream.fromQueue(liveBuffer), - ) - : Stream.fromQueue(liveBuffer); + const afterCatchUp = withCompletionMarker( + liveBuffer, + input.requestCompletionMarker === true, + );Also applies to: 1145-1153, 1238-1246, 1269-1277
🤖 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/server/src/ws.ts` around lines 1110 - 1118, Extract the repeated completion-marker and live-buffer composition into a shared withCompletionMarker helper near the other stream helpers. Have it accept the liveBuffer and requested flag, preserve the marker-before-buffer ordering, and replace the duplicated logic in the shell resume/fallback and thread resume/fallback subscription paths.Source: Coding guidelines
apps/web/src/components/Sidebar.tsx (1)
1789-1791: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated "is thread running" predicate risks divergence.
thread.session?.status === "running" && thread.session.activeTurnId != nullmirrors the guard inuseThreadActions.ts'sarchiveThread(ThreadArchiveBlockedErrorcheck). If one side changes (e.g. a new session status is added), the UI-disabled state and the actual mutation guard can silently disagree.Consider extracting a shared
isThreadBlockingArchive(thread)/isThreadRunning(thread)helper used by both files.As per coding guidelines, "extract shared logic instead of duplicating local implementations."
🤖 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/web/src/components/Sidebar.tsx` around lines 1789 - 1791, Extract the duplicated running-thread predicate into a shared helper, such as isThreadBlockingArchive or isThreadRunning, and use it in both Sidebar’s hasRunningThread calculation and useThreadActions.ts archiveThread’s ThreadArchiveBlockedError guard. Preserve the existing condition and behavior while ensuring both UI and mutation checks rely on the same implementation.Source: Coding guidelines
apps/server/src/provider/opencodeRuntime.ts (2)
659-659: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit:
({} as {})cast is redundant.- const env = input.environment !== undefined ? { environment: input.environment } : ({} as {}); + const env = input.environment !== undefined ? { environment: input.environment } : {};🤖 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/server/src/provider/opencodeRuntime.ts` at line 659, Remove the redundant `as {}` cast from the fallback object in the environment assignment, leaving the existing conditional behavior unchanged.
232-273: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
JSON.parseresult is assigned toAgent["permission"]unchecked.
permissioncomes back asanyand is pushed straight into theAgentshape, so malformed-but-parseable CLI output (e.g. an object instead of a rule array) propagates into the inventory typed as valid. Consider a light shape guard (array check) before accepting the agent.🤖 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/server/src/provider/opencodeRuntime.ts` around lines 232 - 273, Update parseAgentListCliOutput’s flushAgent JSON parsing to validate that the parsed permission value is an array before adding the agent. Skip malformed-but-parseable values that are not arrays, while preserving the existing handling for valid permission arrays and unparseable JSON.apps/server/src/provider/Layers/OpenCodeAdapter.ts (1)
1243-1248: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: use an explicit
undefinedrecovery instead ofEffect.void.
Effect.voidas the not-found branch makesadopted's typeSession | void, so "no resume" reads as an accident rather than intent.- Effect.catchIf( - (cause) => isOpenCodeNotFound(cause), - () => Effect.void, - ), + Effect.catchIf( + (cause) => isOpenCodeNotFound(cause), + () => Effect.succeed(undefined), + ),🤖 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/server/src/provider/Layers/OpenCodeAdapter.ts` around lines 1243 - 1248, Update the not-found recovery branch in the adopted session flow using Effect.catchIf so it explicitly returns undefined instead of Effect.void. Preserve the isOpenCodeNotFound predicate and ensure adopted has the intended Session | undefined type.apps/server/src/provider/Layers/OpenCodeProvider.test.ts (1)
98-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe double never fails, so "degrades gracefully on CLI failure" only exercises the empty-inventory path.
loadInventoryFromClireturns a successful empty inventory wheninventoryErroris set, socheckOpenCodeProviderStatus'sinventoryExit._tag === "Failure"fallback branch stays uncovered. Add a case where the double actually fails (e.g. a separatecliInventoryErrorflag) to pin the error-status path too.Also applies to: 215-222
🤖 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/server/src/provider/Layers/OpenCodeProvider.test.ts` around lines 98 - 104, Update the test double for loadInventoryFromCli and the related runtimeMock state to support an actual failed Effect, using a distinct flag such as cliInventoryError. Add or adjust a test case that enables this flag and verifies checkOpenCodeProviderStatus handles inventoryExit._tag === "Failure" with the expected degraded error status, while preserving the existing empty-inventory scenario.apps/web/src/components/settings/ProviderModelsSection.tsx (1)
113-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueProp doc for
driverKindis now stale.Normalization no longer depends on
driverKind(only the placeholder does), so the JSDoc at Lines 43-47 ("Driver kind for slug normalization + input placeholder") should drop the normalization mention.🤖 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/web/src/components/settings/ProviderModelsSection.tsx` around lines 113 - 118, Update the JSDoc for the driverKind prop in ProviderModelsSection to describe its use only for the input placeholder, removing the outdated reference to slug normalization. Keep the prop behavior and normalization logic unchanged.apps/server/src/provider/Layers/CursorProvider.ts (2)
686-694: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toTitleCaseWordsduplicates the identical helper inClaudeProvider.ts.
apps/server/src/provider/Layers/ClaudeProvider.tsLines 686-694 define a byte-identical function. Extract it into a shared module (e.g.@t3tools/shared/modelor a provider-local util) and import in both places.As per coding guidelines: "extract shared logic instead of duplicating local implementations."
🤖 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/server/src/provider/Layers/CursorProvider.ts` around lines 686 - 694, Extract the duplicated toTitleCaseWords helper into a shared module, then remove the local implementations from CursorProvider and ClaudeProvider and import the shared function in both providers. Preserve its existing whitespace, underscore, hyphen splitting, casing, and joining behavior.Source: Coding guidelines
279-304: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCategory matching is inconsistent with
getCursorConfigOptionCategory.
findCursorEffortConfigOptioncompares categories via the trimming/lowercasing helper, while the context/fast/thinking lookups use rawoption.category === "model_config". Any casing or whitespace variation from the ACP payload would silently drop these descriptors. Consider reusing the normalizer here.♻️ Suggested normalization
- const contextOption = configOptions.find( - (option) => option.category === "model_config" && isCursorContextConfigOption(option), - ); + const contextOption = configOptions.find( + (option) => + getCursorConfigOptionCategory(option) === "model_config" && + isCursorContextConfigOption(option), + );🤖 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/server/src/provider/Layers/CursorProvider.ts` around lines 279 - 304, Update the context, fast, and thinking configuration lookups in the surrounding Cursor provider logic to compare option categories using the same trimming/lowercasing normalization as getCursorConfigOptionCategory and findCursorEffortConfigOption. Preserve the existing option predicates and descriptor construction while ensuring category casing or surrounding whitespace does not exclude valid ACP options.packages/shared/src/model.test.ts (1)
150-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore test coverage for the remaining exported model helpers.
resolveSelectableModel,resolveModelSlugForProvider,isClaudeUltrathinkPrompt,applyClaudePromptEffortPrefix, andtrimOrNullare still exported frompackages/shared/src/model.ts, but they no longer have focused tests here. Add back the covered cases for those helpers; while here, addnull/whitespace-only cases fornormalizeCustomModelSlugto cover its documented return contract.🤖 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 `@packages/shared/src/model.test.ts` around lines 150 - 157, Extend the “model slug normalization” tests in model.test.ts with focused coverage for the exported helpers resolveSelectableModel, resolveModelSlugForProvider, isClaudeUltrathinkPrompt, applyClaudePromptEffortPrefix, and trimOrNull, covering their documented behaviors. Also add normalizeCustomModelSlug assertions for null and whitespace-only input, verifying the documented return contract while preserving the existing exact-slug case.
🤖 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/mobile/src/features/home/HomeScreen.tsx`:
- Line 392: Update the HomeScreen safe-area handling around the paddingTop
calculation and HomeTopContentSpacer at the referenced inset/empty-state paths
so iOS devices without native Liquid Glass retain the existing top inset beneath
the solid header. Preserve the Liquid Glass behavior while adding an equivalent
fallback spacer or content padding for unsupported iOS devices, including the
empty-state layout.
In `@apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts`:
- Around line 736-737: Update rememberTaskDescription to cap description length
before Cache.set, using the task-description schema’s defined maximum if
available or a clearly specified truncation limit. Preserve the existing task
key and cache behavior while ensuring long provider descriptions cannot retain
disproportionate memory.
In `@apps/server/src/provider/Layers/ClaudeAdapter.ts`:
- Around line 1361-1364: Update makeClaudeAdapter and its Claude driver
construction path to provide the services required by
resolveClaudeSdkExecutablePath: HostProcessPlatform, SpawnExecutableResolution,
and ClaudeExecutableFileCheck. Prefer exporting a small runtime layer from the
Claude adapter and merge it into the layer used by ClaudeDriver.create(), or
explicitly extend ClaudeDriverEnv and supply these services at the production
construction site.
In `@apps/server/src/provider/opencodeRuntime.ts`:
- Around line 657-691: Bound both inventory command effects created by
runModelsCli and runAgentsCli with the existing
DEFAULT_OPENCODE_SERVER_TIMEOUT_MS, ensuring timed-out attempts produce failures
handled by the current retry and graceful-degradation flow. Apply the timeout to
each attempt, including retries, without changing the parallel execution or
retry conditions.
In `@apps/web/src/components/ComposerPromptEditor.tsx`:
- Around line 1237-1241: Update the cleanup returned by the effect in
ComposerPromptEditor to remove data-composer-chip-selected from all currently
selected DOM nodes before or alongside unregistering the update, focus, and blur
listeners, ensuring remounts and Strict Mode replays cannot retain stale
highlights.
In `@apps/web/src/components/preview/PreviewView.tsx`:
- Around line 116-153: Update handleSubmitUrl so normalizePreviewUrl runs
outside the try/catch that handles navigateToResolvedUrl failures. Catch
normalization errors separately and surface them through the existing
toast/error-notification mechanism, while preserving the current silent handling
for server-side navigation failures.
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 1783-1788: Derive the displayed and confirmation count from
selectedThreadEntries.length rather than the raw threadKeys.length. Update the
count used by the archive, delete, and mark-unread labels/dialogs after the
parseScopedThreadKey/readThreadShell filtering in the surrounding selection
flow, while keeping the action loops based on selectedThreadEntries.
---
Outside diff comments:
In `@apps/web/src/components/chat/MessagesTimeline.tsx`:
- Around line 702-752: Make the minimap button rendered by the visible timeline
control unfocusable when hitStripWidth is zero by conditionally setting its
tabIndex to -1, while preserving normal keyboard focus when the strip has usable
width. Keep the existing pointer-event behavior and keyboard handlers unchanged.
In `@apps/web/src/hooks/useThreadActions.ts`:
- Around line 91-134: Update attemptArchiveThread to pass an onArchived callback
to archiveThread and track whether archiving succeeded before any follow-up
navigation failure; show the softer post-archive message when that callback ran
instead of the generic failure toast. Preserve archiveThread’s unconditional
refreshArchivedThreadsForEnvironment and callback ordering after mutation
success. Add focused tests covering onArchived tracking and refresh occurring
unconditionally after a successful archive.
---
Nitpick comments:
In `@apps/server/src/provider/Layers/CursorProvider.ts`:
- Around line 686-694: Extract the duplicated toTitleCaseWords helper into a
shared module, then remove the local implementations from CursorProvider and
ClaudeProvider and import the shared function in both providers. Preserve its
existing whitespace, underscore, hyphen splitting, casing, and joining behavior.
- Around line 279-304: Update the context, fast, and thinking configuration
lookups in the surrounding Cursor provider logic to compare option categories
using the same trimming/lowercasing normalization as
getCursorConfigOptionCategory and findCursorEffortConfigOption. Preserve the
existing option predicates and descriptor construction while ensuring category
casing or surrounding whitespace does not exclude valid ACP options.
In `@apps/server/src/provider/Layers/OpenCodeAdapter.ts`:
- Around line 1243-1248: Update the not-found recovery branch in the adopted
session flow using Effect.catchIf so it explicitly returns undefined instead of
Effect.void. Preserve the isOpenCodeNotFound predicate and ensure adopted has
the intended Session | undefined type.
In `@apps/server/src/provider/Layers/OpenCodeProvider.test.ts`:
- Around line 98-104: Update the test double for loadInventoryFromCli and the
related runtimeMock state to support an actual failed Effect, using a distinct
flag such as cliInventoryError. Add or adjust a test case that enables this flag
and verifies checkOpenCodeProviderStatus handles inventoryExit._tag ===
"Failure" with the expected degraded error status, while preserving the existing
empty-inventory scenario.
In `@apps/server/src/provider/opencodeRuntime.ts`:
- Line 659: Remove the redundant `as {}` cast from the fallback object in the
environment assignment, leaving the existing conditional behavior unchanged.
- Around line 232-273: Update parseAgentListCliOutput’s flushAgent JSON parsing
to validate that the parsed permission value is an array before adding the
agent. Skip malformed-but-parseable values that are not arrays, while preserving
the existing handling for valid permission arrays and unparseable JSON.
In `@apps/server/src/ws.ts`:
- Around line 1110-1118: Extract the repeated completion-marker and live-buffer
composition into a shared withCompletionMarker helper near the other stream
helpers. Have it accept the liveBuffer and requested flag, preserve the
marker-before-buffer ordering, and replace the duplicated logic in the shell
resume/fallback and thread resume/fallback subscription paths.
In `@apps/web/src/components/settings/ProviderModelsSection.tsx`:
- Around line 113-118: Update the JSDoc for the driverKind prop in
ProviderModelsSection to describe its use only for the input placeholder,
removing the outdated reference to slug normalization. Keep the prop behavior
and normalization logic unchanged.
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 1789-1791: Extract the duplicated running-thread predicate into a
shared helper, such as isThreadBlockingArchive or isThreadRunning, and use it in
both Sidebar’s hasRunningThread calculation and useThreadActions.ts
archiveThread’s ThreadArchiveBlockedError guard. Preserve the existing condition
and behavior while ensuring both UI and mutation checks rely on the same
implementation.
In `@packages/client-runtime/src/state/shell.ts`:
- Around line 110-118: Rename setReady to reflect that it transitions non-live
state to "synchronizing" rather than a ready state, using a name such as
setConnectedAwaitingSync. Update all references, including the connection-phase
mapping around the relevant call site, while preserving the existing live
short-circuit and state update behavior.
In `@packages/shared/src/model.test.ts`:
- Around line 150-157: Extend the “model slug normalization” tests in
model.test.ts with focused coverage for the exported helpers
resolveSelectableModel, resolveModelSlugForProvider, isClaudeUltrathinkPrompt,
applyClaudePromptEffortPrefix, and trimOrNull, covering their documented
behaviors. Also add normalizeCustomModelSlug assertions for null and
whitespace-only input, verifying the documented return contract while preserving
the existing exact-slug case.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3877e85-4f3b-49bf-b7ed-32a64810de14
⛔ Files ignored due to path filters (3)
packages/effect-codex-app-server/src/_generated/meta.gen.tsis excluded by!**/_generated/**packages/effect-codex-app-server/src/_generated/namespaces.gen.tsis excluded by!**/_generated/**packages/effect-codex-app-server/src/_generated/schema.gen.tsis excluded by!**/_generated/**
📒 Files selected for processing (94)
apps/desktop/src/preview/BrowserSession.test.tsapps/desktop/src/preview/BrowserSession.tsapps/mobile/src/Stack.tsxapps/mobile/src/components/CompactBrandTitle.tsxapps/mobile/src/features/files/FileTreeBrowser.tsxapps/mobile/src/features/home/HomeRouteScreen.tsxapps/mobile/src/features/home/HomeScreen.tsxapps/mobile/src/features/home/WorkspaceConnectionStatus.test.tsapps/mobile/src/features/home/WorkspaceConnectionStatus.tsxapps/mobile/src/features/home/workspace-connection-status.tsapps/mobile/src/features/threads/ThreadDetailScreen.tsxapps/mobile/src/features/threads/ThreadNavigationSidebar.tsxapps/mobile/src/features/threads/ThreadRouteScreen.tsxapps/mobile/src/features/threads/sidebar-navigation-shell.tsxapps/mobile/src/lib/native-glass-capability.test.tsapps/mobile/src/lib/native-glass-capability.tsapps/mobile/src/native/native-glass.tsapps/server/src/bin.test.tsapps/server/src/cli/project.tsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.approval.test.tsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.tsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsapps/server/src/provider/Drivers/ClaudeExecutable.test.tsapps/server/src/provider/Drivers/ClaudeExecutable.tsapps/server/src/provider/Layers/AmpProvider.tsapps/server/src/provider/Layers/ClaudeAdapter.tsapps/server/src/provider/Layers/ClaudeProvider.tsapps/server/src/provider/Layers/CodexAdapter.test.tsapps/server/src/provider/Layers/CodexAdapter.tsapps/server/src/provider/Layers/CodexProvider.tsapps/server/src/provider/Layers/CodexSessionRuntime.test.tsapps/server/src/provider/Layers/CodexSessionRuntime.tsapps/server/src/provider/Layers/CopilotProvider.tsapps/server/src/provider/Layers/CursorProvider.tsapps/server/src/provider/Layers/DroidProvider.tsapps/server/src/provider/Layers/GeminiCliProvider.tsapps/server/src/provider/Layers/GrokProvider.tsapps/server/src/provider/Layers/KiloProvider.tsapps/server/src/provider/Layers/OpenCodeAdapter.test.tsapps/server/src/provider/Layers/OpenCodeAdapter.tsapps/server/src/provider/Layers/OpenCodeProvider.test.tsapps/server/src/provider/Layers/OpenCodeProvider.tsapps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.tsapps/server/src/provider/Layers/ProviderRegistry.test.tsapps/server/src/provider/Layers/codexLaunchArgs.test.tsapps/server/src/provider/Layers/codexLaunchArgs.tsapps/server/src/provider/cursor/CursorSdkMappings.tsapps/server/src/provider/opencodeRuntime.cliParsers.test.tsapps/server/src/provider/opencodeRuntime.tsapps/server/src/provider/providerSnapshot.test.tsapps/server/src/provider/providerSnapshot.tsapps/server/src/server.test.tsapps/server/src/serverSettings.test.tsapps/server/src/textGeneration/CodexTextGeneration.test.tsapps/server/src/textGeneration/CodexTextGeneration.tsapps/server/src/textGeneration/OpenCodeTextGeneration.test.tsapps/server/src/ws.tsapps/web/src/components/AppSidebarLayout.tsxapps/web/src/components/ChatView.tsxapps/web/src/components/ComposerPromptEditor.tsxapps/web/src/components/Sidebar.logic.test.tsapps/web/src/components/Sidebar.logic.tsapps/web/src/components/Sidebar.tsxapps/web/src/components/chat/ComposerPendingApprovalPanel.test.tsxapps/web/src/components/chat/ComposerPendingApprovalPanel.tsxapps/web/src/components/chat/MessagesTimeline.logic.tsapps/web/src/components/chat/MessagesTimeline.test.tsxapps/web/src/components/chat/MessagesTimeline.tsxapps/web/src/components/preview/PreviewView.test.tsxapps/web/src/components/preview/PreviewView.tsxapps/web/src/components/settings/KeybindingsSettings.tsxapps/web/src/components/settings/ProviderModelsSection.tsxapps/web/src/components/settings/ProviderSettingsForm.test.tsapps/web/src/hooks/useThreadActions.tsapps/web/src/index.cssapps/web/src/modelSelection.test.tsapps/web/src/modelSelection.tsapps/web/src/projectScripts.test.tsapps/web/src/projectScripts.tspackages/client-runtime/src/rpc/client.tspackages/client-runtime/src/state/shell-sync.test.tspackages/client-runtime/src/state/shell.tspackages/client-runtime/src/state/threads-sync.test.tspackages/client-runtime/src/state/threads.tspackages/contracts/src/orchestration.tspackages/contracts/src/server.tspackages/contracts/src/settings.test.tspackages/contracts/src/settings.tspackages/effect-codex-app-server/scripts/generate.tspackages/effect-codex-app-server/src/protocol.test.tspackages/shared/src/cliArgs.test.tspackages/shared/src/cliArgs.tspackages/shared/src/model.test.tspackages/shared/src/model.ts
💤 Files with no reviewable changes (7)
- apps/mobile/src/components/CompactBrandTitle.tsx
- apps/server/src/provider/Layers/AmpProvider.ts
- apps/server/src/provider/Layers/GeminiCliProvider.ts
- apps/server/src/provider/Layers/DroidProvider.ts
- apps/server/src/provider/cursor/CursorSdkMappings.ts
- apps/server/src/provider/Layers/CopilotProvider.ts
- apps/server/src/provider/Layers/KiloProvider.ts
| style={{ | ||
| paddingBottom: Math.max(insets.bottom, 24), | ||
| paddingTop: Platform.OS === "ios" ? insets.top + 72 : 0, | ||
| paddingTop: NATIVE_LIQUID_GLASS_SUPPORTED ? insets.top + 72 : 0, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve safe-area insets on iOS without Liquid Glass.
When NATIVE_LIQUID_GLASS_SUPPORTED is false on an iOS device, Line 425 still suppresses HomeTopContentSpacer, but these changes also disable automatic content inset adjustment and remove empty-state top padding. Older or unsupported iOS devices can therefore render content beneath the native header. Keep the existing iOS inset behavior for the solid-header fallback, or add an equivalent fallback spacer/inset path.
Also applies to: 473-474
🤖 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/mobile/src/features/home/HomeScreen.tsx` at line 392, Update the
HomeScreen safe-area handling around the paddingTop calculation and
HomeTopContentSpacer at the referenced inset/empty-state paths so iOS devices
without native Liquid Glass retain the existing top inset beneath the solid
header. Preserve the Liquid Glass behavior while adding an equivalent fallback
spacer or content padding for unsupported iOS devices, including the empty-state
layout.
| const rememberTaskDescription = (threadId: ThreadId, taskId: string, description: string) => | ||
| Cache.set(taskDescriptionByTaskKey, providerTaskKey(threadId, taskId), description); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'task\.started|task\.progress|description' packages/contracts/src/providerRuntime.ts
rg -n -C 3 'taskDescriptionByTaskKey|rememberTaskDescription' apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsRepository: aaditagrawal/t3code
Length of output: 3913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ProviderRuntimeIngestion relevant sizes/config/cache =="
sed -n '700,760p' apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts
sed -n '1720,1770p' apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts
rg -n "TASK_DESCRIPTION_BY_TASK_CACHE_CAPACITY|TASK_DESCRIPTION_BY_TASK_TTL|Cache.make|make<" apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts packages apps/server/src 2>/dev/null | rg "TASK_DESCRIPTION_BY_TASK|Cache.make|make<" || true
echo "== description schema definitions and length constraints =="
sed -n '420,505p' packages/contracts/src/providerRuntime.ts
rg -n "TrimmedNonEmptyStringSchema|NonEmptyStringSchema|StringSchema|length|maxLength|maxLengthBytes" packages/contracts/src apps/server/src -g "*.ts"Repository: aaditagrawal/t3code
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TrimmedNonEmptyString definition =="
rg -n -C 4 "const TrimmedNonEmptyString|TrimmedNonEmptyStringSchema|type TrimmedNonEmptyString|NonEmptyString" packages/contracts/src/providerRuntime.ts packages/contracts/src -g '*.ts' | head -120
echo "== focused length cap occurrences =="
rg -n --max-count 100 "NonEmptyStringSchema|StringSchema|trimmed.*length|maxLength|length:" packages/contracts/src/providerRuntime.ts packages/contracts/src packages -g '*.ts' | head -200Repository: aaditagrawal/t3code
Length of output: 28234
Cap cached task-description payloads before storing them.
TaskStartedPayload.description uses TrimmedNonEmptyStringSchema, while TaskProgressPayload.description is required with the same schema. The cache lets 10,000 task entries stay in memory for 120 minutes, so enforce a specific maximum on the task-description schema or truncate before Cache.set to avoid disproportionate memory retention from long provider descriptions.
🤖 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/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts` around
lines 736 - 737, Update rememberTaskDescription to cap description length before
Cache.set, using the task-description schema’s defined maximum if available or a
clearly specified truncation limit. Preserve the existing task key and cache
behavior while ensuring long provider descriptions cannot retain
disproportionate memory.
Source: Coding guidelines
| const claudeSdkExecutablePath = yield* resolveClaudeSdkExecutablePath( | ||
| claudeSettings.binaryPath, | ||
| claudeEnvironment, | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C4 '\bmakeClaudeAdapter\s*\(' --type=ts
rg -nP -C3 '(HostProcessPlatform|SpawnExecutableResolution|ClaudeExecutableFileCheck)' --type=ts -g '!**/*.test.ts'Repository: aaditagrawal/t3code
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -u
echo "Files named ClaudeAdapter.ts:"
fd -a 'ClaudeAdapter\.ts$' . | sed 's#^\./##'
echo
echo "Git tracked TS files containing makeClaudeAdapter:"
git ls-files '*.ts' '*.tsx' | xargs rg -n -C4 '\bmakeClaudeAdapter\s*\(' || true
echo
echo "Git tracked TS files containing makeClaudeAdapter (no context):"
git ls-files '*.ts' '*.tsx' | xargs rg -n '\bmakeClaudeAdapter\s*\(' || true
echo
echo "Target file outline/size:"
if [ -f apps/server/src/provider/Layers/ClaudeAdapter.ts ]; then
wc -l apps/server/src/provider/Layers/ClaudeAdapter.ts
ast-grep outline apps/server/src/provider/Layers/ClaudeAdapter.ts --match makeClaudeAdapter --view expanded || true
sed -n '1320,1385p' apps/server/src/provider/Layers/ClaudeAdapter.ts
fi
echo
echo "Relevant ClaudeExecutable imports/types/const:"
if [ -f apps/server/src/provider/Drivers/ClaudeExecutable.ts ]; then
wc -l apps/server/src/provider/Drivers/ClaudeExecutable.ts
ast-grep outline apps/server/src/provider/Drivers/ClaudeExecutable.ts --view expanded || true
rg -n -C3 'HostProcessPlatform|SpawnExecutableResolution|ClaudeExecutableFileCheck|resolveClaudeSdkExecutablePath|export const ClaudeExecutableFileCheck|interface|class' apps/server/src/provider/Drivers/ClaudeExecutable.ts
sed -n '1,120p' apps/server/src/provider/Drivers/ClaudeExecutable.ts
fi
echo
echo "All non-test TS matches for required tokens:"
git ls-files '*.ts' '*.tsx' | xargs rg -n -C3 'HostProcessPlatform|SpawnExecutableResolution|ClaudeExecutableFileCheck' || trueRepository: aaditagrawal/t3code
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "ClaudeAdapter.ts relevant definitions/call sites:"
rg -n -C5 'export const ClaudeAdapter|ClaudeAdapter\(\)|makeClaudeAdapter|ClaudeExecutableFileCheck|SpawnExecutableResolution|HostProcessPlatform' apps/server/src/provider/Layers/ClaudeAdapter.ts
echo
echo "ClaudDriver.ts relevant context:"
sed -n '120,170p' apps/server/src/provider/Drivers/ClaudeDriver.ts
rg -n -C4 'ClaudeDriver|ClaudeAdapter|ClaudeExecutableFileCheck|SpawnExecutableResolution|HostProcessPlatform' apps/server/src/provider/Drivers/ClaudeDriver.ts
echo
echo "Other non-test Claude-related references in provider area (concise):"
rg -n -C2 'ClaudeExecutableFileCheck|SpawnExecutableResolution|resolveClaudeSdkExecutablePath|makeClaudeAdapter' apps/server/src/provider -g '!**/*.test.ts'
echo
echo "All non-test TS references to SpawnExecutableResolution in server/provider/apps:"
rg -n '(SpawnExecutableResolution|ClaudeExecutableFileCheck)' apps/server/src/provider apps/server/src -g '!**/*.test.ts'Repository: aaditagrawal/t3code
Length of output: 12244
🏁 Script executed:
#!/bin/bash
set -u
echo "ClaudeProvider.ts imports and ClaudeProvider layer/provider block:"
sed -n '1,90p' apps/server/src/provider/Layers/ClaudeProvider.ts
sed -n '560,680p' apps/server/src/provider/Layers/ClaudeProvider.ts
echo
echo "All non-test TS lines mentioning ClaudeProvider:"
rg -n -C3 'ClaudeProvider|makeClaudeProvider|new ClaudeProvider|ProviderDriver|ProviderDriver<ClaudeSettings' apps/server/src/provider apps/server/src -g '!**/*.test.ts'
echo
echo "Exact resolveClaudeSdkExecutablePath call sites in app/source TS:"
rg -n -C4 'resolveClaudeSdkExecutablePath' apps/server/src --glob '*.ts' --glob '!**/*.test.ts'Repository: aaditagrawal/t3code
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "ClaudeProvider.ts imports and ClaudeProvider driver exports:"
sed -n '1,95p' apps/server/src/provider/Layers/ClaudeProvider.ts
sed -n '600,640p' apps/server/src/provider/Layers/ClaudeProvider.ts
rg -n 'export const ClaudeProvider|ClaudeProviderLive|makeClaudeProvider|ClaudeDriver|Driver kind' apps/server/src/provider/Layers/ClaudeProvider.ts
echo
echo "Registry/driver construction around ClaudeDriver:"
sed -n '100,180p' apps/server/src/provider/Drivers/ClaudeDriver.ts
rg -n -C2 'ClaudeDriver|drivers:|new ClaudeProvider|ProviderRegistry|ProviderInstanceRegistry' apps/server/src/provider -g '*.ts' --glob '!**/*.test.ts'Repository: aaditagrawal/t3code
Length of output: 50375
Provide the Claude SDK resolver services in the Claude driver layer.
makeClaudeAdapter() is called from ClaudeDriver.create(), but that driver only declares ClaudeDriverEnv and ClaudeDriverEnv does not include HostProcessPlatform, SpawnExecutableResolution, or ClaudeExecutableFileCheck (apps/server/src/provider/Drivers/ClaudeDriver.ts:84-92). The one production construction site at apps/server/src/provider/Drivers/ClaudeDriver.ts:149 will not satisfy the new services required by resolveClaudeSdkExecutablePath; export a small runtime layer or add these services to the driver’s context.
🤖 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/server/src/provider/Layers/ClaudeAdapter.ts` around lines 1361 - 1364,
Update makeClaudeAdapter and its Claude driver construction path to provide the
services required by resolveClaudeSdkExecutablePath: HostProcessPlatform,
SpawnExecutableResolution, and ClaudeExecutableFileCheck. Prefer exporting a
small runtime layer from the Claude adapter and merge it into the layer used by
ClaudeDriver.create(), or explicitly extend ClaudeDriverEnv and supply these
services at the production construction site.
| const loadInventoryFromCli: OpenCodeRuntimeShape["loadInventoryFromCli"] = (input) => | ||
| Effect.gen(function* () { | ||
| const env = input.environment !== undefined ? { environment: input.environment } : ({} as {}); | ||
|
|
||
| const runModelsCli = () => | ||
| runOpenCodeCommand({ | ||
| binaryPath: input.binaryPath, | ||
| args: ["models", "--verbose"], | ||
| ...env, | ||
| }).pipe(Effect.exit); | ||
| const runAgentsCli = () => | ||
| runOpenCodeCommand({ binaryPath: input.binaryPath, args: ["agent", "list"], ...env }).pipe( | ||
| Effect.exit, | ||
| ); | ||
|
|
||
| // First attempt — run both in parallel | ||
| let [modelsResult, agentsResult] = yield* Effect.all([runModelsCli(), runAgentsCli()], { | ||
| concurrency: "unbounded", | ||
| }); | ||
|
|
||
| // Retry once after 1s on transient failures (e.g. SQLite "database is locked") | ||
| const needsModelsRetry = modelsResult._tag === "Failure" || modelsResult.value.code !== 0; | ||
| const needsAgentsRetry = agentsResult._tag === "Failure" || agentsResult.value.code !== 0; | ||
| if (needsModelsRetry || needsAgentsRetry) { | ||
| yield* Effect.sleep("1 second"); | ||
| const [m2, a2] = yield* Effect.all( | ||
| [ | ||
| needsModelsRetry ? runModelsCli() : Effect.succeed(modelsResult), | ||
| needsAgentsRetry ? runAgentsCli() : Effect.succeed(agentsResult), | ||
| ], | ||
| { concurrency: "unbounded" }, | ||
| ); | ||
| modelsResult = m2; | ||
| agentsResult = a2; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the CLI inventory commands with a timeout.
runOpenCodeCommand waits on child.exitCode with no deadline, so a wedged opencode models --verbose (or agent list) blocks the provider health check indefinitely — and the retry path can double that. The previous external-server flow at least had DEFAULT_OPENCODE_SERVER_TIMEOUT_MS. Add a timeout so the graceful-degradation branch below is actually reachable.
🛡️ Suggested fix
const runModelsCli = () =>
runOpenCodeCommand({
binaryPath: input.binaryPath,
args: ["models", "--verbose"],
...env,
- }).pipe(Effect.exit);
+ }).pipe(Effect.timeout(OPENCODE_CLI_INVENTORY_TIMEOUT), Effect.exit);
const runAgentsCli = () =>
runOpenCodeCommand({ binaryPath: input.binaryPath, args: ["agent", "list"], ...env }).pipe(
- Effect.exit,
+ Effect.timeout(OPENCODE_CLI_INVENTORY_TIMEOUT),
+ Effect.exit,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const loadInventoryFromCli: OpenCodeRuntimeShape["loadInventoryFromCli"] = (input) => | |
| Effect.gen(function* () { | |
| const env = input.environment !== undefined ? { environment: input.environment } : ({} as {}); | |
| const runModelsCli = () => | |
| runOpenCodeCommand({ | |
| binaryPath: input.binaryPath, | |
| args: ["models", "--verbose"], | |
| ...env, | |
| }).pipe(Effect.exit); | |
| const runAgentsCli = () => | |
| runOpenCodeCommand({ binaryPath: input.binaryPath, args: ["agent", "list"], ...env }).pipe( | |
| Effect.exit, | |
| ); | |
| // First attempt — run both in parallel | |
| let [modelsResult, agentsResult] = yield* Effect.all([runModelsCli(), runAgentsCli()], { | |
| concurrency: "unbounded", | |
| }); | |
| // Retry once after 1s on transient failures (e.g. SQLite "database is locked") | |
| const needsModelsRetry = modelsResult._tag === "Failure" || modelsResult.value.code !== 0; | |
| const needsAgentsRetry = agentsResult._tag === "Failure" || agentsResult.value.code !== 0; | |
| if (needsModelsRetry || needsAgentsRetry) { | |
| yield* Effect.sleep("1 second"); | |
| const [m2, a2] = yield* Effect.all( | |
| [ | |
| needsModelsRetry ? runModelsCli() : Effect.succeed(modelsResult), | |
| needsAgentsRetry ? runAgentsCli() : Effect.succeed(agentsResult), | |
| ], | |
| { concurrency: "unbounded" }, | |
| ); | |
| modelsResult = m2; | |
| agentsResult = a2; | |
| } | |
| const loadInventoryFromCli: OpenCodeRuntimeShape["loadInventoryFromCli"] = (input) => | |
| Effect.gen(function* () { | |
| const env = input.environment !== undefined ? { environment: input.environment } : ({} as {}); | |
| const runModelsCli = () => | |
| runOpenCodeCommand({ | |
| binaryPath: input.binaryPath, | |
| args: ["models", "--verbose"], | |
| ...env, | |
| }).pipe(Effect.timeout(OPENCODE_CLI_INVENTORY_TIMEOUT), Effect.exit); | |
| const runAgentsCli = () => | |
| runOpenCodeCommand({ binaryPath: input.binaryPath, args: ["agent", "list"], ...env }).pipe( | |
| Effect.timeout(OPENCODE_CLI_INVENTORY_TIMEOUT), | |
| Effect.exit, | |
| ); | |
| // First attempt — run both in parallel | |
| let [modelsResult, agentsResult] = yield* Effect.all([runModelsCli(), runAgentsCli()], { | |
| concurrency: "unbounded", | |
| }); | |
| // Retry once after 1s on transient failures (e.g. SQLite "database is locked") | |
| const needsModelsRetry = modelsResult._tag === "Failure" || modelsResult.value.code !== 0; | |
| const needsAgentsRetry = agentsResult._tag === "Failure" || agentsResult.value.code !== 0; | |
| if (needsModelsRetry || needsAgentsRetry) { | |
| yield* Effect.sleep("1 second"); | |
| const [m2, a2] = yield* Effect.all( | |
| [ | |
| needsModelsRetry ? runModelsCli() : Effect.succeed(modelsResult), | |
| needsAgentsRetry ? runAgentsCli() : Effect.succeed(agentsResult), | |
| ], | |
| { concurrency: "unbounded" }, | |
| ); | |
| modelsResult = m2; | |
| agentsResult = a2; | |
| } |
🤖 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/server/src/provider/opencodeRuntime.ts` around lines 657 - 691, Bound
both inventory command effects created by runModelsCli and runAgentsCli with the
existing DEFAULT_OPENCODE_SERVER_TIMEOUT_MS, ensuring timed-out attempts produce
failures handled by the current retry and graceful-degradation flow. Apply the
timeout to each attempt, including retries, without changing the parallel
execution or retry conditions.
| return () => { | ||
| unregisterUpdate(); | ||
| unregisterFocus(); | ||
| unregisterBlur(); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear mirrored chip attributes during cleanup.
The effect unregisters its listeners but does not remove data-composer-chip-selected from currently selected DOM nodes. After a plugin remount or React Strict Mode effect replay, stale highlights can remain because the new selectedKeys set starts empty.
Proposed fix
return () => {
+ applyKeys(new Set());
unregisterUpdate();
unregisterFocus();
unregisterBlur();
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return () => { | |
| unregisterUpdate(); | |
| unregisterFocus(); | |
| unregisterBlur(); | |
| }; | |
| return () => { | |
| applyKeys(new Set()); | |
| unregisterUpdate(); | |
| unregisterFocus(); | |
| unregisterBlur(); | |
| }; |
🤖 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/web/src/components/ComposerPromptEditor.tsx` around lines 1237 - 1241,
Update the cleanup returned by the effect in ComposerPromptEditor to remove
data-composer-chip-selected from all currently selected DOM nodes before or
alongside unregistering the update, focus, and blur listeners, ensuring remounts
and Strict Mode replays cannot retain stale highlights.
| const navigateToResolvedUrl = useCallback( | ||
| async (resolvedUrl: string) => { | ||
| if (tabId && previewBridge) { | ||
| // Drive the webview imperatively; `usePreviewBridge` mirrors the | ||
| // resolved URL back to the server so other clients stay in sync. | ||
| await previewBridge.navigate(tabId, resolvedUrl); | ||
| rememberPreviewUrl(threadRef, resolvedUrl); | ||
| } else { | ||
| await openPreviewSession({ | ||
| openPreview: open, | ||
| threadRef, | ||
| url: resolvedUrl, | ||
| }); | ||
| } | ||
| }, | ||
| [open, tabId, threadRef], | ||
| ); | ||
|
|
||
| const handleSubmitUrl = useCallback( | ||
| async (next: string) => { | ||
| try { | ||
| const resolvedUrl = resolveDiscoveredServerUrl(threadRef.environmentId, next); | ||
| if (tabId && previewBridge) { | ||
| // Drive the webview imperatively; `usePreviewBridge` mirrors the | ||
| // resolved URL back to the server so other clients stay in sync. | ||
| await previewBridge.navigate(tabId, resolvedUrl); | ||
| rememberPreviewUrl(threadRef, resolvedUrl); | ||
| } else { | ||
| await openPreviewSession({ | ||
| openPreview: open, | ||
| threadRef, | ||
| url: resolvedUrl, | ||
| }); | ||
| } | ||
| await navigateToResolvedUrl(normalizePreviewUrl(next)); | ||
| } catch { | ||
| // Server-side `failed` event renders the unreachable view. | ||
| } | ||
| }, | ||
| [open, tabId, threadRef], | ||
| [navigateToResolvedUrl], | ||
| ); | ||
|
|
||
| const handleOpenServerUrl = useCallback( | ||
| async (next: string) => { | ||
| try { | ||
| await navigateToResolvedUrl(resolveDiscoveredServerUrl(threadRef.environmentId, next)); | ||
| } catch { | ||
| // Server-side `failed` event renders the unreachable view. | ||
| } | ||
| }, | ||
| [navigateToResolvedUrl, threadRef.environmentId], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalid URL bar submissions now fail silently.
handleSubmitUrl normalizes next inside the same try/catch that wraps navigateToResolvedUrl. Since normalizePreviewUrl throws before any bridge/server call is made on invalid input (empty, unparsable, non-http(s)), the catch swallows it — but its comment ("Server-side failed event renders the unreachable view") no longer applies here: no server call happens, so no unreachable view ever renders. The user gets no feedback at all when submitting a malformed URL, whereas handleOpenServerUrl's resolveDiscoveredServerUrl falls back to the raw URL on normalization failure instead of leaving the user hanging.
🛡️ Proposed fix: surface normalization failures with a toast
const handleSubmitUrl = useCallback(
async (next: string) => {
+ let resolvedUrl: string;
+ try {
+ resolvedUrl = normalizePreviewUrl(next);
+ } catch {
+ toastManager.add({
+ type: "error",
+ title: "Invalid URL",
+ description: "Enter a valid http(s) URL.",
+ });
+ return;
+ }
try {
- await navigateToResolvedUrl(normalizePreviewUrl(next));
+ await navigateToResolvedUrl(resolvedUrl);
} catch {
// Server-side `failed` event renders the unreachable view.
}
},
[navigateToResolvedUrl],
);🤖 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/web/src/components/preview/PreviewView.tsx` around lines 116 - 153,
Update handleSubmitUrl so normalizePreviewUrl runs outside the try/catch that
handles navigateToResolvedUrl failures. Catch normalization errors separately
and surface them through the existing toast/error-notification mechanism, while
preserving the current silent handling for server-side navigation failures.
| const count = threadKeys.length; | ||
| const selectedThreadEntries = threadKeys.flatMap((threadKey) => { | ||
| const threadRef = parseScopedThreadKey(threadKey); | ||
| const thread = threadRef ? readThreadShell(threadRef) : null; | ||
| return threadRef && thread ? [{ threadKey, threadRef, thread }] : []; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Displayed/confirmed count can outnumber threads actually archived/deleted/marked.
count is taken from the raw selection before selectedThreadEntries filters out keys whose readThreadShell/parseScopedThreadKey lookup fails (e.g. a selected thread was removed via sync while still selected). The menu label ("Archive (N)"), the confirm dialog text, and the delete confirm text all use this pre-filter count, while the actual archive/delete/mark-unread loops only touch selectedThreadEntries. Users can see "Archive (3)" but only 2 threads get archived.
🐛 Proposed fix: derive count after filtering
const threadKeys = [...useThreadSelectionStore.getState().selectedThreadKeys];
if (threadKeys.length === 0) return;
- const count = threadKeys.length;
const selectedThreadEntries = threadKeys.flatMap((threadKey) => {
const threadRef = parseScopedThreadKey(threadKey);
const thread = threadRef ? readThreadShell(threadRef) : null;
return threadRef && thread ? [{ threadKey, threadRef, thread }] : [];
});
+ const count = selectedThreadEntries.length;
+ if (count === 0) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const count = threadKeys.length; | |
| const selectedThreadEntries = threadKeys.flatMap((threadKey) => { | |
| const threadRef = parseScopedThreadKey(threadKey); | |
| const thread = threadRef ? readThreadShell(threadRef) : null; | |
| return threadRef && thread ? [{ threadKey, threadRef, thread }] : []; | |
| }); | |
| const threadKeys = [...useThreadSelectionStore.getState().selectedThreadKeys]; | |
| if (threadKeys.length === 0) return; | |
| const selectedThreadEntries = threadKeys.flatMap((threadKey) => { | |
| const threadRef = parseScopedThreadKey(threadKey); | |
| const thread = threadRef ? readThreadShell(threadRef) : null; | |
| return threadRef && thread ? [{ threadKey, threadRef, thread }] : []; | |
| }); | |
| const count = selectedThreadEntries.length; | |
| if (count === 0) return; |
🤖 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/web/src/components/Sidebar.tsx` around lines 1783 - 1788, Derive the
displayed and confirmation count from selectedThreadEntries.length rather than
the raw threadKeys.length. Update the count used by the archive, delete, and
mark-unread labels/dialogs after the parseScopedThreadKey/readThreadShell
filtering in the surrounding selection flow, while keeping the action loops
based on selectedThreadEntries.
…4168 Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
normalizeCustomModelSlugs no longer takes a provider kind after pingdotgg#4168. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
…call Match the post-pingdotgg#4168 three-argument signature used by other adapters. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
Acquire PubSub subscriptions synchronously for shell/thread streams so events published during snapshot load are not lost to the fork race. Also forward thread deleted/archived lifecycle events to detail subscribers. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
The shell fallback snapshot test must acquire the same PubSub subscription the production path uses after the fork-race fix. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>
20 participants
Summary
Layer 1 of the Jul 25 upstream sync (110 commits on
pingdotgg/t3codesince v0.36.0 ancestry).Ports early provider, orchestration, CLI, and web fixes through preserve-custom-model-slugs (
#4168).Highlights
providerModelsFromSettingswithout provider-kind argTest plan
mainmainSummary by CodeRabbit
New Features
Bug Fixes