feat(mcp): defer thread organization safely#8683
8 unresolved threads · Jump to file
- apps/server/src/orchestration-v2/Orchestrator.ts:7247–7274 · <!-- MURMUR_IGNORE --> `orchestration-v2/Orchestrator.ts:7274` This startup reconciliation repairs
- apps/server/src/orchestration-v2/Orchestrator.ts:2111 · <!-- MURMUR_IGNORE --> 🟠 **High** `orchestration-v2/Orchestrator.ts:2111` Deferred `thread.organiz
- apps/server/src/orchestration-v2/Orchestrator.ts:7422 · ### Apply failure skips queued run promotion **Medium Severity** <!-- DESCRIPTION START --> `apply
- apps/server/src/orchestration-v2/Orchestrator.ts:107 · 🟡 **Medium** `orchestration-v2/Orchestrator.ts:107` A transient projection read failure during def
- docs/user/thread-sidebar.md:30–31 · 🟡 **Medium** `user/thread-sidebar.md:30` A message sent while the provider supports active steerin
- apps/server/src/orchestration-v2/Orchestrator.ts:7247–7268 · The new deferred-organization startup recovery only catches at the outer `Effect.forEach`, so a sing
- apps/server/src/orchestration-v2/Orchestrator.ts:7274–7281 · <!-- MURMUR_IGNORE --> `orchestration-v2/Orchestrator.ts:7281` Per-thread isolation is fixed now, b
- packages/contracts/src/orchestrationV2.ts:2172 · 🟡 **Medium** `src/orchestrationV2.ts:2172` Any connected client can submit `thread.organization.de
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision 858c15ac · right 7247–7274
Original diff
@@ -7052,6 +7238,41 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
// The high-water subscription deliberately skips history, so recover the
// two terminal side effects from current projections instead: one queued
// run per idle thread, plus any app-owned child result not yet transferred.
+ yield* projectionStore.getShellSnapshot().pipe(
+ Effect.flatMap((shell) =>
+ Effect.forEach(
+ [...shell.threads, ...shell.archivedThreads].filter(
+ (thread) => thread.deferredOrganization != null,
+ ),
+ (thread) =>
+ threadDispatch.withLock(
+ thread.id,
+ Effect.gen(function* () {
+ const projection = yield* projectionStore.getThreadProjection(thread.id);
+ const intent = projection.thread.deferredOrganization;
+ if (intent == null) return;
+ const boundRun = projection.runs.find((run) => run.id === intent.runId);
+ const hasNewerRun =
+ boundRun !== undefined &&
+ projection.runs.some((run) => run.ordinal > boundRun.ordinal);
+ if (
+ boundRun === undefined ||
+ hasNewerRun ||
+ ["completed", "failed", "cancelled", "interrupted", "rolled_back"].includes(
+ boundRun.status,
+ )
+ ) {
+ yield* applyDeferredOrganization(thread.id, intent.runId);
+ }
+ }),
+ ),
+ { concurrency: 8, discard: true },
+ ),
+ ),
+ Effect.catchCause((cause) =>
+ Effect.logWarning("Failed to recover deferred thread organization", { cause }),
+ ),apps/server/src/orchestration-v2/Orchestrator.ts · right 7247–7274
orchestration-v2/Orchestrator.ts:7274
This startup reconciliation repairs many independent threads, but a per-thread failure is not isolated: Effect.forEach fails fast (and interrupts the other in-flight fibers at concurrency: 8), so one bad thread aborts recovery for every remaining deferred intent, and the outer catchCause then reduces it — including interruption — to a single warning. The two neighbouring recovery loops below (app-owned subagents, delegated completion delivery) already wrap the per-thread effect in its own catchCause. Consider adding a transient retry plus per-thread isolation here as well, and covering both the retry-success and persistent-failure-continuation paths in DeferredOrganizationRecovery.integration.test.ts (it currently only asserts the stale-discard path).
| 1 | (thread) => |
| 2 | - threadDispatch.withLock( |
| 3 | - thread.id, |
| 4 | - Effect.gen(function* () { |
| 5 | - // …existing intent inspection… |
| 6 | - }), |
| 7 | - ), |
| 8 | + threadDispatch |
| 9 | + .withLock( |
| 10 | + thread.id, |
| 11 | + Effect.gen(function* () { |
| 12 | + // …existing intent inspection… |
| 13 | + }), |
| 14 | + ) |
| 15 | + .pipe( |
| 16 | + Effect.retry({ times: 2 }), |
| 17 | + Effect.catchCause((cause) => |
| 18 | + Effect.logWarning("Failed to recover deferred thread organization", { |
| 19 | + threadId: thread.id, |
| 20 | + cause, |
| 21 | + }), |
| 22 | + ), |
| 23 | + ), |
No longer relevant as of acb2b17f3cc66a207d81763e9e70c7b2926f7978
Fixed in acb2b17f3c with per-thread isolation and focused persistent-failure continuation coverage. I did not add a broad retry: a durable unreadable projection is not made recoverable by immediate retries, while independent intents can continue safely.
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision 271e82b5 · right 2111
Original diff
@@ -1983,6 +1986,168 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
}
});
+ const dispatchDeferredOrganization = Effect.fn("orchestrationV2.dispatch.deferredOrganization")(
+ function* (
+ command: Extract<
+ OrchestrationV2Command,
+ {
+ readonly type:
+ | "thread.organization.defer"
+ | "thread.organization.defer.cancel"
+ | "thread.organization.defer.apply";
+ }
+ >,
+ events: Ref.Ref<Array<OrchestrationV2DomainEvent>>,
+ ) {
+ const projection = yield* projectionStore.getThreadProjection(command.threadId).pipe(
+ Effect.mapError(
+ (cause) =>
+ new OrchestratorProjectionError({
+ threadId: command.threadId,
+ cause,
+ }),
+ ),
+ );
+ const thread = projection.thread;
+ if (thread.deletedAt !== null) {
+ return yield* new OrchestratorDispatchError({
+ commandId: command.commandId,
+ commandType: command.type,
+ cause: `Thread ${command.threadId} is deleted.`,
+ });
+ }
+ const now = yield* DateTime.now;
+ if (command.type === "thread.organization.defer") {
+ if (thread.archivedAt !== null) {
+ return yield* new OrchestratorDispatchError({
+ commandId: command.commandId,
+ commandType: command.type,
+ cause: `Thread ${command.threadId} is archived.`,
+ });
+ }
+ const activeRuns = projection.runs
+ .filter((run) => ["preparing", "starting", "running", "waiting"].includes(run.status))
+ .toSorted((left, right) => right.ordinal - left.ordinal);
+ if (activeRuns[0]?.id !== command.runId) {
+ return yield* new OrchestratorDispatchError({
+ commandId: command.commandId,
+ commandType: command.type,
+ cause: `Run ${command.runId} is not the active run for thread ${command.threadId}.`,
+ });
+ }
+ yield* emit(
+ events,
+ command,
+ )({
+ type: "thread.metadata-updated",
+ threadId: command.threadId,
+ providerInstanceId: thread.providerInstanceId,
+ occurredAt: now,
+ payload: {
+ ...thread,
+ deferredOrganization: {
+ runId: command.runId,
+ action: command.action,
+ requestedAt: now,
+ },
+ updatedAt: now,
+ },
+ });
+ return;
+ }
+
+ if (command.type === "thread.organization.defer.cancel") {
+ yield* emit(
+ events,
+ command,
+ )({
+ type: "thread.metadata-updated",
+ threadId: command.threadId,
+ providerInstanceId: thread.providerInstanceId,
+ occurredAt: now,
+ payload: {
+ ...thread,
+ deferredOrganization: null,
+ updatedAt: thread.deferredOrganization == null ? thread.updatedAt : now,
+ },
+ });
+ return;
+ }
+
+ const intent = thread.deferredOrganization;
+ if (intent == null || intent.runId !== command.runId) {
+ return yield* new OrchestratorDispatchError({
+ commandId: command.commandId,
+ commandType: command.type,
+ cause: `Thread ${command.threadId} has no deferred organization intent for run ${command.runId}.`,
+ });
+ }
+ const boundRun = projection.runs.find((run) => run.id === command.runId);
+ const hasNewerRun =
+ boundRun !== undefined && projection.runs.some((run) => run.ordinal > boundRun.ordinal);
+ const hasPendingWork =
+ projection.runs.some((run) =>
+ ["preparing", "queued", "starting", "running", "waiting"].includes(run.status),
+ ) || projection.runtimeRequests.some((request) => request.status === "pending");
+ const canApply =
+ boundRun?.status === "completed" &&
+ !hasNewerRun &&
+ !hasPendingWork &&
+ thread.titleRegeneration == null &&
+ thread.archivedAt === null;
+ if (!canApply) {
+ yield* emit(
+ events,
+ command,
+ )({
+ type: "thread.metadata-updated",
+ threadId: command.threadId,
+ providerInstanceId: thread.providerInstanceId,
+ occurredAt: now,
+ payload: { ...thread, deferredOrganization: null, updatedAt: now },
+ });
+ return;
+ }
+ if (intent.action === "settle") {🟠 High orchestration-v2/Orchestrator.ts:2111
Deferred thread.organization.defer.apply leaves provider sessions and background work alive after applying settle or archive, and deferred archives retain MCP credentials. These branches emit terminal thread events directly instead of using the teardown logic in dispatchThreadMutation, so they omit provider-session.detached, provider-session.detach, revokeMcpCredential, and terminal.cleanup effects. Route deferred application through the same teardown path as ordinary archive/settle, or emit the equivalent events and effects.
✅ Resolved in d76789f408aaf8c8ee7ef2d2f801528f2f2565db
Fixed in acb2b17f3c. Deferred apply now delegates to the ordinary serialized thread settle/archive mutation with an atomic clear-intent option, inheriting provider detach, archive credential revocation, terminal cleanup, and delegated-completion disposal. Focused coverage opens a real managed provider session, drains the effect worker, verifies terminal effect settlement, and proves receipt replay enqueues no second cleanup.
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision cf20d210 · right 7422
Original diff
@@ -7212,6 +7416,10 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
threadId,
finalizeDelegatedCompletionDelivery(threadId, stored.event.payload.id),
);
+ yield* threadDispatch.withLock(
+ threadId,
+ applyDeferredOrganization(threadId, stored.event.payload.id),
+ );Apply failure skips queued run promotion
Medium Severity
applyDeferredOrganization now runs on the same terminal-run fiber as startNextQueuedRun. A failed projection read or apply dispatch fails that fiber, so the shared handler only logs and never promotes the next queued run. Follow-up work on that thread then sits queued until a later restart.
Reviewed by Cursor Bugbot for commit cf20d21001108280d379a5db99b02d5c6d87d704. Configure here.
Fixed in 71650ee5adf1ac035cd4f30d607ea5bf77bbba21. Terminal handling now isolates a typed deferred-apply failure before startNextQueuedRun, while interrupt-only causes still propagate. The real SQLite/EventSink/ProjectionStore test fails the apply read, observes the queued run become starting, and confirms the stable apply receipt ID was not consumed.
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision a8871601 · right 107
Original diff
@@ -92,6 +94,29 @@ export class OrchestratorProjectionError extends Schema.TaggedErrorClass<Orchest
}
}
+const isProjectionStoreReadError = Schema.is(ProjectionStoreReadError);
+const isOrchestratorDispatchError = Schema.is(OrchestratorDispatchError);
+
+export const runDeferredOrganizationRepair = <A, E, R>(
+ threadId: ThreadId,
+ repair: Effect.Effect<A, E, R>,
+) =>
+ repair.pipe(
+ Effect.retry({
+ times: 1,
+ while: (error) => isProjectionStoreReadError(error) || isOrchestratorDispatchError(error),🟡 Medium orchestration-v2/Orchestrator.ts:107
A transient projection read failure during deferred apply is recorded as a rejected deterministic command, and later recovery replays that rejection instead of applying or clearing the persisted deferred intent. dispatchDeferredOrganization maps the read failure to OrchestratorProjectionError, which is excluded from the while predicate; include that error type in the retry condition so the apply is retried before its receipt is permanently rejected.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 107: |
| 2 | |
| 3 | A transient projection read failure during deferred apply is recorded as a rejected deterministic command, and later recovery replays that rejection instead of applying or clearing the persisted deferred intent. `dispatchDeferredOrganization` maps the read failure to `OrchestratorProjectionError`, which is excluded from the `while` predicate; include that error type in the retry condition so the apply is retried before its receipt is permanently rejected. |
Fixed in 284b9dca6933303a56fe7ce10748847444cfe2e7 at the earlier boundary. thread.organization.defer.apply retries one typed projection failure around the pure plan, before commitRejectedCommand. The real receipt-store test fails that planner read once, then proves the deterministic receipt is accepted and the intent clears.
docs/user/thread-sidebar.md · Earlier revision a8871601 · right 30–31
Original diff
@@ -25,6 +25,11 @@ Agents connected through T3 Code can organize threads too. They can pin, snooze, or change read state in the current project. T3 Code applies the same rules as the sidebar. For example, an agent cannot settle a thread while work or a user request is still pending. +An agent can also ask T3 Code to settle or archive its own thread after the current run finishes. +The request is saved by the server, but it is applied only when that run completes successfully +and no newer, queued, or approval-blocked work remains. A new message or unresolved request keeps +the thread visible and discards the stale organization request.
docs/user/thread-sidebar.md · right 30–31
🟡 Medium user/thread-sidebar.md:30
A message sent while the provider supports active steering does not keep the thread visible: the client selects steer_active, dispatchSteerIntoRun appends it to the current run, and dispatchDeferredOrganization later settles or archives that same thread because no newer run exists. The documentation should limit this behavior to messages that create a newer run.
| 1 | A new message that creates a newer run, or an unresolved request, keeps |
| 2 | the thread visible and discards the stale organization request. |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @docs/user/thread-sidebar.md around lines 30-31: |
| 2 | |
| 3 | A message sent while the provider supports active steering does not keep the thread visible: the client selects `steer_active`, `dispatchSteerIntoRun` appends it to the current run, and `dispatchDeferredOrganization` later settles or archives that same thread because no newer run exists. The documentation should limit this behavior to messages that create a newer run. |
Fixed in 284b9dca6933303a56fe7ce10748847444cfe2e7. The documentation now limits invalidation to a message that creates a newer run; steer_active remains part of the current run.
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision 271e82b5 · right 7247–7268
Original diff
@@ -7052,6 +7238,41 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
// The high-water subscription deliberately skips history, so recover the
// two terminal side effects from current projections instead: one queued
// run per idle thread, plus any app-owned child result not yet transferred.
+ yield* projectionStore.getShellSnapshot().pipe(
+ Effect.flatMap((shell) =>
+ Effect.forEach(
+ [...shell.threads, ...shell.archivedThreads].filter(
+ (thread) => thread.deferredOrganization != null,
+ ),
+ (thread) =>
+ threadDispatch.withLock(
+ thread.id,
+ Effect.gen(function* () {
+ const projection = yield* projectionStore.getThreadProjection(thread.id);
+ const intent = projection.thread.deferredOrganization;
+ if (intent == null) return;
+ const boundRun = projection.runs.find((run) => run.id === intent.runId);
+ const hasNewerRun =
+ boundRun !== undefined &&
+ projection.runs.some((run) => run.ordinal > boundRun.ordinal);
+ if (
+ boundRun === undefined ||
+ hasNewerRun ||
+ ["completed", "failed", "cancelled", "interrupted", "rolled_back"].includes(
+ boundRun.status,
+ )
+ ) {
+ yield* applyDeferredOrganization(thread.id, intent.runId);
+ }
+ }),
+ ),apps/server/src/orchestration-v2/Orchestrator.ts · right 7247–7268
The new deferred-organization startup recovery only catches at the outer Effect.forEach, so a single thread whose projection load or dispatch fails aborts the whole pass and leaves later threads' intents unrecovered. Consider isolating the failure per entity, as the neighbouring subagent and delegated-completion recovery passes do (each wraps the per-thread withLock in its own Effect.catchCause before the outer catch).
| 1 | (thread) => |
| 2 | threadDispatch |
| 3 | .withLock( |
| 4 | thread.id, |
| 5 | Effect.gen(function* () { |
| 6 | const projection = yield* projectionStore.getThreadProjection(thread.id); |
| 7 | const intent = projection.thread.deferredOrganization; |
| 8 | if (intent == null) return; |
| 9 | const boundRun = projection.runs.find((run) => run.id === intent.runId); |
| 10 | const hasNewerRun = |
| 11 | boundRun !== undefined && |
| 12 | projection.runs.some((run) => run.ordinal > boundRun.ordinal); |
| 13 | if ( |
| 14 | boundRun === undefined || |
| 15 | hasNewerRun || |
| 16 | ["completed", "failed", "cancelled", "interrupted", "rolled_back"].includes( |
| 17 | boundRun.status, |
| 18 | ) |
| 19 | ) { |
| 20 | yield* applyDeferredOrganization(thread.id, intent.runId); |
| 21 | } |
| 22 | }), |
| 23 | ) |
| 24 | .pipe( |
| 25 | Effect.catchCause((cause) => |
| 26 | Effect.logWarning("Failed to recover deferred organization for thread", { |
| 27 | threadId: thread.id, |
| 28 | cause, |
| 29 | }), |
| 30 | ), |
| 31 | ), |
A focused test that one persistently failing thread does not prevent a second thread's intent from being applied would lock this in.
Posted via Macroscope — Effect Service Conventions
Fixed in acb2b17f3c. Deferred recovery is now a typed post-provider-reconciliation startup hook, and each thread lock/projection/apply path catches independently. The persistent-failure integration corrupts one thread projection and verifies the next deferred intent is still recovered.
apps/server/src/orchestration-v2/Orchestrator.ts · Earlier revision 4bf90204 · right 7274–7281
Original diff
@@ -7069,6 +7242,54 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
// The high-water subscription deliberately skips history, so recover the
// two terminal side effects from current projections instead: one queued
// run per idle thread, plus any app-owned child result not yet transferred.
+ const recoverDeferredOrganization = projectionStore.getShellSnapshot().pipe(
+ Effect.flatMap((shell) =>
+ Effect.forEach(
+ [...shell.threads, ...shell.archivedThreads].filter(
+ (thread) => thread.deferredOrganization != null,
+ ),
+ (thread) =>
+ threadDispatch
+ .withLock(
+ thread.id,
+ Effect.gen(function* () {
+ const projection = yield* projectionStore.getThreadProjection(thread.id);
+ const intent = projection.thread.deferredOrganization;
+ if (intent == null) return;
+ const boundRun = projection.runs.find((run) => run.id === intent.runId);
+ const hasNewerRun =
+ boundRun !== undefined &&
+ projection.runs.some((run) => run.ordinal > boundRun.ordinal);
+ if (
+ boundRun === undefined ||
+ hasNewerRun ||
+ ["completed", "failed", "cancelled", "interrupted", "rolled_back"].includes(
+ boundRun.status,
+ )
+ ) {
+ yield* applyDeferredOrganization(thread.id, intent.runId);
+ }
+ }),
+ )
+ .pipe(
+ Effect.catchCause((cause) =>
+ Effect.logWarning("Failed to recover deferred thread organization", {
+ threadId: thread.id,
+ cause,
+ }),
+ ),
+ ),apps/server/src/orchestration-v2/Orchestrator.ts · right 7274–7281
orchestration-v2/Orchestrator.ts:7281
Per-thread isolation is fixed now, but this reconciliation still reduces interruption to a warning and never retries a transient repair before readiness. Effect.catchCause catches interrupt causes too, so a shutdown mid-recovery keeps sweeping the remaining threads instead of unwinding; and a one-off projection/dispatch failure permanently skips that thread's intent until the next restart. Consider retrying briefly, then re-failing interrupt-only causes before falling back to the warning, and covering the retry-success path next to the existing persistent-failure continuation test.
| 1 | .pipe( |
| 2 | + Effect.retry({ times: 2, schedule: Schedule.spaced("50 millis") }), |
| 3 | Effect.catchCause((cause) => |
| 4 | - Effect.logWarning("Failed to recover deferred thread organization", { |
| 5 | - threadId: thread.id, |
| 6 | - cause, |
| 7 | - }), |
| 8 | + Cause.hasInterruptsOnly(cause) |
| 9 | + ? Effect.failCause(cause) |
| 10 | + : Effect.logWarning("Failed to recover deferred thread organization", { |
| 11 | + threadId: thread.id, |
| 12 | + cause, |
| 13 | + }), |
| 14 | ), |
| 15 | ), |
Needs import * as Cause from "effect/Cause" and import * as Schedule from "effect/Schedule".
No longer relevant as of afd33efd2d8cd684b11cc34b7c8edcee3e80e083
Fixed in afd33efd2d8cd684b11cc34b7c8edcee3e80e083. The recovery wrapper retries only ProjectionStoreReadError and OrchestratorDispatchError once, immediately. It does not retry rejected receipts, defects, or other causes. Cause.hasInterruptsOnly now propagates so shutdown unwinds; persistent non-interrupt failures keep the prior warning-and-continue behavior. The focused test proves exactly two attempts for one transient typed read failure and separately observes an interrupt-only exit, without timers or polling.
packages/contracts/src/orchestrationV2.ts · Earlier revision cf20d210 · right 2172
Original diff
@@ -2131,6 +2156,24 @@ export const OrchestrationV2Command = Schema.Union([
commandId: CommandId,
threadId: ThreadId,
}),
+ Schema.Struct({
+ type: Schema.Literal("thread.organization.defer"),
+ commandId: CommandId,
+ threadId: ThreadId,
+ runId: RunId,
+ action: Schema.Literals(["settle", "archive"]),
+ }),
+ Schema.Struct({
+ type: Schema.Literal("thread.organization.defer.cancel"),
+ commandId: CommandId,
+ threadId: ThreadId,
+ }),
+ Schema.Struct({
+ type: Schema.Literal("thread.organization.defer.apply"),🟡 Medium src/orchestrationV2.ts:2172
Any connected client can submit thread.organization.defer.apply and prematurely clear deferredOrganization while the bound run is still active, cancelling a pending settle/archive request before its terminal path. Because this variant is part of the public OrchestrationV2Command union, dispatchCommand forwards it to dispatchDeferredOrganization, which reaches !canApply; keep this command in a server-internal schema rather than the client-facing union.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @packages/contracts/src/orchestrationV2.ts around line 2172: |
| 2 | |
| 3 | Any connected client can submit `thread.organization.defer.apply` and prematurely clear `deferredOrganization` while the bound run is still active, cancelling a pending settle/archive request before its terminal path. Because this variant is part of the public `OrchestrationV2Command` union, `dispatchCommand` forwards it to `dispatchDeferredOrganization`, which reaches `!canApply`; keep this command in a server-internal schema rather than the client-facing union. |
Fixed in 71650ee5adf1ac035cd4f30d607ea5bf77bbba21. The public orchestration.dispatchCommand schema rejects only thread.organization.defer.apply; the full internal command schema and server dispatch path still accept it, and defer.cancel remains public. Contract coverage decodes those three boundaries explicitly.