Repository navigation
perf(provider): guide safe read-only shell batching (#28589) - #29977
MattRiddell wants to merge 13 commits into
Conversation
Ports gfsaaser24/t3code PRs 77, 78, 79, 80 (all merged in that fork) onto our fork of pingdotgg/t3code. Upstream pingdotgg#4125 was closed; the fork port is restructured so orchestrator v2 cannot kill it. - provider/openrouter/: self-contained transport module (env ownership, base-URL normalization, live catalog fetch with fallback list). - Drivers/OpenRouterDriver.ts: the V1 driver registration, the only part that retires at the v2 cutover. - ClaudeAdapter.ts is NOT modified; driver identity is re-stamped by a decorator so the churn-heavy file stays merge-clean against upstream. - Web: OpenRouter icon, picker option, settings form, contracts. - PR 78: translate the provider on startSession input. - PR 79: disable Claude Code tool search for OpenRouter sessions. - PR 80: live settings on status checks, API key in the secret store. The fork's turbo-only files were deliberately excluded: .t3-turbo/, SEAM.md, scripts/turbo-customization-manifest.test.ts, and the release.yml TURBO_RELEASE_BRANCH plumbing. Our fork keeps OpenRouter on main so the daily upstream merge has one branch to maintain. Board pingdotgg#7907. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-sync # Conflicts: # apps/server/src/provider/builtInDrivers.ts # apps/web/src/components/chat/providerIconUtils.ts # apps/web/src/components/settings/providerDriverMeta.ts # apps/web/src/lib/contextWindow.ts # apps/web/src/session-logic.ts # packages/contracts/src/model.ts # packages/contracts/src/settings.ts
Upstream removed makeStaticProviderMaintenanceResolver and moved makeManagedServerProvider from a resolved `maintenanceCapabilities` value to a `resolveMaintenance` thunk. OpenRouterDriver still called the deleted helper, so importing builtInDrivers threw at module load and ProviderRegistry.test.ts failed. OpenRouter runs on the Claude Agent CLI and never owns an install, so its capabilities are constant. Use the same manual-only constant that GrokDriver uses and hand it to resolveMaintenance. Board pingdotgg#9630.
Carry the persisted turn into provider recovery and reconcile it against live OpenCode status before changing lifecycle state. Cover provider exits and idle, busy, and unknown resume states. Board: pingdotgg#11564
fix(server): settle stale turns when OpenCode resumes
| }, | ||
| }), | ||
| ), | ||
| customModels: Schema.Array(Schema.String).pipe( |
There was a problem hiding this comment.
🟠 High src/settings.ts:778
Saving a renamed or configured OpenRouter custom model makes the entire provider unavailable, including after restart, because updateCustomModels serializes entries as { slug, name, ... } but this schema accepts only strings, so decoding the saved settings fails. Use CustomModelSetting here so those entries, including in OpenRouterSettingsPatch, decode successfully.
| customModels: Schema.Array(Schema.String).pipe( | |
| customModels: Schema.Array(CustomModelSetting).pipe( |
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/src/settings.ts around line 778:
Saving a renamed or configured OpenRouter custom model makes the entire provider unavailable, including after restart, because `updateCustomModels` serializes entries as `{ slug, name, ... }` but this schema accepts only strings, so decoding the saved settings fails. Use `CustomModelSetting` here so those entries, including in `OpenRouterSettingsPatch`, decode successfully.
| const storedApiKey = Option.isSome(stored) | ||
| ? new TextDecoder().decode(stored.value) | ||
| : undefined; | ||
| const apiKey = resolveOpenRouterApiKey({ |
There was a problem hiding this comment.
🟠 High Drivers/OpenRouterDriver.ts:128
Clearing apiKey does not disconnect OpenRouter: when the settings form omits the cleared value, resolveOpenRouterApiKey falls back to the stored secret, so the old credential continues authenticating after registry recreation or a server restart. Preserve the explicit-clear state and remove the stored secret instead of treating the omitted key as a request to load it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/OpenRouterDriver.ts around line 128:
Clearing `apiKey` does not disconnect OpenRouter: when the settings form omits the cleared value, `resolveOpenRouterApiKey` falls back to the stored secret, so the old credential continues authenticating after registry recreation or a server restart. Preserve the explicit-clear state and remove the stored secret instead of treating the omitted key as a request to load it.
|
|
||
| const updatedAt = yield* nowIso; | ||
| if (running) { | ||
| context.activeTurnId = turnId; |
There was a problem hiding this comment.
🟡 Medium Layers/OpenCodeAdapter.ts:1504
A resumed thread stays running indefinitely if the turn finishes while session.status is in flight: this function applies the stale busy snapshot after the final idle event was ignored because context.activeTurnId was unset. Adopt the turn before querying and reconcile events received during the request before applying its snapshot.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OpenCodeAdapter.ts around line 1504:
A resumed thread stays `running` indefinitely if the turn finishes while `session.status` is in flight: this function applies the stale `busy` snapshot after the final idle event was ignored because `context.activeTurnId` was unset. Adopt the turn before querying and reconcile events received during the request before applying its snapshot.
| } | ||
| } | ||
|
|
||
| if (models.length === 0) { |
There was a problem hiding this comment.
🟡 Medium openrouter/OpenRouterModels.ts:152
A successful response with no usable model IDs is returned as a successful fetch, so provider status reports ready and authenticated with a synthetic default model instead of surfacing the empty-catalog warning. The default is inserted into models before this check, making models.length === 0 unreachable; check catalog.length here so an empty discovered catalog is rejected.
- if (models.length === 0) {
+ if (catalog.length === 0) {🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/openrouter/OpenRouterModels.ts around line 152:
A successful response with no usable model IDs is returned as a successful fetch, so provider status reports `ready` and `authenticated` with a synthetic default model instead of surfacing the empty-catalog warning. The default is inserted into `models` before this check, making `models.length === 0` unreachable; check `catalog.length` here so an empty discovered catalog is rejected.
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — Beyond shell-batching guidance, this comparison adds OpenRouter with enabled-by-default settings and default models. Human review must confirm those product defaults and the broader scope, which the instructions-only description does not explain. Needs human approval because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Internal change: Shared provider instructions only; no visible UI or live install
Problem and change
Separate shell tool calls for already-known independent reads add provider shell starts. Shared runtime instructions now prefer one shell call for those reads, while keeping dependent commands, mutations, approvals and different permissions separate. The guidance preserves command output identity and individual failure reporting. It does not automatically coalesce tool calls or change provider execution.
Board #28589; Matt q6206:1 assigned isolated MattRiddell/t3code source work. Live T3 state, service changes and deployment remain outside this lane's authorization.
Validation
Passive 60-second BEFORE census: 97.6 starts/s; running/ready lanes 22 to 21, mean 21.5; 4.539 starts/s/lane; system+irq+softirq 3.0292%. Raw JSON is attached to #28589 (53993), #19810 (53996) and #24236 (53999), SHA256 8adcc75691f671b7d62470e65e07601b45bc06c6746f21f184cfae662867e0a2. The baseline already meets the absolute target before this change; no reduction is proven.
POST-DEPLOY OWED: controller deploys the accepted source revision; owning lane repeats the passive 60-second census with endpoint lane counts, normalized starts/s/lane and the retained absolute under150 target. Post the result on #28589, #24236 and #19810. These tickets remain open until the required evidence exists.
Rollback: controller reverts this commit and deploys the reverted source revision through the normal T3 delivery process. No lane install or service restart.
Model: GPT-6.1 Sol. Harness: Codex through T3 Code.
Ticket: #28589