sync: merge 10 new upstream commits (v0.0.20)#74
Open
aaditagrawal wants to merge 11 commits intomainfrom
Open
Conversation
…gdotgg#2210) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: = <=>
pingdotgg#2192) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: codex <codex@users.noreply.github.com>
…#2218) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…gdotgg#2224) Co-authored-by: Julius Marminge <julius0216@outlook.com>
Upstream additions: - fix(web): restore manual sort drag and keep per-group expand state (pingdotgg#2221) - fix: Change right panel sheet to be below title bar / action bar (pingdotgg#2224) - Refactor OpenCode lifecycle and structured output handling (pingdotgg#2218) - effect-codex-app-server (pingdotgg#1942) - Redesign model picker with favorites and search (pingdotgg#2153) - fix(server): prevent probeClaudeCapabilities from wasting API requests (pingdotgg#2192) - fix(server): handle OpenCode text response format in commit message gen (pingdotgg#2202) - Devcontainer / IDE updates (pingdotgg#2208) - Expand leading ~ in Codex home paths before exporting CODEX_HOME (pingdotgg#2210) - fix(release): use v<semver> tag format for nightly releases (pingdotgg#2186) Fork adaptations: - Took upstream's redesigned model picker with favorites and search - Removed deleted codexAppServerManager (replaced by effect-codex-app-server) - Stubbed fetchCodexUsage (manager-based readout no longer available) - Extended PROVIDER_ICON_BY_PROVIDER for all 8 fork providers - Extended modelOptionsByProvider test fixtures for all 8 providers - Inline ClaudeSlashCommand type (not yet re-exported from SDK) - Updated SettingsPanels imports for new picker module structure - Preserved fork's CI customizations (ubuntu-24.04 not Blacksmith)
📝 WalkthroughWalkthroughThe PR refactors provider session management from a centralized manager-based architecture to a runtime-based abstraction, removes obsolete Codex utilities and configuration modules, redesigns the model picker UI with sidebar navigation and search functionality, adds keyboard shortcuts for model picker navigation, implements project favorites tracking, and updates configuration files for consistency. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
github-actions
bot
added
vouch:trusted
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/provider/Layers/ClaudeProvider.ts (1)
688-719:⚠️ Potential issue | 🟡 MinorNormalize the Claude binary path before SDK probes.
runClaudeCommandnormalizes blank settings with.trim() || "claude"at line 561, but the SDK probe calls at lines 690 and 718 pass the rawclaudeSettings.binaryPath. With default blank or whitespace-only settings, slash-command and subscription probing will receive an invalid path, causing silent failures while the CLI health check succeeds.Suggested fix
+ const claudeBinaryPath = claudeSettings.binaryPath.trim() || "claude"; + const slashCommands = (resolveSlashCommands - ? yield* resolveSlashCommands(claudeSettings.binaryPath).pipe( + ? yield* resolveSlashCommands(claudeBinaryPath).pipe( Effect.orElseSucceed(() => undefined), ) : undefined) ?? []; @@ if (!subscriptionType && resolveSubscriptionType) { - subscriptionType = yield* resolveSubscriptionType(claudeSettings.binaryPath); + subscriptionType = yield* resolveSubscriptionType(claudeBinaryPath); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/provider/Layers/ClaudeProvider.ts` around lines 688 - 719, The slash-command and SDK probe calls use the raw claudeSettings.binaryPath which can be blank/whitespace and cause silent probe failures; normalize the path first (e.g., const normalizedBinary = claudeSettings.binaryPath.trim() || "claude") and use normalizedBinary when calling resolveSlashCommands, runClaudeCommand (or any helper that probes the CLI), and resolveSubscriptionType so all probes receive the same valid binary path (update references around resolveSlashCommands, slashCommands, authProbe/runClaudeCommand, and resolveSubscriptionType).
🧹 Nitpick comments (5)
apps/web/src/components/CommandPalette.logic.ts (1)
154-173: Prefer object spread overObject.assignfor readability.The refactor is functionally correct, but mixing
Object.assignwithas constis harder to read than a plain object literal with conditional spreads, and it yields a less precise intersection type instead of a single inferred object type. The empty-string defaults on Lines 158 and delimiter on Line 160 also use backticks with no interpolation — regular string literals would be clearer (and your formatter may eventually rewrite them).♻️ Suggested refactor
- return Object.assign( - { - kind: "action" as const, - value: `thread:${thread.id}`, - searchTerms: [thread.title, projectTitle ?? ``, thread.branch ?? ``], - title: thread.title, - description: descriptionParts.join(` · `), - timestamp: formatRelativeTimeLabel( - thread.latestUserMessageAt ?? thread.updatedAt ?? thread.createdAt, - ), - icon: input.icon, - }, - leadingContent ? { titleLeadingContent: leadingContent } : {}, - trailingContent ? { titleTrailingContent: trailingContent } : {}, - { - run: async () => { - await input.runThread(thread); - }, - }, - ); + return { + kind: "action" as const, + value: `thread:${thread.id}`, + searchTerms: [thread.title, projectTitle ?? "", thread.branch ?? ""], + title: thread.title, + description: descriptionParts.join(" · "), + timestamp: formatRelativeTimeLabel( + thread.latestUserMessageAt ?? thread.updatedAt ?? thread.createdAt, + ), + icon: input.icon, + ...(leadingContent ? { titleLeadingContent: leadingContent } : {}), + ...(trailingContent ? { titleTrailingContent: trailingContent } : {}), + run: async () => { + await input.runThread(thread); + }, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/CommandPalette.logic.ts` around lines 154 - 173, Replace the Object.assign return with a single object literal using spread syntax and conditional spreads (e.g., {...(leadingContent && { titleLeadingContent: leadingContent })}) to improve readability and preserve a single inferred object type; keep the "kind: 'action' as const" assertion, compute searchTerms using normal string literals for defaults ('' instead of ``) and use a normal string for the delimiter in descriptionParts.join(' · '); ensure the timestamp still calls formatRelativeTimeLabel(thread.latestUserMessageAt ?? thread.updatedAt ?? thread.createdAt) and the run function still calls await input.runThread(thread) and include input.icon and other fields unchanged.apps/server/src/keybindings.test.ts (1)
195-197: Consider asserting thewhengating too.The new assertions verify the
keyformodelPicker.*defaults but not thatmodelPicker.jump.*are gated bywhen: "modelPickerOpen"(andmodelPicker.toggleby!terminalFocus). SincemodelPicker.jump.1andthread.jump.1sharemod+1, thewhenclause is the actual disambiguator, and a regression on it would silently break the UX without failing this test.♻️ Proposed extension
+ const defaultsByCommandFull = new Map( + DEFAULT_KEYBINDINGS.map((binding) => [binding.command, binding] as const), + ); + assert.equal(defaultsByCommandFull.get("modelPicker.toggle")?.when, "!terminalFocus"); + assert.equal(defaultsByCommandFull.get("modelPicker.jump.1")?.when, "modelPickerOpen"); + assert.equal(defaultsByCommandFull.get("modelPicker.jump.9")?.when, "modelPickerOpen");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/keybindings.test.ts` around lines 195 - 197, The test currently asserts defaultsByCommand.get("modelPicker.toggle"), "modelPicker.jump.1", and "modelPicker.jump.9" only for their key values; add assertions that verify the corresponding `when` gating as well — e.g. assert that the entry for "modelPicker.toggle" has when === "!terminalFocus" and that entries "modelPicker.jump.1" through "modelPicker.jump.9" have when === "modelPickerOpen" — so that defaultsByCommand (and any lookups used in the test) validate the `when` field and prevent regressions/conflicts with similar bindings like "thread.jump.1".apps/web/src/shortcutModifierState.test.ts (1)
23-146: Reset modifier state at the suite boundary.These tests mutate module-level state. A
beforeEach/afterEachreset makes the suite order-independent and protects future tests from leaked modifier state.Suggested isolation hook
-import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ describe("shortcutModifierState", () => { + beforeEach(() => { + clearShortcutModifierState(); + }); + + afterEach(() => { + clearShortcutModifierState(); + }); + it("compares modifier states by value", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/shortcutModifierState.test.ts` around lines 23 - 146, Tests mutate module-level shortcut modifier state and lack isolation; add suite-level hooks to reset state before and/or after each test by calling clearShortcutModifierState inside a beforeEach (and optionally afterEach) within the describe("shortcutModifierState") block so each it() starts with a clean state; locate where tests reference readShortcutModifierState, setShortcutModifierState, syncShortcutModifierStateFromKeyboardEvent and insert the clearShortcutModifierState calls in beforeEach/afterEach to make the suite order-independent.apps/web/src/keybindings.test.ts (1)
410-422: Cover the inverse hint condition for shared jump shortcuts.Since
thread.jump.*andmodelPicker.jump.*share the same shortcuts, also assert thread hints stay hidden whilemodelPickerOpenis active.Suggested assertion
assert.isTrue( shouldShowModelPickerJumpHints(event({ metaKey: true }), DEFAULT_BINDINGS, { platform: "MacIntel", context: { modelPickerOpen: true }, }), ); + assert.isFalse( + shouldShowThreadJumpHints(event({ metaKey: true }), DEFAULT_BINDINGS, { + platform: "MacIntel", + context: { modelPickerOpen: true }, + }), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/keybindings.test.ts` around lines 410 - 422, Add an assertion to cover the inverse condition: when modelPickerOpen is true, verify that thread jump hints remain hidden. In the same test that calls shouldShowModelPickerJumpHints, also call shouldShowThreadJumpHints (the thread hint visibility helper) with the same event(DEFAULT_BINDINGS) and context { platform: "MacIntel", context: { modelPickerOpen: true } } and assert it returns false so shared shortcuts do not show thread hints while the model picker is active.apps/web/src/components/ChatView.browser.tsx (1)
5574-5577: Assert the composer draft is cleared after selecting/model.This currently checks the picker button text, but a regression could leave
/modin the composer and still pass. Add a direct draft/editor assertion so the slash-command replacement behavior is covered.Suggested test tightening
await vi.waitFor(() => { expect(document.querySelector(".model-picker-list")).not.toBeNull(); - expect(findComposerProviderModelPicker()?.textContent).not.toContain("/model"); + const providerModelPicker = findComposerProviderModelPicker(); + expect(providerModelPicker).not.toBeNull(); + expect(providerModelPicker!.textContent).not.toContain("/model"); + expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt ?? "").toBe( + "", + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.browser.tsx` around lines 5574 - 5577, The test currently verifies the model picker UI via document.querySelector(".model-picker-list") and findComposerProviderModelPicker(), but doesn't assert the composer draft itself; update the test inside the vi.waitFor callback to also locate the composer text input/editor (e.g., the composer input element used elsewhere in tests) and assert its value/textContent no longer contains "/model" and is cleared (or equals the expected post-replacement string). Reference the existing helpers used in this file (findComposerProviderModelPicker() and the composer input/editor selector used by other tests) to locate the composer element and add the direct assertion that the draft is cleared after selecting "/model".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 8-9: The scheduled workflow uses GITHUB_REF_NAME (branch) to
derive raw and then validates semver, but scheduled runs set the event to
"schedule" and have a branch like "main", causing semver validation to fail; fix
by detecting scheduled runs and injecting an explicit version branch (e.g., set
raw = "nightly" or "scheduled") before the semver validation step: update the
workflow expression that sets raw (the variable named raw derived from
GITHUB_REF_NAME/GITHUB_REF_NAME) to check github.event_name == 'schedule' (or
compare to the schedule trigger) and assign a stable string like "nightly" for
scheduled runs so the subsequent semver validation logic (the validation step
that currently rejects non-semver raw values) accepts scheduled runs.
In @.vscode/settings.json:
- Line 8: Replace the invalid VS Code setting key "js/ts.tsdk.path" with the
correct key "typescript.tsdk" while keeping the value
"node_modules/typescript/lib"; locate the entry that currently reads
"js/ts.tsdk.path" and rename the key to "typescript.tsdk" so VS Code will use
the workspace TypeScript SDK.
In `@apps/server/src/provider/Layers/CodexAdapter.test.ts`:
- Around line 1030-1033: The test in CodexAdapter.test.ts currently asserts the
native log prefix mistakenly as "NTIVE"; update the assertion to validate the
payload message directly instead of the log prefix: after reading threadLogPath
into contents (variable contents), replace the regex /NTIVE: .*"message":"native
flush test"/ with a regex that only matches the JSON payload message such as
/"message":"native flush test"/ so the test verifies flush behavior rather than
the log prefix.
In `@apps/server/src/provider/Layers/CodexAdapter.ts`:
- Around line 1401-1416: The event consumer fiber (eventFiber) is being forked
into the temporary scope created by startSession (via Effect.scoped) using
Effect.forkChild, so when startSession's scope closes the fiber is interrupted
and runtime.events are dropped; change the fork target to the persistent session
scope (sessionScope) instead of forking as a child of the temporary scope—i.e.,
when creating the fiber from Stream.runForEach(runtime.events,
...)/.pipe(Effect.forkChild) ensure you fork into sessionScope (the session's
persistent scope) so the event pump lives for the full session lifetime rather
than being tied to startSession's ephemeral scope.
In `@apps/server/src/provider/Layers/CodexProvider.ts`:
- Around line 389-394: The probe call in CodexProvider is using process.cwd()
which ignores the configured server workspace; update CodexProviderLive to read
ServerConfig (serverConfig.cwd) and pass that cwd into the probe call (used when
building the options for checkCodexProviderStatus / probe), replacing
process.cwd() with the injected serverConfig.cwd so the probe (function probe
and the checkCodexProviderStatus flow that produces probeResult) runs against
the configured workspace while preserving the existing timeout
(PROVIDER_PROBE_TIMEOUT_MS) and other parameters (binaryPath, homePath,
customModels).
- Around line 218-239: The initialize call is using a hardcoded version
("0.1.0") instead of the helper; replace the inline clientInfo object passed to
client.request("initialize", ...) with the values produced by
buildCodexInitializeParams() (or merge its returned params) so the
clientInfo.version comes from packageJson via buildCodexInitializeParams; locate
the initialization block around the clientContext/CodexClient.layerCommand and
update the initialize variable creation to use buildCodexInitializeParams()
output rather than the hardcoded literal.
In `@apps/server/src/provider/Layers/CodexSessionRuntime.ts`:
- Around line 313-340: The code in buildTurnStartParams is dropping label-only
user-input options (it rejects options when description is missing), causing
valid Codex user-input requests to be suppressed; update the logic that builds
turnInput (inside buildTurnStartParams) to include option objects even when
description is absent—ensure V2TurnStartParams__UserInput entries preserve
label-only options by setting description to undefined or omitting it rather
than filtering the option out, and keep the existing handling for attachments
and text so label-only options are pushed into turnInput the same way other
inputs are.
In `@apps/server/src/provider/Layers/OpenCodeAdapter.test.ts`:
- Around line 79-99: The test double connectToOpenCodeServer currently always
registers a finalizer that mutates runtimeMock.state (closeCalls/closeError),
which makes configured-server sessions (when serverUrl is provided) incorrectly
exercise finalizer failure paths; change connectToOpenCodeServer to only add the
finalizer when serverUrl is not provided (i.e., local/test server path) so
configured external connections return without owning lifecycle, and move any
test that simulates finalizer throws (stopAll close-error coverage) into the
layer or test that calls connectToOpenCodeServer with no serverUrl; reference
connectToOpenCodeServer, runtimeMock.state.closeCalls, and
runtimeMock.state.closeError when making this conditional change.
In `@apps/server/src/provider/opencodeRuntime.ts`:
- Around line 434-440: The code currently masks failures by converting
child.exitCode failures into 0 via Effect.orElseSucceed(() => 0); update the
return so callers can distinguish real success from errors: either remove the
orElseSucceed wrapper and make exitCode an errorful Effect<number,
OpenCodeRuntimeError> (update OpenCodeServerProcess/OpenCodeServerConnection
types accordingly) so failures surface, or, if keeping a successful-effect
signature, replace the mask with a non-colliding sentinel (e.g., -1) and ensure
failures are logged/annotated; locate the return that references child.exitCode
and Effect.orElseSucceed and adjust the behavior and types consistently.
In `@apps/web/src/components/chat/ModelPickerContent.tsx`:
- Around line 330-337: The handler is incorrectly using
targetModelKey.split(":"), which breaks slugs that contain colons; update the
parsing of targetModelKey (used where modelJumpModelKeys and jumpIndex are
referenced and in other handlers around lines noted) to split only on the first
":"—e.g., find the first colon index and take the substring before as provider
and the remainder as slug—then call handleModelSelect(slug, provider);
alternatively centralize encode/decode logic for keys (provider:slug) to a
single helper used by these handlers to avoid truncation bugs (refer to
ProviderKind, targetModelKey, modelJumpModelKeys, jumpIndex, and
handleModelSelect).
In `@apps/web/src/components/chat/ModelPickerSidebar.tsx`:
- Around line 156-207: Remove the hardcoded "coming soon" Gemini and Github
Copilot UI blocks in ModelPickerSidebar.tsx: delete the two
Tooltip/TooltipTrigger blocks that render buttons with
data-model-picker-provider="gemini-coming-soon" and "github-copilot-coming-soon"
(the blocks that include Gemini, GithubCopilotIcon, SOON_BADGE_CLASS and
Clock3Icon and corresponding TooltipPopup using PICKER_TOOLTIP_SIDE and
PICKER_TOOLTIP_CLASS). This eliminates duplicate entries since
AVAILABLE_PROVIDER_OPTIONS now includes copilot and geminiCli and those
providers are rendered elsewhere.
In `@apps/web/src/components/Icons.tsx`:
- Around line 22-27: The CursorIcon component is forwarding a non-SVG prop
`monochrome` to the <svg> because Icon props aren't destructured; update
CursorIcon (and other icons that follow this pattern) to strip `monochrome` from
the props before spreading to the svg—e.g., destructure { className, monochrome,
...props } in the CursorIcon function signature or remove monochrome from the
object prior to {...props} so the svg doesn't receive a non-standard DOM
attribute (follow the same approach used by ClaudeAI).
In `@apps/web/src/environments/runtime/service.ts`:
- Around line 473-480: syncProjectUiFromStore builds logicalKey using
getClientSettings(), so UI keys go stale when grouping settings change; add a
settings change listener that calls syncProjectUiFromStore whenever
sidebarProjectGroupingMode or sidebarProjectGroupingOverrides change (or re-run
the same sync logic) to re-compute logical IDs and call
useUiStateStore.getState().syncProjects again; locate this near the runtime
service initialization where selectProjectsAcrossEnvironments,
getClientSettings, deriveLogicalProjectKeyFromSettings, and
syncProjectUiFromStore are defined and wire the listener to the settings store
or event emitter so changes immediately trigger re-sync.
In `@apps/web/src/hooks/useSettings.ts`:
- Around line 134-140: getClientSettings() currently returns the in-memory
snapshot (getClientSettingsSnapshot()) and can return DEFAULT_CLIENT_SETTINGS if
hydration hasn't started; change the API so there are two clear accessors: keep
getClientSettingsSnapshot() as the synchronous snapshot-only accessor, and make
getClientSettings() an async/hydrating accessor that ensures persisted settings
are loaded before returning (implement an ensureClientSettingsHydrated() or
call/await subscribeClientSettings() startup/hydration flow inside
getClientSettings()), and update callers: runtime init code should await
getClientSettings(), while synchronous callers that truly need a snapshot should
call getClientSettingsSnapshot(); update useSettings() subscribers to continue
starting hydration as before.
---
Outside diff comments:
In `@apps/server/src/provider/Layers/ClaudeProvider.ts`:
- Around line 688-719: The slash-command and SDK probe calls use the raw
claudeSettings.binaryPath which can be blank/whitespace and cause silent probe
failures; normalize the path first (e.g., const normalizedBinary =
claudeSettings.binaryPath.trim() || "claude") and use normalizedBinary when
calling resolveSlashCommands, runClaudeCommand (or any helper that probes the
CLI), and resolveSubscriptionType so all probes receive the same valid binary
path (update references around resolveSlashCommands, slashCommands,
authProbe/runClaudeCommand, and resolveSubscriptionType).
---
Nitpick comments:
In `@apps/server/src/keybindings.test.ts`:
- Around line 195-197: The test currently asserts
defaultsByCommand.get("modelPicker.toggle"), "modelPicker.jump.1", and
"modelPicker.jump.9" only for their key values; add assertions that verify the
corresponding `when` gating as well — e.g. assert that the entry for
"modelPicker.toggle" has when === "!terminalFocus" and that entries
"modelPicker.jump.1" through "modelPicker.jump.9" have when ===
"modelPickerOpen" — so that defaultsByCommand (and any lookups used in the test)
validate the `when` field and prevent regressions/conflicts with similar
bindings like "thread.jump.1".
In `@apps/web/src/components/ChatView.browser.tsx`:
- Around line 5574-5577: The test currently verifies the model picker UI via
document.querySelector(".model-picker-list") and
findComposerProviderModelPicker(), but doesn't assert the composer draft itself;
update the test inside the vi.waitFor callback to also locate the composer text
input/editor (e.g., the composer input element used elsewhere in tests) and
assert its value/textContent no longer contains "/model" and is cleared (or
equals the expected post-replacement string). Reference the existing helpers
used in this file (findComposerProviderModelPicker() and the composer
input/editor selector used by other tests) to locate the composer element and
add the direct assertion that the draft is cleared after selecting "/model".
In `@apps/web/src/components/CommandPalette.logic.ts`:
- Around line 154-173: Replace the Object.assign return with a single object
literal using spread syntax and conditional spreads (e.g., {...(leadingContent
&& { titleLeadingContent: leadingContent })}) to improve readability and
preserve a single inferred object type; keep the "kind: 'action' as const"
assertion, compute searchTerms using normal string literals for defaults (''
instead of ``) and use a normal string for the delimiter in
descriptionParts.join(' · '); ensure the timestamp still calls
formatRelativeTimeLabel(thread.latestUserMessageAt ?? thread.updatedAt ??
thread.createdAt) and the run function still calls await input.runThread(thread)
and include input.icon and other fields unchanged.
In `@apps/web/src/keybindings.test.ts`:
- Around line 410-422: Add an assertion to cover the inverse condition: when
modelPickerOpen is true, verify that thread jump hints remain hidden. In the
same test that calls shouldShowModelPickerJumpHints, also call
shouldShowThreadJumpHints (the thread hint visibility helper) with the same
event(DEFAULT_BINDINGS) and context { platform: "MacIntel", context: {
modelPickerOpen: true } } and assert it returns false so shared shortcuts do not
show thread hints while the model picker is active.
In `@apps/web/src/shortcutModifierState.test.ts`:
- Around line 23-146: Tests mutate module-level shortcut modifier state and lack
isolation; add suite-level hooks to reset state before and/or after each test by
calling clearShortcutModifierState inside a beforeEach (and optionally
afterEach) within the describe("shortcutModifierState") block so each it()
starts with a clean state; locate where tests reference
readShortcutModifierState, setShortcutModifierState,
syncShortcutModifierStateFromKeyboardEvent and insert the
clearShortcutModifierState calls in beforeEach/afterEach to make the suite
order-independent.
🪄 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
Run ID: e5da1bb1-77e4-45e2-acc9-99b2d1e8ef41
⛔ Files ignored due to path filters (5)
.claude/scheduled_tasks.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockpackages/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 (105)
.devcontainer/devcontainer.json.github/workflows/release.yml.oxfmtrc.json.vscode/settings.jsonapps/desktop/src/clientPersistence.test.tsapps/server/package.jsonapps/server/src/codexAppServerManager.test.tsapps/server/src/codexAppServerManager.tsapps/server/src/git/Layers/CodexTextGeneration.tsapps/server/src/git/Layers/CursorTextGeneration.tsapps/server/src/git/Layers/OpenCodeTextGeneration.test.tsapps/server/src/git/Layers/OpenCodeTextGeneration.tsapps/server/src/git/Utils.tsapps/server/src/keybindings.test.tsapps/server/src/keybindings.tsapps/server/src/pathExpansion.test.tsapps/server/src/pathExpansion.tsapps/server/src/provider/CodexDeveloperInstructions.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/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/ProviderAdapterConformance.test.tsapps/server/src/provider/Layers/ProviderRegistry.test.tsapps/server/src/provider/Layers/ProviderRegistry.tsapps/server/src/provider/codexAccount.tsapps/server/src/provider/codexAppServer.tsapps/server/src/provider/codexCliVersion.tsapps/server/src/provider/opencodeRuntime.test.tsapps/server/src/provider/opencodeRuntime.tsapps/server/src/server.test.tsapps/server/src/server.tsapps/web/src/components/AppSidebarLayout.tsxapps/web/src/components/ChatView.browser.tsxapps/web/src/components/ChatView.tsxapps/web/src/components/CommandPalette.logic.tsapps/web/src/components/DiffPanelShell.tsxapps/web/src/components/Icons.tsxapps/web/src/components/Sidebar.logic.test.tsapps/web/src/components/Sidebar.tsxapps/web/src/components/chat/ChatComposer.tsxapps/web/src/components/chat/ComposerCommandMenu.tsxapps/web/src/components/chat/ModelListRow.tsxapps/web/src/components/chat/ModelPickerContent.tsxapps/web/src/components/chat/ModelPickerSidebar.tsxapps/web/src/components/chat/ProviderModelPicker.browser.tsxapps/web/src/components/chat/ProviderModelPicker.tsxapps/web/src/components/chat/TraitsPicker.browser.tsxapps/web/src/components/chat/TraitsPicker.tsxapps/web/src/components/chat/modelPickerModelHighlights.tsapps/web/src/components/chat/modelPickerSearch.test.tsapps/web/src/components/chat/modelPickerSearch.tsapps/web/src/components/chat/providerIconUtils.tsapps/web/src/components/settings/SettingsPanels.tsxapps/web/src/components/ui/autocomplete.tsxapps/web/src/components/ui/combobox.tsxapps/web/src/components/ui/scroll-area.tsxapps/web/src/composer-logic.test.tsapps/web/src/composer-logic.tsapps/web/src/environments/runtime/service.tsapps/web/src/hooks/useHandleNewThread.tsapps/web/src/hooks/useSettings.tsapps/web/src/index.cssapps/web/src/keybindings.test.tsapps/web/src/keybindings.tsapps/web/src/localApi.test.tsapps/web/src/logicalProject.tsapps/web/src/modelPickerOpenState.tsapps/web/src/rightPanelLayout.tsapps/web/src/session-logic.test.tsapps/web/src/session-logic.tsapps/web/src/shortcutModifierState.test.tsapps/web/src/shortcutModifierState.tsapps/web/src/uiStateStore.test.tsapps/web/src/uiStateStore.tspackages/contracts/src/keybindings.test.tspackages/contracts/src/keybindings.tspackages/contracts/src/server.tspackages/contracts/src/settings.tspackages/effect-acp/src/_internal/stdio.tspackages/effect-codex-app-server/package.jsonpackages/effect-codex-app-server/scripts/generate.tspackages/effect-codex-app-server/src/_internal/shared.tspackages/effect-codex-app-server/src/_internal/stdio.tspackages/effect-codex-app-server/src/client.test.tspackages/effect-codex-app-server/src/client.tspackages/effect-codex-app-server/src/errors.tspackages/effect-codex-app-server/src/protocol.test.tspackages/effect-codex-app-server/src/protocol.tspackages/effect-codex-app-server/src/rpc.tspackages/effect-codex-app-server/src/schema.tspackages/effect-codex-app-server/test/examples/codex-app-server-probe.tspackages/effect-codex-app-server/test/fixtures/codex-app-server-mock-peer.tspackages/effect-codex-app-server/tsconfig.jsonpackages/shared/src/model.tsscripts/release-smoke.tsscripts/resolve-nightly-release.test.tsscripts/resolve-nightly-release.tsscripts/resolve-previous-release-tag.ts
💤 Files with no reviewable changes (6)
- apps/server/src/provider/codexCliVersion.ts
- apps/server/src/codexAppServerManager.test.ts
- apps/server/src/provider/opencodeRuntime.test.ts
- apps/server/src/provider/codexAppServer.ts
- apps/server/src/provider/codexAccount.ts
- apps/server/src/codexAppServerManager.ts
Comment on lines
+8
to
+9
| schedule: | ||
| - cron: "0 */3 * * *" |
There was a problem hiding this comment.
Generate a nightly version for scheduled runs.
Line 8 adds scheduled releases, but scheduled workflows resolve raw from GITHUB_REF_NAME, which is the branch name like main, so Line 83 rejects it as an invalid semver. Add an explicit schedule branch before validation.
🐛 Proposed fix
if [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" ]]; then
raw="${{ github.event.inputs.version }}"
+ elif [[ "${GITHUB_EVENT_NAME}" == "schedule" ]]; then
+ base_version="$(sed -nE 's/^[[:space:]]*"version":[[:space:]]*"([^"]+)".*/\1/p' package.json | head -n 1)"
+ base_version="${base_version%%-*}"
+ raw="${base_version}-nightly.$(date -u +%Y%m%d%H%M)"
else
raw="${GITHUB_REF_NAME}"
fiAlso applies to: 76-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 8 - 9, The scheduled workflow
uses GITHUB_REF_NAME (branch) to derive raw and then validates semver, but
scheduled runs set the event to "schedule" and have a branch like "main",
causing semver validation to fail; fix by detecting scheduled runs and injecting
an explicit version branch (e.g., set raw = "nightly" or "scheduled") before the
semver validation step: update the workflow expression that sets raw (the
variable named raw derived from GITHUB_REF_NAME/GITHUB_REF_NAME) to check
github.event_name == 'schedule' (or compare to the schedule trigger) and assign
a stable string like "nightly" for scheduled runs so the subsequent semver
validation logic (the validation step that currently rejects non-semver raw
values) accepts scheduled runs.
| }, | ||
| "oxc.unusedDisableDirectives": "warn", | ||
| "typescript.tsdk": "node_modules/typescript/lib" | ||
| "js/ts.tsdk.path": "node_modules/typescript/lib" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is js/ts.tsdk.path a valid VS Code settings key? What is the current official setting key to point VS Code to a workspace TypeScript SDK (e.g., node_modules/typescript/lib)?
💡 Result:
No, js/ts.tsdk.path is not a valid VS Code settings key. The correct and current official setting key to point VS Code to a workspace TypeScript SDK (e.g., node_modules/typescript/lib) is typescript.tsdk. This is confirmed across multiple authoritative sources including Microsoft VS Code GitHub issues/PRs (e.g., explicit use in PR #138211 and issue #132757), Stack Overflow answers quoting official descriptions, and VS Code documentation pages. While some older or variant docs mention js/ts.tsdk.path (result 8, 20), the predominant and consistent reference in recent sources (up to 2023, with ongoing use in 2026 issues) is typescript.tsdk. Recent unification efforts for JS/TS settings (issues #292934, PR #296677 from Feb 2026) adopt js/ts.* prefixes for other settings like js/ts.validate.enabled, but do not mention changing the tsdk setting, which remains typescript.tsdk. For workspace use: Set in .vscode/settings.json as "typescript.tsdk": "node_modules/typescript/lib". Use with typescript.enablePromptUseWorkspaceTsdk": true to prompt automatic selection.
Citations:
- 1: Allow loading custom typescript.tsdk settings on web hosts microsoft/vscode#138211
- 2: Array of TypeScript SDK paths in project VSCode settings microsoft/vscode#132757
- 3: https://stackoverflow.com/questions/74642723/how-do-i-force-visual-studio-code-to-always-use-my-workspaces-version-of-typesc
- 4: https://code.visualstudio.com/docs/typescript/typescript-transpiling
- 5: https://stackoverflow.com/questions/31308050/can-i-use-a-relative-path-to-configure-typescript-sdk
- 6: https://code.visualstudio.com/docs/typescript/typescript-compiling?from=20423&from_column=20423
Change the VS Code TypeScript SDK setting key from js/ts.tsdk.path to typescript.tsdk.
The key "js/ts.tsdk.path" is not recognized by VS Code and will be ignored. Change Line 8 to:
"typescript.tsdk": "node_modules/typescript/lib"
This ensures VS Code uses your workspace TypeScript version instead of the bundled version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vscode/settings.json at line 8, Replace the invalid VS Code setting key
"js/ts.tsdk.path" with the correct key "typescript.tsdk" while keeping the value
"node_modules/typescript/lib"; locate the entry that currently reads
"js/ts.tsdk.path" and rename the key to "typescript.tsdk" so VS Code will use
the workspace TypeScript SDK.
Comment on lines
+1030
to
+1033
| const threadLogPath = path.join(tempDir, "thread-logger.log"); | ||
| assert.equal(fs.existsSync(threadLogPath), true); | ||
| const contents = fs.readFileSync(threadLogPath, "utf8"); | ||
| assert.match(contents, /NTIVE: .*"message":"native flush test"/); |
There was a problem hiding this comment.
Fix the native log assertion typo.
Line 1033 matches NTIVE, which looks like a missing A from the native log prefix. Prefer asserting the payload message directly so this test only validates the flush behavior.
Proposed fix
- assert.match(contents, /NTIVE: .*"message":"native flush test"/);
+ assert.match(contents, /"message":"native flush test"/);📝 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.
Suggested change
| const threadLogPath = path.join(tempDir, "thread-logger.log"); | |
| assert.equal(fs.existsSync(threadLogPath), true); | |
| const contents = fs.readFileSync(threadLogPath, "utf8"); | |
| assert.match(contents, /NTIVE: .*"message":"native flush test"/); | |
| const threadLogPath = path.join(tempDir, "thread-logger.log"); | |
| assert.equal(fs.existsSync(threadLogPath), true); | |
| const contents = fs.readFileSync(threadLogPath, "utf8"); | |
| assert.match(contents, /"message":"native flush test"/); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/CodexAdapter.test.ts` around lines 1030 -
1033, The test in CodexAdapter.test.ts currently asserts the native log prefix
mistakenly as "NTIVE"; update the assertion to validate the payload message
directly instead of the log prefix: after reading threadLogPath into contents
(variable contents), replace the regex /NTIVE: .*"message":"native flush test"/
with a regex that only matches the JSON payload message such as
/"message":"native flush test"/ so the test verifies flush behavior rather than
the log prefix.
Comment on lines
+1401
to
+1416
| const eventFiber = yield* Stream.runForEach(runtime.events, (event) => | ||
| Effect.gen(function* () { | ||
| yield* writeNativeEvent(event); | ||
| const runtimeEvents = mapToRuntimeEvents(event, event.threadId); | ||
| if (runtimeEvents.length === 0) { | ||
| yield* Effect.logDebug("ignoring unhandled Codex provider event", { | ||
| method: event.method, | ||
| threadId: event.threadId, | ||
| turnId: event.turnId, | ||
| itemId: event.itemId, | ||
| }); | ||
| return; | ||
| } | ||
| yield* Queue.offerAll(runtimeEventQueue, runtimeEvents); | ||
| }), | ||
| }); | ||
| }, | ||
| ); | ||
| ).pipe(Effect.forkChild); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -20 apps/server/src/provider/Layers/CodexAdapter.tsRepository: aaditagrawal/t3code
Length of output: 623
🏁 Script executed:
wc -l apps/server/src/provider/Layers/CodexAdapter.tsRepository: aaditagrawal/t3code
Length of output: 115
🏁 Script executed:
sed -n '1350,1450p' apps/server/src/provider/Layers/CodexAdapter.tsRepository: aaditagrawal/t3code
Length of output: 3768
🏁 Script executed:
sed -n '1300,1380p' apps/server/src/provider/Layers/CodexAdapter.tsRepository: aaditagrawal/t3code
Length of output: 3113
Keep the Codex event pump in the session scope.
startSession is wrapped in Effect.scoped, creating a temporary scope. The event consumer is forked with Effect.forkChild, attaching it to this temporary scope. When startSession returns, that scope closes and interrupts the fiber, so later runtime events are dropped. Fork into sessionScope instead, which persists for the session lifetime.
Proposed fix
const eventFiber = yield* Stream.runForEach(runtime.events, (event) =>
Effect.gen(function* () {
yield* writeNativeEvent(event);
const runtimeEvents = mapToRuntimeEvents(event, event.threadId);
if (runtimeEvents.length === 0) {
yield* Effect.logDebug("ignoring unhandled Codex provider event", {
method: event.method,
threadId: event.threadId,
turnId: event.turnId,
itemId: event.itemId,
});
return;
}
yield* Queue.offerAll(runtimeEventQueue, runtimeEvents);
}),
- ).pipe(Effect.forkChild);
+ ).pipe(Effect.forkIn(sessionScope));📝 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.
Suggested change
| const eventFiber = yield* Stream.runForEach(runtime.events, (event) => | |
| Effect.gen(function* () { | |
| yield* writeNativeEvent(event); | |
| const runtimeEvents = mapToRuntimeEvents(event, event.threadId); | |
| if (runtimeEvents.length === 0) { | |
| yield* Effect.logDebug("ignoring unhandled Codex provider event", { | |
| method: event.method, | |
| threadId: event.threadId, | |
| turnId: event.turnId, | |
| itemId: event.itemId, | |
| }); | |
| return; | |
| } | |
| yield* Queue.offerAll(runtimeEventQueue, runtimeEvents); | |
| }), | |
| }); | |
| }, | |
| ); | |
| ).pipe(Effect.forkChild); | |
| const eventFiber = yield* Stream.runForEach(runtime.events, (event) => | |
| Effect.gen(function* () { | |
| yield* writeNativeEvent(event); | |
| const runtimeEvents = mapToRuntimeEvents(event, event.threadId); | |
| if (runtimeEvents.length === 0) { | |
| yield* Effect.logDebug("ignoring unhandled Codex provider event", { | |
| method: event.method, | |
| threadId: event.threadId, | |
| turnId: event.turnId, | |
| itemId: event.itemId, | |
| }); | |
| return; | |
| } | |
| yield* Queue.offerAll(runtimeEventQueue, runtimeEvents); | |
| }), | |
| ).pipe(Effect.forkIn(sessionScope)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/CodexAdapter.ts` around lines 1401 - 1416,
The event consumer fiber (eventFiber) is being forked into the temporary scope
created by startSession (via Effect.scoped) using Effect.forkChild, so when
startSession's scope closes the fiber is interrupted and runtime.events are
dropped; change the fork target to the persistent session scope (sessionScope)
instead of forking as a child of the temporary scope—i.e., when creating the
fiber from Stream.runForEach(runtime.events, ...)/.pipe(Effect.forkChild) ensure
you fork into sessionScope (the session's persistent scope) so the event pump
lives for the full session lifetime rather than being tied to startSession's
ephemeral scope.
Comment on lines
+218
to
+239
| const clientContext = yield* Layer.build( | ||
| CodexClient.layerCommand({ | ||
| command: input.binaryPath, | ||
| args: ["app-server"], | ||
| cwd: input.cwd, | ||
| ...(input.homePath ? { env: { CODEX_HOME: input.homePath } } : {}), | ||
| }), | ||
| ); | ||
| const client = yield* Effect.service(CodexClient.CodexAppServerClient).pipe( | ||
| Effect.provide(clientContext), | ||
| ); | ||
| const configPath = path.join(codexHome, "config.toml"); | ||
|
|
||
| const content = yield* fileSystem | ||
| .readFileString(configPath) | ||
| .pipe(Effect.orElseSucceed(() => undefined)); | ||
| if (content === undefined) { | ||
| return undefined; | ||
| } | ||
| const initialize = yield* client.request("initialize", { | ||
| clientInfo: { | ||
| name: "t3code_desktop", | ||
| title: "T3 Code Desktop", | ||
| version: "0.1.0", | ||
| }, | ||
| capabilities: { | ||
| experimentalApi: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and find the relevant code
cd /repo && find . -type f -name "CodexProvider.ts" | head -5Repository: aaditagrawal/t3code
Length of output: 118
🏁 Script executed:
# Search for CodexClient.layerCommand definition
rg "layerCommand" --type ts -A 10 | head -50Repository: aaditagrawal/t3code
Length of output: 3874
🏁 Script executed:
# Search for buildCodexInitializeParams definition
rg "buildCodexInitializeParams" --type ts -A 15 | head -80Repository: aaditagrawal/t3code
Length of output: 4253
🏁 Script executed:
# Examine the specific CodexProvider.ts file around lines 218-239
cat -n apps/server/src/provider/Layers/CodexProvider.ts | sed -n '210,250p'Repository: aaditagrawal/t3code
Length of output: 1663
🏁 Script executed:
# Get CodexClient.layerCommand implementation
cat -n packages/effect-codex-app-server/src/client.ts | sed -n '1,150p'Repository: aaditagrawal/t3code
Length of output: 7182
🏁 Script executed:
# Search for the full layerCommand function
rg "export const layerCommand" --type ts -A 30 packages/effect-codex-app-server/src/client.tsRepository: aaditagrawal/t3code
Length of output: 1204
🏁 Script executed:
# Look for ChildProcessSpawner and env handling
rg "env" --type ts -B 3 -A 3 packages/effect-codex-app-server/src/client.ts | head -60Repository: aaditagrawal/t3code
Length of output: 638
Replace hardcoded initialize version with helper function.
Lines 230–239 hardcode the initialization version as "0.1.0", but buildCodexInitializeParams() already exists and uses the correct packageJson.version. Use the helper instead to stay consistent with the session runtime (which correctly imports and uses it).
Proposed fix
- const initialize = yield* client.request("initialize", {
- clientInfo: {
- name: "t3code_desktop",
- title: "T3 Code Desktop",
- version: "0.1.0",
- },
- capabilities: {
- experimentalApi: true,
- },
- });
+ const initialize = yield* client.request("initialize", buildCodexInitializeParams());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/CodexProvider.ts` around lines 218 - 239, The
initialize call is using a hardcoded version ("0.1.0") instead of the helper;
replace the inline clientInfo object passed to client.request("initialize", ...)
with the values produced by buildCodexInitializeParams() (or merge its returned
params) so the clientInfo.version comes from packageJson via
buildCodexInitializeParams; locate the initialization block around the
clientContext/CodexClient.layerCommand and update the initialize variable
creation to use buildCodexInitializeParams() output rather than the hardcoded
literal.
Comment on lines
+330
to
+337
| const targetModelKey = modelJumpModelKeys[jumpIndex]; | ||
| if (!targetModelKey) { | ||
| return; | ||
| } | ||
| const [provider, slug] = targetModelKey.split(":") as [ProviderKind, string]; | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| handleModelSelect(slug, provider); |
There was a problem hiding this comment.
Parse model keys without truncating colon-containing slugs.
These handlers split ${provider}:${slug} with split(":"), so a slug like ollama:llama3 becomes just ollama. Split only on the first delimiter, or centralize key encode/decode.
🐛 Proposed fix
type ModelPickerItem = {
slug: string;
name: string;
shortName?: string;
subProvider?: string;
provider: ProviderKind;
};
+const modelPickerKey = (provider: ProviderKind, slug: string): string => `${provider}:${slug}`;
+
+const parseModelPickerKey = (
+ modelKey: string,
+): { provider: ProviderKind; slug: string } | null => {
+ const separatorIndex = modelKey.indexOf(":");
+ if (separatorIndex < 0) return null;
+ return {
+ provider: modelKey.slice(0, separatorIndex) as ProviderKind,
+ slug: modelKey.slice(separatorIndex + 1),
+ };
+};
+
const EMPTY_MODEL_JUMP_LABELS = new Map<string, string>();- const [provider, slug] = targetModelKey.split(":") as [ProviderKind, string];
+ const parsedTarget = parseModelPickerKey(targetModelKey);
+ if (!parsedTarget) {
+ return;
+ }
event.preventDefault();
event.stopPropagation();
- handleModelSelect(slug, provider);
+ handleModelSelect(parsedTarget.slug, parsedTarget.provider);- const [provider, slug] = modelKey.split(":") as [ProviderKind, string];
- handleModelSelect(slug, provider);
+ const parsedModelKey = parseModelPickerKey(modelKey);
+ if (!parsedModelKey) {
+ return;
+ }
+ handleModelSelect(parsedModelKey.slug, parsedModelKey.provider);- const [provider, slug] = highlightedModelKeyRef.current.split(":") as [
- ProviderKind,
- string,
- ];
- handleModelSelect(slug, provider);
+ const parsedModelKey = parseModelPickerKey(highlightedModelKeyRef.current);
+ if (!parsedModelKey) {
+ return;
+ }
+ handleModelSelect(parsedModelKey.slug, parsedModelKey.provider);Also applies to: 429-434, 461-471
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ModelPickerContent.tsx` around lines 330 - 337,
The handler is incorrectly using targetModelKey.split(":"), which breaks slugs
that contain colons; update the parsing of targetModelKey (used where
modelJumpModelKeys and jumpIndex are referenced and in other handlers around
lines noted) to split only on the first ":"—e.g., find the first colon index and
take the substring before as provider and the remainder as slug—then call
handleModelSelect(slug, provider); alternatively centralize encode/decode logic
for keys (provider:slug) to a single helper used by these handlers to avoid
truncation bugs (refer to ProviderKind, targetModelKey, modelJumpModelKeys,
jumpIndex, and handleModelSelect).
Comment on lines
+156
to
+207
| {/* Gemini button (coming soon) */} | ||
| <Tooltip> | ||
| <TooltipTrigger | ||
| render={ | ||
| <span className="relative block w-full"> | ||
| <button | ||
| className={cn( | ||
| "relative isolate flex w-full aspect-square items-center justify-center rounded opacity-50 cursor-not-allowed transition-colors hover:bg-transparent", | ||
| )} | ||
| disabled | ||
| type="button" | ||
| data-model-picker-provider="gemini-coming-soon" | ||
| aria-label="Gemini — coming soon" | ||
| > | ||
| <Gemini className="size-5 text-muted-foreground/85" aria-hidden /> | ||
| <span className={SOON_BADGE_CLASS} aria-hidden> | ||
| <Clock3Icon className="size-2" /> | ||
| </span> | ||
| </button> | ||
| </span> | ||
| } | ||
| /> | ||
| <TooltipPopup side={PICKER_TOOLTIP_SIDE} align="center" className={PICKER_TOOLTIP_CLASS}> | ||
| Gemini — Coming soon | ||
| </TooltipPopup> | ||
| </Tooltip> | ||
| {/* Github Copilot button (coming soon) */} | ||
| <Tooltip> | ||
| <TooltipTrigger | ||
| render={ | ||
| <span className="relative block w-full"> | ||
| <button | ||
| className={cn( | ||
| "relative isolate flex w-full aspect-square items-center justify-center rounded opacity-50 cursor-not-allowed transition-colors hover:bg-transparent", | ||
| )} | ||
| disabled | ||
| type="button" | ||
| data-model-picker-provider="github-copilot-coming-soon" | ||
| aria-label="Github Copilot — coming soon" | ||
| > | ||
| <GithubCopilotIcon className="size-5 text-muted-foreground/85" aria-hidden /> | ||
| <span className={SOON_BADGE_CLASS} aria-hidden> | ||
| <Clock3Icon className="size-2" /> | ||
| </span> | ||
| </button> | ||
| </span> | ||
| } | ||
| /> | ||
| <TooltipPopup side={PICKER_TOOLTIP_SIDE} align="center" className={PICKER_TOOLTIP_CLASS}> | ||
| Github Copilot — Coming soon | ||
| </TooltipPopup> | ||
| </Tooltip> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether fork's AVAILABLE_PROVIDER_OPTIONS already covers geminiCli and copilot.
fd -t f 'providerIconUtils' | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'
echo "---"
# Sanity: grep values named in AVAILABLE_PROVIDER_OPTIONS
rg -n -C2 --type=ts 'AVAILABLE_PROVIDER_OPTIONS\s*[:=]'
rg -n -C1 --type=ts "value:\s*['\"](geminiCli|copilot)['\"]"Repository: aaditagrawal/t3code
Length of output: 3938
🏁 Script executed:
cat -n apps/web/src/components/chat/ModelPickerSidebar.tsx | sed -n '80,160p'Repository: aaditagrawal/t3code
Length of output: 3434
Remove hardcoded "coming soon" buttons for Gemini and Github Copilot.
The fork's AVAILABLE_PROVIDER_OPTIONS now includes both copilot and geminiCli as available providers, which means they render as live buttons in lines 85–154. The hardcoded "coming soon" buttons at lines 156–207 create duplicate sidebar entries that confuse users and conflict with the fork's actual provider availability. Remove these hardcoded buttons entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ModelPickerSidebar.tsx` around lines 156 - 207,
Remove the hardcoded "coming soon" Gemini and Github Copilot UI blocks in
ModelPickerSidebar.tsx: delete the two Tooltip/TooltipTrigger blocks that render
buttons with data-model-picker-provider="gemini-coming-soon" and
"github-copilot-coming-soon" (the blocks that include Gemini, GithubCopilotIcon,
SOON_BADGE_CLASS and Clock3Icon and corresponding TooltipPopup using
PICKER_TOOLTIP_SIDE and PICKER_TOOLTIP_CLASS). This eliminates duplicate entries
since AVAILABLE_PROVIDER_OPTIONS now includes copilot and geminiCli and those
providers are rendered elsewhere.
Comment on lines
+22
to
+27
| export const CursorIcon: Icon = ({ className, ...props }) => ( | ||
| <svg | ||
| {...props} | ||
| viewBox="0 0 466.73 532.09" | ||
| className={cn("fill-[#26251E] dark:fill-[#EDECEC]", className)} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and get its structure
fd "Icons.tsx" --type fRepository: aaditagrawal/t3code
Length of output: 96
🏁 Script executed:
# Read the file to see the IconProps definition and the relevant icon components
cat -n apps/web/src/components/Icons.tsx | head -100Repository: aaditagrawal/t3code
Length of output: 6780
🏁 Script executed:
# Get the total line count to understand file size
wc -l apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 100
🏁 Script executed:
# Check the OpenAI component around line 350
sed -n '340,365n' apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 45
🏁 Script executed:
# Check the GithubCopilotIcon component around line 686
sed -n '680,705n' apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 45
🏁 Script executed:
# Search for ClaudeAI to see how it handles monochrome
rg "ClaudeAI" apps/web/src/components/Icons.tsx -A 5Repository: aaditagrawal/t3code
Length of output: 296
🏁 Script executed:
# Use correct sed syntax to view the OpenAI component around line 350
sed -n '340,365p' apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 2535
🏁 Script executed:
# Check the GithubCopilotIcon component around line 686
sed -n '680,705p' apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 2691
🏁 Script executed:
# Search for all Icon components and their destructuring patterns
rg "export const \w+: Icon = \(" apps/web/src/components/Icons.tsx -A 3Repository: aaditagrawal/t3code
Length of output: 5480
🏁 Script executed:
# Verify exact line numbers for the three components
grep -n "export const CursorIcon\|export const OpenAI\|export const GithubCopilotIcon" apps/web/src/components/Icons.tsxRepository: aaditagrawal/t3code
Length of output: 265
Strip monochrome before spreading SVG props.
These icons accept IconProps, so callers can pass monochrome; because it is not destructured, it gets forwarded to <svg>. Strip it like ClaudeAI does to avoid leaking a non-SVG prop to the DOM.
Proposed fix
-export const CursorIcon: Icon = ({ className, ...props }) => (
+export const CursorIcon: Icon = ({ monochrome: _monochrome, className, ...props }) => (
<svg
{...props}
viewBox="0 0 466.73 532.09"
className={cn("fill-[`#26251E`] dark:fill-[`#EDECEC`]", className)}
>-export const OpenAI: Icon = ({ className, ...props }) => (
+export const OpenAI: Icon = ({ monochrome: _monochrome, className, ...props }) => (
<svg
{...props}
preserveAspectRatio="xMidYMid"
viewBox="0 0 256 260"
className={cn("fill-black dark:fill-white", className)}
>-export const GithubCopilotIcon: Icon = ({ className, ...props }) => (
+export const GithubCopilotIcon: Icon = ({ monochrome: _monochrome, className, ...props }) => (
<svg
{...props}
preserveAspectRatio="xMidYMid"
viewBox="0 0 256 208"
className={cn("fill-black dark:fill-white", className)}
>📝 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.
Suggested change
| export const CursorIcon: Icon = ({ className, ...props }) => ( | |
| <svg | |
| {...props} | |
| viewBox="0 0 466.73 532.09" | |
| className={cn("fill-[#26251E] dark:fill-[#EDECEC]", className)} | |
| > | |
| export const CursorIcon: Icon = ({ monochrome: _monochrome, className, ...props }) => ( | |
| <svg | |
| {...props} | |
| viewBox="0 0 466.73 532.09" | |
| className={cn("fill-[`#26251E`] dark:fill-[`#EDECEC`]", className)} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/Icons.tsx` around lines 22 - 27, The CursorIcon
component is forwarding a non-SVG prop `monochrome` to the <svg> because Icon
props aren't destructured; update CursorIcon (and other icons that follow this
pattern) to strip `monochrome` from the props before spreading to the svg—e.g.,
destructure { className, monochrome, ...props } in the CursorIcon function
signature or remove monochrome from the object prior to {...props} so the svg
doesn't receive a non-standard DOM attribute (follow the same approach used by
ClaudeAI).
Comment on lines
473
to
480
| function syncProjectUiFromStore() { | ||
| const projects = selectProjectsAcrossEnvironments(useStore.getState()); | ||
| const clientSettings = getClientSettings(); | ||
| useUiStateStore.getState().syncProjects( | ||
| projects.map((project) => ({ | ||
| key: derivePhysicalProjectKey(project), | ||
| logicalKey: deriveLogicalProjectKeyFromSettings(project, clientSettings), | ||
| cwd: project.cwd, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether settings changes trigger project UI resync after logical keys became settings-derived.
# Expected: a settings update path calls useUiStateStore.getState().syncProjects(...) or otherwise invokes the project sync path.
rg -n -C3 '\bsyncProjects\s*\('
rg -n -C3 'sidebarProjectGroupingMode|sidebarProjectGroupingOverrides'
rg -n -C3 'deriveLogicalProjectKeyFromSettings|getClientSettings'Repository: aaditagrawal/t3code
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Find all calls to syncProjectUiFromStore to understand when project UI sync occurs
rg -n 'syncProjectUiFromStore'Repository: aaditagrawal/t3code
Length of output: 305
🏁 Script executed:
#!/bin/bash
# Check useUpdateSettings hook to see if it triggers any project UI sync
rg -n -A10 'useUpdateSettings|persistClientSettings' apps/web/src/hooks/useSettings.tsRepository: aaditagrawal/t3code
Length of output: 2129
🏁 Script executed:
#!/bin/bash
# Search for subscriptions or listeners on settings that might trigger project UI sync
rg -n -C5 'subscribeClientSettings|clientSettingsSnapshot' apps/web/src/Repository: aaditagrawal/t3code
Length of output: 3495
🏁 Script executed:
#!/bin/bash
# Check context around line 499 where syncProjectUiFromStore is called
sed -n '490,510p' apps/web/src/environments/runtime/service.tsRepository: aaditagrawal/t3code
Length of output: 770
🏁 Script executed:
#!/bin/bash
# Check context around line 617 where syncProjectUiFromStore is called
sed -n '605,625p' apps/web/src/environments/runtime/service.tsRepository: aaditagrawal/t3code
Length of output: 865
🏁 Script executed:
#!/bin/bash
# Search for any subscriptions to settings changes in the runtime service
rg -n 'clientSettingsListeners|subscribeClientSettings|addEventListener.*settings' apps/web/src/environments/runtime/service.tsRepository: aaditagrawal/t3code
Length of output: 45
Resync project UI when grouping settings change.
logicalKey now depends on getClientSettings() (lines 475, 550), but these paths only run on project events or snapshot reconciliation. When sidebarProjectGroupingMode or sidebarProjectGroupingOverrides change independently, expansion/collapse state remains keyed by stale logical IDs until the next project event.
Consider adding a settings change listener in the runtime service to re-run project UI sync, or confirm the app design accepts stale grouping until the next sync trigger.
Also applies to: 549-554
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/environments/runtime/service.ts` around lines 473 - 480,
syncProjectUiFromStore builds logicalKey using getClientSettings(), so UI keys
go stale when grouping settings change; add a settings change listener that
calls syncProjectUiFromStore whenever sidebarProjectGroupingMode or
sidebarProjectGroupingOverrides change (or re-run the same sync logic) to
re-compute logical IDs and call useUiStateStore.getState().syncProjects again;
locate this near the runtime service initialization where
selectProjectsAcrossEnvironments, getClientSettings,
deriveLogicalProjectKeyFromSettings, and syncProjectUiFromStore are defined and
wire the listener to the settings store or event emitter so changes immediately
trigger re-sync.
Comment on lines
+134
to
+140
| /** | ||
| * Non-hook accessor for the current merged client settings snapshot. | ||
| * Used by non-React code paths (e.g. runtime services) that need the latest | ||
| * settings without subscribing. | ||
| */ | ||
| export function getClientSettings(): ClientSettings { | ||
| return getClientSettingsSnapshot(); |
There was a problem hiding this comment.
Hydrate before serving non-hook settings.
Line 140 returns the in-memory snapshot, but hydration is only started by subscribeClientSettings(). A runtime service that calls getClientSettings() before any useSettings() subscriber mounts will see DEFAULT_CLIENT_SETTINGS, not the persisted client settings.
Proposed direction
-export function getClientSettings(): ClientSettings {
+export async function getClientSettings(): Promise<ClientSettings> {
+ await hydrateClientSettings();
return getClientSettingsSnapshot();
}Callers that need a synchronous snapshot should use a clearly named snapshot-only accessor, while runtime initialization paths should await the hydrated accessor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/hooks/useSettings.ts` around lines 134 - 140,
getClientSettings() currently returns the in-memory snapshot
(getClientSettingsSnapshot()) and can return DEFAULT_CLIENT_SETTINGS if
hydration hasn't started; change the API so there are two clear accessors: keep
getClientSettingsSnapshot() as the synchronous snapshot-only accessor, and make
getClientSettings() an async/hydrating accessor that ensures persisted settings
are loaded before returning (implement an ensureClientSettingsHydrated() or
call/await subscribeClientSettings() startup/hydration flow inside
getClientSettings()), and update callers: runtime init code should await
getClientSettings(), while synchronous callers that truly need a snapshot should
call getClientSettingsSnapshot(); update useSettings() subscribers to continue
starting hydration as before.
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
10 participants
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
Merges 10 new upstream commits from `pingdotgg/t3code` with proper ancestry recording.
Upstream additions:
effect-codex-app-serverpingdotgg/t3code#1942) — replaces old manager-based architectureFork adaptations:
Test plan
Summary by CodeRabbit
New Features
UI Improvements