Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: SDK stream fiber is detached and unmanaged
- Replaced
Effect.runForkwithEffect.forkChildto keep the fiber in the managed runtime, stored the fiber reference in session context, and added explicitFiber.interruptinstopSessionInternalbefore queue teardown to eliminate the race window.
- Replaced
- ✅ Fixed: Identity function adds unnecessary indirection
- Removed the no-op
asCanonicalTurnIdidentity function and inlined theTurnIdvalue directly at all 10 call sites.
- Removed the no-op
Or push these changes by commenting:
@cursor push 2efc9d6c0c
Preview (2efc9d6c0c)
diff --git a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
--- a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
+++ b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
@@ -34,7 +34,7 @@
ThreadId,
TurnId,
} from "@t3tools/contracts";
-import { Cause, DateTime, Deferred, Effect, Layer, Queue, Random, Ref, Stream } from "effect";
+import { Cause, DateTime, Deferred, Effect, Fiber, Layer, Queue, Random, Ref, Stream } from "effect";
import {
ProviderAdapterProcessError,
@@ -106,6 +106,7 @@
lastAssistantUuid: string | undefined;
lastThreadStartedId: string | undefined;
stopped: boolean;
+ streamFiber: Fiber.Fiber<void, never> | undefined;
}
interface ClaudeQueryRuntime extends AsyncIterable<SDKMessage> {
@@ -144,10 +145,6 @@
return RuntimeItemId.makeUnsafe(value);
}
-function asCanonicalTurnId(value: TurnId): TurnId {
- return value;
-}
-
function asRuntimeRequestId(value: ApprovalRequestId): RuntimeRequestId {
return RuntimeRequestId.makeUnsafe(value);
}
@@ -505,7 +502,7 @@
...(typeof message.session_id === "string"
? { providerThreadId: message.session_id }
: {}),
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
...(itemId ? { itemId: ProviderItemId.makeUnsafe(itemId) } : {}),
payload: message,
},
@@ -613,7 +610,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(turnState ? { turnId: asCanonicalTurnId(turnState.turnId) } : {}),
+ ...(turnState ? { turnId: turnState.turnId } : {}),
payload: {
message,
class: "provider_error",
@@ -640,7 +637,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(turnState ? { turnId: asCanonicalTurnId(turnState.turnId) } : {}),
+ ...(turnState ? { turnId: turnState.turnId } : {}),
payload: {
message,
...(detail !== undefined ? { detail } : {}),
@@ -855,7 +852,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
itemId: asRuntimeItemId(tool.itemId),
payload: {
itemType: tool.itemType,
@@ -896,7 +893,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
itemId: asRuntimeItemId(tool.itemId),
payload: {
itemType: tool.itemType,
@@ -1006,7 +1003,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
providerRefs: {
...providerThreadRef(context),
...(context.turnState ? { providerTurnId: context.turnState.turnId } : {}),
@@ -1165,7 +1162,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
providerRefs: {
...providerThreadRef(context),
...(context.turnState ? { providerTurnId: context.turnState.turnId } : {}),
@@ -1295,6 +1292,11 @@
context.stopped = true;
+ if (context.streamFiber) {
+ yield* Fiber.interrupt(context.streamFiber);
+ context.streamFiber = undefined;
+ }
+
for (const [requestId, pending] of context.pendingApprovals) {
yield* Deferred.succeed(pending.decision, "cancel");
const stamp = yield* makeEventStamp();
@@ -1304,7 +1306,7 @@
provider: PROVIDER,
createdAt: stamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
requestId: asRuntimeRequestId(requestId),
payload: {
requestType: pending.requestType,
@@ -1442,7 +1444,7 @@
provider: PROVIDER,
createdAt: requestedStamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
requestId: asRuntimeRequestId(requestId),
payload: {
requestType,
@@ -1494,7 +1496,7 @@
provider: PROVIDER,
createdAt: resolvedStamp.createdAt,
threadId: context.session.threadId,
- ...(context.turnState ? { turnId: asCanonicalTurnId(context.turnState.turnId) } : {}),
+ ...(context.turnState ? { turnId: context.turnState.turnId } : {}),
requestId: asRuntimeRequestId(requestId),
payload: {
requestType,
@@ -1610,6 +1612,7 @@
lastAssistantUuid: resumeState?.resumeSessionAt,
lastThreadStartedId: undefined,
stopped: false,
+ streamFiber: undefined,
};
yield* Ref.set(contextRef, context);
sessions.set(threadId, context);
@@ -1658,7 +1661,7 @@
providerRefs: {},
});
- Effect.runFork(runSdkStream(context));
+ context.streamFiber = yield* Effect.forkChild(runSdkStream(context));
return {
...session,![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
force-pushed
the
codething/648ca884-claude
branch
from
2b53034 to
1beeff2
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Native event logger missing threadId in event payload
- Added
threadId: context.session.threadIdto the event object and passedcontext.session.threadIdinstead ofnullas the second argument tonativeEventLogger.write(), enabling per-thread log routing consistent with the Codex adapter.
- Added
Or push these changes by commenting:
@cursor push 8489584240
Preview (8489584240)
diff --git a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
--- a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
+++ b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
@@ -502,6 +502,7 @@
provider: PROVIDER,
createdAt: observedAt,
method: sdkNativeMethod(message),
+ threadId: context.session.threadId,
...(typeof message.session_id === "string"
? { providerThreadId: message.session_id }
: {}),
@@ -510,7 +511,7 @@
payload: message,
},
},
- null,
+ context.session.threadId,
);
});
juliusmarminge
force-pushed
the
codething/648ca884-claude
branch
4 times, most recently
from
4afd04a to
3af67f5
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Redundant duplicate
threadIdassignment in session construction- Removed the redundant
...(threadId ? { threadId } : {})spread at line 1587 sincethreadIdis already set directly at line 1581 and is always defined.
- Removed the redundant
Or push these changes by commenting:
@cursor push 1970102289
Preview (1970102289)
diff --git a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
--- a/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
+++ b/apps/server/src/provider/Layers/ClaudeCodeAdapter.ts
@@ -1584,7 +1584,6 @@
runtimeMode: input.runtimeMode,
...(input.cwd ? { cwd: input.cwd } : {}),
...(input.model ? { model: input.model } : {}),
- ...(threadId ? { threadId } : {}),
resumeCursor: {
...(threadId ? { threadId } : {}),
...(resumeState?.resume ? { resume: resumeState.resume } : {}),
| this.runPromise = services ? Effect.runPromiseWith(services) : Effect.runPromise; | ||
| } | ||
|
|
||
| async startSession(input: CodexAppServerStartSessionInput): Promise<ProviderSession> { |
Contributor
There was a problem hiding this comment.
🟠 High src/codexAppServerManager.ts:428
startSession overwrites this.sessions.set(threadId, context) without checking if a session already exists for that threadId. This orphans the previous process (which keeps running) and leaves its exit handler active. When the orphaned process later exits, that handler calls this.sessions.delete(threadId), which removes the new session from the map. Subsequent calls like sendTurn then fail with "Unknown session" because the entry was deleted by the stale handler.
Also found in 1 other location(s)
apps/server/src/provider/Layers/CodexAdapter.ts:1262
The
nativeEventLoggercreated whenoptions.nativeEventLogPathis provided is never closed. Whilemanageris wrapped inEffect.acquireReleasefor cleanup,nativeEventLoggeris instantiated separately and itsclose()method is never called. This causes file handles opened by the logger to leak every time theCodexAdapterlayer is released (e.g., during configuration reloads or tests).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/codexAppServerManager.ts around line 428:
`startSession` overwrites `this.sessions.set(threadId, context)` without checking if a session already exists for that `threadId`. This orphans the previous process (which keeps running) and leaves its `exit` handler active. When the orphaned process later exits, that handler calls `this.sessions.delete(threadId)`, which removes the *new* session from the map. Subsequent calls like `sendTurn` then fail with "Unknown session" because the entry was deleted by the stale handler.
Evidence trail:
apps/server/src/codexAppServerManager.ts: lines 428-471 show `startSession` calls `this.sessions.set(threadId, context)` without checking for existing session; line 943 shows exit handler calls `this.sessions.delete(context.session.threadId)` where `context` is captured in closure; lines 890-893 show `requireSession` throws 'Unknown session' when entry not in map; lines 606-607 show `sendTurn` uses `requireSession`.
Also found in 1 other location(s):
- apps/server/src/provider/Layers/CodexAdapter.ts:1262 -- The `nativeEventLogger` created when `options.nativeEventLogPath` is provided is never closed. While `manager` is wrapped in `Effect.acquireRelease` for cleanup, `nativeEventLogger` is instantiated separately and its `close()` method is never called. This causes file handles opened by the logger to leak every time the `CodexAdapter` layer is released (e.g., during configuration reloads or tests).
Comment on lines
+329
to
+341
| export const OpenCodeIcon: Icon = (props) => ( | ||
| <svg {...props} viewBox="0 0 32 40" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <g clipPath="url(#opencode__clip0_1311_94969)"> | ||
| <path d="M24 32H8V16H24V32Z" fill="#BCBBBB" /> | ||
| <path d="M24 8H8V32H24V8ZM32 40H0V0H32V40Z" fill="#211E1E" /> | ||
| </g> | ||
| <defs> | ||
| <clipPath id="opencode__clip0_1311_94969"> | ||
| <rect width="32" height="40" fill="white" /> | ||
| </clipPath> | ||
| </defs> | ||
| </svg> | ||
| ); |
Contributor
There was a problem hiding this comment.
🟢 Low components/Icons.tsx:329
The OpenCodeIcon component uses hardcoded fill attributes (#211E1E and #BCBBBB) on its SVG paths. In dark mode, the #211E1E dark grey fill renders the icon nearly invisible due to low contrast against dark backgrounds. Unlike GitHubIcon and OpenAI, which use currentColor to inherit the surrounding text color, this icon fails to adapt to the theme.
- <svg {...props} viewBox="0 0 32 40" fill="none" xmlns="http://www.w3.org/2000/svg">
+ <svg {...props} viewBox="0 0 32 40" fill="none" xmlns="http://www.w3.org/2000/svg" fill="currentColor">
<g clipPath="url(#opencode__clip0_1311_94969)">
- <path d="M24 32H8V16H24V32Z" fill="#BCBBBB" />
- <path d="M24 8H8V32H24V8ZM32 40H0V0H32V40Z" fill="#211E1E" />
+ <path d="M24 32H8V16H24V32Z" fill="currentColor" />
+ <path d="M24 8H8V32H24V8ZM32 40H0V0H32V40Z" fill="currentColor" />🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Icons.tsx around lines 329-341:
The `OpenCodeIcon` component uses hardcoded `fill` attributes (`#211E1E` and `#BCBBBB`) on its SVG paths. In dark mode, the `#211E1E` dark grey fill renders the icon nearly invisible due to low contrast against dark backgrounds. Unlike `GitHubIcon` and `OpenAI`, which use `currentColor` to inherit the surrounding text color, this icon fails to adapt to the theme.
Evidence trail:
apps/web/src/components/Icons.tsx lines 329-342 (OpenCodeIcon with hardcoded fills #211E1E and #BCBBBB), lines 5-15 (GitHubIcon with fill="currentColor" at line 12), lines 146-150 (OpenAI with fill="currentColor" at line 147). Verified at commit REVIEWED_COMMIT.
Member
ben-vargas
added a commit
to ben-vargas/ai-t3code
that referenced
this pull request
Mar 7, 2026
|
I prepared a CI fix PR targeting this branch: #243\n\nIt updates stale ClaudeCodeAdapter test expectations for thread identity/providerThreadId behavior and aligns with current adapter semantics. |
Ascinocco
added a commit
to Ascinocco/t3code
that referenced
this pull request
Mar 7, 2026
Fix detached SDK stream fiber by replacing Effect.runFork with Effect.forkChild and adding explicit Fiber.interrupt on session stop. Add missing threadId to native event logger for per-thread log routing. Remove no-op asCanonicalTurnId identity function. Remove redundant threadId spread in session construction. Fix stale test expectations for thread identity behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gabrielMalonso
added a commit
to gabrielMalonso/t3code
that referenced
this pull request
Mar 7, 2026
…apters Merge the Claude Code adapter (PR pingdotgg#179) into main, resolving 45 conflicts caused by the deliberate split of provider stacks on March 5. Key additions: - ClaudeCodeAdapter with full session, turn, and resume lifecycle - Cursor provider support (model catalog, UI, routing) - ProviderKind expanded from "codex" to "codex" | "claudeCode" | "cursor" - Provider model catalogs, aliases, and slug resolution across all providers - UI support for Claude Code and Cursor in ChatView, settings, and composer Conflict resolution strategy: - Kept HEAD's refactored patterns (scoped finalizers, telemetry, serviceTier) - Added PR's new provider routing, adapters, and UI components - Fixed duplicate declarations, missing props, and type mismatches Typecheck: 7/7 packages pass Lint: 0 warnings, 0 errors Tests: 419/422 pass (3 pre-existing failures in ClaudeCodeAdapter.test.ts) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gabrielMalonso
added a commit
to gabrielMalonso/t3code
that referenced
this pull request
Mar 7, 2026
Remove filtro que escondia o Claude Code do seletor de providers, tornando-o selecionável após o merge do adapter (PR pingdotgg#179). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
hameltomor
added a commit
to hameltomor/t3code
that referenced
this pull request
Mar 7, 2026
Add first-class Claude Code support alongside existing Codex provider: - Add ClaudeCodeAdapter (1857 lines) backed by @anthropic-ai/claude-agent-sdk - Extend ProviderKind to accept "codex" | "claudeCode" - Add Claude model catalog (Opus 4.6, Sonnet 4.6, Haiku 4.5) - Register ClaudeCodeAdapter in provider registry and server layers - Add Claude SDK event sources to provider runtime events - Enable Claude Code in UI provider picker - Add ClaudeCodeProviderStartOptions to contracts - Update all tests for multi-provider support Based on upstream PR pingdotgg#179 by juliusmarminge, surgically applied to current main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
souravrs999
added a commit
to souravrs999/t3code
that referenced
this pull request
Mar 8, 2026
Merges codething/648ca884-claude branch which adds full Claude Code provider adapter, orchestration enhancements, and UI surface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
vouch:trusted
This was referenced Mar 18, 2026
|
@juliusmarminge when actually using it as a DMG i noticed that there is a small bug that it doesnt use the system claude binaries but instead the bundles sdks binaries. I filled #1189 into this branch. |
sohamnandi77
added a commit
to sohamnandi77/t3code
that referenced
this pull request
Mar 18, 2026
- Merge upstream Claude traits/model/UI changes from pingdotgg#179. - Add ANTHROPIC_BASE_URL + ANTHROPIC_AUTH_TOKEN overrides and wire them through WS, server, Codex spawn env, and Claude env. - Skip claude auth probe when external Anthropic token is configured. Made-with: Cursor
dafzthomas
added a commit
to dafzthomas/t3code
that referenced
this pull request
Mar 18, 2026
…esolution Merge origin/codething/648ca884-claude into main, resolving conflicts in: - apps/web/src/appSettings.ts (keep both textGenerationModel and customClaudeModels) - apps/web/src/components/ChatView.tsx (take PR's restructured compositor layout) - apps/web/src/composerDraftStore.ts (take PR's modelOptions migration, restore prompt/terminalContexts processing) - packages/contracts/src/model.ts (keep both git text generation model and backward compat exports) Additional fixes on top of the merge: - Add terminalContexts to browser test mock drafts (ClaudeTraitsPicker, CodexTraitsPicker, CompactComposerControlsMenu) - Restore ensureInlineTerminalContextPlaceholders call in draft deserialization - Add terminalContexts and onRemoveTerminalContext props to ComposerPromptEditor - Use DEFAULT_MODEL_BY_PROVIDER[selectedProvider] instead of hardcoded .codex - Persist provider/model before clearComposerDraftContent to prevent reset Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
dafzthomas
added a commit
to dafzthomas/t3code
that referenced
this pull request
Mar 18, 2026
Merges latest commits from pingdotgg#179 including: - Storage refactor (composerDraftStore extracted resolveModelOptions) - setState pattern refactored in browser test fixtures - Upstream main merged in (terminal context, sidebar fixes, git text gen) - Button overflow fix Conflict resolution: - Keep Haven Code fork identity (Bedrock settings, enableCodexProvider, claude-haiku-4-5 as default git text gen model, Bedrock badge in picker) - Accept upstream refactors (draftsByThreadId variable pattern, resolveModelOptions callback, CSS improvements) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Im not sure if this is ready for reviews and such yet, but these errors when testing it out. On NixOS btw |
Co-authored-by: codex <codex@users.noreply.github.com>
Comment on lines
+2511
to
+2519
| const sendTurn: ClaudeAdapterShape["sendTurn"] = (input) => | ||
| Effect.gen(function* () { | ||
| const context = yield* requireSession(input.threadId); | ||
|
|
||
| if (context.turnState) { | ||
| // Auto-close a stale synthetic turn (from background agent responses | ||
| // between user prompts) to prevent blocking the user's next turn. | ||
| yield* completeTurn(context, "completed"); | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Medium Layers/ClaudeAdapter.ts:2511
In sendTurn, the code at lines 2515-2519 calls completeTurn(context, "completed") whenever context.turnState exists, without checking whether the turn is a synthetic background turn or an active user-initiated turn. This causes any ongoing user turn to be incorrectly marked as completed when the user sends a new message, even if the previous turn was still processing. The comment claims this only closes "stale synthetic turns," but the condition if (context.turnState) matches all turns. Consider adding a flag to distinguish synthetic turns from user turns, or checking that the turn is actually synthetic before auto-completing it.
- if (context.turnState) {
- // Auto-close a stale synthetic turn (from background agent responses
- // between user prompts) to prevent blocking the user's next turn.
- yield* completeTurn(context, "completed");
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeAdapter.ts around lines 2511-2519:
In `sendTurn`, the code at lines 2515-2519 calls `completeTurn(context, "completed")` whenever `context.turnState` exists, without checking whether the turn is a synthetic background turn or an active user-initiated turn. This causes any ongoing user turn to be incorrectly marked as completed when the user sends a new message, even if the previous turn was still processing. The comment claims this only closes "stale synthetic turns," but the condition `if (context.turnState)` matches all turns. Consider adding a flag to distinguish synthetic turns from user turns, or checking that the turn is actually synthetic before auto-completing it.
Evidence trail:
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 2515-2518 (REVIEWED_COMMIT): shows `if (context.turnState)` condition with comment claiming it's for synthetic turns only.
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 85-93 (REVIEWED_COMMIT): `ClaudeTurnState` interface has no flag to distinguish synthetic vs user turns.
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 1592-1606 (REVIEWED_COMMIT): synthetic turn creation - same ClaudeTurnState structure as user turns.
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 2545-2554 (REVIEWED_COMMIT): user turn creation - identical ClaudeTurnState structure with no distinguishing flag.
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 2043-2053 (REVIEWED_COMMIT): `requireSession` only checks session existence, not status, so `sendTurn` can be called while a turn is active.
|
@JustYannicc If I'm looking to build the desktop app using this branch should I rather use the one for #1189 ? |
- add decoding defaults in `AppSettingsSchema` so older persisted settings load safely - export shared `Schema.Literals` types for `EnvMode` and `TimestampFormat` - add a regression test covering pre-new-key settings hydration
|
@TaylorJonesTRT if you just want to use the build command. It doesn't matter. But if you want to build a DMG then you need to use the other Branch I created. It's just a small bug that only happens in the DMG but not with build commands. |
dafzthomas
added a commit
to dafzthomas/t3code
that referenced
this pull request
Mar 19, 2026
…update Merge upstream PR pingdotgg#179 updates (Claude adapter + storage refactor)
|
Hi, I got an error while using Claude model. I was using this branch, built the DMG installation file and installed it on my mac (macbook pro m1, macos tahoe 26.2). The error log says: |
|
how are you building it for macOS? It’s failing when I try to package it. @JustYannicc, could you share the scripts you’re using? |
- Map unknown pending approval/user-input errors to explicit stale-request details - Clear stale pending approvals and user-input prompts in session derivation logic - Treat reused Claude text block indexes as new assistant messages to avoid item ID reuse - Persist latest runtime metadata before provider stop-all and expand regression coverage
| @@ -498,6 +526,17 @@ const makeProviderService = (options?: ProviderServiceLiveOptions) => | |||
| const runStopAll = () => | |||
Contributor
There was a problem hiding this comment.
🟡 Medium Layers/ProviderService.ts:526
In runStopAll, the loop at lines 534-539 saves active session state via upsertSessionBinding, but the subsequent loop at lines 543-553 immediately overwrites every thread binding with a minimal runtimePayload that only contains activeTurnId, lastRuntimeEvent, and lastRuntimeEventAt. This drops cwd, model, modelOptions, and providerOptions that were just preserved, breaking session recovery after restart. Consider removing the second loop or merging it with the first to preserve the full session state.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ProviderService.ts around line 526:
In `runStopAll`, the loop at lines 534-539 saves active session state via `upsertSessionBinding`, but the subsequent loop at lines 543-553 immediately overwrites every thread binding with a minimal `runtimePayload` that only contains `activeTurnId`, `lastRuntimeEvent`, and `lastRuntimeEventAt`. This drops `cwd`, `model`, `modelOptions`, and `providerOptions` that were just preserved, breaking session recovery after restart. Consider removing the second loop or merging it with the first to preserve the full session state.
Evidence trail:
- apps/server/src/provider/Layers/ProviderService.ts lines 526-557 (the `runStopAll` function showing both loops)
- apps/server/src/provider/Layers/ProviderService.ts lines 171-188 (`upsertSessionBinding` function calling `directory.upsert` with `toRuntimePayloadFromSession`)
- apps/server/src/provider/Layers/ProviderService.ts lines 89-109 (`toRuntimePayloadFromSession` function returning `cwd`, `model`, `activeTurnId`, `lastError` from session)
- First loop (lines 534-539) saves session state including `cwd`, `model` via `upsertSessionBinding`
- Second loop (lines 543-553) calls `directory.upsert` directly with minimal `runtimePayload` containing only `activeTurnId`, `lastRuntimeEvent`, `lastRuntimeEventAt`
- Both loops use same `directory.upsert`, so second loop overwrites first loop's data
| return typeof value === "number" && Number.isFinite(value) ? value : undefined; | ||
| } | ||
|
|
||
| function toTurnId(value: string | undefined): TurnId | undefined { |
Contributor
There was a problem hiding this comment.
🟢 Low Layers/CodexAdapter.ts:112
toTurnId checks value?.trim() for truthiness but then passes the original untrimmed value to TurnId.makeUnsafe. If the input contains leading/trailing whitespace (e.g., " abc "), the check passes but a TurnId containing whitespace is created. Consider using TurnId.makeUnsafe(value.trim()) to match the validation logic.
-function toTurnId(value: string | undefined): TurnId | undefined {
- return value?.trim() ? TurnId.makeUnsafe(value) : undefined;
-}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/CodexAdapter.ts around line 112:
`toTurnId` checks `value?.trim()` for truthiness but then passes the original untrimmed `value` to `TurnId.makeUnsafe`. If the input contains leading/trailing whitespace (e.g., `" abc "`), the check passes but a `TurnId` containing whitespace is created. Consider using `TurnId.makeUnsafe(value.trim())` to match the validation logic.
Evidence trail:
apps/server/src/provider/Layers/CodexAdapter.ts lines 112-113 at REVIEWED_COMMIT - shows `toTurnId` function checking `value?.trim()` but passing untrimmed `value` to `makeUnsafe`.
packages/contracts/src/baseSchemas.ts lines 14-29 at REVIEWED_COMMIT - shows `TurnId` is created via `makeEntityId` using `TrimmedNonEmptyString` schema, confirming values should be trimmed.
- Reuse persisted `resumeCursor` when restarting a Claude session for the same thread/provider - Generate a fresh Claude `sessionId` only for new sessions; avoid reusing stale checkpoint fields - Keep resume state when only model options change, with tests covering restart and resume behavior
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+2512
to
+2516
| if (context.turnState) { | ||
| // Auto-close a stale synthetic turn (from background agent responses | ||
| // between user prompts) to prevent blocking the user's next turn. | ||
| yield* completeTurn(context, "completed"); | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Medium Layers/ClaudeAdapter.ts:2512
When sendTurn completes a stale turn at line 2512 and then yields multiple times before setting the new turnState at line 2554, the runSdkStream fiber can interleave and receive an SDK message. In handleAssistantMessage at line 1587, the missing turnState causes a synthetic turn to be auto-created with a new turnId, emitting a turn.started event. The original sendTurn then overwrites context.turnState, leaving the synthetic turn without a matching turn.completed event and corrupting the runtime event stream. Consider atomically swapping the turn state or ensuring no yields occur between clearing the old turn and establishing the new one.
if (context.turnState) {
- // Auto-close a stale synthetic turn (from background agent responses
- // between user prompts) to prevent blocking the user's next turn.
- yield* completeTurn(context, "completed");
+ // Auto-close a stale synthetic turn (from background agent responses
+ // between user prompts) to prevent blocking the user's next turn.
+ // Immediately swap turnState to prevent race with SDK stream handler.
+ const staleTurnState = context.turnState;
+ context.turnState = undefined;
+ yield* completeTurn(context, "completed", undefined, undefined, staleTurnState);
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeAdapter.ts around lines 2512-2516:
When `sendTurn` completes a stale turn at line 2512 and then yields multiple times before setting the new `turnState` at line 2554, the `runSdkStream` fiber can interleave and receive an SDK message. In `handleAssistantMessage` at line 1587, the missing `turnState` causes a synthetic turn to be auto-created with a new `turnId`, emitting a `turn.started` event. The original `sendTurn` then overwrites `context.turnState`, leaving the synthetic turn without a matching `turn.completed` event and corrupting the runtime event stream. Consider atomically swapping the turn state or ensuring no yields occur between clearing the old turn and establishing the new one.
Evidence trail:
apps/server/src/provider/Layers/ClaudeAdapter.ts lines 2501 (runSdkStream forked), 2510-2514 (completeTurn called), 1241 (completeTurn sets context.turnState = undefined), 2517-2543 (multiple yield* between completeTurn and setting new turnState), 2554 (context.turnState = turnState overwrites), 1575-1612 (handleAssistantMessage auto-creates synthetic turn when turnState is missing)
Comment on lines
+312
to
+316
| const effectiveResumeCursor = | ||
| input.resumeCursor ?? | ||
| (persistedBinding?.provider === input.provider | ||
| ? persistedBinding.resumeCursor | ||
| : undefined); |
Contributor
There was a problem hiding this comment.
🟢 Low Layers/ProviderService.ts:312
The effectiveResumeCursor computation at lines 312-316 uses ?? which passes through null values from persistedBinding.resumeCursor. Since null !== undefined is true, the code spreads { resumeCursor: null } into the adapter input, causing the adapter to receive an unexpected null instead of omitting the field. This contradicts the explicit null/undefined checks in recoverSessionForThread (lines 215-216) and likely violates the adapter's expectations for omitted vs. null resume cursors.
- const effectiveResumeCursor =
- input.resumeCursor ??
- (persistedBinding?.provider === input.provider
- ? persistedBinding.resumeCursor
- : undefined);
+ const effectiveResumeCursor =
+ input.resumeCursor ??
+ (persistedBinding?.provider === input.provider &&
+ persistedBinding.resumeCursor !== null &&
+ persistedBinding.resumeCursor !== undefined
+ ? persistedBinding.resumeCursor
+ : undefined);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ProviderService.ts around lines 312-316:
The `effectiveResumeCursor` computation at lines 312-316 uses `??` which passes through `null` values from `persistedBinding.resumeCursor`. Since `null !== undefined` is `true`, the code spreads `{ resumeCursor: null }` into the adapter input, causing the adapter to receive an unexpected `null` instead of omitting the field. This contradicts the explicit `null`/`undefined` checks in `recoverSessionForThread` (lines 215-216) and likely violates the adapter's expectations for omitted vs. null resume cursors.
Evidence trail:
apps/server/src/provider/Layers/ProviderService.ts lines 312-319 (effectiveResumeCursor computation and spread)
apps/server/src/provider/Layers/ProviderService.ts lines 215-216 (explicit null/undefined checks in recoverSessionForThread)
apps/server/src/provider/Services/ProviderSessionDirectory.ts:20 (resumeCursor type: `unknown | null`)
packages/contracts/src/provider.ts:50-61 (ProviderSessionStartInput schema with `resumeCursor: Schema.optional(Schema.Unknown)`)
- Read image attachments from the server state dir - Validate supported Claude image MIME types before sending - Add coverage for embedding attachment bytes in user prompts
- Return a deny permission result when the user cancels a pending Claude question - Add coverage for aborting AskUserQuestion while user input is pending
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
14 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
This PR adds the Claude Code adapter on top of the core orchestration branch in #103.
It includes:
Stack
codething/648ca884Validation
bun lintbun typecheckcd apps/server && bun run test -- --run src/provider/Layers/ProviderAdapterRegistry.test.tscd apps/web && bun run test -- --run src/session-logic.test.tsNote
High Risk
Adds a new
claudeAgentprovider with session lifecycle, resume, approvals/user-input bridging, and rollback behavior, plus changes to provider/session validation and restart rules; mistakes could break turn execution, approval gating, or checkpoint reverts across providers.Overview
Adds first-class support for the
claudeAgentprovider end-to-end, including a newClaudeAdapterLivebacked by@anthropic-ai/claude-agent-sdkthat streams canonicalProviderRuntimeEvents, supports interrupts, approvals/user-input requests, resume cursors, and thread rollback.Updates orchestration/provider plumbing to be provider-aware:
ProviderKindand model catalogs now include Claude models and options,ProviderCommandReactorenforces provider/model compatibility, restarts Claude sessions when ClaudemodelOptionschange, and surfaces turn-start failures as thread activities instead of crashing.Extends ingestion and checkpointing tests/behavior for Claude events (turn lifecycle, task progress summaries, checkpoint capture/revert), and adds new integration coverage for first-turn Claude selection, stopAll recovery via persisted resume state, approval response forwarding, and interrupt forwarding. Also refreshes docs/README to reflect Claude as supported.
Written by Cursor Bugbot for commit a75a9b4. This will update automatically on new commits. Configure here.
Note
Add Claude (claudeAgent) as a selectable provider alongside Codex
ClaudeAdapterLiveserver implementation that integrates with the@anthropic-ai/claude-agent-sdkto manage Claude sessions, streaming, permissions, and state recovery.ProviderKindfrom a single'codex'literal to a union with'claudeAgent'; registers the Claude adapter in the provider registry and health service.supportsClaudeFastMode,supportsClaudeAdaptiveReasoning,supportsClaudeUltrathinkKeyword, etc.) and aClaudeTraitsPickerUI component for effort, thinking, and fast mode controls.'claudeAgent'.effort/fastModefields with a provider-scopedmodelOptionsshape; legacy v1 data is migrated on load.sourceProposedPlanthrough projection pipeline, DB schema, and snapshot queries so plan context is preserved across turns.Macroscope summarized fda3fa3.