feat: Copilot interaction mode, plan support, and usage UI improvements#4
Merged
aaditagrawal merged 4 commits intomainfrom Mar 9, 2026
Merged
Conversation
- Add Copilot plan/default interaction mode switching via session.rpc.mode.set - Emit proposed plan events (turn.plan.updated, turn.proposed.completed) on plan changes - Rewrite Copilot health check to detect auth status before fetching models/quota - Map Codex secondary rate limit bucket to weekly for newer CLI compatibility - Add CopilotAdapter test suite for interaction mode and plan event emission - Refine sidebar usage bars: grouped Codex quotas, Copilot quota prioritization, visual polish - Add Git & GitHub fork safety policy to AGENTS.md
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds Copilot interaction-mode and plan RPCs, reshapes Copilot health probing, maps Codex secondary limits into a weekly quota, centralizes thread provider inference, updates sidebar quota UI, adds related tests, and introduces a Git & GitHub policy note and app settings for grayscale provider logos. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Adapter
participant Session as CopilotSession (RPC)
participant Manager as CopilotAdapter / EventHandler
participant Subscriber as Event Subscriber
Client->>Manager: sendTurn(input)
Manager->>Manager: syncInteractionMode(record, input.mode?)
Manager->>Session: rpc.mode.set({mode})
Session-->>Manager: {mode}
Manager->>Session: issueTurn(...)
Session->>Manager: emit session.mode_changed
Manager->>Manager: update record.interactionMode
Session->>Manager: emit session.plan_changed
Manager->>Session: rpc.plan.read()
Session-->>Manager: {exists, content, path}
Manager->>Manager: emitLatestProposedPlan(record)
Manager->>Subscriber: emit proposed-plan event
Subscriber-->>Client: receive proposed-plan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
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/CopilotAdapter.ts (1)
1128-1133:⚠️ Potential issue | 🟠 MajorReset
interactionModewhen you swap out the session handle.
reconfigureSession()keepsrecord.interactionModefrom the destroyed session. If the user is in plan mode and a turn triggers reconfiguration,sendTurn()will immediately see"plan"already recorded and skipsession.rpc.mode.set()on the new session, so that first post-reconfigure turn runs in the SDK's default interactive mode instead.🛠️ Proposed fix
record.session = nextSession; + record.interactionMode = undefined; record.model = input.model; record.reasoningEffort = input.reasoningEffort; record.updatedAt = new Date().toISOString();Also applies to: 1427-1428
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/provider/Layers/CopilotAdapter.ts` around lines 1128 - 1133, When swapping the session handle in reconfigureSession, reset record.interactionMode to undefined (or the SDK default) so the new session doesn't inherit the old session's mode; update the block where you assign record.session = nextSession (and the analogous spot around the other session-swap) to set record.interactionMode = undefined after assigning nextSession and before wiring nextSession.on, ensuring sendTurn and session.rpc.mode.set will correctly initialize the mode on the fresh session.
🤖 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/provider/Layers/CodexAdapter.test.ts`:
- Around line 221-251: The test fails because CodexAdapter's fetchCodexUsage
builds the response using the misspelled identifier "quoas" instead of "quotas",
causing a ReferenceError; update the return payload construction in
fetchCodexUsage (and any other references in CodexAdapter.ts) to use the correct
"quotas" identifier so that usage.quotas is populated and usage.quota =
usage.quotas?.[0] works as intended.
In `@apps/server/src/provider/Layers/ProviderHealth.ts`:
- Around line 483-493: ProviderHealth currently calls getStatus() without
existence checks or a .catch(), so any failure aborts the probe; update the
block that calls (client as ...).getStatus(), getAuthStatus(), and
rpc.account.getQuota() to first check those methods exist before invoking them,
wrap each call in .catch(() => undefined), and ensure the Promise.all() used to
fetch [status, authStatus] and later [models, quota] returns undefined on
missing methods or errors so the probe can continue to report auth-driven
warning/error states; apply the same existence-check + .catch(() => undefined)
pattern to the rpc.account.getQuota() access in CopilotAdapter (around the
rpc.account.getQuota() call referenced in CopilotAdapter.ts:1714).
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 450-452: The reset-date <p> uses only group-hover (class
"group-hover/bar:opacity-100") so keyboard/touch users can't reveal it; update
the reveal behavior to also respond to focus (add
"group-focus-within/bar:opacity-100" or similar) and ensure the surrounding card
container (the element using the "group/bar" class) is keyboard-focusable (e.g.,
add tabIndex={0} or include a focusable control) so focusable descendants exist;
apply the same change to the other occurrence that uses formatUsageResetLabel
(lines ~529-531).
---
Outside diff comments:
In `@apps/server/src/provider/Layers/CopilotAdapter.ts`:
- Around line 1128-1133: When swapping the session handle in reconfigureSession,
reset record.interactionMode to undefined (or the SDK default) so the new
session doesn't inherit the old session's mode; update the block where you
assign record.session = nextSession (and the analogous spot around the other
session-swap) to set record.interactionMode = undefined after assigning
nextSession and before wiring nextSession.on, ensuring sendTurn and
session.rpc.mode.set will correctly initialize the mode on the fresh session.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7630e03e-4213-444b-b935-4834298f5d0e
📒 Files selected for processing (9)
AGENTS.mdapps/server/src/codexAppServerManager.test.tsapps/server/src/codexAppServerManager.tsapps/server/src/provider/Layers/CodexAdapter.test.tsapps/server/src/provider/Layers/CopilotAdapter.test.tsapps/server/src/provider/Layers/CopilotAdapter.tsapps/server/src/provider/Layers/ProviderHealth.tsapps/server/src/provider/Layers/copilotTurnTracking.tsapps/web/src/components/Sidebar.tsx
Comment on lines
+221
to
+251
| it.effect("maps Codex secondary rate limit bucket into weekly usage", () => | ||
| Effect.gen(function* () { | ||
| validationManager.readRateLimitsImpl.mockResolvedValueOnce({ | ||
| primary: { | ||
| usedPercent: 4, | ||
| windowDurationMins: 300, | ||
| resetsAt: 1_773_075_410, | ||
| }, | ||
| weekly: { | ||
| usedPercent: 44, | ||
| windowDurationMins: 10_080, | ||
| resetsAt: 1_773_532_873, | ||
| }, | ||
| }); | ||
|
|
||
| const usage = yield* Effect.promise(() => fetchCodexUsage()); | ||
|
|
||
| assert.equal(usage.provider, "codex"); | ||
| assert.deepStrictEqual(usage.quotas, [ | ||
| { | ||
| plan: "Session (5 hrs)", | ||
| percentUsed: 4, | ||
| resetDate: "2026-03-09T16:56:50.000Z", | ||
| }, | ||
| { | ||
| plan: "Weekly", | ||
| percentUsed: 44, | ||
| resetDate: "2026-03-15T00:01:13.000Z", | ||
| }, | ||
| ]); | ||
| assert.deepStrictEqual(usage.quota, usage.quotas?.[0]); |
There was a problem hiding this comment.
fetchCodexUsage() still throws on the happy path.
This new test calls fetchCodexUsage(), but apps/server/src/provider/Layers/CodexAdapter.ts still builds the return payload with quoas.length instead of quotas.length. Any non-empty quota response will raise a ReferenceError before these assertions run.
🐛 Minimal fix in apps/server/src/provider/Layers/CodexAdapter.ts
- ...(quoas.length > 0 ? { quotas } : {}),
+ ...(quotas.length > 0 ? { quotas } : {}),🤖 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 221 - 251,
The test fails because CodexAdapter's fetchCodexUsage builds the response using
the misspelled identifier "quoas" instead of "quotas", causing a ReferenceError;
update the return payload construction in fetchCodexUsage (and any other
references in CodexAdapter.ts) to use the correct "quotas" identifier so that
usage.quotas is populated and usage.quota = usage.quotas?.[0] works as intended.
Comment on lines
+483
to
+493
| const [status, authStatus] = await Promise.all([ | ||
| (client as unknown as { getStatus(): Promise<{ version?: string }> }).getStatus(), | ||
| (client as unknown as { getAuthStatus(): Promise<{ isAuthenticated?: boolean; statusMessage?: string }> }).getAuthStatus().catch(() => undefined), | ||
| ]); | ||
| const [models, quota] = | ||
| authStatus?.isAuthenticated === true | ||
| ? await Promise.all([ | ||
| client.listModels().catch(() => undefined), | ||
| (client as unknown as { rpc: { account: { getQuota: () => Promise<{ quotaSnapshots?: unknown }> } } }).rpc.account.getQuota().catch(() => undefined), | ||
| ]) | ||
| : [undefined, undefined]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd 'copilot-sdk.d.ts$' . -x sh -c 'echo "== $1 =="; sed -n "1,220p" "$1"' sh {}
rg -n 'getStatus|getAuthStatus|getQuota|rpc' apps/server/srcRepository: aaditagrawal/t3code
Length of output: 4960
🏁 Script executed:
sed -n '475,500p' apps/server/src/provider/Layers/ProviderHealth.ts | cat -n
sed -n '1710,1720p' apps/server/src/provider/Layers/CopilotAdapter.ts | cat -n
grep -A 5 -B 5 'public readonly rpc' apps/server/src/provider/Layers/CopilotAdapter.test.ts | head -20Repository: aaditagrawal/t3code
Length of output: 2278
Unguarded getStatus() call breaks the granular health probe logic.
The Copilot SDK surface in copilot-sdk.d.ts only declares listModels(), but the probe calls getStatus(), getAuthStatus(), and rpc.account.getQuota() through type casts. Only getAuthStatus() and the nested calls are protected with .catch() handlers; getStatus() in the Promise.all() lacks error handling, so any failure (method missing or network timeout) rejects the entire probe and reports Copilot as unavailable instead of returning the auth-driven warning/error state this refactor attempts to preserve.
Wrap all three calls with existence checks and .catch() handlers to match the expected behavior:
- Check that
getStatus,getAuthStatus, andrpc.account.getQuotaexist before calling - Return
undefinedif any call fails or the method doesn't exist - Let the probe continue and report granular auth state regardless
Also review and apply the same pattern in CopilotAdapter.ts:1714 where rpc.account.getQuota() is accessed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/ProviderHealth.ts` around lines 483 - 493,
ProviderHealth currently calls getStatus() without existence checks or a
.catch(), so any failure aborts the probe; update the block that calls (client
as ...).getStatus(), getAuthStatus(), and rpc.account.getQuota() to first check
those methods exist before invoking them, wrap each call in .catch(() =>
undefined), and ensure the Promise.all() used to fetch [status, authStatus] and
later [models, quota] returns undefined on missing methods or errors so the
probe can continue to report auth-driven warning/error states; apply the same
existence-check + .catch(() => undefined) pattern to the rpc.account.getQuota()
access in CopilotAdapter (around the rpc.account.getQuota() call referenced in
CopilotAdapter.ts:1714).
- Add .catch() to getStatus() in Copilot health probe to prevent unhandled rejections from aborting the entire probe - Reset interactionMode on session reconfiguration so new sessions don't inherit stale mode from destroyed sessions
Remove hover-only opacity on reset date labels so they are accessible to keyboard and touch users without requiring a mouse hover.
…action - Extract inferProviderForThreadModel and resolveThreadProvider into shared lib/threadProvider module, removing duplicate logic from store - Show provider icon next to each thread in the sidebar with tooltip - Add grayscaleProviderLogos app setting with settings UI toggle - Persist thread.provider field so closed sessions retain their provider - Infer provider on draft thread creation from the selected model - Always show Copilot percent labels in usage bars
This was referenced Mar 10, 2026
This was referenced Mar 18, 2026
This was referenced Mar 28, 2026
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
1 participant
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
session.rpc.mode.set(), synced before each turn. Emitturn.plan.updatedandturn.proposed.completedevents when plans change.getAuthStatus) before fetching models/quota, with granular error states (error vs timeout vs unauthenticated).secondarybucket toweeklyfor backwards compatibility.Test plan
bun lintpasses (0 errors)bun typecheckpasses (0 errors across 7 packages)bun run testforcodexAppServerManager.test.ts— verify rate limit mappingbun run testforCodexAdapter.test.ts— verify secondary→weekly usage mappingbun run testforCopilotAdapter.test.ts— verify interaction mode switching and plan event emissionSummary by CodeRabbit
New Features
Improvements
Tests
Documentation