feat(mobile): nested subagent threads behind a toggle#3871
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
- Initialize provider as unchecked in a pending state - Update initial probe message to reflect session-local status
- Type the runtime effect with `Scope` - Build the ACP session runtime without wrapping it in `Effect.scoped`
- Use strict TurnId and ProviderItemId parsing in Codex session routing - Decode in-memory stdio chunks in streaming mode to avoid split UTF-8 corruption
- Transfer session-owned scopes into adapter state - Ensure runtime scopes close on stop and startup failure - Add regression coverage for scoped lifecycle cleanup
- Close the managed native event logger when the adapter layer tears down - Make session runtime close idempotent with an atomic closed flag - Add coverage for flushing thread native logs on shutdown
- Use codex app-server snapshots for auth, models, and skills - Remove legacy CLI/config discovery paths and related helpers - Update tests for the new provider status flow
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Document the target orchestration graph, IDs, lifecycles, and capability model - Add Codex app-server probe fixtures and update the probe test harness
- Introduce orchestration v2 service interfaces and error types - Add replay runtime, fixtures, and integration coverage - Update shared contracts and probe transcripts Co-authored-by: codex <codex@users.noreply.github.com>
- Add Codex adapter and replay harness wiring - Introduce in-memory orchestration projections and provider registry - Expand orchestration contracts for turn and runtime events
Co-authored-by: codex <codex@users.noreply.github.com>
- Add context transfer IDs, schemas, and projections - Support cheap fork creation and Codex native fork rollback - Cover fork idempotency and replay behavior in tests
- Track remaining projection, context transfer, rollback, capability, and subagent work - Clarify current V2 baseline and debugger-only follow-ups
- Map fork and merge-back turns into stored handoffs and transfer resolutions - Add shell snapshot projection support plus coverage tests - Update replay fixtures and web contracts for the new turn flow
Co-authored-by: codex <codex@users.noreply.github.com>
- Move Codex replay recording into `apps/server` - Add Claude Agent SDK replay fixtures and test harness - Update orchestration-v2 fixture scenarios and docs
- Move Claude provider runtime logic into its own module - Share the SDK query runner between live and replay paths - Add replay driver error wrapping for unexpected failures
Port orchestration V2 provider adapter wiring to the provider-instance driver registry. Co-authored-by: codex <codex@users.noreply.github.com>
- persist the selected model on run records - surface run model selection in the debug UI - update replay fixtures and contracts for the new field
- Record Claude SDK transcripts across multiple prompts and restart/query modes - Add approval and tool-call replay coverage for new orchestration fixtures - Update Claude adapter testkit to model open/prompt/permission frames
- Derive Claude SDK query options from runtime policy - Add read-only replay fixture and policy mapping tests - Reuse shared approval-policy fixtures across orchestrator tests Co-authored-by: codex <codex@users.noreply.github.com>
- add active steering and interrupt-restart replay fixtures - update Claude adapter/orchestrator turn handling for steering - refresh replay and integration test coverage
- add interrupt and mid-tool replay fixtures for Claude and Codex - log Claude Agent SDK protocol frames to native event traces - project Codex commandExecution start events into orchestration updates
- Map Cursor SDK agents and runs to V2 thread and turn lifecycles - Update MCP capability, tool, and testing guidance for SDK-based injection
github-actions
Bot
added
the
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d03b0c3. Configure here.
| depth: 0, | ||
| hasSubagentChildren: false, | ||
| isSubagentBranchExpanded: false, | ||
| })); |
There was a problem hiding this comment.
Open subagent missing sidebar row
High Severity
When "Show nested subagents" is off, buildHomeListLayout incorrectly uses getSubagentThreadTreeRoots to determine total and visible thread counts. This causes child subagent threads to be excluded from the home list and navigation sidebar, even though they were previously displayed as flat rows. Consequently, these threads are not visible or selectable in the sidebar.
Reviewed by Cursor Bugbot for commit d03b0c3. Configure here.
| depth: 0, | ||
| hasSubagentChildren: false, | ||
| isSubagentBranchExpanded: false, | ||
| })); |
There was a problem hiding this comment.
Paginated roots hide selected subagent
Medium Severity
The new subagent display logic can inadvertently hide threads. When nested subagents are enabled, threads whose parent roots are paginated out, or subagent threads with collapsed parents during search, are not included in the visible list. This can cause selected or searched subagent threads to be missing from the home list.
Reviewed by Cursor Bugbot for commit d03b0c3. Configure here.
The composer labeled a send as Steer whenever the active turn was busy, even when the server had runs queued for the thread — the send would actually join that queue. Server-queued runs now count as backlog.
Extract the environment-scoped subagent tree projection with cycle and orphan fallbacks so every client renders the same rows. The web sidebar adopts it separately in pingdotgg#3861; this copy carries only the shared helpers the mobile tree needs.
With the nested-subagent toggle off, the home list and sidebar were still paginating tree roots only, so subagent child threads vanished from the flat list and the recency baseline shifted. Gate all root/tree projection on the preference so the off state matches the pre-feature flat behavior exactly. With the toggle on, two nested-mode holes could hide threads: search matches under collapsed parents never rendered, and root-based pagination could slice out the branch of the routed selection. Force every branch open while searching, and pin the selected thread's root past the pagination cut so its branch always renders.
PixPMusic
force-pushed
the
pixpmusic/mobile-subagent-tree
branch
from
d03b0c3 to
ebef3b8
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const service = yield* OrchestratorMcpService; | ||
| return yield* service.createThreads(scope, input); | ||
| }), | ||
| t3_thread_start: (input) => |
There was a problem hiding this comment.
🟡 Medium orchestrator/handlers.ts:62
The t3_thread_start handler only calls service.createThreads and returns result.threads[0]!, so it creates an idle thread and never starts the first turn. The tool description says it "immediately start[s] its first turn," so callers invoking t3_thread_wait / t3_thread_read with the returned runId will observe an idle thread with no active run. The handler needs to also issue a start/send to the new thread after creation so the first turn is actually running.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/mcp/toolkits/orchestrator/handlers.ts around line 62:
The `t3_thread_start` handler only calls `service.createThreads` and returns `result.threads[0]!`, so it creates an idle thread and never starts the first turn. The tool description says it "immediately start[s] its first turn," so callers invoking `t3_thread_wait` / `t3_thread_read` with the returned `runId` will observe an idle thread with no active run. The handler needs to also issue a start/send to the new thread after creation so the first turn is actually running.
| preview: toolCall.result.value.results, | ||
| }, | ||
| ]; | ||
| default: |
There was a problem hiding this comment.
🟡 Medium Adapters/CursorAdapterV2.ts:464
When cursorToolSearchResults receives an ls or readLints tool call that succeeded, it returns [] instead of the result entries. The default branch of the switch discards the successful result data, so the emitted file_search turn item loses all entries for those tool types. Consider adding case entries for ls and readLints to map their results into the expected { fileName, line?, preview? } shape.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts around line 464:
When `cursorToolSearchResults` receives an `ls` or `readLints` tool call that succeeded, it returns `[]` instead of the result entries. The `default` branch of the `switch` discards the successful result data, so the emitted `file_search` turn item loses all entries for those tool types. Consider adding `case` entries for `ls` and `readLints` to map their results into the expected `{ fileName, line?, preview? }` shape.
| ...base, | ||
| sessions: { | ||
| ...base.sessions, | ||
| supportsModelSwitchInSession: setup.models != null || hasModelConfig, |
There was a problem hiding this comment.
🟡 Medium Adapters/AcpAdapterV2.ts:262
negotiatedCapabilities sets supportsModelSwitchInSession to true whenever sessionSetupResult.models is present, but ACP's models field reports current model state and does not imply session/set_model support. When an agent returns models without supporting model switching, the orchestrator treats in-session model changes as allowed, so configureSession calls runtime.setSessionModel(...) and the turn fails instead of forcing a restart. The hasModelConfig check (whether a "model" config option exists) is the reliable signal for switch support; setup.models != null should not also enable it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts around line 262:
`negotiatedCapabilities` sets `supportsModelSwitchInSession` to `true` whenever `sessionSetupResult.models` is present, but ACP's `models` field reports current model state and does not imply `session/set_model` support. When an agent returns `models` without supporting model switching, the orchestrator treats in-session model changes as allowed, so `configureSession` calls `runtime.setSessionModel(...)` and the turn fails instead of forcing a restart. The `hasModelConfig` check (whether a `"model"` config option exists) is the reliable signal for switch support; `setup.models != null` should not also enable it.
| }); | ||
| } | ||
|
|
||
| const diff = yield* checkpointStore |
There was a problem hiding this comment.
🟡 Medium checkpointing/CheckpointDiffQuery.ts:175
getTurnDiff always resolves both checkpoint refs inside toScope.cwd, but the from checkpoint may have been captured in a different scope's worktree. When a later run changed the runtime cwd, the from ref doesn't exist under toScope.cwd, so checkpointStore.diffCheckpoints fails with a "ref unavailable" error instead of returning the cross-run diff. Consider resolving the from ref in its own scope's cwd (or falling back to HEAD) when the scopes differ.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/checkpointing/CheckpointDiffQuery.ts around line 175:
`getTurnDiff` always resolves both checkpoint refs inside `toScope.cwd`, but the `from` checkpoint may have been captured in a different scope's worktree. When a later run changed the runtime `cwd`, the `from` ref doesn't exist under `toScope.cwd`, so `checkpointStore.diffCheckpoints` fails with a "ref unavailable" error instead of returning the cross-run diff. Consider resolving the `from` ref in its own scope's `cwd` (or falling back to HEAD) when the scopes differ.
| }), | ||
| ), | ||
| ), | ||
| offer: (message) => |
There was a problem hiding this comment.
🟠 High Adapters/ClaudeAdapterV2.ts:522
offer silently drops user prompts after the session is closed. Queue.shutdown(promptQueue) causes Queue.offer to return false, but Effect.asVoid discards that value, so callers see success while the message is never delivered to queryRuntime. The prompt.offer protocol log is also emitted, falsely claiming the message was sent.
Check the Queue.offer result and fail when it returns false so callers are notified that the prompt was not enqueued.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts around line 522:
`offer` silently drops user prompts after the session is closed. `Queue.shutdown(promptQueue)` causes `Queue.offer` to return `false`, but `Effect.asVoid` discards that value, so callers see success while the message is never delivered to `queryRuntime`. The `prompt.offer` protocol log is also emitted, falsely claiming the message was sent.
Check the `Queue.offer` result and fail when it returns `false` so callers are notified that the prompt was not enqueued.
| input.runtimePolicy.sandboxPolicy === undefined | ||
| ? runtimeModeDefaults.sandboxPolicy | ||
| : yield* decodeTurnSandboxPolicy(input.runtimePolicy.sandboxPolicy); | ||
| const selectedEffort = getModelSelectionStringOptionValue( |
There was a problem hiding this comment.
🟡 Medium Adapters/CodexAdapterV2.ts:530
buildCodexTurnStartParams ignores input.runtimePolicy.reasoningEffort and only reads reasoningEffort from input.modelSelection. When a runtime policy sets reasoningEffort, that override is silently dropped and the Codex turn runs with the model default or user-selected effort instead of the enforced value. Consider preferring input.runtimePolicy.reasoningEffort (when present) before falling back to the model-selection option.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts around line 530:
`buildCodexTurnStartParams` ignores `input.runtimePolicy.reasoningEffort` and only reads `reasoningEffort` from `input.modelSelection`. When a runtime policy sets `reasoningEffort`, that override is silently dropped and the Codex turn runs with the model default or user-selected effort instead of the enforced value. Consider preferring `input.runtimePolicy.reasoningEffort` (when present) before falling back to the model-selection option.
| const [respondingApprovalId, setRespondingApprovalId] = useState<RuntimeRequestId | null>(null); | ||
| const [respondingUserInputId, setRespondingUserInputId] = useState<RuntimeRequestId | null>(null); | ||
|
|
||
| const pendingRequests = useMemo( |
There was a problem hiding this comment.
🟡 Medium state/use-selected-thread-requests.ts:70
useSelectedThreadRequests picks activePendingApprovals[0] and activePendingUserInputs[0] as the active request, but derivePendingThreadRequests returns requests in projection.runtimeRequests insertion order without sorting by createdAt. When requests arrive or are upserted out of chronological order, the hook surfaces and submits the wrong (not oldest) pending approval or user-input request. The previous code path sorted via derivePendingApprovals / derivePendingUserInputs; this regression is specific to the new mobile path. Consider sorting the derived approvals and user inputs by createdAt before selecting index 0.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/state/use-selected-thread-requests.ts around line 70:
`useSelectedThreadRequests` picks `activePendingApprovals[0]` and `activePendingUserInputs[0]` as the active request, but `derivePendingThreadRequests` returns requests in `projection.runtimeRequests` insertion order without sorting by `createdAt`. When requests arrive or are upserted out of chronological order, the hook surfaces and submits the wrong (not oldest) pending approval or user-input request. The previous code path sorted via `derivePendingApprovals` / `derivePendingUserInputs`; this regression is specific to the new mobile path. Consider sorting the derived approvals and user inputs by `createdAt` before selecting index `0`.
| }; | ||
| } | ||
|
|
||
| function nestedGrepWorkspaceResults(success: Record<string, unknown>): Record<string, unknown> { |
There was a problem hiding this comment.
🟡 Medium Adapters/CursorAdapterV2.ts:524
nestedGrepWorkspaceResults always builds a { type: "content" } object and reads only content.matches, so nested grep calls made with outputMode set to "files" or "count" lose their actual results and return empty matches. The renderer handles files and count results separately, so subagent grep calls in those modes display incorrect or missing tool output. Consider branching on outputMode (or the presence of content.files / content.count) so each mode's data is preserved instead of being projected into the content shape.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts around line 524:
`nestedGrepWorkspaceResults` always builds a `{ type: "content" }` object and reads only `content.matches`, so nested grep calls made with `outputMode` set to `"files"` or `"count"` lose their actual results and return empty matches. The renderer handles `files` and `count` results separately, so subagent grep calls in those modes display incorrect or missing tool output. Consider branching on `outputMode` (or the presence of `content.files` / `content.count`) so each mode's data is preserved instead of being projected into the content shape.
| }): ClaudeAgentSdkQueryOptions { | ||
| const compiledSelection = compileClaudeModelSelection(input.modelSelection); | ||
| const extraArgs = | ||
| input.settings === undefined ? {} : parseCliArgs(input.settings.launchArgs).flags; |
There was a problem hiding this comment.
🟡 Medium Adapters/ClaudeAdapterV2.ts:637
makeClaudeQueryOptions parses input.settings.launchArgs with parseCliArgs, which splits the string on raw whitespace without honoring shell quoting. A multi-word argument like --append-system-prompt "always think step by step" is broken into separate tokens — "always becomes the value for --append-system-prompt, and the remaining words are dropped as positionals. Users who set CLI arguments with spaces get silently truncated values at runtime. Consider using a shell-aware parser (e.g., parseArgsStringToArgs or shell-quote) so quoted values are preserved as a single token.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts around line 637:
`makeClaudeQueryOptions` parses `input.settings.launchArgs` with `parseCliArgs`, which splits the string on raw whitespace without honoring shell quoting. A multi-word argument like `--append-system-prompt "always think step by step"` is broken into separate tokens — `"always` becomes the value for `--append-system-prompt`, and the remaining words are dropped as positionals. Users who set CLI arguments with spaces get silently truncated values at runtime. Consider using a shell-aware parser (e.g., `parseArgsStringToArgs` or `shell-quote`) so quoted values are preserved as a single token.
ApprovabilityVerdict: Needs human review 15 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
Scope note on the Macroscope sweep: because this stack merges One of them is already fixed upstream in this stack: the Review scope for this PR is the commits after |
| ): ReadonlyArray<OrchestrationV2ProviderTurn> => | ||
| providerTurns.filter((turn) => turn.providerThreadId === providerThread.id); | ||
|
|
||
| const countTerminalTurnsAfterBoundary = ( |
There was a problem hiding this comment.
🟡 Medium Adapters/CodexAdapterV2.ts:648
resolveCodexForkRollbackTurnCount only counts terminal turns (completed/interrupted/failed/cancelled) after the fork boundary, so when later turns are still running or pending it returns 0. The forked Codex thread then skips native rollback and starts from the newest in-progress state instead of the requested historical turn. Consider counting all turns after the boundary, or at least all turns that have been initiated, so the rollback count reflects the actual conversation distance.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts around line 648:
`resolveCodexForkRollbackTurnCount` only counts terminal turns (completed/interrupted/failed/cancelled) after the fork boundary, so when later turns are still `running` or `pending` it returns `0`. The forked Codex thread then skips native rollback and starts from the newest in-progress state instead of the requested historical turn. Consider counting all turns after the boundary, or at least all turns that have been initiated, so the rollback count reflects the actual conversation distance.
| return error; | ||
| } | ||
|
|
||
| function codexTimestamp(seconds: number | null | undefined): DateTime.Utc { |
There was a problem hiding this comment.
🟠 High Adapters/CodexAdapterV2.ts:213
providerThreadFromCodexThread sets createdAt and updatedAt to NaN when the Codex thread response carries ISO 8601 strings for createdAt/updatedAt. codexTimestamp multiplies the value by 1000 and passes it to DateTime.makeUnsafe, so an ISO string like "2026-04-18T00:00:00.000Z" produces NaN, corrupting the timestamps on every provider thread. The function should parse ISO strings instead of assuming numeric Unix seconds.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts around line 213:
`providerThreadFromCodexThread` sets `createdAt` and `updatedAt` to `NaN` when the Codex thread response carries ISO 8601 strings for `createdAt`/`updatedAt`. `codexTimestamp` multiplies the value by `1000` and passes it to `DateTime.makeUnsafe`, so an ISO string like `"2026-04-18T00:00:00.000Z"` produces `NaN`, corrupting the timestamps on every provider thread. The function should parse ISO strings instead of assuming numeric Unix seconds.
| if (!requiresApproval && sandboxType === "workspaceWrite") { | ||
| const writableRoots = recordValue(sandboxPolicy, "writableRoots"); | ||
| if (Array.isArray(writableRoots)) { | ||
| for (const root of writableRoots) { | ||
| if (typeof root === "string" && root.trim().length > 0) { | ||
| rules.push({ | ||
| permission: "external_directory", | ||
| pattern: `${root.replace(/\/$/, "")}/*`, | ||
| action: "allow", | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Medium Adapters/OpenCodeAdapterV2.ts:569
In openCodePermissionRules, the workspaceWrite branch only emits external_directory allow rules for entries in sandboxPolicy.writableRoots and never checks sandboxPolicy.readOnlyAccess. When a policy sets readOnlyAccess: { type: "fullAccess" } (e.g. WORKSPACE_NEVER_POLICY with type: "workspaceWrite" and approvalPolicy: "never"), reads from external directories outside the workspace remain denied, so OpenCode sessions that should have read-only access to external repos or files will fail or silently reject those reads. Consider adding an external_directory allow rule (pattern * or a scoped path) when readOnlyAccess.type === "fullAccess" in the workspaceWrite branch, mirroring the handling already present in the readOnly branch.
if (!requiresApproval && sandboxType === "workspaceWrite") {
+ const readOnlyAccess = recordValue(sandboxPolicy, "readOnlyAccess");
+ if (recordString(readOnlyAccess, "type") === "fullAccess") {
+ rules.push({ permission: "external_directory", pattern: "*", action: "allow" });
+ }
const writableRoots = recordValue(sandboxPolicy, "writableRoots");🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts around lines 569-582:
In `openCodePermissionRules`, the `workspaceWrite` branch only emits `external_directory` allow rules for entries in `sandboxPolicy.writableRoots` and never checks `sandboxPolicy.readOnlyAccess`. When a policy sets `readOnlyAccess: { type: "fullAccess" }` (e.g. `WORKSPACE_NEVER_POLICY` with `type: "workspaceWrite"` and `approvalPolicy: "never"`), reads from external directories outside the workspace remain denied, so OpenCode sessions that should have read-only access to external repos or files will fail or silently reject those reads. Consider adding an `external_directory` allow rule (pattern `*` or a scoped path) when `readOnlyAccess.type === "fullAccess"` in the `workspaceWrite` branch, mirroring the handling already present in the `readOnly` branch.
| const relativePath = | ||
| resolveWorkspaceRelativeFilePath(props.workspaceRoot, link.path) ?? | ||
| (link.path.startsWith("/") ? null : link.path); |
There was a problem hiding this comment.
🟡 Medium threads/ThreadActivityInspector.tsx:99
File links with Windows absolute paths like C:\repo\file.ts or UNC paths like \\server\share\file are incorrectly treated as relative paths and remain tappable, navigating to ThreadFile with an invalid relative path instead of being disabled. The fallback on line 100 only rejects paths starting with /, so Windows-style absolute paths pass through because they don't begin with /. Consider using isAbsolutePath (which already handles Windows paths) instead of the link.path.startsWith("/") check.
| const relativePath = | |
| resolveWorkspaceRelativeFilePath(props.workspaceRoot, link.path) ?? | |
| (link.path.startsWith("/") ? null : link.path); | |
| const relativePath = | |
| resolveWorkspaceRelativeFilePath(props.workspaceRoot, link.path) ?? | |
| (isAbsolutePath(link.path) ? null : link.path); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadActivityInspector.tsx around lines 99-101:
File links with Windows absolute paths like `C:\repo\file.ts` or UNC paths like `\\server\share\file` are incorrectly treated as relative paths and remain tappable, navigating to `ThreadFile` with an invalid relative path instead of being disabled. The fallback on line 100 only rejects paths starting with `/`, so Windows-style absolute paths pass through because they don't begin with `/`. Consider using `isAbsolutePath` (which already handles Windows paths) instead of the `link.path.startsWith("/")` check.
| CheckpointRollbackServiceV2Shape | ||
| >()("t3/orchestration-v2/CheckpointRollbackService/CheckpointRollbackServiceV2") {} | ||
|
|
||
| export const layer: Layer.Layer< |
There was a problem hiding this comment.
🟠 High orchestration-v2/CheckpointRollbackService.ts:46
session.rollbackThread(...) is called before the rollback outcome is durably recorded via eventSink.write. If the process crashes after rollbackThread succeeds but before the events are written, a retry rebuilds runsToRollback from the stale projection and calls rollbackThread a second time, causing the provider conversation to be rewound too far or to fail against an already-rolled-back thread. Consider writing the rollback events before invoking session.rollbackThread, or guarding against re-entrant rollback with a persisted marker so a retry does not re-execute the provider-side rewind.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/CheckpointRollbackService.ts around line 46:
`session.rollbackThread(...)` is called before the rollback outcome is durably recorded via `eventSink.write`. If the process crashes after `rollbackThread` succeeds but before the events are written, a retry rebuilds `runsToRollback` from the stale projection and calls `rollbackThread` a second time, causing the provider conversation to be rewound too far or to fail against an already-rolled-back thread. Consider writing the rollback events before invoking `session.rollbackThread`, or guarding against re-entrant rollback with a persisted marker so a retry does not re-execute the provider-side rewind.
| } satisfies ProjectionVerificationV2; | ||
| }); | ||
|
|
||
| const rebuild = Effect.gen(function* () { |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionMaintenance.ts:139
rebuild reads all events into memory before opening its delete-and-replay transaction. If a new orchestration event is committed after that read completes but before/during the transaction, EventSink projects the newer event and then rebuild wipes it out by replaying only the stale prefix it already read. The method ends with projection tables missing the latest committed event and projection_metadata.last_sequence rolled back to an earlier value, leaving runtime data inconsistent until another repair runs. Consider moving readAllEvents inside sql.withTransaction (or otherwise acquiring the event-snapshot and projection-wipe under the same transaction/lock) so concurrent appends cannot interleave.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionMaintenance.ts around line 139:
`rebuild` reads all events into memory *before* opening its delete-and-replay transaction. If a new orchestration event is committed after that read completes but before/during the transaction, `EventSink` projects the newer event and then `rebuild` wipes it out by replaying only the stale prefix it already read. The method ends with projection tables missing the latest committed event and `projection_metadata.last_sequence` rolled back to an earlier value, leaving runtime data inconsistent until another repair runs. Consider moving `readAllEvents` inside `sql.withTransaction` (or otherwise acquiring the event-snapshot and projection-wipe under the same transaction/lock) so concurrent appends cannot interleave.
| activeProviderThreadId: projection.thread.activeProviderThreadId, | ||
| latestRunId: latestRun?.id ?? null, | ||
| activeRunId: activeRun?.id ?? null, | ||
| status: latestRun?.status ?? "idle", |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionStore.ts:767
threadShellFromProjection sets status from latestRun?.status instead of activeRun?.status, so when an older run is still executing and a newer run is queued, the shell reports status: "queued" while activeRunId points at the running run. Consumers that gate UI or actions on status will treat an actively executing thread as merely queued and suppress running-only behavior. Consider deriving status from activeRun instead of latestRun.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around line 767:
`threadShellFromProjection` sets `status` from `latestRun?.status` instead of `activeRun?.status`, so when an older run is still executing and a newer run is queued, the shell reports `status: "queued"` while `activeRunId` points at the running run. Consumers that gate UI or actions on `status` will treat an actively executing thread as merely queued and suppress running-only behavior. Consider deriving `status` from `activeRun` instead of `latestRun`.
| parts: new Map(), | ||
| partIdsByMessage: new Map(), | ||
| providerTurn, | ||
| nextItemOrdinal: 1, |
There was a problem hiding this comment.
🟡 Medium Adapters/OpenCodeAdapterV2.ts:1872
Every nested subagent turn initializes nextItemOrdinal to 1, so a second turn in the same child thread reuses ordinals 1, 2, etc. ProjectionStore.sortMessagesByTurnItemOrder orders thread messages by the smallest associated turnItem.ordinal, which means a later turn's messages sort ahead of an earlier turn's messages with the same ordinals. The child conversation appears out of order once a child thread has more than one turn.
The ordinal should continue from where the previous turn in the same child thread left off, rather than resetting to 1 for each turn.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts around line 1872:
Every nested subagent turn initializes `nextItemOrdinal` to `1`, so a second turn in the same child thread reuses ordinals `1`, `2`, etc. `ProjectionStore.sortMessagesByTurnItemOrder` orders thread messages by the smallest associated `turnItem.ordinal`, which means a later turn's messages sort ahead of an earlier turn's messages with the same ordinals. The child conversation appears out of order once a child thread has more than one turn.
The ordinal should continue from where the previous turn in the same child thread left off, rather than resetting to `1` for each turn.
| } | ||
| } | ||
|
|
||
| function toFeedActivity(row: OrchestrationV2ProjectedTurnItem): ThreadFeedActivity { |
There was a problem hiding this comment.
🟠 High lib/threadActivity.ts:289
toFeedActivity eagerly calls JSON.stringify on the entire item for every visible activity row, embedding unbounded payloads (e.g. file_change.diffStr, command_execution.output, proposed_plan.markdown) into fullDetail as a string. A thread with large diffs or tool output allocates a massive serialized string for every row, which can freeze or OOM the mobile UI while building the feed. Consider deferring serialization until the user actually expands a row, or serializing only a bounded preview of item.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/lib/threadActivity.ts around line 289:
`toFeedActivity` eagerly calls `JSON.stringify` on the entire `item` for every visible activity row, embedding unbounded payloads (e.g. `file_change.diffStr`, `command_execution.output`, `proposed_plan.markdown`) into `fullDetail` as a string. A thread with large diffs or tool output allocates a massive serialized string for every row, which can freeze or OOM the mobile UI while building the feed. Consider deferring serialization until the user actually expands a row, or serializing only a bounded preview of `item`.
| stopWithFailure("Unexpected outbound ACP frame", actual); | ||
| return; | ||
| } | ||
| if (actual.kind === "request" && message.id !== undefined && message.id !== null) { |
There was a problem hiding this comment.
🟡 Medium scripts/acp-replay-agent.ts:259
When the client sends two concurrent requests with the same ACP method, pendingClientRequestIds (keyed by actual.method) silently overwrites the first request id on line 260. When the recorded response is later emitted, it replies to the wrong request id, and the first request is never answered. Consider keying pending requests by id instead of method, or tracking multiple outstanding ids per method.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/scripts/acp-replay-agent.ts around line 259:
When the client sends two concurrent requests with the same ACP method, `pendingClientRequestIds` (keyed by `actual.method`) silently overwrites the first request id on line 260. When the recorded response is later emitted, it replies to the wrong request id, and the first request is never answered. Consider keying pending requests by id instead of method, or tracking multiple outstanding ids per method.
| const bootstrap: ProjectService["Service"]["bootstrap"] = Effect.fn("ProjectService.bootstrap")( | ||
| function* (input) { | ||
| const existing = yield* getByWorkspaceRoot(input.workspaceRoot); | ||
| if (Option.isSome(existing)) return { project: existing.value, created: false }; | ||
| return { project: yield* create(input), created: true }; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🟡 Medium project/ProjectService.ts:340
bootstrap calls getByWorkspaceRoot(input.workspaceRoot) before create(input), and getByWorkspaceRoot normalizes the path via workspacePaths.normalizeWorkspaceRoot without passing createIfMissing. So when input.createWorkspaceRootIfMissing is true and the directory does not yet exist, bootstrap throws a ProjectOperationError instead of creating the workspace and bootstrapping the project — the createWorkspaceRootIfMissing option is never honored.
function* (input) {
- const existing = yield* getByWorkspaceRoot(input.workspaceRoot);
- if (Option.isSome(existing)) return { project: existing.value, created: false };
+ const existing = yield* getByWorkspaceRoot(input.workspaceRoot, {
+ includeDeleted: true,
+ }).pipe(
+ Effect.catchTag("WorkspaceRootNotExistsError", () => Option.none()),
+ );
+ if (Option.isSome(existing)) {
+ if (existing.value.deletedAt !== null) {
+ return { project: yield* create(input), created: true };
+ }
+ return { project: existing.value, created: false };
+ }
return { project: yield* create(input), created: true };🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/ProjectService.ts around lines 340-346:
`bootstrap` calls `getByWorkspaceRoot(input.workspaceRoot)` before `create(input)`, and `getByWorkspaceRoot` normalizes the path via `workspacePaths.normalizeWorkspaceRoot` without passing `createIfMissing`. So when `input.createWorkspaceRootIfMissing` is `true` and the directory does not yet exist, `bootstrap` throws a `ProjectOperationError` instead of creating the workspace and bootstrapping the project — the `createWorkspaceRootIfMissing` option is never honored.
|
|
||
| const maxTurnCount = threadContext.value.checkpoints.reduce( | ||
| (max, checkpoint) => Math.max(max, checkpoint.checkpointTurnCount), | ||
| const projection = yield* threads.getThreadProjection(input.threadId).pipe( |
There was a problem hiding this comment.
🟡 Medium checkpointing/CheckpointDiffQuery.ts:102
getTurnDiff catches every error from threads.getThreadProjection and replaces it with CheckpointThreadNotFoundError, so a persistence or decode failure is reported to callers as "thread not found" instead of the real error. This hides backend outages and makes diff requests return a misleading result during infrastructure faults. The Effect.mapError at line 103 discards the original error regardless of its type. Consider narrowing the mapping to only the thread-not-found case (or rethrowing non-not-found errors) so genuine failures propagate.
Also found in 1 other location(s)
apps/server/src/orchestration-v2/ThreadManagementService.ts:241
getProjectThreadmaps everyorchestrator.getThreadProjectionfailure tothread_not_found.OrchestratorV2.getThreadProjectionwraps both missing-thread and projection-read/setup failures inOrchestratorProjectionError, so a database/read failure will now be reported as 'thread not found' instead of surfacing an orchestration error. Calls such assendToThread,waitForThread, andinterruptThreadwill therefore return the wrong failure reason and can mislead callers into treating transient backend failures as a missing thread.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/checkpointing/CheckpointDiffQuery.ts around line 102:
`getTurnDiff` catches every error from `threads.getThreadProjection` and replaces it with `CheckpointThreadNotFoundError`, so a persistence or decode failure is reported to callers as "thread not found" instead of the real error. This hides backend outages and makes diff requests return a misleading result during infrastructure faults. The `Effect.mapError` at line 103 discards the original error regardless of its type. Consider narrowing the mapping to only the thread-not-found case (or rethrowing non-not-found errors) so genuine failures propagate.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/ThreadManagementService.ts:241 -- `getProjectThread` maps every `orchestrator.getThreadProjection` failure to `thread_not_found`. `OrchestratorV2.getThreadProjection` wraps both missing-thread and projection-read/setup failures in `OrchestratorProjectionError`, so a database/read failure will now be reported as 'thread not found' instead of surfacing an orchestration error. Calls such as `sendToThread`, `waitForThread`, and `interruptThread` will therefore return the wrong failure reason and can mislead callers into treating transient backend failures as a missing thread.
| TurnItemPositionStoreV2Shape | ||
| >()("t3/orchestration-v2/TurnItemPositionStore/TurnItemPositionStoreV2") {} | ||
|
|
||
| export const layer: Layer.Layer<TurnItemPositionStoreV2, never, SqlClient.SqlClient> = Layer.effect( |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/TurnItemPositionStore.ts:35
The allocate function can abort the entire event write with an unhandled unique-constraint error when two concurrent calls for different turnItemIds in the same thread compute the same next ordinal. The INSERT ... ON CONFLICT(thread_id, turn_item_id) DO NOTHING only handles conflicts on the (thread_id, turn_item_id) unique constraint, but the table also enforces UNIQUE (thread_id, ordinal). Both transactions read the same MAX(ordinal), both try to insert the same next value, and the second one hits the (thread_id, ordinal) constraint — which is not covered by the ON CONFLICT clause — causing a hard failure instead of retrying or returning an existing position. Consider retrying the allocation on a unique-violation error or using ON CONFLICT(thread_id, ordinal) with an update/do-nothing fallback that re-reads the existing row.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/TurnItemPositionStore.ts around line 35:
The `allocate` function can abort the entire event write with an unhandled unique-constraint error when two concurrent calls for different `turnItemId`s in the same thread compute the same next `ordinal`. The `INSERT ... ON CONFLICT(thread_id, turn_item_id) DO NOTHING` only handles conflicts on the `(thread_id, turn_item_id)` unique constraint, but the table also enforces `UNIQUE (thread_id, ordinal)`. Both transactions read the same `MAX(ordinal)`, both try to insert the same next value, and the second one hits the `(thread_id, ordinal)` constraint — which is not covered by the `ON CONFLICT` clause — causing a hard failure instead of retrying or returning an existing position. Consider retrying the allocation on a unique-violation error or using `ON CONFLICT(thread_id, ordinal)` with an update/do-nothing fallback that re-reads the existing row.
| yield* sql` | ||
| UPDATE orchestration_v2_events | ||
| SET | ||
| driver = provider, | ||
| provider_instance_id = COALESCE( | ||
| json_extract(payload_json, '$.providerInstanceId'), | ||
| provider | ||
| ) | ||
| `; |
There was a problem hiding this comment.
🟡 Medium Migrations/035_OrchestrationV2Foundation.ts:18
The migration backfills orchestration_v2_events.driver from the legacy provider column, but that column also held provider instance IDs (e.g. codex_personal) in the pre-split schema. When an upgraded database has custom instance IDs, driver ends up storing an instance ID instead of a valid ProviderDriverKind. Migration 038_ApplicationEventSource later copies this into application-event metadata, and the event decoder expects driver to be a valid ProviderDriverKind, so replaying or loading historical V2 events can fail after upgrade. Consider deriving driver from the JSON payload's driver field (falling back to a normalized default) instead of copying provider verbatim.
yield* sql`
- UPDATE orchestration_v2_events
- SET
- driver = provider,
- provider_instance_id = COALESCE(
- json_extract(payload_json, '$.providerInstanceId'),
- provider
- )
+ UPDATE orchestration_v2_events
+ SET
+ driver = COALESCE(json_extract(payload_json, '$.driver'), provider),
+ provider_instance_id = COALESCE(
+ json_extract(payload_json, '$.providerInstanceId'),
+ provider
+ )
`;🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/persistence/Migrations/035_OrchestrationV2Foundation.ts around lines 18-26:
The migration backfills `orchestration_v2_events.driver` from the legacy `provider` column, but that column also held provider instance IDs (e.g. `codex_personal`) in the pre-split schema. When an upgraded database has custom instance IDs, `driver` ends up storing an instance ID instead of a valid `ProviderDriverKind`. Migration `038_ApplicationEventSource` later copies this into application-event metadata, and the event decoder expects `driver` to be a valid `ProviderDriverKind`, so replaying or loading historical V2 events can fail after upgrade. Consider deriving `driver` from the JSON payload's `driver` field (falling back to a normalized default) instead of copying `provider` verbatim.
| export const executorLayer: Layer.Layer< | ||
| OrchestrationEffectExecutorV2, | ||
| never, | ||
| | ProviderSessionManagerV2 | ||
| | RunFinalizationService | ||
| | CheckpointRollbackServiceV2 | ||
| | ProviderTurnControlServiceV2 | ||
| | ProviderTurnStartServiceV2 | ||
| | RuntimeRequestServiceV2 | ||
| > = Layer.effect( |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/EffectWorker.ts:39
executorLayer declares its required environment without ResourceCleanupService, but the layer body does yield* ResourceCleanupService and uses it to handle terminal.cleanup and attachment.cleanup effects. Because ResourceCleanupService is a Context.Reference with a no-op default, the layer can build successfully without a real cleanup service, so terminal.cleanup and attachment.cleanup effects silently succeed without deleting anything. Add ResourceCleanupService to the Layer.Layer type parameter's union so the compiler enforces providing a real implementation.
| export const executorLayer: Layer.Layer< | |
| OrchestrationEffectExecutorV2, | |
| never, | |
| | ProviderSessionManagerV2 | |
| | RunFinalizationService | |
| | CheckpointRollbackServiceV2 | |
| | ProviderTurnControlServiceV2 | |
| | ProviderTurnStartServiceV2 | |
| | RuntimeRequestServiceV2 | |
| > = Layer.effect( | |
| export const executorLayer: Layer.Layer< | |
| OrchestrationEffectExecutorV2, | |
| never, | |
| | ProviderSessionManagerV2 | |
| | RunFinalizationService | |
| | CheckpointRollbackServiceV2 | |
| | ProviderTurnControlServiceV2 | |
| | ProviderTurnStartServiceV2 | |
| | RuntimeRequestServiceV2 | |
| | ResourceCleanupService | |
| > = Layer.effect( |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/EffectWorker.ts around lines 39-48:
`executorLayer` declares its required environment without `ResourceCleanupService`, but the layer body does `yield* ResourceCleanupService` and uses it to handle `terminal.cleanup` and `attachment.cleanup` effects. Because `ResourceCleanupService` is a `Context.Reference` with a no-op default, the layer can build successfully without a real cleanup service, so `terminal.cleanup` and `attachment.cleanup` effects silently succeed without deleting anything. Add `ResourceCleanupService` to the `Layer.Layer` type parameter's union so the compiler enforces providing a real implementation.
| break; | ||
| case "queue_message": | ||
| messageIndex += 1; | ||
| pushDispatch( |
There was a problem hiding this comment.
🟡 Medium fixtures/shared.ts:462
The queue_message branch calls dispatchMessageCommand without setting dispatchMode, so it defaults to start_immediately instead of { type: "queue_after_active" }. A queue_message step that should produce a queued turn is dispatched as a normal immediate message, causing the wrong run.status and inputIntent and invalidating queued-turn replay coverage. Consider passing dispatchMode: { type: "queue_after_active" } in the queue_message branch.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/fixtures/shared.ts around line 462:
The `queue_message` branch calls `dispatchMessageCommand` without setting `dispatchMode`, so it defaults to `start_immediately` instead of `{ type: "queue_after_active" }`. A `queue_message` step that should produce a queued turn is dispatched as a normal immediate message, causing the wrong `run.status` and `inputIntent` and invalidating queued-turn replay coverage. Consider passing `dispatchMode: { type: "queue_after_active" }` in the `queue_message` branch.
| }); | ||
| } | ||
|
|
||
| for (let remaining = 1_000; remaining > 0; remaining -= 1) { |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderTurnControlService.ts:173
interruptAndAwaitTerminal waits for terminalization using a fixed 1000-iteration loop of setImmediate yields. setImmediate only advances the Node event loop by one tick — it is not a real-time delay — so the entire budget can elapse in milliseconds. If projection ingestion of the provider terminal event takes longer than 1000 event-loop turns, the method fails spuriously with Provider turn ... did not terminalize before restart even though the turn would settle shortly after. Consider replacing the fixed tick budget with a time-based timeout that sleeps between checks, so the wait duration reflects wall-clock time rather than event-loop turn count.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnControlService.ts around line 173:
`interruptAndAwaitTerminal` waits for terminalization using a fixed 1000-iteration loop of `setImmediate` yields. `setImmediate` only advances the Node event loop by one tick — it is not a real-time delay — so the entire budget can elapse in milliseconds. If projection ingestion of the provider terminal event takes longer than 1000 event-loop turns, the method fails spuriously with `Provider turn ... did not terminalize before restart` even though the turn would settle shortly after. Consider replacing the fixed tick budget with a time-based timeout that sleeps between checks, so the wait duration reflects wall-clock time rather than event-loop turn count.
| const request = projection.runtimeRequests.find( | ||
| (candidate) => candidate.id === input.requestId, | ||
| ); | ||
| if (request === undefined) { |
There was a problem hiding this comment.
🟠 High orchestration-v2/RuntimeRequestService.ts:58
The respond function allows duplicate responses for an already-resolved runtime request. projection.runtimeRequests retains resolved requests, so a second call with the same requestId passes the existence and session guards and forwards another respondToRuntimeRequest call to the provider, causing duplicate approvals or answers. The missing check is a request.status === "pending" (or equivalent) guard before calling session.value.respondToRuntimeRequest.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/RuntimeRequestService.ts around line 58:
The `respond` function allows duplicate responses for an already-resolved runtime request. `projection.runtimeRequests` retains resolved requests, so a second call with the same `requestId` passes the existence and session guards and forwards another `respondToRuntimeRequest` call to the provider, causing duplicate approvals or answers. The missing check is a `request.status === "pending"` (or equivalent) guard before calling `session.value.respondToRuntimeRequest`.
| }), | ||
| ), | ||
| ); | ||
| const row = (yield* readRows()).find( |
There was a problem hiding this comment.
🟡 Medium project/ProjectService.ts:188
getByWorkspaceRoot scans rows in oldest-first order and returns the first row whose workspaceRoot matches. When includeDeleted is true and a workspace was soft-deleted then reused by a newer active project, the method returns the older deleted project instead of the current active one. The .find() predicate does not prefer non-deleted rows, so callers get stale project data. Consider selecting the non-deleted row when one exists, or sorting by recency before the search.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/ProjectService.ts around line 188:
`getByWorkspaceRoot` scans rows in oldest-first order and returns the first row whose `workspaceRoot` matches. When `includeDeleted` is `true` and a workspace was soft-deleted then reused by a newer active project, the method returns the older deleted project instead of the current active one. The `.find()` predicate does not prefer non-deleted rows, so callers get stale project data. Consider selecting the non-deleted row when one exists, or sorting by recency before the search.
Align getSubagentThreadAncestorKeys with the sidebar-threads branch so both carry an identical implementation: stop at projected tree roots and never report parents that are missing from the projection. Keeps the shared file byte-identical across the two PRs that touch it.
| ), | ||
| ...(discoveryWarning ? { discoveryWarning } : {}), | ||
| parsed: { | ||
| version: null, |
There was a problem hiding this comment.
🟡 Medium Layers/CursorProvider.ts:335
checkCursorProviderStatus hard-codes version: null in the success path, so every healthy Cursor snapshot reports no installed version even when the SDK probe succeeds. Downstream consumers that read provider.version or versionAdvisory (e.g. settings UI and update/maintenance handling) can never surface a version or version-advisory state for Cursor providers. Consider populating version from the catalog or probe result instead of null.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CursorProvider.ts around line 335:
`checkCursorProviderStatus` hard-codes `version: null` in the success path, so every healthy Cursor snapshot reports no installed version even when the SDK probe succeeds. Downstream consumers that read `provider.version` or `versionAdvisory` (e.g. settings UI and update/maintenance handling) can never surface a version or version-advisory state for Cursor providers. Consider populating `version` from the catalog or probe result instead of `null`.
| }), | ||
| ), | ||
| ); | ||
| const currentSnapshot = () => |
There was a problem hiding this comment.
🟡 Medium Drivers/AcpRegistryDriver.ts:113
makeSnapshot copies the checkedAt timestamp captured at instance creation into every later snapshot.refresh() and snapshot.getSnapshot() call, so refreshes always report the original creation time instead of when the refresh actually ran. This makes the provider appear permanently stale — downstream cache/UI code cannot tell that a fresh check completed. Consider generating the timestamp inside currentSnapshot (e.g. via DateTime.formatIso(yield* DateTime.now)) so each refresh produces an up-to-date checkedAt.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/AcpRegistryDriver.ts around line 113:
`makeSnapshot` copies the `checkedAt` timestamp captured at instance creation into every later `snapshot.refresh()` and `snapshot.getSnapshot()` call, so refreshes always report the original creation time instead of when the refresh actually ran. This makes the provider appear permanently stale — downstream cache/UI code cannot tell that a fresh check completed. Consider generating the timestamp inside `currentSnapshot` (e.g. via `DateTime.formatIso(yield* DateTime.now)`) so each refresh produces an up-to-date `checkedAt`.
| // persistently and get tombstoned instead of published as completed. | ||
| if (thread.latestTurn?.state === "interrupted" && thread.latestTurn.completedAt !== null) { | ||
| return "completed"; | ||
| if (thread.pendingRuntimeRequest !== null) { |
There was a problem hiding this comment.
🟡 Medium src/agentAwareness.ts:90
A thread whose only pending runtime request is auth_refresh is incorrectly published with phase waiting_for_approval, because the fallback at line 90 treats every non-user_input pendingRuntimeRequest the same way. This marks the thread as interruptive in the relay with the headline "Approval needed" even though no user approval is actually pending. Consider checking for auth_refresh explicitly and excluding it from the waiting_for_approval branch, so such threads resolve via thread.status instead.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/shared/src/agentAwareness.ts around line 90:
A thread whose only pending runtime request is `auth_refresh` is incorrectly published with phase `waiting_for_approval`, because the fallback at line 90 treats every non-`user_input` `pendingRuntimeRequest` the same way. This marks the thread as interruptive in the relay with the headline "Approval needed" even though no user approval is actually pending. Consider checking for `auth_refresh` explicitly and excluding it from the `waiting_for_approval` branch, so such threads resolve via `thread.status` instead.
| yield* agentAwarenessRelay.start(); | ||
| }), | ||
| ), | ||
| autoBootstrap: (serverConfig.autoBootstrapProjectFromCwd |
There was a problem hiding this comment.
🟠 High src/serverRuntimeStartup.ts:438
When serverConfig.autoBootstrapProjectFromCwd is enabled, a failure in resolveAutoBootstrapWelcomeTargets (e.g. projects.bootstrap(...), threads.getShellSnapshot(), or threadLaunch.launch(...)) now aborts the entire startup effect and calls commandGate.failCommandReady(...), preventing the server from ever reaching command readiness. Previously this bootstrap work was best-effort and failures were logged but did not block startup. Consider wrapping the autoBootstrap effect with Effect.catch so bootstrap failures degrade gracefully instead of failing the whole startup.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/serverRuntimeStartup.ts around line 438:
When `serverConfig.autoBootstrapProjectFromCwd` is enabled, a failure in `resolveAutoBootstrapWelcomeTargets` (e.g. `projects.bootstrap(...)`, `threads.getShellSnapshot()`, or `threadLaunch.launch(...)`) now aborts the entire `startup` effect and calls `commandGate.failCommandReady(...)`, preventing the server from ever reaching command readiness. Previously this bootstrap work was best-effort and failures were logged but did not block startup. Consider wrapping the `autoBootstrap` effect with `Effect.catch` so bootstrap failures degrade gracefully instead of failing the whole startup.
| messageId: input.message.messageId, | ||
| text: input.message.text, | ||
| attachments, | ||
| }, |
There was a problem hiding this comment.
🟠 High operations/commands.ts:447
startThreadTurn silently drops input.bootstrap.runSetupScript when launching via ORCHESTRATION_V2_WS_METHODS.launchThread. The launchThread request object built around line 432 never includes the runSetupScript flag, so the server never receives it and setup scripts stop running on first-message launches. Consider forwarding input.bootstrap.runSetupScript in the launchThread payload.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/operations/commands.ts around line 447:
`startThreadTurn` silently drops `input.bootstrap.runSetupScript` when launching via `ORCHESTRATION_V2_WS_METHODS.launchThread`. The `launchThread` request object built around line 432 never includes the `runSetupScript` flag, so the server never receives it and setup scripts stop running on first-message launches. Consider forwarding `input.bootstrap.runSetupScript` in the `launchThread` payload.
| if (runId === input.unsettledRunId || interruptedRunIds.has(runId)) { |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.logic.ts:383
deriveTurnFolds never creates a turn-fold for any run that has a run_interrupt_request or run_interrupt_result event. Lines 320–329 collect every interrupted runId, and line 383 skips all of them unconditionally — not just the latest one. As a result, old interrupted runs never collapse and stay permanently expanded in the timeline.
The interrupt check should only suppress folding for the latest run (matching isLatestInterruptedTurn below), not for every run that was ever interrupted. Consider gating the skip on input.latestRun?.runId === runId.
- if (runId === input.unsettledRunId || interruptedRunIds.has(runId)) {
+ if (runId === input.unsettledRunId || (input.latestRun?.runId === runId && interruptedRunIds.has(runId))) {🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.logic.ts around line 383:
`deriveTurnFolds` never creates a turn-fold for any run that has a `run_interrupt_request` or `run_interrupt_result` event. Lines 320–329 collect every interrupted `runId`, and line 383 skips all of them unconditionally — not just the latest one. As a result, old interrupted runs never collapse and stay permanently expanded in the timeline.
The interrupt check should only suppress folding for the *latest* run (matching `isLatestInterruptedTurn` below), not for every run that was ever interrupted. Consider gating the skip on `input.latestRun?.runId === runId`.
| @@ -32,27 +35,36 @@ export function buildLocalDraftThread( | |||
| draftThread: DraftThreadState, | |||
| fallbackModelSelection: ModelSelection, | |||
| ): Thread { | |||
| return { | |||
| const timestamp = DateTime.makeUnsafe(draftThread.createdAt); | |||
There was a problem hiding this comment.
🟡 Medium components/ChatView.logic.ts:38
buildLocalDraftThread parses draftThread.createdAt with DateTime.makeUnsafe, which throws on invalid date strings. Persisted draft sessions are rehydrated from composerDraftStore with any non-empty createdAt preserved as-is, so a malformed value in local storage will crash draft-thread rendering instead of falling back to a usable draft. Opening the affected draft becomes impossible until storage is cleared. Consider wrapping the parse in a try/catch (or validating the date) and falling back to the current time on failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.logic.ts around line 38:
`buildLocalDraftThread` parses `draftThread.createdAt` with `DateTime.makeUnsafe`, which throws on invalid date strings. Persisted draft sessions are rehydrated from `composerDraftStore` with any non-empty `createdAt` preserved as-is, so a malformed value in local storage will crash draft-thread rendering instead of falling back to a usable draft. Opening the affected draft becomes impossible until storage is cleared. Consider wrapping the parse in a try/catch (or validating the date) and falling back to the current time on failure.
| questions: item.questions.map((question) => ({ ...question, multiSelect: false })), |
There was a problem hiding this comment.
🟡 Medium state/threadRequests.ts:58
derivePendingThreadRequests overwrites every user_input_request question with multiSelect: false, so any pending prompt that actually allows multiple selections is presented to clients as single-select. The UI will only let the user choose one option and will submit the wrong answer shape for multi-select requests. Consider preserving question.multiSelect instead of hardcoding false.
| questions: item.questions.map((question) => ({ ...question, multiSelect: false })), | |
| questions: item.questions.map((question) => ({ ...question })), |
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threadRequests.ts around line 58:
`derivePendingThreadRequests` overwrites every `user_input_request` question with `multiSelect: false`, so any pending prompt that actually allows multiple selections is presented to clients as single-select. The UI will only let the user choose one option and will submit the wrong answer shape for multi-select requests. Consider preserving `question.multiSelect` instead of hardcoding `false`.
| const plans = projection.plans.filter((plan) => plan.kind === "todo_list"); | ||
| const plan = | ||
| [...plans].toReversed().find((candidate) => candidate.runId === latestRunId) ?? | ||
| plans.at(-1) ?? | ||
| null; |
There was a problem hiding this comment.
🟡 Medium src/session-logic.ts:233
deriveActivePlanState returns the last-inserted todo_list plan instead of the most recently updated one when no plan matches latestRunId. When an older plan is updated after a newer plan is created, plans.at(-1) still yields the newer (untouched) plan, so the Tasks panel can display stale steps/explanation for the wrong active plan. Consider sorting candidates by planItemTime(projection, plan.id) (descending) so the most recently updated plan is selected.
const plans = projection.plans.filter((plan) => plan.kind === "todo_list");
- const plan =
- [...plans].toReversed().find((candidate) => candidate.runId === latestRunId) ??
- plans.at(-1) ??
- null;
+ const byRecency = [...plans].toSorted(
+ (left, right) =>
+ planItemTime(projection, left.id).localeCompare(planItemTime(projection, right.id)) ||
+ left.id.localeCompare(right.id),
+ );
+ const plan =
+ [...byRecency].toReversed().find((candidate) => candidate.runId === latestRunId) ??
+ byRecency.at(-1) ??
+ null;Also found in 2 other location(s)
apps/mobile/src/state/use-thread-selection.ts:71
threadDetailToShellselectspendingRuntimeRequestwithprojection.runtimeRequests.find(...), which returns the first pending request in array order instead of the newest one. If a thread has multiple pending requests, an older stale request can win over the current one, causingpresentThreadShellto derive the wrong pending state (hasPendingApprovalsvshasPendingUserInput) and show the wrong thread status until the shell list materializes.
apps/server/src/project/ProjectService.ts:188
getByWorkspaceRootscansreadRows()in creation order and uses.find(...)even whenoptions?.includeDeleted === true. If a project is soft-deleted and a new active project is later created for the sameworkspaceRoot, this method returns the older deleted row instead of the current active project, becauselistAll()is documented to return oldest-first. Callers asking for a workspace lookup with deleted rows included will get stale project data.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/session-logic.ts around lines 233-237:
`deriveActivePlanState` returns the last-inserted `todo_list` plan instead of the most recently updated one when no plan matches `latestRunId`. When an older plan is updated after a newer plan is created, `plans.at(-1)` still yields the newer (untouched) plan, so the Tasks panel can display stale steps/explanation for the wrong active plan. Consider sorting candidates by `planItemTime(projection, plan.id)` (descending) so the most recently updated plan is selected.
Also found in 2 other location(s):
- apps/mobile/src/state/use-thread-selection.ts:71 -- `threadDetailToShell` selects `pendingRuntimeRequest` with `projection.runtimeRequests.find(...)`, which returns the first pending request in array order instead of the newest one. If a thread has multiple pending requests, an older stale request can win over the current one, causing `presentThreadShell` to derive the wrong pending state (`hasPendingApprovals` vs `hasPendingUserInput`) and show the wrong thread status until the shell list materializes.
- apps/server/src/project/ProjectService.ts:188 -- `getByWorkspaceRoot` scans `readRows()` in creation order and uses `.find(...)` even when `options?.includeDeleted === true`. If a project is soft-deleted and a new active project is later created for the same `workspaceRoot`, this method returns the older deleted row instead of the current active project, because `listAll()` is documented to return oldest-first. Callers asking for a workspace lookup with deleted rows included will get stale project data.
| const base = { ...projection, updatedAt: event.occurredAt }; |
There was a problem hiding this comment.
🟡 Medium state/orchestrationV2Projection.ts:97
Every non-thread.* event updates projection.updatedAt but leaves projection.thread.updatedAt stale, so callers like use-thread-selection.ts that sort thread shells by thread.updatedAt show an old timestamp even after incremental run.*, turn-item.updated, etc. events have updated the projection. The case "thread.*" branch replaces thread wholesale with event.payload, but all other branches only update projection.updatedAt via base and never propagate event.occurredAt to thread.updatedAt. Consider updating thread.updatedAt in base (e.g. by spreading event.occurredAt into the thread) so the thread shell timestamp tracks projection activity.
| const base = { ...projection, updatedAt: event.occurredAt }; | |
| const base = { ...projection, updatedAt: event.occurredAt, thread: { ...projection.thread, updatedAt: event.occurredAt } }; |
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/orchestrationV2Projection.ts around line 97:
Every non-`thread.*` event updates `projection.updatedAt` but leaves `projection.thread.updatedAt` stale, so callers like `use-thread-selection.ts` that sort thread shells by `thread.updatedAt` show an old timestamp even after incremental `run.*`, `turn-item.updated`, etc. events have updated the projection. The `case "thread.*"` branch replaces `thread` wholesale with `event.payload`, but all other branches only update `projection.updatedAt` via `base` and never propagate `event.occurredAt` to `thread.updatedAt`. Consider updating `thread.updatedAt` in `base` (e.g. by spreading `event.occurredAt` into the thread) so the thread shell timestamp tracks projection activity.
| useEffect(() => { | ||
| if (!enabled) return; | ||
| const handler = (event: globalThis.KeyboardEvent) => { | ||
| if (!isOpenFavoriteEditorShortcut(event, keybindings)) return; |
There was a problem hiding this comment.
🟡 Medium chat/OpenInPickerShortcut.ts:28
isOpenFavoriteEditorShortcut is called without passing the current shortcut context (e.g., terminalFocus, previewFocus), so when clauses on editor.openFavorite bindings are evaluated against stale defaults. A user keybinding like editor.openFavorite with when: "!terminalFocus" will still fire while the terminal is focused, because the live context state is never supplied to the matcher. Consider passing the current context into isOpenFavoriteEditorShortcut so when clauses resolve against the actual UI state.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/OpenInPickerShortcut.ts around line 28:
`isOpenFavoriteEditorShortcut` is called without passing the current shortcut context (e.g., `terminalFocus`, `previewFocus`), so `when` clauses on `editor.openFavorite` bindings are evaluated against stale defaults. A user keybinding like `editor.openFavorite` with `when: "!terminalFocus"` will still fire while the terminal is focused, because the live context state is never supplied to the matcher. Consider passing the current context into `isOpenFavoriteEditorShortcut` so `when` clauses resolve against the actual UI state.
| ? `${relationshipLabel(primaryParent.edge, props.threadId)}: ${primaryNode?.thread?.title ?? "related thread"}` | ||
| : "Agent session connected"; | ||
|
|
||
| const openThread = (threadId: ThreadId, archivedThread: boolean) => { |
There was a problem hiding this comment.
🟡 Medium threads/ThreadRelationshipsBanner.tsx:109
When a user taps a row pointing at an archived thread, openThread() discards the threadId and navigates to the generic "SettingsArchive" screen, so the specific archived thread the user selected is never opened — they land on the full archive list with no way to distinguish which conversation they wanted. Consider passing threadId (or an environment + search filter) into the navigation params so the archive screen can preselect or deep-link to the tapped thread.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadRelationshipsBanner.tsx around line 109:
When a user taps a row pointing at an archived thread, `openThread()` discards the `threadId` and navigates to the generic `"SettingsArchive"` screen, so the specific archived thread the user selected is never opened — they land on the full archive list with no way to distinguish which conversation they wanted. Consider passing `threadId` (or an environment + search filter) into the navigation params so the archive screen can preselect or deep-link to the tapped thread.
| }; | ||
|
|
||
| for (const root of input.roots) visit(root, 0); | ||
| return rows; |
There was a problem hiding this comment.
🟡 Medium state/threadRelationships.ts:266
walkThreadRelationships marks a thread as visited the first time it is reached, so when the same thread pair is connected by two different edge kinds (e.g. a lineage edge and a transfer edge), only the first edge is emitted and the second is silently dropped. The returned walk is missing valid relationship rows, hiding edges from callers. The visited check should be keyed on the (threadId, edge) pair (or edge key) rather than on relatedId alone, so distinct relationships to the same thread are all included.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threadRelationships.ts around line 266:
`walkThreadRelationships` marks a thread as visited the first time it is reached, so when the same thread pair is connected by two different edge kinds (e.g. a lineage edge and a `transfer` edge), only the first edge is emitted and the second is silently dropped. The returned walk is missing valid relationship rows, hiding edges from callers. The `visited` check should be keyed on the `(threadId, edge)` pair (or edge key) rather than on `relatedId` alone, so distinct relationships to the same thread are all included.
| const providerSession = | ||
| providerThread?.providerSessionId == null | ||
| ? null | ||
| : (projection.providerSessions.find( | ||
| (candidate) => candidate.id === providerThread.providerSessionId, | ||
| ) ?? null); | ||
| const providerTurn = | ||
| item.providerTurnId === null | ||
| ? null | ||
| : (projection.providerTurns.find((candidate) => candidate.id === item.providerTurnId) ?? | ||
| null); | ||
| const requestId = |
There was a problem hiding this comment.
🟡 Medium state/itemSupport.ts:64
resolveV2ItemSupport returns providerSession: null for items whose session is only reachable through runtimeRequest.responseCapability.providerSessionId when providerThread.providerSessionId is null (e.g., live runtime requests with an ad-hoc session). The test fixture models exactly this case (providerThread.providerSessionId: null, runtimeRequest.responseCapability.providerSessionId: session-1). Consumers lose session/model/cwd details for the item despite the projection containing the session record. Consider deriving providerSession from the runtime request's providerSessionId as a fallback before giving up.
const providerSession =
providerThread?.providerSessionId == null
? null
: (projection.providerSessions.find(
(candidate) => candidate.id === providerThread.providerSessionId,
) ?? null);
+ const fallbackProviderSession =
+ requestId != null
+ ? (projection.runtimeRequests.find((candidate) => candidate.id === requestId) ?? null)
+ : null;
const providerTurn =🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/itemSupport.ts around lines 64-75:
`resolveV2ItemSupport` returns `providerSession: null` for items whose session is only reachable through `runtimeRequest.responseCapability.providerSessionId` when `providerThread.providerSessionId` is `null` (e.g., live runtime requests with an ad-hoc session). The test fixture models exactly this case (`providerThread.providerSessionId: null`, `runtimeRequest.responseCapability.providerSessionId: session-1`). Consumers lose session/model/cwd details for the item despite the projection containing the session record. Consider deriving `providerSession` from the runtime request's `providerSessionId` as a fallback before giving up.
| @@ -157,15 +166,18 @@ export const OpenInPicker = memo(function OpenInPicker({ | |||
| availableEditors, | |||
| openInCwd, | |||
| compact = false, | |||
| enableShortcut = true, | |||
| displayMode = "toolbar", | |||
There was a problem hiding this comment.
🟡 Medium chat/OpenInPicker.tsx:169
Removing the per-instance keyboard shortcut handler means editor.openFavorite always opens the thread root gitCwd instead of the file shown in a file preview. FilePreviewPanel renders a compact OpenInPicker with openInCwd={absolutePath}, so pressing the shortcut previously opened that specific file. After this change only ChatView's global handler remains, so the same shortcut always targets the thread root directory regardless of which file is being previewed. Consider restoring per-instance shortcut handling (or forwarding the preview path to the global handler) so the shortcut opens the correct file.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/OpenInPicker.tsx around line 169:
Removing the per-instance keyboard shortcut handler means `editor.openFavorite` always opens the thread root `gitCwd` instead of the file shown in a file preview. `FilePreviewPanel` renders a compact `OpenInPicker` with `openInCwd={absolutePath}`, so pressing the shortcut previously opened that specific file. After this change only `ChatView`'s global handler remains, so the same shortcut always targets the thread root directory regardless of which file is being previewed. Consider restoring per-instance shortcut handling (or forwarding the preview path to the global handler) so the shortcut opens the correct file.
| return available; | ||
| }); | ||
|
|
||
| const invalidate: ProjectEnrichmentService["Service"]["invalidate"] = Effect.fn( |
There was a problem hiding this comment.
🟡 Medium project/ProjectEnrichmentService.ts:259
invalidate removes entries from the caches but does not cancel or deduplicate work already queued or in-flight for the same workspaceRoot. A resolver that started before the invalidation can write its old result back into the cache afterward, so the caller immediately sees stale metadata after requesting a refresh. Consider tracking in-flight or queued roots per lane and skipping or re-queueing stale completions, or canceling pending work for the invalidated roots.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/ProjectEnrichmentService.ts around line 259:
`invalidate` removes entries from the caches but does not cancel or deduplicate work already queued or in-flight for the same `workspaceRoot`. A resolver that started before the invalidation can write its old result back into the cache afterward, so the caller immediately sees stale metadata after requesting a refresh. Consider tracking in-flight or queued roots per lane and skipping or re-queueing stale completions, or canceling pending work for the invalidated roots.
| checkpoints: upsertById(base.checkpoints, event.payload), | ||
| }; | ||
| case "checkpoint.rollback-requested": | ||
| return base; |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionStore.ts:278
When a context-transfer.created or context-transfer.updated event is applied, the transfer is only inserted into the projection for event.threadId. The SQL-backed store exposes a transfer on both sourceThreadId and targetThreadId, so a transfer event emitted for one side is missing from the other thread's memory projection. Callers reading the non-emitting thread miss transfers involving their thread.
applyToProjection only knows about the single projection it receives, so the fix belongs in applyToProjectionReplayState (which has access to all projections) — it should upsert the transfer into both the sourceThreadId and targetThreadId projections, similar to how provider-session.attached/provider-session.updated already fan out across bound threads.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around line 278:
When a `context-transfer.created` or `context-transfer.updated` event is applied, the transfer is only inserted into the projection for `event.threadId`. The SQL-backed store exposes a transfer on **both** `sourceThreadId` and `targetThreadId`, so a transfer event emitted for one side is missing from the other thread's memory projection. Callers reading the non-emitting thread miss transfers involving their thread.
`applyToProjection` only knows about the single projection it receives, so the fix belongs in `applyToProjectionReplayState` (which has access to all projections) — it should upsert the transfer into both the `sourceThreadId` and `targetThreadId` projections, similar to how `provider-session.attached`/`provider-session.updated` already fan out across bound threads.
| const throughOrdinal = Math.max( | ||
| 0, | ||
| ...projection.runs.map((run) => run.ordinal), | ||
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | ||
| ); |
There was a problem hiding this comment.
🟠 High orchestration-v2/CheckpointCleanupService.ts:43
When a thread accumulates many runs and checkpoints, Math.max(0, ...projection.runs.map(...), ...projection.checkpoints.map(...)) throws a RangeError: Maximum call stack size exceeded because it spreads every ordinal as a function argument. This aborts cleanup and leaves checkpoint refs undeleted. Consider computing the max with a reducer instead of spreading into Math.max.
| const throughOrdinal = Math.max( | |
| 0, | |
| ...projection.runs.map((run) => run.ordinal), | |
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | |
| ); | |
| const throughOrdinal = [ | |
| ...projection.runs.map((run) => run.ordinal), | |
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | |
| ].reduce((max, n) => (n > max ? n : max), 0); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/CheckpointCleanupService.ts around lines 43-47:
When a thread accumulates many runs and checkpoints, `Math.max(0, ...projection.runs.map(...), ...projection.checkpoints.map(...))` throws a `RangeError: Maximum call stack size exceeded` because it spreads every ordinal as a function argument. This aborts `cleanup` and leaves checkpoint refs undeleted. Consider computing the max with a reducer instead of spreading into `Math.max`.
| return next; | ||
| }); | ||
| }, []); | ||
| const listLayout = useMemo( |
There was a problem hiding this comment.
🟡 Medium home/HomeScreen.tsx:268
In nested-subagent mode, a currently open subagent child thread disappears from the phone home list when its root falls past the initial page. HomeScreen calls buildHomeListLayout without pinnedThreadKey, and expandedSubagentThreadKeys starts empty, so if a deep-linked child lives under an older root the list renders only the first page of roots with no expanded ancestors — the open thread is unreachable until the user manually taps "Show more" and expands each ancestor. buildHomeListLayout already has pinnedThreadKey support for exactly this scenario; consider passing the currently selected thread's key so its branch root is kept visible.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/home/HomeScreen.tsx around line 268:
In nested-subagent mode, a currently open subagent child thread disappears from the phone home list when its root falls past the initial page. `HomeScreen` calls `buildHomeListLayout` without `pinnedThreadKey`, and `expandedSubagentThreadKeys` starts empty, so if a deep-linked child lives under an older root the list renders only the first page of roots with no expanded ancestors — the open thread is unreachable until the user manually taps "Show more" and expands each ancestor. `buildHomeListLayout` already has `pinnedThreadKey` support for exactly this scenario; consider passing the currently selected thread's key so its branch root is kept visible.
| const providerEventIngestor = yield* ProviderEventIngestorV2; | ||
| const serverSettings = yield* ServerSettingsService; | ||
|
|
||
| const writeFinalRunEvents = (input: { |
There was a problem hiding this comment.
🟠 High orchestration-v2/RunExecutionService.ts:300
writeFinalRunEvents writes terminal run.updated, node.updated, and provider-thread.updated events via eventSink.writeWithEffects without a transactional run-ownership guard. If a restart supersedes this attempt after shouldFinalizeRun() is checked but before the write commits, the superseded attempt overwrites the newer attempt's run state and enqueues a stale checkpoint capture. The shouldFinalizeRun() callback is a non-atomic precondition, so the window between the check and the unguarded write allows a superseded attempt to incorrectly persist terminal status for the current run. Consider routing the terminal events through eventSink.writeIfRunCurrent (with allowSupersededAttemptTerminalArtifacts where appropriate) so the write is rejected when the attempt no longer owns the run.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/RunExecutionService.ts around line 300:
`writeFinalRunEvents` writes terminal `run.updated`, `node.updated`, and `provider-thread.updated` events via `eventSink.writeWithEffects` without a transactional run-ownership guard. If a restart supersedes this attempt after `shouldFinalizeRun()` is checked but before the write commits, the superseded attempt overwrites the newer attempt's run state and enqueues a stale checkpoint capture. The `shouldFinalizeRun()` callback is a non-atomic precondition, so the window between the check and the unguarded write allows a superseded attempt to incorrectly persist terminal status for the current run. Consider routing the terminal events through `eventSink.writeIfRunCurrent` (with `allowSupersededAttemptTerminalArtifacts` where appropriate) so the write is rejected when the attempt no longer owns the run.
| function* ( | ||
| expectedStatus: OrchestrationV2Run["status"], | ||
| ): Effect.fn.Return<CurrentAttemptState, never> { | ||
| const currentOption = yield* Effect.option( |
There was a problem hiding this comment.
🟠 High orchestration-v2/ProviderTurnStartService.ts:149
Transient storage errors during projectionStore.getThreadProjection(...) are silently swallowed by getCurrentAttemptState, causing start() to exit successfully without advancing the run. The run stays stuck in starting with no retry or surfaced error, so provider-turn starts silently hang on ordinary database failures.
getCurrentAttemptState wraps the projection read in Effect.option(...) and then Effect.catchCause(...), converting every failure — including transient store/DB errors — into "run_stale". continueIfCurrent then returns false and start() returns normally. Consider distinguishing transient errors (which should propagate) from a genuinely stale or missing run (which may warrant the current no-op behavior), so storage failures are not masked as stale-state exits.
Also found in 1 other location(s)
apps/server/src/orchestration-v2/CheckpointService.ts:299
isGitCheckpointableat line299converts everyCheckpointStore.isGitRepositoryfailure intofalse. If VCS detection fails because of a real runtime problem (for example a transientVcsError, permission problem, or driver failure),captureBaseline,materializeBaselineCheckpoint, andcaptureall silently behave as though the workspace is not a Git repo and emit checkpoints with status"missing"/"error"instead of surfacing the failure. That leaves runs completing without a usable rollback checkpoint even though checkpointing was expected to work.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around line 149:
Transient storage errors during `projectionStore.getThreadProjection(...)` are silently swallowed by `getCurrentAttemptState`, causing `start()` to exit successfully without advancing the run. The run stays stuck in `starting` with no retry or surfaced error, so provider-turn starts silently hang on ordinary database failures.
`getCurrentAttemptState` wraps the projection read in `Effect.option(...)` and then `Effect.catchCause(...)`, converting every failure — including transient store/DB errors — into `"run_stale"`. `continueIfCurrent` then returns `false` and `start()` returns normally. Consider distinguishing transient errors (which should propagate) from a genuinely stale or missing run (which may warrant the current no-op behavior), so storage failures are not masked as stale-state exits.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/CheckpointService.ts:299 -- `isGitCheckpointable` at line `299` converts every `CheckpointStore.isGitRepository` failure into `false`. If VCS detection fails because of a real runtime problem (for example a transient `VcsError`, permission problem, or driver failure), `captureBaseline`, `materializeBaselineCheckpoint`, and `capture` all silently behave as though the workspace is not a Git repo and emit checkpoints with status `"missing"`/`"error"` instead of surfacing the failure. That leaves runs completing without a usable rollback checkpoint even though checkpointing was expected to work.
|
|
||
| function readThreadSubtree(thread: EnvironmentThreadShell): readonly EnvironmentThreadShell[] { | ||
| const snapshot = appAtomRegistry.get(environmentSnapshotAtom(thread.environmentId)); | ||
| if (snapshot === null) return [thread]; |
There was a problem hiding this comment.
🟠 High home/useThreadListActions.ts:43
readThreadSubtree returns only [thread] when the environment snapshot is null, so actionSubagentDescendants sees zero descendants. On the archived-threads screen, where appAtomRegistry.get(environmentSnapshotAtom(...)) is null, this causes useThreadAction to skip the confirmation dialog for a parent that still owns subagent children. Because the server applies thread.delete and thread.unarchive recursively, deleting or unarchiving a parent permanently removes or restores hidden child threads with no warning. Consider sourcing the subtree from the same archived-threads snapshot the screen already uses instead of returning [thread] when the live shell snapshot is null.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/home/useThreadListActions.ts around line 43:
`readThreadSubtree` returns only `[thread]` when the environment snapshot is `null`, so `actionSubagentDescendants` sees zero descendants. On the archived-threads screen, where `appAtomRegistry.get(environmentSnapshotAtom(...))` is `null`, this causes `useThreadAction` to skip the confirmation dialog for a parent that still owns subagent children. Because the server applies `thread.delete` and `thread.unarchive` recursively, deleting or unarchiving a parent permanently removes or restores hidden child threads with no warning. Consider sourcing the subtree from the same archived-threads snapshot the screen already uses instead of returning `[thread]` when the live shell snapshot is `null`.
| yield* trackChildLifecycle(event); | ||
| }), | ||
| ), | ||
| Stream.takeUntilEffect(() => shouldStopProviderEventIngestion), |
There was a problem hiding this comment.
🟠 High orchestration-v2/RunExecutionService.ts:691
Stream.takeUntilEffect(() => shouldStopProviderEventIngestion) stops the stream as soon as the root turn.terminal event arrives and activeChildProviderTurns and activeChildSubagents are both still empty. This drops owned child artifacts that arrive after the root terminal event — for example a late app_thread.created, provider_thread.updated, or message.updated for a nested subagent thread still in flight. The takeUntilEffect check runs immediately after the terminal event is processed (in the same Stream.tap callback), so if no child provider turns or subagents have been observed yet, the stream closes before the late child events can be ingested. Consider waiting for a grace period after the root terminal before stopping the stream, or deferring the stop check until at least one stream iteration after the terminal event.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/RunExecutionService.ts around line 691:
`Stream.takeUntilEffect(() => shouldStopProviderEventIngestion)` stops the stream as soon as the root `turn.terminal` event arrives and `activeChildProviderTurns` and `activeChildSubagents` are both still empty. This drops owned child artifacts that arrive *after* the root terminal event — for example a late `app_thread.created`, `provider_thread.updated`, or `message.updated` for a nested subagent thread still in flight. The `takeUntilEffect` check runs immediately after the terminal event is processed (in the same `Stream.tap` callback), so if no child provider turns or subagents have been observed yet, the stream closes before the late child events can be ingested. Consider waiting for a grace period after the root terminal before stopping the stream, or deferring the stop check until at least one stream iteration after the terminal event.
| }): HomeListLayout { | ||
| const items: HomeListItem[] = []; | ||
| const stickyHeaderIndices: number[] = []; | ||
| const allThreads = input.groups.flatMap((group) => group.threads); |
There was a problem hiding this comment.
🟡 Medium home/homeListItems.ts:158
hasUnavailableSubagentParent is called with allThreads, which is built from group.threads — the threads that remain after search/filter grouping. When a subagent child is visible because its project matched, but its parent thread lives in a different project group that was filtered out, the function reports the parent as unavailable and the row renders a false Parent unavailable label. The parent thread still exists; it is just hidden by the current view filter. Consider passing the full, unfiltered thread list (e.g., all threads across all projects/environments before grouping) so the availability check reflects actual thread existence rather than current visibility.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/home/homeListItems.ts around line 158:
`hasUnavailableSubagentParent` is called with `allThreads`, which is built from `group.threads` — the threads that remain after search/filter grouping. When a subagent child is visible because its project matched, but its parent thread lives in a different project group that was filtered out, the function reports the parent as unavailable and the row renders a false `Parent unavailable` label. The parent thread still exists; it is just hidden by the current view filter. Consider passing the full, unfiltered thread list (e.g., all threads across all projects/environments before grouping) so the availability check reflects actual thread existence rather than current visibility.
| ownerNodeId: providerThread.ownerNodeId, | ||
| firstRunOrdinal: providerThread.firstRunOrdinal ?? run.ordinal, | ||
| lastRunOrdinal: run.ordinal, | ||
| handoffIds: providerThread.handoffIds, |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderTurnStartService.ts:342
When session.resumeThread(...) fails and a portable fallback handoff is created, the persisted provider-thread.updated event sets handoffIds to providerThread.handoffIds without appending the new handoff.id. This silently drops the linkage between the replacement provider thread and the fallback context handoff — any code that later reads providerThread.handoffIds will not find the handoff that was just written. Consider updating runningProviderThread.handoffIds to include the new handoff ID when effectiveHandoffs has been extended.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around line 342:
When `session.resumeThread(...)` fails and a portable fallback handoff is created, the persisted `provider-thread.updated` event sets `handoffIds` to `providerThread.handoffIds` without appending the new `handoff.id`. This silently drops the linkage between the replacement provider thread and the fallback context handoff — any code that later reads `providerThread.handoffIds` will not find the handoff that was just written. Consider updating `runningProviderThread.handoffIds` to include the new handoff ID when `effectiveHandoffs` has been extended.
| ), | ||
| ); | ||
|
|
||
| yield* recorder.writeRecord({ |
There was a problem hiding this comment.
🟡 Medium scripts/record-codex-app-server-replay-fixture.ts:1303
The provider_thread_resume scenario writes a runtime_exit record between the first and second app-server sessions, but the replay driver treats runtime_exit as a permanent end-of-transcript marker. As a result, replaying the generated fixture stops after the first turn and never reaches thread/resume or the second turn. Consider removing the intermediate runtime_exit record so the second session's records remain reachable during replay.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/scripts/record-codex-app-server-replay-fixture.ts around line 1303:
The `provider_thread_resume` scenario writes a `runtime_exit` record between the first and second app-server sessions, but the replay driver treats `runtime_exit` as a permanent end-of-transcript marker. As a result, replaying the generated fixture stops after the first turn and never reaches `thread/resume` or the second turn. Consider removing the intermediate `runtime_exit` record so the second session's records remain reachable during replay.
|
|
||
| private throwFailure(): void { | ||
| if (this.failure !== null) throw this.failure; | ||
| } |
There was a problem hiding this comment.
🟡 Medium Adapters/OpenCodeAdapterV2.testkit.ts:243
response() can hang forever waiting for an entry that is already available. When response() sees an emit_inbound entry whose frame type is not sdk.response, it calls await this.changed() to wait for the cursor to advance. But advance() clears all current waiters before response() registers its own waiter — if another consumer (e.g. events()) consumes the entry and calls advance() in the window between response() reading this.cursor and registering the waiter, response() sleeps with no guaranteed future advance() to wake it. The same lost-wakeup race exists in events(). Consider re-checking the cursor after registering the waiter, or adding the waiter before reading the current entry so no advance() is missed.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.testkit.ts around line 243:
`response()` can hang forever waiting for an entry that is already available. When `response()` sees an `emit_inbound` entry whose frame type is not `sdk.response`, it calls `await this.changed()` to wait for the cursor to advance. But `advance()` clears all current waiters before `response()` registers its own waiter — if another consumer (e.g. `events()`) consumes the entry and calls `advance()` in the window between `response()` reading `this.cursor` and registering the waiter, `response()` sleeps with no guaranteed future `advance()` to wake it. The same lost-wakeup race exists in `events()`. Consider re-checking the cursor after registering the waiter, or adding the waiter before reading the current entry so no `advance()` is missed.
| activeRun, | ||
| queuedRuns, | ||
| canReorder: capabilities?.supportsQueuedMessages === true, | ||
| canPromoteToSteer: |
There was a problem hiding this comment.
🟡 Medium state/threadWorkflows.ts:75
canPromoteToSteer is true whenever activeRun is non-null, including runs with status "preparing", "starting", or "waiting". The server only allows steering when the run is "running" with an active provider turn, so promoting in those other states fails with thread_not_sendable. The check at line 76 should additionally require activeRun.status === "running".
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threadWorkflows.ts around line 75:
`canPromoteToSteer` is `true` whenever `activeRun` is non-null, including runs with status `"preparing"`, `"starting"`, or `"waiting"`. The server only allows steering when the run is `"running"` with an active provider turn, so promoting in those other states fails with `thread_not_sendable`. The check at line 76 should additionally require `activeRun.status === "running"`.
| @@ -0,0 +1,1428 @@ | |||
| import * as NodeRuntime from "@effect/platform-node/NodeRuntime"; | |||
There was a problem hiding this comment.
🟡 Medium scripts/record-codex-app-server-replay-fixture.ts:1
The steered-turn flow deadlocks when the initial prompt triggers an approval request before turn/started fires. The approval handlers for item/commandExecution/requestApproval, item/fileChange/requestApproval, and item/permissions/requestApproval all call beforeApprovalResponse(), which awaits approvalGate. But approvalGate is only released after turn/steer is sent, and turn/steer is sent only after Deferred.await(started) completes — which requires the server to emit turn/started. If the server blocks on an approval request before emitting turn/started, both sides wait on each other indefinitely. Consider releasing the gate or restructuring the steer sequencing so approval requests are not blocked on turn/started.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/scripts/record-codex-app-server-replay-fixture.ts around line 1:
The steered-turn flow deadlocks when the initial prompt triggers an approval request before `turn/started` fires. The approval handlers for `item/commandExecution/requestApproval`, `item/fileChange/requestApproval`, and `item/permissions/requestApproval` all call `beforeApprovalResponse()`, which awaits `approvalGate`. But `approvalGate` is only released after `turn/steer` is sent, and `turn/steer` is sent only after `Deferred.await(started)` completes — which requires the server to emit `turn/started`. If the server blocks on an approval request before emitting `turn/started`, both sides wait on each other indefinitely. Consider releasing the gate or restructuring the steer sequencing so approval requests are not blocked on `turn/started`.
| providerSessions: upsertById(base.providerSessions, event.payload), | ||
| }; | ||
| case "provider-session.detached": | ||
| return { |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionStore.ts:224
applyToProjection handles provider-thread.updated by upserting the payload only into the current thread's providerThreads array. When a provider thread belongs to a subagent node in a different thread, the parent-thread projection built by the memory-backed store never receives that provider thread, so its providerThreads list is missing entries that getThreadProjection on the SQL store returns. Any feature that inspects subagent provider threads sees different results depending on which projection store backs the thread. Consider mirroring the cross-thread fan-out already done for provider-session.* events (via providerSessionThreadIds) so provider-thread.updated events also upsert into every bound parent-thread projection, or document why the memory store intentionally omits this relationship.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around line 224:
`applyToProjection` handles `provider-thread.updated` by upserting the payload only into the current thread's `providerThreads` array. When a provider thread belongs to a subagent node in a different thread, the parent-thread projection built by the memory-backed store never receives that provider thread, so its `providerThreads` list is missing entries that `getThreadProjection` on the SQL store returns. Any feature that inspects subagent provider threads sees different results depending on which projection store backs the thread. Consider mirroring the cross-thread fan-out already done for `provider-session.*` events (via `providerSessionThreadIds`) so `provider-thread.updated` events also upsert into every bound parent-thread projection, or document why the memory store intentionally omits this relationship.
| const make = Effect.gen(function* () { | ||
| const orchestrator = yield* OrchestratorV2; | ||
|
|
||
| const getProjectThread: ThreadManagementServiceShape["getProjectThread"] = (input) => |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ThreadManagementService.ts:239
getProjectThread maps every orchestrator.getThreadProjection failure to thread_not_found, including projection-read failures. When the orchestrator fails due to a database or read error (wrapped in OrchestratorProjectionError), callers of sendToThread, waitForThread, and interruptThread receive a thread_not_found error code instead of an orchestration error, causing transient backend failures to be misreported as missing threads. Consider distinguishing projection-read failures from genuinely missing threads — e.g., by inspecting the OrchestratorProjectionError or mapping non-not-found causes to orchestration_error.
Also found in 2 other location(s)
apps/server/src/checkpointing/CheckpointDiffQuery.ts:103
getTurnDiffnow wraps every failure fromthreads.getThreadProjectioninCheckpointThreadNotFoundError. If projection loading fails for any reason other than a missing thread (for example a persistence/decode error insideOrchestratorV2), callers will incorrectly receive a "thread not found" response instead of the real checkpoint/orchestration failure, which hides outages and makes diff requests behave incorrectly during backend faults.
apps/server/src/project/http.ts:73
The catch-all at lines
70-73converts everyProjectServicefailure intoEnvironmentInternalError. That means ordinary user mistakes such asProjectConflictError(creating a project on an already-owned workspace) orProjectNotFoundError(updating/deleting an unknown project) are returned from/api/projects/mutateasinternal_errorinstead of the declaredEnvironmentRequestInvalidError, so HTTP clients cannot distinguish invalid input from a server failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadManagementService.ts around line 239:
`getProjectThread` maps every `orchestrator.getThreadProjection` failure to `thread_not_found`, including projection-read failures. When the orchestrator fails due to a database or read error (wrapped in `OrchestratorProjectionError`), callers of `sendToThread`, `waitForThread`, and `interruptThread` receive a `thread_not_found` error code instead of an orchestration error, causing transient backend failures to be misreported as missing threads. Consider distinguishing projection-read failures from genuinely missing threads — e.g., by inspecting the `OrchestratorProjectionError` or mapping non-not-found causes to `orchestration_error`.
Also found in 2 other location(s):
- apps/server/src/checkpointing/CheckpointDiffQuery.ts:103 -- `getTurnDiff` now wraps every failure from `threads.getThreadProjection` in `CheckpointThreadNotFoundError`. If projection loading fails for any reason other than a missing thread (for example a persistence/decode error inside `OrchestratorV2`), callers will incorrectly receive a "thread not found" response instead of the real checkpoint/orchestration failure, which hides outages and makes diff requests behave incorrectly during backend faults.
- apps/server/src/project/http.ts:73 -- The catch-all at lines `70`-`73` converts every `ProjectService` failure into `EnvironmentInternalError`. That means ordinary user mistakes such as `ProjectConflictError` (creating a project on an already-owned workspace) or `ProjectNotFoundError` (updating/deleting an unknown project) are returned from `/api/projects/mutate` as `internal_error` instead of the declared `EnvironmentRequestInvalidError`, so HTTP clients cannot distinguish invalid input from a server failure.
| }, | ||
| }), | ||
| ); | ||
| if (input.fixtureInput.steps[stepIndex + 1]?.type !== "approve_next_runtime_request") { |
There was a problem hiding this comment.
🟡 Medium fixtures/shared.ts:560
A steer or restart step followed by answer_next_user_input_request deadlocks the scenario. When the steered/restarted run pauses for user input, it enters waiting status, which await_thread_idle treats as active. Because await_thread_idle is inserted before the answer_next_user_input_request step runs, the scenario waits for a run that is blocked on the very answer that comes next.
The guard at lines 560 and 594 only suppresses await_thread_idle when the next step is approve_next_runtime_request, but not for answer_next_user_input_request. Include answer_next_user_input_request in that condition so the answer step runs before waiting for idle.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/fixtures/shared.ts around line 560:
A `steer` or `restart` step followed by `answer_next_user_input_request` deadlocks the scenario. When the steered/restarted run pauses for user input, it enters `waiting` status, which `await_thread_idle` treats as active. Because `await_thread_idle` is inserted before the `answer_next_user_input_request` step runs, the scenario waits for a run that is blocked on the very answer that comes next.
The guard at lines 560 and 594 only suppresses `await_thread_idle` when the next step is `approve_next_runtime_request`, but not for `answer_next_user_input_request`. Include `answer_next_user_input_request` in that condition so the answer step runs before waiting for idle.
| prefix: `t3-orchestration-v2-codex-${scenario}-`, | ||
| }); | ||
| const stateDir = path.join(baseDir, "userdata"); |
There was a problem hiding this comment.
🟡 Medium Adapters/CodexAdapterV2.testkit.ts:69
makeReplayServerConfig passes the raw scenario string into fs.makeTempDirectory as the temp directory prefix. Scenarios like provider_thread_resume/codex:first-runtime contain /, and Node's fs.mkdtemp appends random characters directly to the prefix, so the embedded path separator turns the prefix into a nested path whose parent does not exist. The result is an ENOENT error when building the Codex replay layer for those scenarios, instead of starting the replay harness. Consider sanitizing the scenario (e.g. replacing / and : with -) before using it in the prefix.
| prefix: `t3-orchestration-v2-codex-${scenario}-`, | |
| }); | |
| const stateDir = path.join(baseDir, "userdata"); | |
| const baseDir = yield* fs.makeTempDirectory({ | |
| prefix: `t3-orchestration-v2-codex-${scenario.replace(/[^a-zA-Z0-9]/g, "-")}-`, | |
| }); |
Also found in 1 other location(s)
apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts:519
makeReplayServerConfiginterpolatesscenariodirectly into thefs.makeTempDirectoryprefix. Unlike the shared replay helper, it never sanitizes path separators, so a transcript whosescenariocontains/or\makes the prefix point into a non-existent subdirectory andmakeTempDirectoryfails withENOENT. The decoded transcript schema accepts any string here, so replay harness setup breaks for scenario names that include path separators.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.testkit.ts around lines 69-71:
`makeReplayServerConfig` passes the raw `scenario` string into `fs.makeTempDirectory` as the temp directory prefix. Scenarios like `provider_thread_resume/codex:first-runtime` contain `/`, and Node's `fs.mkdtemp` appends random characters directly to the prefix, so the embedded path separator turns the prefix into a nested path whose parent does not exist. The result is an `ENOENT` error when building the Codex replay layer for those scenarios, instead of starting the replay harness. Consider sanitizing the scenario (e.g. replacing `/` and `:` with `-`) before using it in the prefix.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts:519 -- `makeReplayServerConfig` interpolates `scenario` directly into the `fs.makeTempDirectory` prefix. Unlike the shared replay helper, it never sanitizes path separators, so a transcript whose `scenario` contains `/` or `\` makes the prefix point into a non-existent subdirectory and `makeTempDirectory` fails with `ENOENT`. The decoded transcript schema accepts any string here, so replay harness setup breaks for scenario names that include path separators.
| const safeScenario = scenario.replace(/[^a-z0-9_-]+/gi, "-"); | ||
| return Effect.gen(function* () { | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const path = yield* Path.Path; |
There was a problem hiding this comment.
🟡 Medium testkit/ProviderReplayHarness.ts:71
makeReplayServerConfig creates a temp directory via fs.makeTempDirectory but never registers cleanup, so every call leaks the entire replay workspace (database, logs, worktrees, attachments) in the system temp directory. Repeated test runs will steadily consume disk and can fail when the temp volume fills. Consider scoping the temp directory so it is removed when the effect's scope closes.
Also found in 2 other location(s)
apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.testkit.ts:46
makeAcpRegistryProviderAdapterRegistryReplayLayerallocatesreplayDirwithfileSystem.makeTempDirectoryat line 46 and never removes it. Each ACP Registry replay leaves itsstatus.jsondirectory behind after the layer is released, so repeated replay runs accumulate orphaned temp directories on disk.
apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.testkit.ts:39
makeGrokProviderAdapterRegistryReplayLayercreatesreplayDirwithfileSystem.makeTempDirectoryat line 39 without any finalizer or explicit removal. Every Grok replay run leaks that temporary directory and itsstatus.json, causing unbounded temp-file growth across repeated test executions.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts around line 71:
`makeReplayServerConfig` creates a temp directory via `fs.makeTempDirectory` but never registers cleanup, so every call leaks the entire replay workspace (database, logs, worktrees, attachments) in the system temp directory. Repeated test runs will steadily consume disk and can fail when the temp volume fills. Consider scoping the temp directory so it is removed when the effect's scope closes.
Also found in 2 other location(s):
- apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.testkit.ts:46 -- `makeAcpRegistryProviderAdapterRegistryReplayLayer` allocates `replayDir` with `fileSystem.makeTempDirectory` at line 46 and never removes it. Each ACP Registry replay leaves its `status.json` directory behind after the layer is released, so repeated replay runs accumulate orphaned temp directories on disk.
- apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.testkit.ts:39 -- `makeGrokProviderAdapterRegistryReplayLayer` creates `replayDir` with `fileSystem.makeTempDirectory` at line 39 without any finalizer or explicit removal. Every Grok replay run leaks that temporary directory and its `status.json`, causing unbounded temp-file growth across repeated test executions.
| for (const { parameter } of parameters) { | ||
| const nativeId = parameter.id.trim(); | ||
| const id = cursorSdkProviderOptionId(nativeId); | ||
| if (!nativeId || !id || seen.has(id)) { |
There was a problem hiding this comment.
🟡 Medium Layers/CursorProvider.ts:112
buildCursorCapabilitiesFromSdkModel adds the option id to seen before validating that the parameter has any usable values. When a duplicated SDK parameter appears first with only blank/invalid values, its id is still recorded, so the later duplicate with real values is skipped by the seen.has(id) check. This silently drops a valid model option from the discovered capabilities. Consider moving seen.add(id) to after the values.length === 0 check so that empty entries don't block later valid ones.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/CursorProvider.ts around line 112:
`buildCursorCapabilitiesFromSdkModel` adds the option `id` to `seen` before validating that the parameter has any usable values. When a duplicated SDK parameter appears first with only blank/invalid values, its `id` is still recorded, so the later duplicate with real values is skipped by the `seen.has(id)` check. This silently drops a valid model option from the discovered capabilities. Consider moving `seen.add(id)` to after the `values.length === 0` check so that empty entries don't block later valid ones.
| const pendingRequest = | ||
| projection.runtimeRequests.find((request) => request.status === "pending") ?? null; |
There was a problem hiding this comment.
🟡 Medium state/use-thread-selection.ts:70
threadDetailToShell selects the pending runtime request via projection.runtimeRequests.find((request) => request.status === "pending"), which returns the first match in array order rather than the most recent. When a thread has multiple pending requests, an older stale request wins over the newer one, so presentThreadShell derives the wrong pending state (e.g. hasPendingApprovals vs hasPendingUserInput) and the shell shows incorrect thread status until the real shell list materializes. Consider finding the pending request with the latest createdAt.
const pendingRequest =
- projection.runtimeRequests.find((request) => request.status === "pending") ?? null;
+ [...projection.runtimeRequests]
+ .sort((left, right) => right.createdAt.localeCompare(left.createdAt))
+ .find((request) => request.status === "pending") ?? null;🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/state/use-thread-selection.ts around lines 70-71:
`threadDetailToShell` selects the pending runtime request via `projection.runtimeRequests.find((request) => request.status === "pending")`, which returns the first match in array order rather than the most recent. When a thread has multiple pending requests, an older stale request wins over the newer one, so `presentThreadShell` derives the wrong pending state (e.g. `hasPendingApprovals` vs `hasPendingUserInput`) and the shell shows incorrect thread status until the real shell list materializes. Consider finding the pending request with the latest `createdAt`.
| assert.equal(subagent.result, "initial subagent response"); |
There was a problem hiding this comment.
🟡 Medium subagent_continue/codex_output.ts:38
assertSubagentContinueOutput asserts subagent.result equals "initial subagent response", but the Codex adapter overwrites result on every subagent.updated event. After the second turn the final projection carries "continued subagent response", so this assertion fails for the valid continued-subagent transcript. The assertion should check that the child thread produced both responses instead of pinning subagent.result to the first one.
| assert.equal(subagent.result, "initial subagent response"); | |
| assert.isTrue( | |
| [ | |
| "initial subagent response", | |
| "continued subagent response", | |
| ].includes(subagent.result), | |
| ); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/fixtures/subagent_continue/codex_output.ts around line 38:
`assertSubagentContinueOutput` asserts `subagent.result` equals `"initial subagent response"`, but the Codex adapter overwrites `result` on every `subagent.updated` event. After the second turn the final projection carries `"continued subagent response"`, so this assertion fails for the valid continued-subagent transcript. The assertion should check that the child thread produced both responses instead of pinning `subagent.result` to the first one.
| frame: makeClaudePromptOfferFrame(message), | ||
| }); | ||
|
|
||
| try { |
There was a problem hiding this comment.
🟡 Medium Adapters/ClaudeAdapterV2.testkit.ts:1418
In recordClaudeRestartingQueries, when query() or iteration throws, the catch block closes only promptQueue and rethrows without calling queryRuntime.close(). This leaks the Claude SDK runtime/subprocess for that turn. The queryRuntime variable is scoped inside the try block so it is unreachable from the catch, and even on the success path queryRuntime.close() is never called. Every other recording path explicitly closes the runtime on both success and failure, so an error here (or normal completion) can leave the subprocess alive and cause transcript generation or tests to hang. Consider hoisting queryRuntime above the try and calling queryRuntime.close() in both the success and error paths.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.testkit.ts around line 1418:
In `recordClaudeRestartingQueries`, when `query()` or iteration throws, the `catch` block closes only `promptQueue` and rethrows without calling `queryRuntime.close()`. This leaks the Claude SDK runtime/subprocess for that turn. The `queryRuntime` variable is scoped inside the `try` block so it is unreachable from the `catch`, and even on the success path `queryRuntime.close()` is never called. Every other recording path explicitly closes the runtime on both success and failure, so an error here (or normal completion) can leave the subprocess alive and cause transcript generation or tests to hang. Consider hoisting `queryRuntime` above the `try` and calling `queryRuntime.close()` in both the success and error paths.
| ...(mutation.defaultModelSelection === undefined | ||
| ? {} | ||
| : { defaultModelSelection: mutation.defaultModelSelection }), | ||
| ...(mutation.scripts === undefined ? {} : { scripts: mutation.scripts }), |
There was a problem hiding this comment.
🟡 Medium project/http.ts:48
The project.create HTTP handler silently drops the createWorkspaceRootIfMissing field. When an API caller sends a mutation payload with createWorkspaceRootIfMissing: true, the value is never forwarded to projects.create(...), so auto-creation of a missing workspace directory does not happen even though the caller requested it. This means requests that succeed over WebSocket fail or behave differently over HTTP for the same payload. The fix is to forward mutation.createWorkspaceRootIfMissing into the projects.create(...) call, matching the conditional-spread pattern used for the other optional fields.
| ...(mutation.defaultModelSelection === undefined | |
| ? {} | |
| : { defaultModelSelection: mutation.defaultModelSelection }), | |
| ...(mutation.scripts === undefined ? {} : { scripts: mutation.scripts }), | |
| ...(mutation.defaultModelSelection === undefined | |
| ? {} | |
| : { defaultModelSelection: mutation.defaultModelSelection }), | |
| ...(mutation.scripts === undefined ? {} : { scripts: mutation.scripts }), | |
| ...(mutation.createWorkspaceRootIfMissing === undefined | |
| ? {} | |
| : { createWorkspaceRootIfMissing: mutation.createWorkspaceRootIfMissing }), |
Also found in 1 other location(s)
apps/server/src/project/ProjectService.ts:342
bootstrapcallsgetByWorkspaceRoot(input.workspaceRoot)beforecreate(input).getByWorkspaceRootalways normalizes viaworkspacePaths.normalizeWorkspaceRoot(workspaceRoot)without passingcreateIfMissing, so a missing directory fails withProjectOperationErrorinstead of allowingcreate()to create it. Any bootstrap request that setscreateWorkspaceRootIfMissing: trueon a not-yet-created workspace root will be rejected instead of bootstrapping the project.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/http.ts around lines 48-51:
The `project.create` HTTP handler silently drops the `createWorkspaceRootIfMissing` field. When an API caller sends a mutation payload with `createWorkspaceRootIfMissing: true`, the value is never forwarded to `projects.create(...)`, so auto-creation of a missing workspace directory does not happen even though the caller requested it. This means requests that succeed over WebSocket fail or behave differently over HTTP for the same payload. The fix is to forward `mutation.createWorkspaceRootIfMissing` into the `projects.create(...)` call, matching the conditional-spread pattern used for the other optional fields.
Also found in 1 other location(s):
- apps/server/src/project/ProjectService.ts:342 -- `bootstrap` calls `getByWorkspaceRoot(input.workspaceRoot)` before `create(input)`. `getByWorkspaceRoot` always normalizes via `workspacePaths.normalizeWorkspaceRoot(workspaceRoot)` without passing `createIfMissing`, so a missing directory fails with `ProjectOperationError` instead of allowing `create()` to create it. Any bootstrap request that sets `createWorkspaceRootIfMissing: true` on a not-yet-created workspace root will be rejected instead of bootstrapping the project.
| frame: { type: "query.interrupt" }, | ||
| }); | ||
|
|
||
| try { |
There was a problem hiding this comment.
🟡 Medium Adapters/ClaudeAdapterV2.testkit.ts:1961
recordInterruptedClaudeQuery records runtime_exit with status: "cancelled" for any exception thrown by runtime.interrupt() or recordMessagesUntilIteratorDone(), not just CancelledError. Unexpected SDK/runtime failures during interrupt or stream draining are silently reclassified as a normal cancellation instead of failing the recording, so the replay transcript hides real errors and replay treats them the same as success.
The try/catch on lines 1963–1967 and 1970–1978 both store the error into cancelledError unconditionally, collapsing all exceptions into the "cancelled" bucket. Consider re-throwing non-cancellation errors or recording status: "error" for them instead of overwriting cancelledError.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.testkit.ts around line 1961:
`recordInterruptedClaudeQuery` records `runtime_exit` with `status: "cancelled"` for *any* exception thrown by `runtime.interrupt()` or `recordMessagesUntilIteratorDone()`, not just `CancelledError`. Unexpected SDK/runtime failures during interrupt or stream draining are silently reclassified as a normal cancellation instead of failing the recording, so the replay transcript hides real errors and replay treats them the same as success.
The `try/catch` on lines 1963–1967 and 1970–1978 both store the error into `cancelledError` unconditionally, collapsing all exceptions into the `"cancelled"` bucket. Consider re-throwing non-cancellation errors or recording `status: "error"` for them instead of overwriting `cancelledError`.
| } | ||
| if (entry.type === "expect_outbound") { | ||
| const signal = cursorAdvanced; | ||
| yield* Effect.promise(() => signal.promise); |
There was a problem hiding this comment.
🟡 Medium Adapters/CursorAdapterV2.testkit.ts:321
In waitForRun, when the next transcript entry is expect_outbound, the function awaits cursorAdvanced.promise — but that promise is only resolved by advance() inside assertOutbound. If the code under test never emits the expected outbound frame (e.g. it forgets to call run.cancel during an interrupt scenario), nothing ever resolves the promise, so run.wait hangs forever instead of surfacing a replay mismatch. This turns transcript mismatches into stuck tests. Consider adding a timeout or a different synchronization mechanism so a missing outbound frame is reported as a replay error rather than an indefinite hang.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts around line 321:
In `waitForRun`, when the next transcript entry is `expect_outbound`, the function awaits `cursorAdvanced.promise` — but that promise is only resolved by `advance()` inside `assertOutbound`. If the code under test never emits the expected outbound frame (e.g. it forgets to call `run.cancel` during an interrupt scenario), nothing ever resolves the promise, so `run.wait` hangs forever instead of surfacing a replay mismatch. This turns transcript mismatches into stuck tests. Consider adding a timeout or a different synchronization mechanism so a missing outbound frame is reported as a replay error rather than an indefinite hang.
| : { id: pendingRequest.id, kind: pendingRequest.kind, createdAt: pendingRequest.createdAt }, | ||
| latestVisibleMessage: null, | ||
| latestUserMessageAt: latestUserMessageAt(projection), | ||
| hasActionableProposedPlan: false, |
There was a problem hiding this comment.
🟡 Medium state/use-thread-selection.ts:96
threadDetailToShell hard-codes hasActionableProposedPlan: false, so for a newly created thread that already has an actionable proposed plan, the optimistic shell incorrectly reports no actionable plan. The mobile UI hides the Plan Ready indicator until the real shell snapshot arrives. Consider deriving this flag from projection.plans to match the canonical shell builder.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/state/use-thread-selection.ts around line 96:
`threadDetailToShell` hard-codes `hasActionableProposedPlan: false`, so for a newly created thread that already has an actionable proposed plan, the optimistic shell incorrectly reports no actionable plan. The mobile UI hides the `Plan Ready` indicator until the real shell snapshot arrives. Consider deriving this flag from `projection.plans` to match the canonical shell builder.
| assert.isDefined(assistant); | ||
| assert.isBelow(assistant.text.indexOf(expected.first), assistant.text.indexOf(expected.second)); |
There was a problem hiding this comment.
🟡 Medium grok_subagent_lineage/output.ts:83
assertGrokSubagentLineageOutput does not verify that the child assistant message actually contains expected.first. At line 84, assert.isBelow(assistant.text.indexOf(expected.first), assistant.text.indexOf(expected.second)) passes even when expected.first is absent, because indexOf returns -1 and -1 < indexOf(expected.second). A broken projection can drop the opening chunk of a child subagent response without failing this contract test. Add an explicit assert.include(assistant.text, expected.first) before the ordering check.
| assert.isDefined(assistant); | |
| assert.isBelow(assistant.text.indexOf(expected.first), assistant.text.indexOf(expected.second)); | |
| assert.isDefined(assistant); | |
| assert.include(assistant.text, expected.first); | |
| assert.isBelow(assistant.text.indexOf(expected.first), assistant.text.indexOf(expected.second)); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/fixtures/grok_subagent_lineage/output.ts around lines 83-84:
`assertGrokSubagentLineageOutput` does not verify that the child assistant message actually contains `expected.first`. At line 84, `assert.isBelow(assistant.text.indexOf(expected.first), assistant.text.indexOf(expected.second))` passes even when `expected.first` is absent, because `indexOf` returns `-1` and `-1 < indexOf(expected.second)`. A broken projection can drop the opening chunk of a child subagent response without failing this contract test. Add an explicit `assert.include(assistant.text, expected.first)` before the ordering check.
564fe38 to
be1bb7e
Compare
Mobile UI for the owned-subtree lifecycle cascade in pingdotgg#3861: thread list actions archive, unarchive, and delete a root together with its recursively owned subagent threads using the shared subtree helpers and confirmation copy from client-runtime, and thread lists flag recovery roots whose subagent parent is no longer available. Depends on the server-side cascade in pingdotgg#3861.
PixPMusic
force-pushed
the
pixpmusic/mobile-subagent-tree
branch
from
be1bb7e to
b93a7bd
Compare
| const nativeEventLogger = yield* makeEventNdjsonLogger(providerEventLogPath, { | ||
| stream: "native", | ||
| }); |
There was a problem hiding this comment.
🟡 Medium Adapters/CursorAgentSdk.ts:306
The EventNdjsonLogger created in cursorAgentSdkRunnerLiveLayer is never closed. Each thread writer owns a batched logger scope with a 200 ms flush window and an open RotatingFileSink; when the layer is released those buffers are never flushed and the file handles stay open, silently discarding final buffered protocol events and leaking file descriptors. The logger exposes close for exactly this purpose, but the layer never registers it as a finalizer. Consider adding yield* Effect.addFinalizer(() => nativeEventLogger.close()) (or guarding against undefined) so the writers are flushed and closed on layer release.
const nativeEventLogger = yield* makeEventNdjsonLogger(providerEventLogPath, {
stream: "native",
});
+ if (nativeEventLogger !== undefined) {
+ yield* Effect.addFinalizer(() => nativeEventLogger.close());
+ }
Also found in 2 other location(s)
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts:468
The logger allocated by
makeEventNdjsonLoggeris never registered for cleanup in thisLayer.effect.EventNdjsonLoggerbuffers writes viaLogger.batchedand only its explicitclose()closes the batching scope and flushes pending records. On layer/server shutdown, recent Claude protocol events (up to the batch window) can therefore be dropped, and writer scopes remain open. RegisternativeEventLogger.close()as a scoped finalizer (the Codex allocation at line 1108 has the same lifecycle problem).
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.testkit.ts:1948
recordMessagesUntilFirstToolUseis awaited before the cleanuptrybegins. If the query returns aresult, ends, oriterator.next()rejects before a tool use, that helper throws andrecordInterruptedClaudeQueryexits without callingpromptQueue.close()orruntime.close()/runtime.interrupt(). A failedinterruptAfter: "tool_use"recording can therefore leave the Claude runtime/subprocess alive and hang or leak resources in the replay-recording process. Move this wait inside atry/finallythat always closes the queue and runtime.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/CursorAgentSdk.ts around lines 306-308:
The `EventNdjsonLogger` created in `cursorAgentSdkRunnerLiveLayer` is never closed. Each thread writer owns a batched logger scope with a 200 ms flush window and an open `RotatingFileSink`; when the layer is released those buffers are never flushed and the file handles stay open, silently discarding final buffered protocol events and leaking file descriptors. The logger exposes `close` for exactly this purpose, but the layer never registers it as a finalizer. Consider adding `yield* Effect.addFinalizer(() => nativeEventLogger.close())` (or guarding against `undefined`) so the writers are flushed and closed on layer release.
Also found in 2 other location(s):
- apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts:468 -- The logger allocated by `makeEventNdjsonLogger` is never registered for cleanup in this `Layer.effect`. `EventNdjsonLogger` buffers writes via `Logger.batched` and only its explicit `close()` closes the batching scope and flushes pending records. On layer/server shutdown, recent Claude protocol events (up to the batch window) can therefore be dropped, and writer scopes remain open. Register `nativeEventLogger.close()` as a scoped finalizer (the Codex allocation at line 1108 has the same lifecycle problem).
- apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.testkit.ts:1948 -- `recordMessagesUntilFirstToolUse` is awaited before the cleanup `try` begins. If the query returns a `result`, ends, or `iterator.next()` rejects before a tool use, that helper throws and `recordInterruptedClaudeQuery` exits without calling `promptQueue.close()` or `runtime.close()`/`runtime.interrupt()`. A failed `interruptAfter: "tool_use"` recording can therefore leave the Claude runtime/subprocess alive and hang or leak resources in the replay-recording process. Move this wait inside a `try/finally` that always closes the queue and runtime.
| }; | ||
| } | ||
|
|
||
| export function makeSubagentConversationArtifacts(input: { |
There was a problem hiding this comment.
🟠 High orchestration-v2/SubagentProjection.ts:77
makeSubagentConversationArtifacts hard-codes runId: null on both the message and the turn item, so the assistant result is never matched by subagentResultForRun. That function filters for artifacts whose runId === run.id, which never holds when runId is null, so the parent always gets the fallback text "Child task completed without an assistant result." instead of the real child response. Pass the child run ID into this helper and assign it to both artifacts.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/SubagentProjection.ts around line 77:
`makeSubagentConversationArtifacts` hard-codes `runId: null` on both the message and the turn item, so the assistant result is never matched by `subagentResultForRun`. That function filters for artifacts whose `runId === run.id`, which never holds when `runId` is `null`, so the parent always gets the fallback text `"Child task completed without an assistant result."` instead of the real child response. Pass the child run ID into this helper and assign it to both artifacts.
| const base = { | ||
| ...projection, | ||
| thread: { | ||
| ...projection.thread, | ||
| updatedAt: event.occurredAt, | ||
| }, | ||
| updatedAt: event.occurredAt, | ||
| }; |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionStore.ts:166
applyToProjection overwrites projection.thread.updatedAt with event.occurredAt for every domain event, including run.*, message.updated, and provider-session.* events. The persisted SQL projection only updates the thread row for thread.* events, so the in-memory thread.updatedAt diverges from the store after any non-thread activity. Consumers that rely on thread.updatedAt to detect thread metadata changes observe a spurious update on every ordinary event. Consider updating projection.updatedAt only (not thread.updatedAt) in the shared base, and let the thread.* cases set thread.updatedAt via event.payload.
- const base = {
- ...projection,
- thread: {
- ...projection.thread,
- updatedAt: event.occurredAt,
- },
- updatedAt: event.occurredAt,
- };
+ const base = {
+ ...projection,
+ updatedAt: event.occurredAt,
+ };🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around lines 166-173:
`applyToProjection` overwrites `projection.thread.updatedAt` with `event.occurredAt` for every domain event, including `run.*`, `message.updated`, and `provider-session.*` events. The persisted SQL projection only updates the thread row for `thread.*` events, so the in-memory `thread.updatedAt` diverges from the store after any non-thread activity. Consumers that rely on `thread.updatedAt` to detect thread metadata changes observe a spurious update on every ordinary event. Consider updating `projection.updatedAt` only (not `thread.updatedAt`) in the shared `base`, and let the `thread.*` cases set `thread.updatedAt` via `event.payload`.
| (left, right) => | ||
| DateTime.toEpochMillis(right.createdAt) - DateTime.toEpochMillis(left.createdAt), | ||
| )[0] ?? null; | ||
| const latestVisibleMessage = |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProjectionStore.ts:738
threadShellFromProjection selects latestVisibleMessage and latestUserMessageAt from all projection.messages, including messages from rolled-back runs. Rolled-back messages stay in projection.messages but their turn items are removed from visibleTurnItems, so after a rollback the thread-list shell can still preview message text that is no longer part of the visible conversation. latestUserMessageAt has the same stale-history problem. Consider filtering messages against the visible run set before sorting, so the shell reflects only messages the user can actually see.
Also found in 1 other location(s)
apps/mobile/src/state/use-thread-selection.ts:94
threadDetailToShellhard-codeslatestVisibleMessagetonulleven though the detail projection already containsprojection.messages. During the documented optimistic window before the shell list materializes, a newly created thread with messages is therefore presented as having no latest message, so thread-list/sidebar previews remain blank until a separate shell update arrives. The canonicalthreadShellFromProjectionderives the latest message from the projection instead.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around line 738:
`threadShellFromProjection` selects `latestVisibleMessage` and `latestUserMessageAt` from all `projection.messages`, including messages from rolled-back runs. Rolled-back messages stay in `projection.messages` but their turn items are removed from `visibleTurnItems`, so after a rollback the thread-list shell can still preview message text that is no longer part of the visible conversation. `latestUserMessageAt` has the same stale-history problem. Consider filtering messages against the visible run set before sorting, so the shell reflects only messages the user can actually see.
Also found in 1 other location(s):
- apps/mobile/src/state/use-thread-selection.ts:94 -- `threadDetailToShell` hard-codes `latestVisibleMessage` to `null` even though the detail projection already contains `projection.messages`. During the documented optimistic window before the shell list materializes, a newly created thread with messages is therefore presented as having no latest message, so thread-list/sidebar previews remain blank until a separate shell update arrives. The canonical `threadShellFromProjection` derives the latest message from the projection instead.
| const resolveFavicon = Effect.fn("ProjectEnrichmentService.resolveFavicon")(function* ( | ||
| workspaceRoot: string, | ||
| ) { | ||
| const faviconPath = yield* Cache.get(faviconCache, workspaceRoot); | ||
| yield* logFailure(workspaceRoot, "faviconPath", faviconPath); | ||
| }); |
There was a problem hiding this comment.
🟡 Medium project/ProjectEnrichmentService.ts:173
resolveFavicon fetches and caches the favicon but never publishes a ProjectEnrichmentChange, so subscribers never learn that the favicon resolved. When repository identity resolution finishes first, its notification carries faviconPath: null; once the favicon later resolves, no follow-up notification is sent, so consumers relying on subscribeChanges keep seeing faviconPath: null until some unrelated refresh happens. The worker should publish a change (with the current cached repository identity and favicon result) after the favicon resolves, similar to what resolveRepositoryIdentity already does.
const resolveFavicon = Effect.fn("ProjectEnrichmentService.resolveFavicon")(function* (
workspaceRoot: string,
) {
const faviconPath = yield* Cache.get(faviconCache, workspaceRoot);
yield* logFailure(workspaceRoot, "faviconPath", faviconPath);
+ const repositoryIdentity = yield* Cache.getSuccess(repositoryIdentityCache, workspaceRoot);
+ yield* PubSub.publish(changes, {
+ workspaceRoot,
+ repositoryIdentityResolved: Option.isSome(repositoryIdentity) && Exit.isSuccess(Option.getUnsafe(repositoryIdentity)),
+ enrichment: {
+ repositoryIdentity: availableValue(repositoryIdentity),
+ faviconPath: availableValue(Option.some(faviconPath)),
+ },
+ });
});🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/ProjectEnrichmentService.ts around lines 173-178:
`resolveFavicon` fetches and caches the favicon but never publishes a `ProjectEnrichmentChange`, so subscribers never learn that the favicon resolved. When repository identity resolution finishes first, its notification carries `faviconPath: null`; once the favicon later resolves, no follow-up notification is sent, so consumers relying on `subscribeChanges` keep seeing `faviconPath: null` until some unrelated refresh happens. The worker should publish a change (with the current cached repository identity and favicon result) after the favicon resolves, similar to what `resolveRepositoryIdentity` already does.
| terminalStatusQuality: "strong", | ||
| }, | ||
| streaming: { | ||
| streamsAssistantText: true, |
There was a problem hiding this comment.
🟡 Medium Adapters/ClaudeAdapterV2.ts:143
ClaudeProviderCapabilitiesV2.streaming.streamsAssistantText is true, but the adapter never processes partial stream_event messages from the Claude SDK. assistantTextFromSdkMessage only handles full assistant messages, and buildAssistantArtifacts emits them with streaming: false and status: "completed". Callers that rely on this capability expect incremental assistant text, but instead receive nothing until the entire assistant message is finished. Set streamsAssistantText to false, or emit/update streaming artifacts from partial stream events.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts around line 143:
`ClaudeProviderCapabilitiesV2.streaming.streamsAssistantText` is `true`, but the adapter never processes partial `stream_event` messages from the Claude SDK. `assistantTextFromSdkMessage` only handles full `assistant` messages, and `buildAssistantArtifacts` emits them with `streaming: false` and `status: "completed"`. Callers that rely on this capability expect incremental assistant text, but instead receive nothing until the entire assistant message is finished. Set `streamsAssistantText` to `false`, or emit/update streaming artifacts from partial stream events.
| Effect.andThen(onCommitted), | ||
| ); | ||
|
|
||
| const assertWorkspaceAvailable = Effect.fn("ProjectService.assertWorkspaceAvailable")(function* ( |
There was a problem hiding this comment.
🟡 Medium project/ProjectService.ts:234
assertWorkspaceAvailable performs a read-then-write without atomicity, so two concurrent create calls for the same workspace root both pass the conflict check and dispatch valid commands. This leaves two active projects owning the same workspaceRoot, violating the collision invariant the service is supposed to enforce. The check must be enforced atomically with command dispatch/persistence (e.g., a unique constraint or compare-and-set in the projection/persistence layer) rather than via a read-then-write that races with concurrent operations.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/project/ProjectService.ts around line 234:
`assertWorkspaceAvailable` performs a read-then-write without atomicity, so two concurrent `create` calls for the same workspace root both pass the conflict check and dispatch valid commands. This leaves two active projects owning the same `workspaceRoot`, violating the collision invariant the service is supposed to enforce. The check must be enforced atomically with command dispatch/persistence (e.g., a unique constraint or compare-and-set in the projection/persistence layer) rather than via a read-then-write that races with concurrent operations.
| intervalMinutes: | ||
| schedule.type === "interval" | ||
| ? String(Math.max(1, Math.round(schedule.everyMs / 60_000))) | ||
| : "15", |
There was a problem hiding this comment.
🟡 Medium settings/ScheduledTasksSettings.tsx:221
taskToDraft rounds everyMs to whole minutes, so editing and saving an existing sub-minute interval task silently changes its cadence. For example, a 90-second interval becomes 2 minutes and a 30-second interval becomes 1 minute, even when the schedule was not modified. The intervalMinutes field is a whole-minute string, so the original sub-minute value is lost as soon as the task is opened in the edit dialog. Consider preserving the original everyMs in the draft and only recomputing it when the user actually edits the interval, or allow editing sub-minute intervals in the UI.
- intervalMinutes:
- schedule.type === "interval"
- ? String(Math.max(1, Math.round(schedule.everyMs / 60_000)))
- : "15",🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ScheduledTasksSettings.tsx around lines 221-224:
`taskToDraft` rounds `everyMs` to whole minutes, so editing and saving an existing sub-minute interval task silently changes its cadence. For example, a 90-second interval becomes 2 minutes and a 30-second interval becomes 1 minute, even when the schedule was not modified. The `intervalMinutes` field is a whole-minute string, so the original sub-minute value is lost as soon as the task is opened in the edit dialog. Consider preserving the original `everyMs` in the draft and only recomputing it when the user actually edits the interval, or allow editing sub-minute intervals in the UI.
| import { McpInvocationContext } from "../../McpInvocationContext.ts"; | ||
| import { OrchestratorMcpService } from "../../OrchestratorMcpService.ts"; | ||
|
|
||
| const handlers = { |
There was a problem hiding this comment.
🟠 High orchestrator/handlers.ts:7
Removing expiresAt from McpInvocationScope causes McpSessionRegistry.resolve to return stored provider-session tokens indefinitely, so a leaked bearer token continues authorizing MCP preview and orchestration operations with no time bound. Previously the token was rejected once expiresAt passed; now records are only removed by explicit revocation. If removing credential lifetime enforcement is intentional, document the rationale and confirm revocation is the sole remaining control.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/mcp/toolkits/orchestrator/handlers.ts around line 7:
Removing `expiresAt` from `McpInvocationScope` causes `McpSessionRegistry.resolve` to return stored provider-session tokens indefinitely, so a leaked bearer token continues authorizing MCP preview and orchestration operations with no time bound. Previously the token was rejected once `expiresAt` passed; now records are only removed by explicit revocation. If removing credential lifetime enforcement is intentional, document the rationale and confirm revocation is the sole remaining control.
| normalized === "external_directory" || | ||
| normalizedTool === "read" || | ||
| normalizedTool.includes("glob") || | ||
| normalizedTool.includes("grep") || |
There was a problem hiding this comment.
🟡 Medium Adapters/OpenCodeAdapterV2.ts:443
openCodePermissionRequestKind returns "file-read" for any tool name containing search, so OpenCode's websearch permission (projected elsewhere as web_search) is classified as a file-read request. This presents a network operation as a local file-read approval, misleading users about what they are authorizing. The normalizedTool.includes("search") match on line 444 should be removed (or scoped to local search tools only) so websearch falls through to "command".
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.ts around line 443:
`openCodePermissionRequestKind` returns `"file-read"` for any tool name containing `search`, so OpenCode's `websearch` permission (projected elsewhere as `web_search`) is classified as a file-read request. This presents a network operation as a local file-read approval, misleading users about what they are authorizing. The `normalizedTool.includes("search")` match on line 444 should be removed (or scoped to local search tools only) so `websearch` falls through to `"command"`.
| ), | ||
| Effect.map((snapshot) => | ||
| snapshot.threads | ||
| .filter((thread) => thread.projectId === input.projectId) |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ThreadManagementService.ts:271
listProjectThreads returns soft-deleted threads because it only filters by projectId and lineage, never excluding shells where deletedAt is non-null. This is inconsistent with getProjectThread, which treats the same deleted threads as not found, so callers receive deleted threads in normal project listings and in pagination/total counts. Add a thread.deletedAt === null filter.
Also found in 1 other location(s)
apps/server/src/orchestration-v2/ProjectionStore.ts:738
threadShellFromProjectioncomputeslatestVisibleMessagefrom every entry inprojection.messageswithout applying run/attempt visibility. Rollback retains messages fromrolled_backruns (while turn-item visibility removes them), so after a rollback the shell preview can still show the discarded assistant/user message instead of the latest visible conversation message.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ThreadManagementService.ts around line 271:
`listProjectThreads` returns soft-deleted threads because it only filters by `projectId` and lineage, never excluding shells where `deletedAt` is non-null. This is inconsistent with `getProjectThread`, which treats the same deleted threads as not found, so callers receive deleted threads in normal project listings and in pagination/total counts. Add a `thread.deletedAt === null` filter.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/ProjectionStore.ts:738 -- `threadShellFromProjection` computes `latestVisibleMessage` from every entry in `projection.messages` without applying run/attempt visibility. Rollback retains messages from `rolled_back` runs (while turn-item visibility removes them), so after a rollback the shell preview can still show the discarded assistant/user message instead of the latest visible conversation message.
| const parentKey = subagentParentThreadKey(thread); | ||
| return parentKey === null || !threadKeys.has(parentKey); | ||
| }); | ||
| const placedKeys = new Set<string>(); |
There was a problem hiding this comment.
🟠 High state/threadRelationships.ts:221
getSubagentThreadTreeRoots uses recursive markPlaced calls to traverse every subagent child, so a sufficiently deep valid subagent chain exhausts the JavaScript call stack and throws before the thread list can render. The visited set bounds cycles but does not bound recursion depth. Consider replacing the recursion with an explicit stack or queue, as getOwnedSubagentDescendants already does.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threadRelationships.ts around line 221:
`getSubagentThreadTreeRoots` uses recursive `markPlaced` calls to traverse every subagent child, so a sufficiently deep valid subagent chain exhausts the JavaScript call stack and throws before the thread list can render. The `visited` set bounds cycles but does not bound recursion depth. Consider replacing the recursion with an explicit stack or queue, as `getOwnedSubagentDescendants` already does.
| }); | ||
| yield* Effect.logInfo("V2 orchestration recovery completed", recovery); | ||
|
|
||
| yield* Effect.logDebug("Accepting commands"); |
There was a problem hiding this comment.
🟠 High src/serverRuntimeStartup.ts:448
commandGate.signalCommandReady is called before the welcome.publish phase. If lifecycleEvents.publish fails after readiness is signaled, commands have already been allowed to run, but the outer Effect.exit(startup) observes the failure and calls commandGate.failCommandReady, transitioning the gate from ready to an error state after callers already saw readiness. This exposes a contradictory state where commands execute during partial initialization and the gate retroactively fails. Move commandGate.signalCommandReady to after all failure-capable startup phases (including welcome.publish) have completed.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/serverRuntimeStartup.ts around line 448:
`commandGate.signalCommandReady` is called before the `welcome.publish` phase. If `lifecycleEvents.publish` fails after readiness is signaled, commands have already been allowed to run, but the outer `Effect.exit(startup)` observes the failure and calls `commandGate.failCommandReady`, transitioning the gate from `ready` to an error state after callers already saw readiness. This exposes a contradictory state where commands execute during partial initialization and the gate retroactively fails. Move `commandGate.signalCommandReady` to after all failure-capable startup phases (including `welcome.publish`) have completed.
| const questions = extractXAiAskUserQuestions(params).map((question) => ({ | ||
| id: question.id, | ||
| header: question.header, | ||
| question: question.question, | ||
| options: [...question.options], | ||
| })); |
There was a problem hiding this comment.
🟠 High Adapters/GrokAdapterV2.ts:110
The mapping of extractXAiAskUserQuestions(params) drops the multiSelect field, so when a Grok request sends multiSelect: true the question is presented as single-select. OrchestrationV2UserInputQuestion defaults an omitted multiSelect to false, which prevents users from selecting multiple labels as the provider requested. Add multiSelect: question.multiSelect to the mapped object.
| const questions = extractXAiAskUserQuestions(params).map((question) => ({ | |
| id: question.id, | |
| header: question.header, | |
| question: question.question, | |
| options: [...question.options], | |
| })); | |
| const questions = extractXAiAskUserQuestions(params).map((question) => ({ | |
| id: question.id, | |
| header: question.header, | |
| question: question.question, | |
| multiSelect: question.multiSelect, | |
| options: [...question.options], | |
| })); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.ts around lines 110-115:
The mapping of `extractXAiAskUserQuestions(params)` drops the `multiSelect` field, so when a Grok request sends `multiSelect: true` the question is presented as single-select. `OrchestrationV2UserInputQuestion` defaults an omitted `multiSelect` to `false`, which prevents users from selecting multiple labels as the provider requested. Add `multiSelect: question.multiSelect` to the mapped object.
| return { valueType: typeof payload }; | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
🟠 High provider/NativeProtocolLogging.ts:29
summarizeNativeProtocolPayload copies provider-controlled tag into the observability stream as method whenever it matches the structural regex, and errorTag(record) can expose the original _tag value. Strings like Bearer-token-123 satisfy /^[A-Za-z][A-Za-z0-9._:/-]*$/, so credential or prompt-derived values are leaked despite the function's guarantee that payload content is never retained. Emit only fixed classifications or allowlisted protocol tags rather than the original values.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/NativeProtocolLogging.ts around line 29:
`summarizeNativeProtocolPayload` copies provider-controlled `tag` into the observability stream as `method` whenever it matches the structural regex, and `errorTag(record)` can expose the original `_tag` value. Strings like `Bearer-token-123` satisfy `/^[A-Za-z][A-Za-z0-9._:/-]*$/`, so credential or prompt-derived values are leaked despite the function's guarantee that payload content is never retained. Emit only fixed classifications or allowlisted protocol tags rather than the original values.
| updatedAt: now, | ||
| archivedAt: null, | ||
| deletedAt: null, | ||
| ...input, |
There was a problem hiding this comment.
🟡 Medium src/test-fixtures.ts:50
The trailing ...input at the end of makeRawThreadShell overwrites every fixture default with the caller's undefined values. makeThreadShellFixture always passes id, projectId, and title from its overrides even when those properties are absent, so Partial<EnvironmentThreadShell> yields undefined for those keys. The resulting fixture has id, projectId, and title set to undefined, and lineage.rootThreadId (still the generated id) no longer matches id. This silently produces malformed shells. Consider filtering out undefined values before spreading, e.g. ...stripUndefined(input).
| ...input, | |
| ...(Object.fromEntries(Object.entries(input).filter(([, v]) => v !== undefined)) as Partial<OrchestrationV2ThreadShell>), |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/test-fixtures.ts around line 50:
The trailing `...input` at the end of `makeRawThreadShell` overwrites every fixture default with the caller's `undefined` values. `makeThreadShellFixture` always passes `id`, `projectId`, and `title` from its `overrides` even when those properties are absent, so `Partial<EnvironmentThreadShell>` yields `undefined` for those keys. The resulting fixture has `id`, `projectId`, and `title` set to `undefined`, and `lineage.rootThreadId` (still the generated `id`) no longer matches `id`. This silently produces malformed shells. Consider filtering out `undefined` values before spreading, e.g. `...stripUndefined(input)`.
| const runOrdinal = suppliedRunOrdinal ?? runRows[0]?.ordinal ?? null; | ||
| const lowerBound = runOrdinal === null ? 0 : runOrdinal * 1_000_000; | ||
| const upperBound = runOrdinal === null ? 999_999 : lowerBound + 999_999; | ||
| yield* sql` |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/TurnItemPositionStore.ts:59
When a run's partition already holds 999,999 positions, allocate returns upperBound + 1 instead of failing or waiting. COALESCE(MAX(ordinal), lowerBound) + 1 ignores the upperBound check, so the new ordinal spills into the next run's range — either violating the unique constraint or silently placing the item in another run's ordering partition. Consider rejecting allocation when MAX(ordinal) >= upperBound before inserting.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/TurnItemPositionStore.ts around line 59:
When a run's partition already holds 999,999 positions, `allocate` returns `upperBound + 1` instead of failing or waiting. `COALESCE(MAX(ordinal), lowerBound) + 1` ignores the `upperBound` check, so the new ordinal spills into the next run's range — either violating the unique constraint or silently placing the item in another run's ordering partition. Consider rejecting allocation when `MAX(ordinal) >= upperBound` before inserting.
| id: item.planId, | ||
| runId: item.runId, | ||
| planMarkdown: item.markdown, | ||
| status: "active" as const, |
There was a problem hiding this comment.
🟡 Medium src/session-logic.ts:558
deriveTimelineEntriesFromVisibleTurnItems hard-codes status: "active" for every proposed_plan timeline card, so superseded, completed, or draft plans are all presented as actionable active plans. Users can be offered plan actions for plans that are no longer active. The status should be resolved from the actual plan artifact instead of synthesized.
Also found in 2 other location(s)
apps/mobile/src/state/use-thread-selection.ts:89
threadDetailToShellderivesstatusfromactiveRunbeforelatestRun, unlike the authoritative shell projection, which uses the latest run's status and exposes an older concurrent active run separately viaactiveRunId. If a newer run has completed while an older run remains active, this optimistic shell reports the thread as running/waiting rather than completed, so thread status and controls are incorrect until the shell list materializes.
apps/web/src/components/chat/MessagesTimeline.tsx:2353
SimpleWorkEntryRowtreats every neutral tool status as successful once the turn settles. V2 mapscancelledandinterrupteditems totoolLifecycleStatus: "stopped", which is neutral, so after the run ends these stopped tool calls render a checkmark with the “Completed” tooltip instead of indicating that they were cancelled/interrupted.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/session-logic.ts around line 558:
`deriveTimelineEntriesFromVisibleTurnItems` hard-codes `status: "active"` for every `proposed_plan` timeline card, so superseded, completed, or draft plans are all presented as actionable active plans. Users can be offered plan actions for plans that are no longer active. The status should be resolved from the actual plan artifact instead of synthesized.
Also found in 2 other location(s):
- apps/mobile/src/state/use-thread-selection.ts:89 -- `threadDetailToShell` derives `status` from `activeRun` before `latestRun`, unlike the authoritative shell projection, which uses the latest run's status and exposes an older concurrent active run separately via `activeRunId`. If a newer run has completed while an older run remains active, this optimistic shell reports the thread as running/waiting rather than completed, so thread status and controls are incorrect until the shell list materializes.
- apps/web/src/components/chat/MessagesTimeline.tsx:2353 -- `SimpleWorkEntryRow` treats every neutral tool status as successful once the turn settles. V2 maps `cancelled` and `interrupted` items to `toolLifecycleStatus: "stopped"`, which is neutral, so after the run ends these stopped tool calls render a checkmark with the “Completed” tooltip instead of indicating that they were cancelled/interrupted.
| checkpointScope: (input) => | ||
| Effect.succeed( | ||
| CheckpointScopeId.make( | ||
| joinId("checkpoint-scope", "thread", input.threadId, "name", input.name), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🔴 Critical orchestration-v2/IdAllocator.ts:295
allocate.checkpointScope derives the ID only from threadId and name, so two runs in the same thread that both use the name "root" get the same CheckpointScopeId. This aliases distinct checkpoint scopes and makes their checkpoint IDs and refs collide, corrupting checkpoint history for multi-run threads. The runId is not included in the derivation, so the scope is not unique per run. Consider including the runId in the scope ID, or otherwise adding a per-run component, so each run gets a distinct scope.
checkpointScope: (input) =>
Effect.succeed(
CheckpointScopeId.make(
- joinId("checkpoint-scope", "thread", input.threadId, "name", input.name),
+ joinId("checkpoint-scope", "thread", input.threadId, "name", input.name, "run", input.runId),
),
),Also found in 1 other location(s)
apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts:589
fallbackCallIdis based only onwrapperName, so multiple nested conversation steps of the same tool type withouttoolCallIdall receive the same ID (for example, two reads both becomenested-readToolCall). The caller prefixes this value identically andemitToolArtifactsderives node and turn-item IDs from it, causing later calls to collide with/overwrite the earlier projected artifact instead of displaying every nested tool call. Include the step index or another per-call value in the fallback ID.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/IdAllocator.ts around lines 295-300:
`allocate.checkpointScope` derives the ID only from `threadId` and `name`, so two runs in the same thread that both use the name `"root"` get the same `CheckpointScopeId`. This aliases distinct checkpoint scopes and makes their checkpoint IDs and refs collide, corrupting checkpoint history for multi-run threads. The `runId` is not included in the derivation, so the scope is not unique per run. Consider including the `runId` in the scope ID, or otherwise adding a per-run component, so each run gets a distinct scope.
Also found in 1 other location(s):
- apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts:589 -- `fallbackCallId` is based only on `wrapperName`, so multiple nested conversation steps of the same tool type without `toolCallId` all receive the same ID (for example, two reads both become `nested-readToolCall`). The caller prefixes this value identically and `emitToolArtifacts` derives node and turn-item IDs from it, causing later calls to collide with/overwrite the earlier projected artifact instead of displaying every nested tool call. Include the step index or another per-call value in the fallback ID.
3 participants
Stacking
Warning
Stacked on #3870 (which itself requires #2829). Only the top two commits belong to this PR —
refactor(client-runtime): share subagent thread tree projectionandfeat(mobile): show collapsible nested subagent threads. Everything below them is the #3870 stack.Summary
Opt-in nested subagent threads for the mobile home screen and thread sidebar, kept separate from #3870 on purpose so it can be dropped or deferred independently if the feature isn't wanted:
client-runtimewith cycle and orphan fallbacks, so clients cannot drift on which rows count as roots; the web sidebar adopts the same projection in feat(sidebar): reveal nested subagent threads #3861 — whichever lands first, the other's copy of the twothreadRelationshipsfiles rebases away cleanlyValidation
vp check,vp run typecheck— greent3testAVD and a Pixel 7 Pro from the integration branch: toggle appears in the menu, nested Codex/Claude children expand and collapse, and the preference survives a cold restartNote
Medium Risk
Low-risk list UX when the toggle stays off; medium overall because the diff also touches mobile thread navigation, feed, and desktop userdata paths where regressions would affect daily use.
Overview
Adds an opt-in Show nested subagents control on the mobile home thread list (persisted in device preferences, default off). When enabled, the list uses shared
client-runtimehelpers (getSubagentThreadTreeRoots,flattenSubagentThreadTree) to indent subagent child threads under parents with per-branch expand/collapse; Show more pagination still counts root threads so expanded nests do not eat preview slots. Recent-thread previews and grouping now derive from roots as well.The same diff also carries broader mobile orchestration V2 work alongside that feature: home/archive fixtures move to V2 shell types, orchestration cache schema alignment, thread detail feed changes (
latestRun, fork-from-response, expandable work log +ThreadActivityInspector), queue reorder UI, relationship banner, runtime-request IDs, and checkpoint summaries from projection. Desktop production state paths shift fromuserdatatouserdata-v2(and related branding/test expectations). Large.plansdocs and a small marketing harness label update are documentation-only.Reviewed by Cursor Bugbot for commit d03b0c3. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add nested subagent thread display behind a toggle in mobile home and sidebar
useSubagentThreadListPreferencehook.ThreadDetailsPanel,ThreadRelationshipsPanel,ThreadRelationshipsBanner,ThreadQueueControl,ThreadAutomationsPanel,V2ItemInspector, andV2LifecycleRowon web and mobile.orchestrationAdapterinstead ofadapter.userdatatouserdata-v2(and desktop fromt3codetot3code-v2), and the SSH tunnel script targetsuserdata-v2; existing V1 persisted state will not be read by default.📊 Macroscope summarized d03b0c3. 189 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Depends on #3861 for the server-side owned-subtree lifecycle cascade: mobile dispatches a single root command for archive/unarchive/delete and relies on the orchestrator to cascade the transition and cancel active runs across the owned subagent subtree.