Integrate Gemini PR 54 onto current main#58
Merged
aaditagrawal merged 6 commits intomainfrom Apr 11, 2026
Merged
Conversation
…d persistence layers
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR centralizes cross-platform command resolution, refactors Gemini CLI process spawning and interrupt handling, normalizes legacy provider names across runtime and persistence (with migrations and tests), consolidates model-option logic for the web UI, and updates docs and contributor guidance. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/components/ui/sidebar.tsx (1)
113-133:⚠️ Potential issue | 🟡 MinorAdd
openProptosetOpendependencies to prevent stale closure during mode transitions.
setOpenreadsopenPropat line 120 to determine whether to update internal state, butopenPropis missing from theuseCallbackdependency array. WhenopenPropchanges butopenremains the same (e.g., during controlled ↔ uncontrolled transitions), the callback won't be recreated and will reference a staleopenPropvalue.Suggested fix
const setOpen = React.useCallback( async (value: boolean | ((value: boolean) => boolean)) => { const openState = typeof value === "function" ? value(open) : value; if (setOpenProp) { setOpenProp(openState); } if (openProp === undefined) { _setOpen(openState); } // This sets the cookie to keep the sidebar state. await cookieStore.set({ expires: Date.now() + SIDEBAR_COOKIE_MAX_AGE * 1000, name: SIDEBAR_COOKIE_NAME, path: "/", value: String(openState), }); }, - [setOpenProp, open], + [setOpenProp, open, openProp], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/sidebar.tsx` around lines 113 - 133, The setOpen callback captures openProp but doesn’t include it in its dependency array, causing a stale closure when controlled/uncontrolled mode changes; update the dependency list of the React.useCallback that defines setOpen to include openProp so the callback is recreated when openProp changes, ensuring the conditional that calls _setOpen(openState) behaves correctly; keep the existing logic that sets the cookie (cookieStore.set) and references like SIDEBAR_COOKIE_MAX_AGE, SIDEBAR_COOKIE_NAME and _setOpen unchanged.apps/web/src/components/settings/SettingsPanels.tsx (1)
762-772:⚠️ Potential issue | 🟡 MinorBuilt-in duplicate checks currently depend on a live snapshot.
This only looks at
serverProviders, so a provider with no published model list can still save a built-in slug as “custom”. Now thatproviderModelOptionsalready carries the fallback catalog, use that merged list for the duplicate check instead of live models only.💡 Proposed fix
- if ( - serverProviders - .find((candidate) => candidate.provider === provider) - ?.models.some((option) => !option.isCustom && option.slug === normalized) - ) { + if ( + providerModelOptions[provider].some( + (option) => option.isCustom !== true && option.slug === normalized, + ) + ) { setCustomModelErrorByProvider((existing) => ({ ...existing, [provider]: "That model is already built in.", })); return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/settings/SettingsPanels.tsx` around lines 762 - 772, The duplicate-built-in check currently inspects only serverProviders; update it to use the merged/fallback list found in providerModelOptions instead so providers without a live published list still block built-in slugs. Replace the search over serverProviders with a lookup into providerModelOptions for the given provider (e.g., providerModelOptions[provider] or find by .provider) and test its .models array for any non-custom option whose slug matches normalized, then call setCustomModelErrorByProvider(...) as before if a match is found.packages/contracts/src/model.ts (1)
369-397: 🛠️ Refactor suggestion | 🟠 MajorMove these provider model tables out of
@t3tools/contracts.The new opencode/kilo catalogs, defaults, and alias maps add more runtime provider metadata to the contracts package. That keeps pushing
@t3tools/contractsbeyond schemas/types and into runtime behavior, which makes the package boundary harder to maintain.As per coding guidelines,
packages/contracts/**/*.ts: Keep the@t3tools/contractspackage schema-only — no runtime logic.Also applies to: 410-413, 486-501
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/contracts/src/model.ts` around lines 369 - 397, The arrays opencode, geminiCli, amp, kilo and the related catalog/default/alias maps are runtime provider metadata and must be removed from the contracts package; extract these constants into a new runtime package (e.g., a providers/provider-models package) and export them from there, leaving only types/interfaces in packages/contracts (keep contracts schema-only). Update all imports that referenced opencode/geminiCli/amp/kilo or the alias maps to point to the new runtime package, and ensure packages/contracts/src/model.ts retains only type definitions (no runtime tables or default data).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/commandPath.ts`:
- Around line 42-61: The current logic drops the original command when it has an
extension not in PATHEXT; update the candidate generation in the
extension-present branch (variables: extension, normalizedExtension,
windowsPathExtensions, commandWithoutExtension) to always include the original
command string as a candidate. Concretely, when extension.length > 0, return a
Set that contains command plus the existing variants (`commandWithoutExtension +
normalizedExtension` and its lowercase) so explicit paths with unknown
extensions are preserved.
In `@apps/web/src/components/settings/SettingsPanels.tsx`:
- Around line 1215-1217: The current lookup uses PROVIDER_LABEL_BY_PROVIDER
which contains entries for every provider and therefore always overrides
providerCard.title; change the logic in the providerDisplayName assignment (the
providerDisplayName variable) to render providerCard.title directly (or flip the
precedence so providerCard.title is used before the map), or alternatively
update the shared PROVIDER_LABEL_BY_PROVIDER/PROVIDER_DISPLAY_NAMES mapping
before this render so the new exact-text labels are present; locate the
providerDisplayName assignment and replace the lookup with providerCard.title
(or swap the null-coalescing order) to ensure the renamed provider titles are
actually displayed.
In `@apps/web/src/providerModelOptions.ts`:
- Around line 59-69: The Copilot branch currently replaces the built-in fallback
catalog with only the discovered snapshot (enriched + customOnly), removing any
built-in models not present in discovery; instead merge the enriched snapshot
back into the existing catalog like other providers do: compute enriched as you
already do (using baseTiers and dedupedModels), then combine/merge that enriched
list with the originals from base[provider] (preserving built-in entries not in
dedupedModels) before assigning to result[provider] so the fallback copilot
models remain available. Reference: provider variable, baseTiers, enriched,
customOnly and result[provider].
In `@CONTRIBUTING.md`:
- Line 29: Update the example GitHub CLI command in CONTRIBUTING.md to
explicitly set the base branch to main by adding the --base main flag to the
provided gh pr create command (the current example is "gh pr create --repo
aaditagrawal/t3code"); modify that line so the command reads with --base main to
ensure PRs target the fork's main branch.
---
Outside diff comments:
In `@apps/web/src/components/settings/SettingsPanels.tsx`:
- Around line 762-772: The duplicate-built-in check currently inspects only
serverProviders; update it to use the merged/fallback list found in
providerModelOptions instead so providers without a live published list still
block built-in slugs. Replace the search over serverProviders with a lookup into
providerModelOptions for the given provider (e.g.,
providerModelOptions[provider] or find by .provider) and test its .models array
for any non-custom option whose slug matches normalized, then call
setCustomModelErrorByProvider(...) as before if a match is found.
In `@apps/web/src/components/ui/sidebar.tsx`:
- Around line 113-133: The setOpen callback captures openProp but doesn’t
include it in its dependency array, causing a stale closure when
controlled/uncontrolled mode changes; update the dependency list of the
React.useCallback that defines setOpen to include openProp so the callback is
recreated when openProp changes, ensuring the conditional that calls
_setOpen(openState) behaves correctly; keep the existing logic that sets the
cookie (cookieStore.set) and references like SIDEBAR_COOKIE_MAX_AGE,
SIDEBAR_COOKIE_NAME and _setOpen unchanged.
In `@packages/contracts/src/model.ts`:
- Around line 369-397: The arrays opencode, geminiCli, amp, kilo and the related
catalog/default/alias maps are runtime provider metadata and must be removed
from the contracts package; extract these constants into a new runtime package
(e.g., a providers/provider-models package) and export them from there, leaving
only types/interfaces in packages/contracts (keep contracts schema-only). Update
all imports that referenced opencode/geminiCli/amp/kilo or the alias maps to
point to the new runtime package, and ensure packages/contracts/src/model.ts
retains only type definitions (no runtime tables or default data).
🪄 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: 9a0b63d5-4111-44e2-848e-76078496bc93
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
CONTRIBUTING.mdREADME.mdapps/desktop/package.jsonapps/server/package.jsonapps/server/src/commandPath.tsapps/server/src/geminiCliServerManager.test.tsapps/server/src/geminiCliServerManager.tsapps/server/src/open.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.tsapps/server/src/persistence/Layers/OrchestrationEventStore.test.tsapps/server/src/persistence/Layers/OrchestrationEventStore.tsapps/server/src/persistence/Migrations.tsapps/server/src/persistence/Migrations/020_NormalizeLegacyProviderKinds.test.tsapps/server/src/persistence/Migrations/020_NormalizeLegacyProviderKinds.tsapps/server/src/persistence/Migrations/021_RepairProjectionThreadProposedPlanImplementationColumns.test.tsapps/server/src/persistence/Migrations/021_RepairProjectionThreadProposedPlanImplementationColumns.tsapps/server/src/provider/Layers/CursorAdapter.tsapps/server/src/provider/Layers/ProviderSessionDirectory.test.tsapps/server/src/provider/Layers/ProviderSessionDirectory.tsapps/server/src/provider/providerKind.tsapps/web/src/components/ChatView.tsxapps/web/src/components/chat/ProviderModelPicker.logic.test.tsapps/web/src/components/chat/ProviderModelPicker.tsxapps/web/src/components/settings/SettingsPanels.browser.tsxapps/web/src/components/settings/SettingsPanels.tsxapps/web/src/components/ui/sidebar.tsxapps/web/src/providerModelOptions.tsapps/web/src/providerModels.tsapps/web/src/routes/_chat.$environmentId.$threadId.tsxpackage.jsonpackages/contracts/src/model.tspackages/shared/src/model.tspackages/shared/src/shell.test.tspackages/shared/src/shell.ts
Owner
Author
|
Pushed follow-up commit Addressed here:
Local verification:
|
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
2 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.
Supersedes the conflicted cross-repo PR #54 with a fork-owned integration branch.
Why this exists:
aaditagrawal/t3code:mainfrommtdewwolf/t3code-gemini:main.main, plus the required follow-up fixes so it remains compatible with the fork's downstream QoL and multi-provider behavior.Included here:
Validation:
bun fmtbun lintbun typecheckReference:
Summary by CodeRabbit