From 4753c0f4b82809bf2b4b27644744dd03e37f639d Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 11:22:36 +0000 Subject: [PATCH 01/18] fix(server): only the newest plan stays actionable (TEC-1331) A superseded plan could come back as the thread's current plan and be implemented instead of the approved revision. - The provider event ingestor supersedes every other active proposed plan when a new one arrives, for every adapter (only Claude did it, in memory). - Continuing a thread in any interaction mode completes the plan the turn implements and supersedes any other open plan. A background-task wake does not count as a continuation. - Plan review's decided-plan write-back now reaches V2 through the new thread.proposed-plan.retire command; before, V2 rejected the legacy upsert and the error was swallowed. - The web no longer falls back to an earlier run's plan. - The approval prompt names the approved revision and plan id, and repeats the full plan when the agent wrote more than one revision. Co-Authored-By: Claude Opus 5.5 --- .../Layers/OrchestrationEngine.ts | 17 +++ .../Orchestrator.control-reads.test.ts | 131 ++++++++++++++++++ .../src/orchestration-v2/Orchestrator.ts | 77 ++++++++-- .../src/orchestration-v2/ProjectionStore.ts | 28 ++++ .../ProviderEventIngestor.test.ts | 50 +++++++ .../orchestration-v2/ProviderEventIngestor.ts | 18 +++ .../ProviderTurnControlService.test.ts | 1 + .../testkit/OrchestratorScenario.ts | 1 + .../PlanReviewContextPolicy.test.ts | 7 + .../src/planreview/PlanReviewContextPolicy.ts | 13 +- .../src/planreview/PlanReviewService.test.ts | 40 +++++- .../src/planreview/PlanReviewService.ts | 4 + apps/web/src/session-logic.test.ts | 22 +++ apps/web/src/session-logic.ts | 11 +- packages/contracts/src/orchestrationFork.ts | 10 ++ packages/shared/src/planReview.test.ts | 28 +++- packages/shared/src/planReview.ts | 15 +- 17 files changed, 439 insertions(+), 34 deletions(-) diff --git a/apps/server/src/orchestration-v2/Layers/OrchestrationEngine.ts b/apps/server/src/orchestration-v2/Layers/OrchestrationEngine.ts index 64c451480f4ef..2dc101fa10af8 100644 --- a/apps/server/src/orchestration-v2/Layers/OrchestrationEngine.ts +++ b/apps/server/src/orchestration-v2/Layers/OrchestrationEngine.ts @@ -274,6 +274,23 @@ const make = Effect.gen(function* () { } case "thread.activity.append": return yield* dispatchNative(command, actorUserId); + // T3-CUSTOM(expbkt3): plan review records a decided plan through the legacy + // upsert; V2 keeps the decision as the plan artifact's status. + case "thread.proposed-plan.upsert": + if (command.proposedPlan.implementedAt === null) { + return yield* dispatchNative(command, actorUserId); + } + return yield* dispatchNative( + { + type: "thread.proposed-plan.retire", + commandId: command.commandId, + threadId: command.threadId, + planId: command.proposedPlan.id, + status: + command.proposedPlan.implementationThreadId === null ? "superseded" : "completed", + }, + actorUserId, + ); default: return yield* dispatchNative(command, actorUserId); } diff --git a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts index c68fc0ace5e93..32d72d5567589 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts @@ -334,6 +334,137 @@ it.effect("implements a proposed plan that the command projection leaves out", ( }).pipe(Effect.provide(layerTest)), ); +const createPlanThread = (name: string) => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const threadId = ThreadId.make(`thread:${name}`); + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make(`create-${name}`), + threadId, + projectId: ProjectId.make(`project:${name}`), + title: "Plan", + modelSelection, + runtimeMode: "full-access", + interactionMode: "plan", + branch: null, + worktreePath: null, + createdBy: "user", + creationSource: "web", + }); + return threadId; + }); + +const applyProposedPlan = ( + threadId: ThreadId, + name: string, + status: "active" | "completed" | "superseded" = "active", +) => + Effect.gen(function* () { + const projections = yield* ProjectionStore.ProjectionStoreV2; + const planId = PlanId.make(`plan:${name}`); + yield* projections.apply({ + id: EventId.make(`plan:${name}:${status}`), + type: "plan.updated", + threadId, + occurredAt: yield* DateTime.now, + payload: { + id: planId, + threadId, + runId: null, + nodeId: NodeId.make(`node:${name}`), + kind: "proposed_plan", + status, + markdown: `# Plan ${name}`, + }, + }); + return planId; + }); + +const continueThread = ( + threadId: ThreadId, + name: string, + sourcePlanRef?: { readonly threadId: ThreadId; readonly planId: PlanId }, +) => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + yield* orchestrator.dispatch({ + type: "message.dispatch", + commandId: CommandId.make(`continue-${name}`), + threadId, + messageId: MessageId.make(`continue-${name}-input`), + text: "What about the rollout?", + attachments: [], + ...(sourcePlanRef === undefined ? {} : { sourcePlanRef }), + dispatchMode: { type: "defer_start" }, + createdBy: "user", + creationSource: "web", + }); + }); + +it.effect("supersedes an open plan when the thread continues without it", () => + Effect.gen(function* () { + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* createPlanThread("continue-plan"); + const planId = yield* applyProposedPlan(threadId, "continue-plan"); + + yield* continueThread(threadId, "continue-plan"); + + assert.equal((yield* projections.getPlan(threadId, planId))?.status, "superseded"); + assert.deepEqual(yield* projections.getActiveProposedPlans(threadId), []); + }).pipe(Effect.provide(layerTest)), +); + +it.effect("completes the implemented plan and supersedes every other open plan", () => + Effect.gen(function* () { + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* createPlanThread("implement-newest"); + const older = yield* applyProposedPlan(threadId, "implement-newest-r2"); + const newest = yield* applyProposedPlan(threadId, "implement-newest-r3"); + + yield* continueThread(threadId, "implement-newest", { threadId, planId: newest }); + + assert.equal((yield* projections.getPlan(threadId, newest))?.status, "completed"); + assert.equal((yield* projections.getPlan(threadId, older))?.status, "superseded"); + }).pipe(Effect.provide(layerTest)), +); + +it.effect("implements a superseded plan that the turn names", () => + Effect.gen(function* () { + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* createPlanThread("implement-superseded"); + const planId = yield* applyProposedPlan(threadId, "implement-superseded", "superseded"); + + yield* continueThread(threadId, "implement-superseded", { threadId, planId }); + + assert.equal((yield* projections.getPlan(threadId, planId))?.status, "completed"); + }).pipe(Effect.provide(layerTest)), +); + +it.effect("retires an open plan whose review was decided", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* createPlanThread("retire-plan"); + const open = yield* applyProposedPlan(threadId, "retire-plan-open"); + const done = yield* applyProposedPlan(threadId, "retire-plan-done", "completed"); + + for (const planId of [open, done]) { + yield* orchestrator.dispatch({ + type: "thread.proposed-plan.retire", + commandId: CommandId.make(`retire-${planId}`), + threadId, + planId, + status: "superseded", + }); + } + + assert.equal((yield* projections.getPlan(threadId, open))?.status, "superseded"); + // A finished plan keeps its outcome. + assert.equal((yield* projections.getPlan(threadId, done))?.status, "completed"); + }).pipe(Effect.provide(layerTest)), +); + // Stop's settle follow-up runs after the provider interrupt returns, possibly // long after the Stop (retries) or again (an effect replayed after a crash). // A later run's background work is not that Stop's to end. diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 461a918dd6490..0ff555b882c4b 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -464,6 +464,8 @@ function commandThreadId(command: OrchestrationV2ServerCommand): ThreadId { case "thread.visit": case "thread.mark-unread": case "thread.activity.append": + // T3-CUSTOM(expbkt3): a decided plan review retires its plan. + case "thread.proposed-plan.retire": case "thread.metadata.update": case "thread.pull-request.link": case "thread.pull-request.unlink": @@ -4890,27 +4892,54 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio cause: `Proposed plan ${command.sourcePlanRef?.planId} belongs to a different project.`, }); } - if (sourcePlan !== null && sourcePlan.status !== "active") { + // A superseded plan may still be implemented when the turn names it: a + // reviewer can approve the exact version after a plan-mode question. + if ( + sourcePlan !== null && + sourcePlan.status !== "active" && + sourcePlan.status !== "superseded" + ) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, commandType: command.type, cause: `Proposed plan ${sourcePlan.id} is not active.`, }); } + // Continuing a thread ends its open plan in every interaction mode: the + // plan this turn implements is completed, any other is superseded, so an + // earlier plan can never become actionable again. A background-task wake + // is not a continuation and leaves a plan that waits for review alone. const completeSourcePlan = (occurredAt: DateTime.Utc) => - sourcePlan === null - ? Effect.void - : emit( - events, - command, - )({ - type: "plan.updated", - threadId: sourcePlan.threadId, - ...(sourcePlan.runId === null ? {} : { runId: sourcePlan.runId }), - nodeId: sourcePlan.nodeId, - occurredAt, - payload: { ...sourcePlan, status: "completed" }, - }); + Effect.gen(function* () { + const openPlans = + delegatedCompletion === undefined + ? yield* projectionStore + .getActiveProposedPlans(command.threadId) + .pipe(mapDispatchError(command)) + : []; + const retired = [ + ...openPlans + .filter((plan) => plan.id !== sourcePlan?.id) + .map((plan) => ({ ...plan, status: "superseded" as const })), + ...(sourcePlan === null ? [] : [{ ...sourcePlan, status: "completed" as const }]), + ]; + yield* Effect.forEach( + retired, + (plan) => + emit( + events, + command, + )({ + type: "plan.updated", + threadId: plan.threadId, + ...(plan.runId === null ? {} : { runId: plan.runId }), + nodeId: plan.nodeId, + occurredAt, + payload: plan, + }), + { discard: true }, + ); + }); if (dispatchMode.type === "steer_active" || dispatchMode.type === "restart_active") { yield* dispatchSteerIntoRun({ @@ -10379,6 +10408,26 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio readonly reason: string; } | undefined; + // T3-CUSTOM(expbkt3): a plan review decided outside a turn start ends its plan. + if (command.type === "thread.proposed-plan.retire") { + const plan = yield* projectionStore + .getPlan(command.threadId, command.planId) + .pipe(mapDispatchError(command)); + if (plan?.kind === "proposed_plan" && plan.status === "active") { + yield* emit( + events, + command, + )({ + type: "plan.updated", + threadId: plan.threadId, + ...(plan.runId === null ? {} : { runId: plan.runId }), + nodeId: plan.nodeId, + occurredAt: yield* DateTime.now, + payload: { ...plan, status: command.status }, + }); + } + return { events: yield* Ref.get(events), effects: [] }; + } // T3-CUSTOM(expbkt3): fork tool/activity payloads share the V2 event and receipt transaction. if (command.type === "thread.activity.append") { const projection = yield* projectionStore diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 2ded30eabfcd7..bab683493d994 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -418,6 +418,13 @@ export interface ProjectionStoreV2Shape { threadId: ThreadId, planId: PlanId, ) => Effect.Effect; + /** Proposed plans on the thread that still wait for a decision. */ + readonly getActiveProposedPlans: ( + threadId: ThreadId, + ) => Effect.Effect< + ReadonlyArray>, + ProjectionStoreV2Error + >; readonly hasUnpairedRunInterruptRequest: ( threadId: ThreadId, requestId: TurnItemId, @@ -1052,6 +1059,10 @@ const decodeMessagePayload = Schema.decodeUnknownEffect( ); const decodePlanArtifact = Schema.decodeUnknownEffect(OrchestrationV2PlanArtifactSchema); const decodePlanPayload = (json: string) => decodePlanArtifact(parseEncodedPayload(json)); +const isProposedPlanArtifact = ( + plan: OrchestrationV2PlanArtifact, +): plan is Extract => + plan.kind === "proposed_plan"; const decodeTurnItemPayload = Schema.decodeUnknownEffect( Schema.fromJsonString(OrchestrationV2TurnItemJsonSchema), ); @@ -4297,6 +4308,14 @@ export const layer: Layer.Layer = ) .pipe(Effect.mapError(controlReadError(threadId))); + const getActiveProposedPlans: ProjectionStoreV2Shape["getActiveProposedPlans"] = (threadId) => + Effect.gen(function* () { + const rows = yield* sql`SELECT payload_json FROM orchestration_v2_projection_plans + WHERE thread_id = ${threadId} AND kind = 'proposed_plan' AND status = 'active'`; + const plans = yield* decodeRows(decodePlanPayload, threadId)(rows); + return plans.filter(isProposedPlanArtifact); + }).pipe(Effect.mapError(controlReadError(threadId))); + const hasUnpairedRunInterruptRequest: ProjectionStoreV2Shape["hasUnpairedRunInterruptRequest"] = (threadId, requestId, resultId) => Effect.gen(function* () { @@ -5782,6 +5801,7 @@ export const layer: Layer.Layer = getThreadRecords, getRuntimeRequest, getPlan, + getActiveProposedPlans, getProviderControlContext, getLimitRecoveryCandidates, getRecoveryThreadIds, @@ -6013,6 +6033,14 @@ export const layerMemory: Layer.Layer = Layer.effect( return yield* new ProjectionStoreThreadNotFoundError({ threadId }); return projection.plans.find((plan) => plan.id === planId); }), + getActiveProposedPlans: (threadId) => + Effect.gen(function* () { + const projection = (yield* Ref.get(replayState)).projections.get(threadId); + if (projection === undefined) return []; + return projection.plans + .filter(isProposedPlanArtifact) + .filter((plan) => plan.status === "active"); + }), hasUnpairedRunInterruptRequest: (threadId, requestId, resultId) => Effect.gen(function* () { const projection = (yield* Ref.get(replayState)).projections.get(threadId); diff --git a/apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts b/apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts index c9740adfbf0c4..1f03e67498956 100644 --- a/apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts +++ b/apps/server/src/orchestration-v2/ProviderEventIngestor.test.ts @@ -434,6 +434,56 @@ layer("ProviderEventIngestorV2", (it) => { }), ); + it.effect("keeps only the newest proposed plan actionable, for every provider", () => + Effect.gen(function* () { + const now = yield* DateTime.now; + const eventSink = yield* EventSink.EventSinkV2; + const projectionStore = yield* ProjectionStore.ProjectionStoreV2; + const ingestor = yield* ProviderEventIngestor.ProviderEventIngestorV2; + const idAllocator = yield* IdAllocator.IdAllocatorV2; + const threadEvent = yield* threadCreatedEvent(now); + const providerSessionId = yield* idAllocator.allocate.providerSession({ + providerInstanceId: modelSelection.instanceId, + threadId: threadEvent.threadId, + }); + const proposePlan = (revision: number) => + ingestor.ingestNormalized({ + providerSessionId, + providerInstanceId: modelSelection.instanceId, + threadId: threadEvent.threadId, + event: { + type: "plan.updated", + driver: CODEX_DRIVER, + plan: { + id: PlanId.make(`plan:supersede-r${revision}`), + threadId: threadEvent.threadId, + runId: null, + nodeId: NodeId.make(`node:supersede-r${revision}`), + kind: "proposed_plan", + status: "active", + markdown: `# Plan revision ${revision}`, + }, + }, + }); + + yield* eventSink.write({ events: [threadEvent] }); + yield* proposePlan(2); + const stored = yield* proposePlan(3); + // A streamed update of the newest plan retires nothing more. + const restreamed = yield* proposePlan(3); + + const statuses = (yield* projectionStore.getThreadProjection(threadEvent.threadId)).plans + .map((plan) => [plan.id, plan.status]) + .toSorted(([left], [right]) => String(left).localeCompare(String(right))); + assert.deepEqual(statuses, [ + ["plan:supersede-r2", "superseded"], + ["plan:supersede-r3", "active"], + ]); + assert.equal(stored.length, 2); + assert.equal(restreamed.length, 1); + }), + ); + it.effect( "treats successful provider terminal markers as non-persisted orchestration control signals", () => diff --git a/apps/server/src/orchestration-v2/ProviderEventIngestor.ts b/apps/server/src/orchestration-v2/ProviderEventIngestor.ts index ad5d3a7ddb1b2..ee7a8523ff4e0 100644 --- a/apps/server/src/orchestration-v2/ProviderEventIngestor.ts +++ b/apps/server/src/orchestration-v2/ProviderEventIngestor.ts @@ -500,7 +500,25 @@ export const layer: Layer.Layer< occurredAt, ) : plan; + // A thread has one actionable proposed plan: the newest. Retiring the + // others here covers every adapter and survives restarts. + const superseded = + plan.kind === "proposed_plan" && plan.status === "active" + ? (yield* projections.getActiveProposedPlans(plan.threadId)).filter( + (candidate) => candidate.id !== plan.id, + ) + : []; return [ + ...(yield* Effect.forEach(superseded, (previousPlan) => + makeDomainEvent(input, { + type: "plan.updated", + threadId: previousPlan.threadId, + payload: { ...previousPlan, status: "superseded" }, + runId: previousPlan.runId, + nodeId: previousPlan.nodeId, + occurredAt, + }), + )), yield* makeDomainEvent(input, { type: "plan.updated", threadId: plan.threadId, diff --git a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts index 7918f819eee2a..92ae881ce3a50 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts @@ -222,6 +222,7 @@ it.effect( getTurnStartHistory: () => Effect.die("unused"), getRuntimeRecoveryProjection: () => Effect.die("unused getRuntimeRecoveryProjection"), getPlan: () => Effect.die("unused"), + getActiveProposedPlans: () => Effect.die("unused"), hasUnpairedRunInterruptRequest: () => Effect.die("unused interrupt read"), getThreadAttachmentIds: () => Effect.die("Unused attachment lookup"), getTimelinePage: () => Effect.die("Unused timeline read"), diff --git a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts index 8136cf696aab6..b08c590ebb007 100644 --- a/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts +++ b/apps/server/src/orchestration-v2/testkit/OrchestratorScenario.ts @@ -135,6 +135,7 @@ function commandThreadIds(command: OrchestrationV2Command): ReadonlyArray { @@ -21,6 +22,12 @@ describe("decidePlanResend", () => { expect(decision.reason).toContain("different session"); }); + it("resends when the agent revised the plan", () => { + const decision = decidePlanResend({ ...baseSignals, agentRevisionCount: 3 }); + expect(decision.shouldResend).toBe(true); + expect(decision.reason).toContain("only this revision is approved"); + }); + it("resends when the thread compacted after the plan was written", () => { const decision = decidePlanResend({ ...baseSignals, diff --git a/apps/server/src/planreview/PlanReviewContextPolicy.ts b/apps/server/src/planreview/PlanReviewContextPolicy.ts index 784ecb865147d..f287b62171c78 100644 --- a/apps/server/src/planreview/PlanReviewContextPolicy.ts +++ b/apps/server/src/planreview/PlanReviewContextPolicy.ts @@ -20,6 +20,8 @@ export interface PlanResendSignals { readonly targetThreadId: string; /** Provider session status bound to the target thread, null when unbound. */ readonly providerSessionStatus: string | null; + /** How many versions of the plan the agent wrote. */ + readonly agentRevisionCount: number; } export interface PlanResendDecision { @@ -34,7 +36,7 @@ export interface PlanResendDecision { /** * Returns whether the approval prompt must carry the whole plan. * - * Three signals force a resend; anything else keeps the prompt to one line. + * Four signals force a resend; anything else keeps the prompt to one line. */ export function decidePlanResend(signals: PlanResendSignals): PlanResendDecision { if (signals.targetThreadId !== signals.planThreadId) { @@ -44,6 +46,15 @@ export function decidePlanResend(signals: PlanResendSignals): PlanResendDecision }; } + // Earlier revisions are still in context, and "the plan above" has been read + // as one of them. Naming the approved text removes the ambiguity. + if (signals.agentRevisionCount > 1) { + return { + shouldResend: true, + reason: "the plan was revised, and only this revision is approved", + }; + } + if (signals.latestCompactionAt !== null && signals.latestCompactionAt > signals.planCreatedAt) { return { shouldResend: true, diff --git a/apps/server/src/planreview/PlanReviewService.test.ts b/apps/server/src/planreview/PlanReviewService.test.ts index f55c3e8a31211..0e527ce70d660 100644 --- a/apps/server/src/planreview/PlanReviewService.test.ts +++ b/apps/server/src/planreview/PlanReviewService.test.ts @@ -379,7 +379,7 @@ describe("PlanReviewService approval", () => { expect(result.resentPlan).toBe(false); expect(result.prompt).toBe( - "Plan approved. Implement the plan you proposed above, exactly as written.", + "Plan approved (revision 1, plan plan:a). Implement the plan you proposed above, exactly as written.", ); const commands = yield* Ref.get(dispatched); @@ -456,13 +456,43 @@ describe("PlanReviewService approval", () => { expect(result.resentPlan).toBe(true); const text = turnText(yield* Ref.get(dispatched)); - expect(text).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN:"); + expect(text).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN (revision 1, plan plan:a):"); expect(text).toContain("3. Flip the flag"); expect(text).toContain("compacted its context"); }), ), ); + it.effect("implements the approved revision, not an earlier one", () => + runWithService({ sessionStatus: "running", compactionAt: null }, ({ service, dispatched }) => + Effect.gen(function* () { + const document = yield* capturePlan(service, "plan:a"); + yield* capturePlan(service, "plan:b", `${PLAN}\n4. Announce it`); + const result = yield* service.submit({ + documentId: document.documentId, + decision: "approved", + globalComment: "", + editedMarkdown: null, + actorUserId: reviewerId, + actorLabel: "Tushar", + }); + + // Earlier revisions are still in the agent's context, so the approved + // one is named and repeated in full. + expect(result.resentPlan).toBe(true); + const commands = yield* Ref.get(dispatched); + const text = turnText(commands); + expect(text).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN (revision 2, plan plan:b):"); + expect(text).toContain("4. Announce it"); + expect(text).toContain("only this revision is approved"); + const turn = commands.find((command) => command.type === "thread.turn.start"); + expect(turn?.type === "thread.turn.start" && turn.sourceProposedPlan?.planId).toBe( + "plan:b", + ); + }), + ), + ); + it.effect("re-sends the plan when no provider session is bound", () => runWithService({ sessionStatus: null, compactionAt: null }, ({ service }) => Effect.gen(function* () { @@ -495,7 +525,7 @@ describe("PlanReviewService approval", () => { }); const text = turnText(yield* Ref.get(dispatched)); - expect(text).toContain("Plan approved with reviewer edits."); + expect(text).toContain("Plan approved with reviewer edits (revision 2, plan plan:a)."); expect(text).toContain("1. Add the migration\n2. Backfill the rows"); expect(text).toContain("3. Flip the flag behind a kill switch"); expect(text).not.toContain("```diff"); @@ -534,7 +564,9 @@ describe("PlanReviewService approval", () => { const text = turnText(yield* Ref.get(dispatched)); // T3-CUSTOM(expbkt3): approving with comments open means "start now, and // fold these in" — so the prompt must not also say "exactly as written". - expect(text).toContain("Plan approved — start implementing it now."); + expect(text).toContain( + "Plan approved (revision 1, plan plan:a) — start implementing it now.", + ); expect(text).toContain("apply them as you implement"); expect(text).not.toContain("exactly as written"); expect(text).toContain("Reviewer notes:\nStart with the migration."); diff --git a/apps/server/src/planreview/PlanReviewService.ts b/apps/server/src/planreview/PlanReviewService.ts index eec89e50dcbd2..54d89e0f37b5c 100644 --- a/apps/server/src/planreview/PlanReviewService.ts +++ b/apps/server/src/planreview/PlanReviewService.ts @@ -833,12 +833,14 @@ export const make = Effect.gen(function* () { .sort() .at(-1) ?? null; + const agentRevisions = snapshot.versions.filter(({ authorKind }) => authorKind === "agent"); const resend = decidePlanResend({ latestCompactionAt, planCreatedAt: agentBaseline.createdAt, planThreadId: document.threadId, targetThreadId: document.threadId, providerSessionStatus: thread.session?.status ?? null, + agentRevisionCount: agentRevisions.length, }); resentPlan = resend.shouldResend; @@ -851,6 +853,8 @@ export const make = Effect.gen(function* () { hasReviewerEdits || resend.shouldResend ? approvedVersion.contentMarkdown : null, fullPlanReason: hasReviewerEdits ? null : resend.reason, wasEdited: hasReviewerEdits, + revision: approvedVersion.revision, + planId: approvedVersion.sourcePlanId ?? agentBaseline.sourcePlanId, }); } else { prompt = buildPlanReviewFeedbackPrompt({ diff --git a/apps/web/src/session-logic.test.ts b/apps/web/src/session-logic.test.ts index f7d61b8f65a70..33f14f9e49a36 100644 --- a/apps/web/src/session-logic.test.ts +++ b/apps/web/src/session-logic.test.ts @@ -239,6 +239,28 @@ describe("V2 session presentation", () => { expect(plan?.planMarkdown).toBe("Plan"); }); + it("does not resurface an earlier run's plan when the latest run has none", () => { + const baseProjection = makeThreadProjectionFixture(); + const plan = findLatestProposedPlan( + { + ...baseProjection, + plans: [ + { + id: PlanId.make("plan-old"), + threadId: baseProjection.thread.id, + nodeId: NodeId.make("node-plan-old"), + kind: "proposed_plan" as const, + markdown: "Old plan", + status: "active" as const, + runId: RunId.make("run-1"), + }, + ], + }, + RunId.make("run-2"), + ); + expect(plan).toBeNull(); + }); + it("assigns run rollback to the turn-start message instead of a later steer", () => { const runId = RunId.make("run-steered"); const turnStartMessageId = MessageId.make("message-turn-start"); diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index 241b13574bdeb..86c8c60b9a9ad 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -311,10 +311,13 @@ export function findLatestProposedPlan( projection: OrchestrationV2ThreadProjection | null, latestRunId: RunId | string | null | undefined, ): LatestProposedPlanState | null { - if (projection === null) return null; - const plans = projection.plans.filter((plan) => plan.kind === "proposed_plan"); - const candidates = latestRunId ? plans.filter((plan) => plan.runId === latestRunId) : plans; - const plan = [...(candidates.length > 0 ? candidates : plans)] + // T3-CUSTOM(expbkt3): only the latest run's plan is current. Falling back to an + // earlier run resurfaced a superseded plan in the banner and implement prompt. + if (projection === null || !latestRunId) return null; + const candidates = projection.plans.filter( + (plan) => plan.kind === "proposed_plan" && plan.runId === latestRunId, + ); + const plan = candidates .toSorted( (left, right) => planItemTime(projection, left.id).localeCompare(planItemTime(projection, right.id)) || diff --git a/packages/contracts/src/orchestrationFork.ts b/packages/contracts/src/orchestrationFork.ts index 6e20871bd6c2b..3e3003316af7b 100644 --- a/packages/contracts/src/orchestrationFork.ts +++ b/packages/contracts/src/orchestrationFork.ts @@ -5,6 +5,7 @@ import { CommandId, EnvironmentId, IsoDateTime, + PlanId, ProjectId, ThreadId, TrimmedNonEmptyString, @@ -121,6 +122,15 @@ export const ForkThreadCommands = [ threadId: ThreadId, activity: OrchestrationThreadActivity, }), + // Ends a proposed plan whose review was decided outside a turn start, such as + // a discard. Only an active plan changes. + Schema.Struct({ + type: Schema.Literal("thread.proposed-plan.retire"), + commandId: CommandId, + threadId: ThreadId, + planId: PlanId, + status: Schema.Literals(["completed", "superseded"]), + }), Schema.Struct({ type: Schema.Literal("thread.member.add"), commandId: CommandId, diff --git a/packages/shared/src/planReview.test.ts b/packages/shared/src/planReview.test.ts index 6cdde6175f5e1..76cdafbb3d255 100644 --- a/packages/shared/src/planReview.test.ts +++ b/packages/shared/src/planReview.test.ts @@ -262,13 +262,15 @@ describe("buildPlanReviewApprovalPrompt", () => { fullPlanMarkdown: null, fullPlanReason: null, wasEdited: false, + revision: 3, + planId: "plan-3", }; it("sends one short line when the plan is still in context", () => { const prompt = buildPlanReviewApprovalPrompt(base); expect(prompt).toBe( - "Plan approved. Implement the plan you proposed above, exactly as written.", + "Plan approved (revision 3, plan plan-3). Implement the plan you proposed above, exactly as written.", ); }); @@ -289,7 +291,7 @@ describe("buildPlanReviewApprovalPrompt", () => { wasEdited: true, }); - expect(prompt).toContain("Plan approved with reviewer edits."); + expect(prompt).toContain("Plan approved with reviewer edits (revision 3, plan plan-3)."); expect(prompt).toContain("3. Flip the flag safely"); expect(prompt).not.toContain("```diff"); expect(prompt.split("# Auth rewrite")).toHaveLength(2); @@ -334,7 +336,9 @@ describe("buildPlanReviewApprovalPrompt", () => { it("tells the agent to implement now and apply the refinements", () => { const prompt = buildPlanReviewApprovalPrompt(withComment); - expect(prompt).toContain("Plan approved — start implementing it now."); + expect(prompt).toContain( + "Plan approved (revision 3, plan plan-3) — start implementing it now.", + ); expect(prompt).toContain("Treat them as"); expect(prompt).toContain("apply them as you implement"); expect(prompt).toContain("Do not go back"); @@ -348,7 +352,9 @@ describe("buildPlanReviewApprovalPrompt", () => { it("treats overall notes alone as refinements too", () => { const prompt = buildPlanReviewApprovalPrompt({ ...base, notes: "Ship behind a flag." }); - expect(prompt).toContain("Plan approved — start implementing it now."); + expect(prompt).toContain( + "Plan approved (revision 3, plan plan-3) — start implementing it now.", + ); expect(prompt).toContain("Reviewer notes:\nShip behind a flag."); expect(prompt).not.toContain("exactly as written"); }); @@ -357,7 +363,7 @@ describe("buildPlanReviewApprovalPrompt", () => { const prompt = buildPlanReviewApprovalPrompt(base); expect(prompt).toBe( - "Plan approved. Implement the plan you proposed above, exactly as written.", + "Plan approved (revision 3, plan plan-3). Implement the plan you proposed above, exactly as written.", ); }); @@ -368,12 +374,20 @@ describe("buildPlanReviewApprovalPrompt", () => { fullPlanReason: "this session compacted its context after the plan was written", }); - expect(prompt).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN:"); + expect(prompt).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN (revision 3, plan plan-3):"); expect(prompt).toContain("apply them as you implement"); expect(prompt).toContain("Batch this."); }); }); + it("names the approved revision even without a source plan", () => { + const prompt = buildPlanReviewApprovalPrompt({ ...base, revision: 4, planId: null }); + + expect(prompt).toBe( + "Plan approved (revision 4). Implement the plan you proposed above, exactly as written.", + ); + }); + it("repeats the plan and explains why when the policy demands it", () => { const prompt = buildPlanReviewApprovalPrompt({ ...base, @@ -381,7 +395,7 @@ describe("buildPlanReviewApprovalPrompt", () => { fullPlanReason: "this session compacted its context after the plan was written", }); - expect(prompt).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN:"); + expect(prompt).toContain("PLEASE IMPLEMENT THIS APPROVED PLAN (revision 3, plan plan-3):"); expect(prompt).toContain("3. Flip the flag"); expect(prompt).toContain("(The full plan is repeated because this session compacted"); }); diff --git a/packages/shared/src/planReview.ts b/packages/shared/src/planReview.ts index 8423da33ef649..8d396feaefd7c 100644 --- a/packages/shared/src/planReview.ts +++ b/packages/shared/src/planReview.ts @@ -49,6 +49,11 @@ export interface PlanReviewApprovalInput { readonly fullPlanMarkdown: string | null; readonly fullPlanReason: string | null; readonly wasEdited: boolean; + /** The approved document revision, named in the prompt so the agent and the + * transcript both record exactly which version was approved. */ + readonly revision: number; + /** The proposed plan the approved revision came from, when one exists. */ + readonly planId: string | null; } /** @@ -226,12 +231,14 @@ export function buildPlanReviewApprovalPrompt(input: PlanReviewApprovalInput): s * a list of changes contradicts itself, so the instruction changes instead. */ const hasRefinements = input.comments.length > 0 || notes.length > 0; + const planLabel = input.planId === null ? "" : `, plan ${input.planId}`; + const version = `revision ${input.revision}${planLabel}`; if (input.fullPlanMarkdown !== null) { sections.push( input.wasEdited - ? "Plan approved with reviewer edits. Implement this complete revised plan:" - : "PLEASE IMPLEMENT THIS APPROVED PLAN:", + ? `Plan approved with reviewer edits (${version}). Implement this complete revised plan:` + : `PLEASE IMPLEMENT THIS APPROVED PLAN (${version}):`, ); sections.push(planReviewFence("markdown", input.fullPlanMarkdown)); if (input.fullPlanReason !== null) { @@ -240,8 +247,8 @@ export function buildPlanReviewApprovalPrompt(input: PlanReviewApprovalInput): s } else { sections.push( hasRefinements - ? "Plan approved — start implementing it now." - : "Plan approved. Implement the plan you proposed above, exactly as written.", + ? `Plan approved (${version}) — start implementing it now.` + : `Plan approved (${version}). Implement the plan you proposed above, exactly as written.`, ); } From f9554a0e8c16710455136b07f09e9f0cbe3ed94f Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 11:35:27 +0000 Subject: [PATCH 02/18] fix(server): address review of plan supersession (TEC-1331) - A retire that finds the plan already closed is an expected no-op, not a rejected command; plan review keeps closing the rest if one fails. - `/compact`, `/logout` and background-task wakes do not end an open plan; a queued message completes only the plan it names. - The web falls back to an earlier run's plan only while it is active, so a wake or a subagent thread keeps the banner and a closed plan never returns. - Fix the web type narrowing, fork markers and formatting in ProjectionStore. Co-Authored-By: Claude Opus 5.5 --- .../Orchestrator.control-reads.test.ts | 15 +++++++- .../src/orchestration-v2/Orchestrator.ts | 20 +++++++---- .../src/orchestration-v2/ProjectionStore.ts | 12 +++++-- .../src/planreview/PlanReviewService.ts | 3 +- apps/web/src/session-logic.test.ts | 35 +++++++++---------- apps/web/src/session-logic.ts | 15 +++++--- 6 files changed, 65 insertions(+), 35 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts index 32d72d5567589..f1b9859a40fd2 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts @@ -385,6 +385,7 @@ const continueThread = ( threadId: ThreadId, name: string, sourcePlanRef?: { readonly threadId: ThreadId; readonly planId: PlanId }, + text = "What about the rollout?", ) => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; @@ -393,7 +394,7 @@ const continueThread = ( commandId: CommandId.make(`continue-${name}`), threadId, messageId: MessageId.make(`continue-${name}-input`), - text: "What about the rollout?", + text, attachments: [], ...(sourcePlanRef === undefined ? {} : { sourcePlanRef }), dispatchMode: { type: "defer_start" }, @@ -415,6 +416,18 @@ it.effect("supersedes an open plan when the thread continues without it", () => }).pipe(Effect.provide(layerTest)), ); +it.effect("leaves an open plan alone for a maintenance command", () => + Effect.gen(function* () { + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = yield* createPlanThread("compact-plan"); + const planId = yield* applyProposedPlan(threadId, "compact-plan"); + + yield* continueThread(threadId, "compact-plan", undefined, "/compact"); + + assert.equal((yield* projections.getPlan(threadId, planId))?.status, "active"); + }).pipe(Effect.provide(layerTest)), +); + it.effect("completes the implemented plan and supersedes every other open plan", () => Effect.gen(function* () { const projections = yield* ProjectionStore.ProjectionStoreV2; diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 0ff555b882c4b..98d3f2a39c8b0 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4908,11 +4908,17 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio // Continuing a thread ends its open plan in every interaction mode: the // plan this turn implements is completed, any other is superseded, so an // earlier plan can never become actionable again. A background-task wake - // is not a continuation and leaves a plan that waits for review alone. - const completeSourcePlan = (occurredAt: DateTime.Utc) => + // or a maintenance command (`/compact`) is not a continuation, and a + // queued message waits for its run, so those leave an open plan alone. + const continuesThread = + delegatedCompletion === undefined && !isNativeMaintenanceCommand(command); + const completeSourcePlan = ( + occurredAt: DateTime.Utc, + options: { readonly supersedeOpenPlans: boolean } = { supersedeOpenPlans: true }, + ) => Effect.gen(function* () { const openPlans = - delegatedCompletion === undefined + continuesThread && options.supersedeOpenPlans ? yield* projectionStore .getActiveProposedPlans(command.threadId) .pipe(mapDispatchError(command)) @@ -5185,7 +5191,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio occurredAt: now, payload: run, }); - yield* completeSourcePlan(now); + yield* completeSourcePlan(now, { supersedeOpenPlans: false }); yield* emitEvent({ type: "run-attempt.created", threadId: command.threadId, @@ -10933,10 +10939,12 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio Effect.flatMap((planned) => // A settle that finds the provider already ended everything, or a // stop that finds nothing running, has nothing to record. That is - // its expected outcome, not a failure. + // its expected outcome, not a failure. So is retiring a plan that a + // turn already completed or superseded. planned.events.length > 0 || command.type === "thread.background-work.settle" || - command.type === "thread.stop" + command.type === "thread.stop" || + command.type === "thread.proposed-plan.retire" ? Effect.succeed(planned) : Effect.fail( new OrchestratorDispatchError({ diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index bab683493d994..ccd482284336c 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -418,7 +418,7 @@ export interface ProjectionStoreV2Shape { threadId: ThreadId, planId: PlanId, ) => Effect.Effect; - /** Proposed plans on the thread that still wait for a decision. */ + // T3-CUSTOM(expbkt3): proposed plans on the thread that still wait for a decision. readonly getActiveProposedPlans: ( threadId: ThreadId, ) => Effect.Effect< @@ -1059,6 +1059,7 @@ const decodeMessagePayload = Schema.decodeUnknownEffect( ); const decodePlanArtifact = Schema.decodeUnknownEffect(OrchestrationV2PlanArtifactSchema); const decodePlanPayload = (json: string) => decodePlanArtifact(parseEncodedPayload(json)); +// T3-CUSTOM(expbkt3): narrows a plan artifact to a proposed plan. const isProposedPlanArtifact = ( plan: OrchestrationV2PlanArtifact, ): plan is Extract => @@ -4308,10 +4309,13 @@ export const layer: Layer.Layer = ) .pipe(Effect.mapError(controlReadError(threadId))); + // T3-CUSTOM(expbkt3): one indexed read, so plan supersession never loads the thread. const getActiveProposedPlans: ProjectionStoreV2Shape["getActiveProposedPlans"] = (threadId) => Effect.gen(function* () { - const rows = yield* sql`SELECT payload_json FROM orchestration_v2_projection_plans - WHERE thread_id = ${threadId} AND kind = 'proposed_plan' AND status = 'active'`; + const rows = yield* sql` + SELECT payload_json FROM orchestration_v2_projection_plans + WHERE thread_id = ${threadId} AND kind = 'proposed_plan' AND status = 'active' + `; const plans = yield* decodeRows(decodePlanPayload, threadId)(rows); return plans.filter(isProposedPlanArtifact); }).pipe(Effect.mapError(controlReadError(threadId))); @@ -5801,6 +5805,7 @@ export const layer: Layer.Layer = getThreadRecords, getRuntimeRequest, getPlan, + // T3-CUSTOM(expbkt3): plan supersession read. getActiveProposedPlans, getProviderControlContext, getLimitRecoveryCandidates, @@ -6033,6 +6038,7 @@ export const layerMemory: Layer.Layer = Layer.effect( return yield* new ProjectionStoreThreadNotFoundError({ threadId }); return projection.plans.find((plan) => plan.id === planId); }), + // T3-CUSTOM(expbkt3): plan supersession read. getActiveProposedPlans: (threadId) => Effect.gen(function* () { const projection = (yield* Ref.get(replayState)).projections.get(threadId); diff --git a/apps/server/src/planreview/PlanReviewService.ts b/apps/server/src/planreview/PlanReviewService.ts index 54d89e0f37b5c..7af8634be13aa 100644 --- a/apps/server/src/planreview/PlanReviewService.ts +++ b/apps/server/src/planreview/PlanReviewService.ts @@ -315,7 +315,8 @@ export const make = Effect.gen(function* () { }, createdAt: implementedAt, }); - }), + // One plan that cannot be closed must not keep the rest open. + }).pipe(Effect.ignore), { discard: true }, ); }).pipe(Effect.ignore); diff --git a/apps/web/src/session-logic.test.ts b/apps/web/src/session-logic.test.ts index 33f14f9e49a36..ec631c2bd1a5b 100644 --- a/apps/web/src/session-logic.test.ts +++ b/apps/web/src/session-logic.test.ts @@ -239,26 +239,23 @@ describe("V2 session presentation", () => { expect(plan?.planMarkdown).toBe("Plan"); }); - it("does not resurface an earlier run's plan when the latest run has none", () => { + it("never resurfaces a closed plan from an earlier run", () => { const baseProjection = makeThreadProjectionFixture(); - const plan = findLatestProposedPlan( - { - ...baseProjection, - plans: [ - { - id: PlanId.make("plan-old"), - threadId: baseProjection.thread.id, - nodeId: NodeId.make("node-plan-old"), - kind: "proposed_plan" as const, - markdown: "Old plan", - status: "active" as const, - runId: RunId.make("run-1"), - }, - ], - }, - RunId.make("run-2"), - ); - expect(plan).toBeNull(); + const earlierPlan = (name: string, status: "active" | "completed" | "superseded") => ({ + id: PlanId.make(`plan-${name}`), + threadId: baseProjection.thread.id, + nodeId: NodeId.make(`node-plan-${name}`), + kind: "proposed_plan" as const, + markdown: `Plan ${name}`, + status, + runId: RunId.make("run-1"), + }); + const latest = (plans: ReadonlyArray>) => + findLatestProposedPlan({ ...baseProjection, plans: [...plans] }, RunId.make("run-2")); + + expect(latest([earlierPlan("old", "superseded"), earlierPlan("done", "completed")])).toBeNull(); + // A background wake starts a run without ending the plan that waits for review. + expect(latest([earlierPlan("waiting", "active")])?.planMarkdown).toBe("Plan waiting"); }); it("assigns run rollback to the turn-start message instead of a later steer", () => { diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index 86c8c60b9a9ad..eb85501e8c141 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -311,12 +311,17 @@ export function findLatestProposedPlan( projection: OrchestrationV2ThreadProjection | null, latestRunId: RunId | string | null | undefined, ): LatestProposedPlanState | null { - // T3-CUSTOM(expbkt3): only the latest run's plan is current. Falling back to an - // earlier run resurfaced a superseded plan in the banner and implement prompt. - if (projection === null || !latestRunId) return null; - const candidates = projection.plans.filter( - (plan) => plan.kind === "proposed_plan" && plan.runId === latestRunId, + // T3-CUSTOM(expbkt3): an earlier run's plan is current only while it is still + // active. Falling back to any earlier plan resurfaced a superseded one in the + // banner and implement prompt; the server keeps at most one plan active. + if (projection === null) return null; + const plans = projection.plans.filter( + (plan): plan is Extract => + plan.kind === "proposed_plan", ); + const latestRunPlans = latestRunId ? plans.filter((plan) => plan.runId === latestRunId) : []; + const candidates = + latestRunPlans.length > 0 ? latestRunPlans : plans.filter((plan) => plan.status === "active"); const plan = candidates .toSorted( (left, right) => From cc197621a2ff22e715ad76344f07951e75dbaad3 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 11:38:10 +0000 Subject: [PATCH 03/18] fix(server): a queued continuation ends the open plan when it starts (TEC-1331) Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 98d3f2a39c8b0..ad6b5b5faf246 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -1738,6 +1738,24 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio session.status !== "stopped" && session.status !== "error", ); + // A queued continuation ends the thread's open plan when it starts, not + // when it was queued, so cancelling it leaves the plan open. Wakes and + // maintenance commands are not continuations. + const supersededPlans = + queuedRun.delegatedCompletion === undefined && + queuedRun.workStartedAt === undefined && + !isNativeMaintenanceCommand(queuedMessage) + ? (yield* projectionStore.getActiveProposedPlans(threadId).pipe( + Effect.mapError( + (cause) => + new OrchestratorDispatchError({ + commandId, + commandType: "message.dispatch", + cause, + }), + ), + )).filter((plan) => plan.id !== queuedRun.sourcePlanRef?.planId) + : []; yield* writeSystemEvents( [ ...(selectionChanged @@ -1858,6 +1876,15 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio occurredAt: now, payload: notificationTurnItem(userTurnItem, queuedMessage, projection.subagents), }, + ...supersededPlans.map((plan) => ({ + type: "plan.updated" as const, + threadId: plan.threadId, + ...(plan.runId === null ? {} : { runId: plan.runId }), + nodeId: plan.nodeId, + providerInstanceId: queuedRun.providerInstanceId, + occurredAt: now, + payload: { ...plan, status: "superseded" as const }, + })), { type: "run.updated", threadId, From 650ab2332d6bad1dbca3c56fcda86048df3b4164 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 11:39:45 +0000 Subject: [PATCH 04/18] fix(server): detect wakes from the message, including notifications (TEC-1331) Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/Orchestrator.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index ad6b5b5faf246..f74e544e08b35 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -1742,8 +1742,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio // when it was queued, so cancelling it leaves the plan open. Wakes and // maintenance commands are not continuations. const supersededPlans = - queuedRun.delegatedCompletion === undefined && - queuedRun.workStartedAt === undefined && + queuedMessage.delegatedCompletion === undefined && + queuedMessage.notification === undefined && !isNativeMaintenanceCommand(queuedMessage) ? (yield* projectionStore.getActiveProposedPlans(threadId).pipe( Effect.mapError( @@ -4938,7 +4938,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio // or a maintenance command (`/compact`) is not a continuation, and a // queued message waits for its run, so those leave an open plan alone. const continuesThread = - delegatedCompletion === undefined && !isNativeMaintenanceCommand(command); + delegatedCompletion === undefined && + command.notification === undefined && + !isNativeMaintenanceCommand(command); const completeSourcePlan = ( occurredAt: DateTime.Utc, options: { readonly supersedeOpenPlans: boolean } = { supersedeOpenPlans: true }, From 8dec884073f15a70a1026d85c36c24fd313aeabd Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 11:40:55 +0000 Subject: [PATCH 05/18] chore(server): mark the queued-run plan supersession as a fork edit (TEC-1331) Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/Orchestrator.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index f74e544e08b35..3d765d75a819a 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -1738,7 +1738,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio session.status !== "stopped" && session.status !== "error", ); - // A queued continuation ends the thread's open plan when it starts, not + // T3-CUSTOM(expbkt3): a queued continuation ends the thread's open plan when it starts, not // when it was queued, so cancelling it leaves the plan open. Wakes and // maintenance commands are not continuations. const supersededPlans = @@ -1876,6 +1876,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio occurredAt: now, payload: notificationTurnItem(userTurnItem, queuedMessage, projection.subagents), }, + // T3-CUSTOM(expbkt3): retire the plans this queued continuation ends. ...supersededPlans.map((plan) => ({ type: "plan.updated" as const, threadId: plan.threadId, From e925a5bf529057fad55bf1b68b436b4a34c223d8 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 15:38:24 +0000 Subject: [PATCH 06/18] fix(web): the BK sidebar drops the PLAN tag once plan mode is off A thread kept its PLAN pill, Plan Ready group and needs-human count for as long as its plan stayed active, even after plan mode was turned off. A plan now waits only while its thread is in plan mode, matching upstream's Plan Ready rule, so stale tags clear without a migration. Co-Authored-By: Claude Opus 5.5 --- .../sidebar/PhaseGroupedSidebar.logic.test.ts | 73 ++++++++++++++----- .../sidebar/PhaseSidebarTree.logic.test.ts | 20 ++++- .../sidebar/sidebarSessionCounters.test.ts | 14 +++- .../client-runtime/src/state/phaseSidebar.ts | 34 ++++++--- 4 files changed, 109 insertions(+), 32 deletions(-) diff --git a/apps/web/src/components/sidebar/PhaseGroupedSidebar.logic.test.ts b/apps/web/src/components/sidebar/PhaseGroupedSidebar.logic.test.ts index 289dc04aa8b38..16e63abd789de 100644 --- a/apps/web/src/components/sidebar/PhaseGroupedSidebar.logic.test.ts +++ b/apps/web/src/components/sidebar/PhaseGroupedSidebar.logic.test.ts @@ -809,7 +809,11 @@ describe("phase sidebar lifecycle", () => { ).toBe("implementing"); expect( resolvePhaseSidebarPhase( - makeThread({ hasActionableProposedPlan: true, backgroundLiveness: null }), + makeThread({ + interactionMode: "plan", + hasActionableProposedPlan: true, + backgroundLiveness: null, + }), ), ).toBe("plan_ready"); }); @@ -853,15 +857,29 @@ describe("phase sidebar lifecycle", () => { ]); }); - // T3-CUSTOM(expbkt3): BEGIN — Plan Ready means a plan is waiting, in any mode. - it("files a plan submitted from a default-mode turn under Plan Ready", () => { - // `t3_submit_plan` never changes interactionMode, so gating the group on - // plan mode hid every plan an agent submitted mid-turn. + // T3-CUSTOM(expbkt3): BEGIN — Plan Ready means a plan is waiting, and a plan + // waits only while its thread is in plan mode. + it("files a held plan under Plan Ready only while the thread is in plan mode", () => { expect( resolvePhaseSidebarPhase( - makeThread({ interactionMode: "default", hasActionableProposedPlan: true }), + makeThread({ interactionMode: "plan", hasActionableProposedPlan: true }), ), ).toBe("plan_ready"); + // Turning plan mode off clears a stale plan out of the group. + expect( + resolvePhaseSidebarPhase( + makeThread({ interactionMode: "default", hasActionableProposedPlan: true }), + ), + ).toBe("ready"); + expect( + resolvePhaseSidebarPhase( + makeThread({ + interactionMode: "default", + hasActionableProposedPlan: true, + runtime: makeSession("error"), + }), + ), + ).toBe("ready"); }); it("keeps a waiting plan out of Plan Ready while the agent is still running", () => { @@ -979,6 +997,7 @@ describe("phase sidebar lifecycle", () => { expect( resolvePhaseSidebarPhase( makeThread({ + interactionMode: "plan", hasPendingAsyncUserInput: true, hasActionableProposedPlan: true, }), @@ -991,7 +1010,11 @@ describe("phase sidebar lifecycle", () => { // this deliberately stays "plan": both badges have to survive together. expect( resolvePhaseSidebarAttentionKind( - makeThread({ hasActionableProposedPlan: true, hasPendingAsyncUserInput: true }), + makeThread({ + interactionMode: "plan", + hasActionableProposedPlan: true, + hasPendingAsyncUserInput: true, + }), ), ).toBe("plan"); expect(resolvePhaseSidebarAttentionKind(makeThread({ hasPendingAsyncUserInput: true }))).toBe( @@ -1074,35 +1097,47 @@ describe("phase sidebar attention badges", () => { // T3-CUSTOM(expbkt3): BEGIN — a plan waiting on a human is attention too. it("reports a waiting plan as attention, ranked below every blocking kind", () => { - expect(resolvePhaseSidebarAttentionKind(makeThread({ hasActionableProposedPlan: true }))).toBe( - "plan", - ); + const plan = { interactionMode: "plan", hasActionableProposedPlan: true } as const; + expect(resolvePhaseSidebarAttentionKind(makeThread(plan))).toBe("plan"); // A question, an approval and a failure each outrank it. expect( - resolvePhaseSidebarAttentionKind( - makeThread({ hasActionableProposedPlan: true, hasPendingUserInput: true }), - ), + resolvePhaseSidebarAttentionKind(makeThread({ ...plan, hasPendingUserInput: true })), ).toBe("input"); + expect( + resolvePhaseSidebarAttentionKind(makeThread({ ...plan, hasPendingApprovals: true })), + ).toBe("approval"); + expect( + resolvePhaseSidebarAttentionKind(makeThread({ ...plan, runtime: makeSession("error") })), + ).toBe("error"); + }); + + it("drops the plan badge once plan mode is off", () => { expect( resolvePhaseSidebarAttentionKind( - makeThread({ hasActionableProposedPlan: true, hasPendingApprovals: true }), + makeThread({ interactionMode: "default", hasActionableProposedPlan: true }), ), - ).toBe("approval"); + ).toBeNull(); + // An async question on the same thread still flies its own badge. expect( resolvePhaseSidebarAttentionKind( makeThread({ + interactionMode: "default", hasActionableProposedPlan: true, - runtime: makeSession("error"), + hasPendingAsyncUserInput: true, }), ), - ).toBe("error"); + ).toBe("ask"); }); it("badges a plan that arrives while the agent is still working", () => { - // The row stays in Implementing, but it still has to say a plan is there. + // The row stays in Planning, but it still has to say a plan is there. expect( resolvePhaseSidebarAttentionKind( - makeThread({ hasActionableProposedPlan: true, runtime: makeSession("running") }), + makeThread({ + interactionMode: "plan", + hasActionableProposedPlan: true, + runtime: makeSession("running"), + }), ), ).toBe("plan"); }); diff --git a/apps/web/src/components/sidebar/PhaseSidebarTree.logic.test.ts b/apps/web/src/components/sidebar/PhaseSidebarTree.logic.test.ts index 06b78eaa65758..a64377ff6fbc8 100644 --- a/apps/web/src/components/sidebar/PhaseSidebarTree.logic.test.ts +++ b/apps/web/src/components/sidebar/PhaseSidebarTree.logic.test.ts @@ -71,7 +71,7 @@ function makeRow( readonly archived?: boolean; readonly pendingApproval?: boolean; readonly unread?: boolean; - // T3-CUSTOM(expbkt3): the row holds an actionable plan. + // T3-CUSTOM(expbkt3): the row holds an actionable plan, in plan mode. readonly plan?: boolean; } = {}, ): PhaseSidebarRow { @@ -87,7 +87,7 @@ function makeRow( : {}), ...(options.archived ? { archivedAt: now } : {}), ...(options.pendingApproval ? { hasPendingApprovals: true } : {}), - ...(options.plan ? { hasActionableProposedPlan: true } : {}), + ...(options.plan ? { interactionMode: "plan" as const, hasActionableProposedPlan: true } : {}), }); return { thread, @@ -455,6 +455,22 @@ describe("resolvePhaseSidebarTreePhase", () => { expect(resolvePhaseSidebarTreePhase(tree[0] as never)).toBe("implementing"); }); + it("does not hoist a stale plan once the child left plan mode", () => { + // Turning plan mode off files the child under Ready, so the collapsed + // parent must not keep reporting a plan underneath it. + const child = makeRow("child", { parent: "parent", phaseId: "ready", plan: true }); + const tree = buildPhaseSidebarTree( + [ + makeRow("parent", { phaseId: "ready" }), + { ...child, thread: { ...child.thread, interactionMode: "default" } }, + ], + { compareSiblings: byId }, + ); + + expect(tree[0]?.descendantAttention).toBeNull(); + expect(resolvePhaseSidebarTreePhase(tree[0] as never)).toBe("ready"); + }); + it("still goes red when the subtree holds both a plan and a question", () => { const tree = buildPhaseSidebarTree( [ diff --git a/apps/web/src/components/sidebar/sidebarSessionCounters.test.ts b/apps/web/src/components/sidebar/sidebarSessionCounters.test.ts index abe9669b6bf29..df5cfc6b789b3 100644 --- a/apps/web/src/components/sidebar/sidebarSessionCounters.test.ts +++ b/apps/web/src/components/sidebar/sidebarSessionCounters.test.ts @@ -102,10 +102,22 @@ describe("sidebar session counters", () => { it("classifies approvals, input, plans, and failures as human attention", () => { expect(threadNeedsHumanAttention(makeThread({ hasPendingApprovals: true }))).toBe(true); expect(threadNeedsHumanAttention(makeThread({ hasPendingUserInput: true }))).toBe(true); - expect(threadNeedsHumanAttention(makeThread({ hasActionableProposedPlan: true }))).toBe(true); + expect( + threadNeedsHumanAttention( + makeThread({ interactionMode: "plan", hasActionableProposedPlan: true }), + ), + ).toBe(true); expect(threadNeedsHumanAttention(makeThread({ runtime: makeSession("error") }))).toBe(true); }); + it("stops counting a held plan once plan mode is off", () => { + expect( + threadNeedsHumanAttention( + makeThread({ interactionMode: "default", hasActionableProposedPlan: true }), + ), + ).toBe(false); + }); + it("counts starting and running sessions as running", () => { expect(threadIsRunning(makeThread({ runtime: makeSession("running") }))).toBe(true); expect(threadIsRunning(makeThread({ runtime: makeSession("starting") }))).toBe(true); diff --git a/packages/client-runtime/src/state/phaseSidebar.ts b/packages/client-runtime/src/state/phaseSidebar.ts index bd8801ca02e97..1660d7b2ddbde 100644 --- a/packages/client-runtime/src/state/phaseSidebar.ts +++ b/packages/client-runtime/src/state/phaseSidebar.ts @@ -970,6 +970,17 @@ export function phaseSidebarHasAsyncQuestion( return thread.hasPendingAsyncUserInput === true; } +/** + * T3-CUSTOM(expbkt3): a plan waits on a human only while its thread is in plan + * mode. Turning plan mode off clears the PLAN pill, the Plan Ready group and + * the needs-human count, the same rule as upstream's Plan Ready pill. + */ +export function phaseSidebarHasWaitingPlan( + thread: Pick, +): boolean { + return thread.hasActionableProposedPlan && thread.interactionMode === "plan"; +} + /** * T3-CUSTOM(expbkt3): is an agent actually working on this thread right now? * @@ -1011,6 +1022,8 @@ export function resolvePhaseSidebarAttentionKind( | "hasPendingApprovals" | "hasPendingUserInput" | "hasActionableProposedPlan" + // T3-CUSTOM(expbkt3): a plan waits only in plan mode (`phaseSidebarHasWaitingPlan`). + | "interactionMode" // T3-CUSTOM(expbkt3): an async question is attention that leaves the agent running. | "hasPendingAsyncUserInput" // T3-CUSTOM(expbkt3): a session error is a failure the badge has to show. @@ -1035,7 +1048,7 @@ export function resolvePhaseSidebarAttentionKind( // It deliberately loses to `plan` in the row badges rather than replacing it: // callers render ASK from `phaseSidebarHasAsyncQuestion` alongside whatever // this returns, so a thread holding both flies both badges. - if (thread.hasActionableProposedPlan) return "plan"; + if (phaseSidebarHasWaitingPlan(thread)) return "plan"; if (phaseSidebarHasAsyncQuestion(thread)) return "ask"; return null; } @@ -1070,8 +1083,8 @@ export function resolvePhaseSidebarPhase( // Sidebar V2's reliability ordering: a failure must not be hidden by // liveness that can linger while background work winds down. if (hasFailure) { - // T3-CUSTOM(expbkt3): a failure still files a held plan under Plan Ready. - return thread.hasActionableProposedPlan ? "plan_ready" : "ready"; + // T3-CUSTOM(expbkt3): a failure still files a waiting plan under Plan Ready. + return phaseSidebarHasWaitingPlan(thread) ? "plan_ready" : "ready"; } // A settled foreground turn can still own native subagents, workflows, or @@ -1085,12 +1098,12 @@ export function resolvePhaseSidebarPhase( return thread.interactionMode === "plan" ? "planning" : "implementing"; } - // T3-CUSTOM(expbkt3): Plan Ready means a plan is waiting for a human, in any - // interaction mode. `t3_submit_plan` never changes interactionMode, so gating - // this on plan mode hid every plan submitted from a default-mode turn. A - // settled turn holding an actionable plan outranks a failure: the row's whole - // job is the decision, and the failure still flies its own badge. - if (thread.hasActionableProposedPlan) return "plan_ready"; + // T3-CUSTOM(expbkt3): Plan Ready means a plan is waiting for a human, and a + // plan waits only while its thread is in plan mode + // (`phaseSidebarHasWaitingPlan`). Turning plan mode off files a stale plan + // under Ready. A settled turn holding a waiting plan outranks a failure: the + // row's whole job is the decision, and the failure still flies its own badge. + if (phaseSidebarHasWaitingPlan(thread)) return "plan_ready"; // T3-CUSTOM(expbkt3): an idle plan-mode thread with nothing to decide is just // idle. It used to land in Plan Ready, which made the group unscannable — the @@ -1513,7 +1526,8 @@ export function threadNeedsHumanAttention(thread: ThreadShell): boolean { // T3-CUSTOM(expbkt3): an async question is a human step too, even though it // leaves the agent running. phaseSidebarHasAsyncQuestion(thread) || - thread.hasActionableProposedPlan || + // T3-CUSTOM(expbkt3): a plan counts only while the thread is in plan mode. + phaseSidebarHasWaitingPlan(thread) || thread.runtime?.status === "failed" ); } From 0d144384c4dc684befbe9137d602916c3b3b03e5 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 15:38:26 +0000 Subject: [PATCH 07/18] feat(web): plan cards render like chat, go wide, and take comments - Remove the plan card's Preview button; Expand plan stays. - New header: icon, title, state, step count, reading time, and a Widen plan toggle that lets cards use the free width beside the chat. - Plan bodies get the chat's markdown features (skills, runnable shell commands, image preview, artifact templates). - HTML in a plan renders as a live themed page, like html_render: a whole HTML plan, a raw HTML document, or an html-render fence. - Selecting plan text offers Cite and the comment actions; highlights, pins and scroll-to-quote work on plans, and the panel labels them. - Plan-mode turns tell the agent how to add an html-render visual. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/RunExecutionService.ts | 6 +- .../forkAgentPlanInstructions.expbkt3.test.ts | 21 ++- .../forkAgentPlanInstructions.expbkt3.ts | 25 ++- apps/web/src/components/ChatView.tsx | 8 - .../src/components/chat/MessagesTimeline.tsx | 37 ++-- .../src/components/chat/ProposedPlanCard.tsx | 83 +++------ .../chat/useAssistantCitationTarget.ts | 19 ++- .../web/src/fork/planCard/PlanCardHeading.tsx | 96 +++++++++++ apps/web/src/fork/planCard/PlanCardRow.tsx | 94 +++++++++++ apps/web/src/fork/planCard/PlanHtmlFrame.tsx | 45 +++++ apps/web/src/fork/planCard/PlanMarkdown.tsx | 66 ++++++++ apps/web/src/fork/planCard/planCard.css | 25 +++ apps/web/src/fork/planCard/planCard.test.ts | 159 ++++++++++++++++++ apps/web/src/fork/planCard/planCardContext.ts | 36 ++++ apps/web/src/fork/planCard/planCardLayout.ts | 44 +++++ apps/web/src/fork/planCard/planCardSeams.ts | 57 +++++++ .../src/fork/planCard/planCardWidthStore.ts | 36 ++++ apps/web/src/fork/planCard/planContent.ts | 114 +++++++++++++ apps/web/src/fork/planCard/planHtml.ts | 32 ++++ apps/web/src/fork/planCard/planSummary.ts | 56 ++++++ .../threadComments/ThreadCommentsPanel.tsx | 4 +- .../web/src/fork/threadComments/model.test.ts | 11 ++ apps/web/src/fork/threadComments/model.ts | 14 +- docs/operations/expbkt3-customizations.md | 3 +- docs/user/chat-comments.md | 4 +- docs/user/plan-review.md | 22 ++- 26 files changed, 995 insertions(+), 122 deletions(-) create mode 100644 apps/web/src/fork/planCard/PlanCardHeading.tsx create mode 100644 apps/web/src/fork/planCard/PlanCardRow.tsx create mode 100644 apps/web/src/fork/planCard/PlanHtmlFrame.tsx create mode 100644 apps/web/src/fork/planCard/PlanMarkdown.tsx create mode 100644 apps/web/src/fork/planCard/planCard.css create mode 100644 apps/web/src/fork/planCard/planCard.test.ts create mode 100644 apps/web/src/fork/planCard/planCardContext.ts create mode 100644 apps/web/src/fork/planCard/planCardLayout.ts create mode 100644 apps/web/src/fork/planCard/planCardSeams.ts create mode 100644 apps/web/src/fork/planCard/planCardWidthStore.ts create mode 100644 apps/web/src/fork/planCard/planContent.ts create mode 100644 apps/web/src/fork/planCard/planHtml.ts create mode 100644 apps/web/src/fork/planCard/planSummary.ts diff --git a/apps/server/src/orchestration-v2/RunExecutionService.ts b/apps/server/src/orchestration-v2/RunExecutionService.ts index db5ea97545e20..e14a84f27dd39 100644 --- a/apps/server/src/orchestration-v2/RunExecutionService.ts +++ b/apps/server/src/orchestration-v2/RunExecutionService.ts @@ -1387,7 +1387,11 @@ export const layer: Layer.Layer< : // T3-CUSTOM(expbkt3): command history stays unchanged; the agent gets the plan policy. { ...input.message, - text: appendAgentPlanInstructions(comments.text, agentPlanSubmissionEnabled), + text: appendAgentPlanInstructions( + comments.text, + agentPlanSubmissionEnabled, + input.runtimePolicy.interactionMode, + ), }; // A context read that fails costs the agent the apps' notes for // this turn, not the turn itself. diff --git a/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.test.ts b/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.test.ts index 9cf970267b8d0..9644a8afea953 100644 --- a/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.test.ts +++ b/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vite-plus/test"; -import { appendAgentPlanInstructions } from "./forkAgentPlanInstructions.expbkt3.ts"; +import { + appendAgentPlanInstructions, + PLAN_FORMAT_INSTRUCTIONS, +} from "./forkAgentPlanInstructions.expbkt3.ts"; describe("server agent plan instructions", () => { it("keeps the complete provider message unchanged when submission is enabled", () => { @@ -16,10 +19,24 @@ describe("server agent plan instructions", () => { expect(result).toContain("Do not call t3_submit_plan"); }); + it("tells a plan-mode turn how to put live HTML in the plan card", () => { + const result = appendAgentPlanInstructions("Plan the change.", true, "plan"); + expect(result).toBe(`Plan the change.\n\n${PLAN_FORMAT_INSTRUCTIONS}`); + expect(result).toContain("`html-render`"); + }); + + it("adds the plan format only in plan mode, after the submission policy", () => { + expect(appendAgentPlanInstructions("Build it.", true, "default")).toBe("Build it."); + const result = appendAgentPlanInstructions("Plan it.", false, "plan"); + expect(result.indexOf("")).toBeLessThan( + result.indexOf(""), + ); + }); + it.each(["/compact", " /review current changes", "/model"])( "keeps the native provider command %s unchanged", (text) => { - expect(appendAgentPlanInstructions(text, false)).toBe(text); + expect(appendAgentPlanInstructions(text, false, "plan")).toBe(text); }, ); }); diff --git a/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.ts b/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.ts index dbdbd0f359e5a..9a3aadda2a1f8 100644 --- a/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.ts +++ b/apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.ts @@ -1,6 +1,25 @@ // T3-CUSTOM(expbkt3): one plan policy reaches every provider through native turn input. -export function appendAgentPlanInstructions(text: string, submissionEnabled: boolean): string { + +/** What a plan in plan mode may contain; the web plan card renders these parts. */ +export const PLAN_FORMAT_INSTRUCTIONS = [ + "", + "The T3 plan card renders your plan's Markdown like a chat reply. To show a visual inside the plan (a chart, diagram, comparison or mockup), add a fenced code block whose language is `html-render` and that holds one self-contained HTML document. The card shows it as a live page in T3's theme: style it with var(--foreground), var(--muted-foreground), var(--border), var(--card), var(--accent), var(--chart-1) to var(--chart-6) and var(--font-sans), leave the page background transparent, and let the content set the height. A plan written entirely as one HTML document also renders as a page. Plain `html` code blocks stay as code.", + "", +].join("\n"); + +export function appendAgentPlanInstructions( + text: string, + submissionEnabled: boolean, + interactionMode?: string, +): string { // Native slash commands must stay intact for the provider CLI. - if (submissionEnabled || text.trimStart().startsWith("/")) return text; - return `${text}\n\n\nThe T3 plan submission tool is disabled for this server. Write the complete plan directly in your main chat response. Do not call t3_submit_plan or use another T3 tool to create a plan panel.\n`; + if (text.trimStart().startsWith("/")) return text; + const blocks: string[] = []; + if (!submissionEnabled) { + blocks.push( + "\nThe T3 plan submission tool is disabled for this server. Write the complete plan directly in your main chat response. Do not call t3_submit_plan or use another T3 tool to create a plan panel.\n", + ); + } + if (interactionMode === "plan") blocks.push(PLAN_FORMAT_INSTRUCTIONS); + return blocks.length === 0 ? text : `${text}\n\n${blocks.join("\n\n")}`; } diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index e2d496d9bf025..63a8f5b56ef0b 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -2,7 +2,6 @@ import { PlanReviewPanel, useOpenPlanReviewDocumentId } from "../fork/planReviewSurface"; import { PlanReviewTakeover } from "../fork/planReviewTakeover"; import { PlanReviewComposerDock } from "../fork/planReviewComposerDock"; -import { usePlanReviewTakeoverStore } from "../planReviewTakeoverStore"; import { THREAD_COMMENTS_EMPTY_SEND_TEXT, ThreadCommentsPanel, @@ -3492,10 +3491,6 @@ export default function ChatView(props: ChatViewProps) { } return findLatestProposedPlan(serverProjection, activeLatestRun?.runId ?? null); }, [activeLatestRun?.runId, latestRunSettled, serverProjection]); - // T3-CUSTOM(expbkt3): a native plan opens the review takeover. - const openPlanReviewSurface = useCallback((documentId: string) => { - usePlanReviewTakeoverStore.getState().undismiss(documentId); - }, []); const planReviewDocumentId = useOpenPlanReviewDocumentId( activeThreadRef?.environmentId ?? null, activeThreadRef?.threadId ?? null, @@ -11521,9 +11516,6 @@ export default function ChatView(props: ChatViewProps) { {/* Messages — LegendList handles virtualization and scrolling internally */} void) | undefined; - planReviewDocumentId: string | null | undefined; citationRequest: AssistantCitationTarget | null; listRef: React.RefObject; timestampFormat: TimestampFormat; @@ -450,9 +448,6 @@ export interface MessagesTimelineHistoryControls { } interface MessagesTimelineProps { - // T3-CUSTOM(expbkt3): a card opens the shared native review. - onOpenPlanReview?: ((documentId: string) => void) | undefined; - planReviewDocumentId?: string | null | undefined; citationRequest?: AssistantCitationRequest | null; citationHistoryLoading?: boolean; onCiteAssistantText?: ( @@ -548,9 +543,6 @@ interface MessagesTimelineProps { // --------------------------------------------------------------------------- export const MessagesTimeline = memo(function MessagesTimeline({ - // T3-CUSTOM(expbkt3): callback keeps the review controller outside virtual rows. - onOpenPlanReview, - planReviewDocumentId, citationRequest = null, citationHistoryLoading = false, onCiteAssistantText, @@ -1224,9 +1216,6 @@ export const MessagesTimeline = memo(function MessagesTimeline({ const sharedState = useMemo( () => ({ - // T3-CUSTOM(expbkt3): keep card callbacks stable across native item updates. - onOpenPlanReview, - planReviewDocumentId, citationRequest: readyCitationRequest, listRef, timestampFormat, @@ -1265,8 +1254,6 @@ export const MessagesTimeline = memo(function MessagesTimeline({ workGroupViewState, }), [ - onOpenPlanReview, - planReviewDocumentId, readyCitationRequest, listRef, timestampFormat, @@ -2790,17 +2777,17 @@ function ProposedPlanTimelineRow({ return (
- + {/* T3-CUSTOM(expbkt3): BEGIN — plans share the chat's markdown, citations and comments, and can go wide. */} + + + + {/* T3-CUSTOM(expbkt3): END */}
); } diff --git a/apps/web/src/components/chat/ProposedPlanCard.tsx b/apps/web/src/components/chat/ProposedPlanCard.tsx index b19f28f4a908b..b03449f184ac9 100644 --- a/apps/web/src/components/chat/ProposedPlanCard.tsx +++ b/apps/web/src/components/chat/ProposedPlanCard.tsx @@ -9,20 +9,25 @@ import { type ScopedThreadRef, } from "@t3tools/contracts"; import { - buildCollapsedProposedPlanPreviewMarkdown, buildProposedPlanMarkdownFilename, downloadPlanAsTextFile, normalizePlanMarkdownForExport, proposedPlanTitle, stripDisplayedPlanMarkdown, } from "../../proposedPlan"; -import ChatMarkdown from "../ChatMarkdown"; -import { ArrowRightIcon, EllipsisIcon, LoaderCircleIcon } from "lucide-react"; // T3-CUSTOM(expbkt3): icons for the native plan review entry point. +// T3-CUSTOM(expbkt3): BEGIN — plans render like chat (HTML parts, citations, comments) and stay mounted when folded. +import ChatMarkdown from "../../fork/planCard/PlanMarkdown"; +import { + buildCollapsedProposedPlanPreviewMarkdown, + useExpandPlanForCitation, +} from "../../fork/planCard/planCardSeams"; +import { PlanCardHeading } from "../../fork/planCard/PlanCardHeading"; +// T3-CUSTOM(expbkt3): END +import { EllipsisIcon } from "lucide-react"; import { Button } from "../ui/button"; import { Input } from "../ui/input"; import { Menu, MenuItem, MenuPopup, MenuTrigger } from "../ui/menu"; import { cn } from "~/lib/utils"; -import { Badge } from "../ui/badge"; import { Dialog, DialogDescription, @@ -44,24 +49,15 @@ export const ProposedPlanCard = memo(function ProposedPlanCard({ threadRef, cwd, workspaceRoot, - // T3-CUSTOM(expbkt3): BEGIN — native timeline plan review entry point. - onOpenPlanReview, - planReviewDocumentId, - reviewable = false, - // T3-CUSTOM(expbkt3): END }: { planMarkdown: string; environmentId: EnvironmentId; threadRef?: ScopedThreadRef | undefined; cwd: string | undefined; workspaceRoot: string | undefined; - // T3-CUSTOM(expbkt3): BEGIN — native timeline plan review entry point. - onOpenPlanReview?: ((documentId: string) => void) | undefined; - planReviewDocumentId?: string | null | undefined; - reviewable?: boolean | undefined; - // T3-CUSTOM(expbkt3): END }) { const [expanded, setExpanded] = useState(false); + useExpandPlanForCitation(setExpanded); // T3-CUSTOM(expbkt3): a quote inside a folded plan opens it. const canWriteFiles = useEnvironmentScope(environmentId, AuthFilesystemWriteScope); const [isSaveDialogOpen, setIsSaveDialogOpen] = useState(false); const [savePath, setSavePath] = useState(""); @@ -165,12 +161,8 @@ export const ProposedPlanCard = memo(function ProposedPlanCard({ return (
-
- Plan - {/* Same heading level as the message author headings in the timeline, - so a plan's own headings nest beneath it in the outline. */} -

{title}

-
+ {/* T3-CUSTOM(expbkt3): icon, title, state, size and the width control. */} + } @@ -216,47 +208,16 @@ export const ProposedPlanCard = memo(function ProposedPlanCard({
) : null}
- {canCollapse || (reviewable && onOpenPlanReview) ? ( // T3-CUSTOM(expbkt3): native timeline plan review entry point. -
- {/* T3-CUSTOM(expbkt3): BEGIN — layout host for the plan review button alongside the collapse button. */} - {canCollapse ? ( - - ) : null} - {/* T3-CUSTOM(expbkt3): END */} - {/* T3-CUSTOM(expbkt3): BEGIN — native timeline plan review entry point. */} - {reviewable && onOpenPlanReview ? ( - planReviewDocumentId ? ( - - ) : ( - - ) - ) : null} - {/* T3-CUSTOM(expbkt3): END */} + {canCollapse ? ( +
+
) : null}
diff --git a/apps/web/src/components/chat/useAssistantCitationTarget.ts b/apps/web/src/components/chat/useAssistantCitationTarget.ts index 958e2531eeee0..f89b6dfaccf1b 100644 --- a/apps/web/src/components/chat/useAssistantCitationTarget.ts +++ b/apps/web/src/components/chat/useAssistantCitationTarget.ts @@ -5,6 +5,12 @@ import type { TimelineEntry } from "../../session-logic"; import type { MessagesTimelineRow } from "./MessagesTimeline.logic"; import type { AssistantCitationRequest, AssistantCitationTarget } from "./AssistantCitationSource"; import { toastManager } from "../ui/toast"; +// T3-CUSTOM(expbkt3): a citation or comment can also quote a plan card. +import { + citationSourceRunId, + isCitableSource, + isCitationSource, +} from "../../fork/planCard/planCardSeams"; export interface CitationHistoryPage { readonly loading: boolean; @@ -84,8 +90,7 @@ export function useAssistantCitationTarget({ toastManager.add({ type: "warning", title, description }); }; const source = entries.find( - (entry) => - entry.kind === "message" && entry.message.id === navigation.target.citation.messageId, + (entry) => isCitationSource(entry, navigation.target.citation.messageId), // T3-CUSTOM(expbkt3): plans too. ); if (!source) { if (loadEarlier) { @@ -108,7 +113,8 @@ export function useAssistantCitationTarget({ ); return; } - if (source.kind !== "message" || source.message.role !== "assistant") { + if (!isCitableSource(source)) { + // T3-CUSTOM(expbkt3): an assistant response or a plan. fail( "The citation does not refer to an assistant response", "The selected text is still saved in your citation.", @@ -116,10 +122,11 @@ export function useAssistantCitationTarget({ return; } const index = rows.findIndex( - (row) => row.kind === "message" && row.message.id === navigation.target.citation.messageId, + (row) => isCitationSource(row, navigation.target.citation.messageId), // T3-CUSTOM(expbkt3): plans too. ); if (index < 0) { - if (source.message.runId) onExpandTurn(source.message.runId); + const runId = citationSourceRunId(source); // T3-CUSTOM(expbkt3): a plan's run too. + if (runId) onExpandTurn(runId); // T3-CUSTOM(expbkt3): a plan's run too. return; } if (listLoaded && listRef.current) setReady(navigation.target); @@ -180,7 +187,7 @@ export function useAssistantCitationTarget({ const positioning = request !== null && finishedKey !== request.key; const sourceRow = target && positioning - ? rows.find((row) => row.kind === "message" && row.message.id === target.citation.messageId) + ? rows.find((row) => isCitationSource(row, target.citation.messageId)) // T3-CUSTOM(expbkt3): plans too. : undefined; return { target, diff --git a/apps/web/src/fork/planCard/PlanCardHeading.tsx b/apps/web/src/fork/planCard/PlanCardHeading.tsx new file mode 100644 index 0000000000000..d7487f867c384 --- /dev/null +++ b/apps/web/src/fork/planCard/PlanCardHeading.tsx @@ -0,0 +1,96 @@ +/** + * T3-CUSTOM(expbkt3): the left side of a plan card's header. + * + * Replaces upstream's "Plan" badge and title with an icon, the title, the + * plan's state, its size, and the control that widens or narrows the card. + * It takes the header's free width, so the width control sits just before + * upstream's actions menu. + */ +import { FoldHorizontalIcon, ListChecksIcon, UnfoldHorizontalIcon } from "lucide-react"; +import { use, useMemo } from "react"; + +import { Badge } from "../../components/ui/badge"; +import { Button } from "../../components/ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../../components/ui/tooltip"; +import { PlanCardContext } from "./planCardContext"; +import { usePlanCardWidthStore } from "./planCardWidthStore"; +import { + PLAN_STATUS_LABEL, + PLAN_STATUS_VARIANT, + planCardTitle, + summarizePlan, +} from "./planSummary"; + +export function PlanCardHeading({ + planMarkdown, + title, +}: { + readonly planMarkdown: string; + readonly title: string; +}) { + const card = use(PlanCardContext); + const summary = useMemo(() => summarizePlan(planMarkdown), [planMarkdown]); + const status = card?.plan.status; + return ( +
+ +
+ {/* Same heading level as the message author headings in the timeline, + so a plan's own headings nest beneath it in the outline. */} +

+ {planCardTitle(planMarkdown, title)} +

+

+ Plan + {summary.steps > 0 ? ( + + · {summary.steps} {summary.steps === 1 ? "step" : "steps"} + + ) : null} + · {summary.readMinutes} min read +

+
+ {status ? ( + {PLAN_STATUS_LABEL[status]} + ) : null} + {card ? : null} +
+ ); +} + +function PlanCardWidthToggle({ canWiden }: { readonly canWiden: boolean }) { + const width = usePlanCardWidthStore((state) => state.width); + const toggle = usePlanCardWidthStore((state) => state.toggle); + const wide = width === "wide"; + // A narrow window has no room beside the chat column, so there is nothing to toggle. + if (!wide && !canWiden) return null; + const label = wide ? "Fit plan to the chat column" : "Widen plan"; + return ( + + + } + > + {wide ? ( + + {label} + + ); +} diff --git a/apps/web/src/fork/planCard/PlanCardRow.tsx b/apps/web/src/fork/planCard/PlanCardRow.tsx new file mode 100644 index 0000000000000..8244bac72f6a5 --- /dev/null +++ b/apps/web/src/fork/planCard/PlanCardRow.tsx @@ -0,0 +1,94 @@ +/** + * T3-CUSTOM(expbkt3): the timeline row seam for plan cards. + * + * Wraps upstream's `ProposedPlanCard` to give it the chat's timeline state + * (see `planCardContext`) and to draw it wide when the reader asks. A wide + * card leaves the chat column for the free width of the conversation area; + * the column itself is measured from the DOM, because the timeline centres it + * inside padding that the chat canvas changes as floating cards come and go. + */ +import type { ReactNode } from "react"; +import { useLayoutEffect, useMemo, useRef, useState } from "react"; + +import { useChatCanvas } from "../../components/chat/ChatCanvasContext"; +import type { ProposedPlan } from "../../types"; +import "./planCard.css"; +import { PlanCardContext, type PlanCardTimeline } from "./planCardContext"; +import { resolveWidePlanBox, type WidePlanBox } from "./planCardLayout"; +import { usePlanCardWidthStore } from "./planCardWidthStore"; + +// Floating things a wide card must stop before, inside the chat canvas. +const OBSTACLE_SELECTOR = '[data-thread-details-panel="inline"], [data-preview-mini-player]'; + +function sameBox(left: WidePlanBox | null, right: WidePlanBox | null) { + return left?.offset === right?.offset && left?.width === right?.width; +} + +export function PlanCardRow({ + plan, + itemKey, + timeline, + children, +}: { + readonly plan: ProposedPlan; + readonly itemKey: string; + readonly timeline: PlanCardTimeline; + readonly children: ReactNode; +}) { + const width = usePlanCardWidthStore((state) => state.width); + const canvas = useChatCanvas(); + const ref = useRef(null); + const [box, setBox] = useState(null); + + // The canvas context changes whenever the column, a floating card or the + // preview moves, so it re-runs the measurement along with resizes. + useLayoutEffect(() => { + const element = ref.current; + const column = element?.parentElement; + const area = element?.closest(".messages-timeline-scroll"); + if (!element || !column || !area) return; + const measure = () => { + const columnRect = column.getBoundingClientRect(); + const columnStyle = getComputedStyle(column); + const areaRect = area.getBoundingClientRect(); + const areaLeft = areaRect.left + area.clientLeft; + const obstacles = Array.from( + element.closest("[data-chat-canvas]")?.querySelectorAll(OBSTACLE_SELECTOR) ?? [], + (obstacle) => obstacle.getBoundingClientRect().left, + ); + const next = resolveWidePlanBox({ + lane: { + left: columnRect.left + Number.parseFloat(columnStyle.paddingLeft), + right: columnRect.right - Number.parseFloat(columnStyle.paddingRight), + }, + area: { left: areaLeft, right: areaLeft + area.clientWidth }, + padding: canvas?.lane.padding ?? 12, + obstacles, + }); + setBox((current) => (sameBox(current, next) ? current : next)); + }; + measure(); + const observer = new ResizeObserver(measure); + observer.observe(column); + observer.observe(area); + return () => observer.disconnect(); + }, [canvas]); + + const wide = width === "wide" && box !== null; + const value = useMemo( + () => ({ plan, itemKey, timeline, canWiden: box !== null }), + [plan, itemKey, timeline, box], + ); + return ( + +
+ {children} +
+ + ); +} diff --git a/apps/web/src/fork/planCard/PlanHtmlFrame.tsx b/apps/web/src/fork/planCard/PlanHtmlFrame.tsx new file mode 100644 index 0000000000000..e1a7cadbda57d --- /dev/null +++ b/apps/web/src/fork/planCard/PlanHtmlFrame.tsx @@ -0,0 +1,45 @@ +/** + * T3-CUSTOM(expbkt3): one HTML part of a plan, shown like an agent's HTML render. + * + * It reuses the chat's `HtmlRenderDocument`, so the page gets the same sandbox, + * the app's theme variables before first paint, live theme changes, link + * handling, and a frame that grows to the page's height. The plan's HTML has + * no stored attachment, so the bootstrapped page is served from a blob URL that + * lives as long as the frame. + */ +import { useEffect, useState } from "react"; + +import { HtmlRenderDocument } from "../../components/files/BrowserDocumentFrame"; +import { planHtmlDocument, planHtmlFrameHeight } from "./planHtml"; + +export function PlanHtmlFrame({ html, title }: { readonly html: string; readonly title: string }) { + const [src, setSrc] = useState(null); + const [contentHeight, setContentHeight] = useState(); + useEffect(() => { + const url = URL.createObjectURL( + new Blob([planHtmlDocument(html)], { type: "text/html;charset=utf-8" }), + ); + // oxlint-disable-next-line react/set-state-in-effect -- The URL exists only while this effect owns it. + setSrc(url); + return () => URL.revokeObjectURL(url); + }, [html]); + + return ( +
+ {src !== null ? ( + + ) : null} +
+ ); +} diff --git a/apps/web/src/fork/planCard/PlanMarkdown.tsx b/apps/web/src/fork/planCard/PlanMarkdown.tsx new file mode 100644 index 0000000000000..cf19daae37aeb --- /dev/null +++ b/apps/web/src/fork/planCard/PlanMarkdown.tsx @@ -0,0 +1,66 @@ +/** + * T3-CUSTOM(expbkt3): the plan card's body, a drop-in for `ChatMarkdown`. + * + * Upstream's `ProposedPlanCard` imports this under the name `ChatMarkdown`, so + * its JSX stays unchanged. Inside a timeline row it renders the plan the way + * an assistant message renders: the same Markdown features (skills, runnable + * shell commands, image preview, artifact templates), HTML parts as live + * themed pages, and a citation source so a selection offers Cite and the + * comment actions, with comment highlights painted back on the text. + */ +import { MessageId } from "@t3tools/contracts"; +import { type ComponentProps, use, useMemo } from "react"; + +import { AssistantCitationSource } from "../../components/chat/AssistantCitationSource"; +import { shouldPreserveAssistantLineBreaks } from "../../components/chat/MessagesTimeline.logic"; +import ChatMarkdown from "../../components/ChatMarkdown"; +import { ThreadCommentMarks } from "../threadComments/threadCommentsSurface"; +import { PlanCardContext } from "./planCardContext"; +import { splitPlanSegments } from "./planContent"; +import { PlanHtmlFrame } from "./PlanHtmlFrame"; + +export default function PlanMarkdown(props: ComponentProps) { + const card = use(PlanCardContext); + const segments = useMemo(() => splitPlanSegments(props.text), [props.text]); + const timeline = card?.timeline; + const body = segments.map((segment, index) => + segment.kind === "html" ? ( + + ) : ( + + ), + ); + if (!card || !timeline) return <>{body}; + + return ( + // Comment marks position against this box, and a collapsed card clips them + // together with the text they mark. +
+ + {body} + + {timeline.threadRef ? ( + + ) : null} +
+ ); +} diff --git a/apps/web/src/fork/planCard/planCard.css b/apps/web/src/fork/planCard/planCard.css new file mode 100644 index 0000000000000..731e9df88a754 --- /dev/null +++ b/apps/web/src/fork/planCard/planCard.css @@ -0,0 +1,25 @@ +/* T3-CUSTOM(expbkt3): plan card styles. Fork-owned, loaded by PlanCardRow. */ + +/* A wide plan card leaves the chat column, so its timeline row must not clip it. + The card stays inside the scrolling area, which still clips horizontally. */ +[data-timeline-root]:has([data-plan-card-width="wide"]) { + overflow-x: visible; +} + +/* Plans read as a sequence of sections: each top-level section heading after + the first gets a quiet rule above it. */ +[data-plan-card-body] .chat-markdown > :is(h1, h2):not(:first-child) { + margin-top: 1.5rem; + padding-top: 1rem; + border-top: 1px solid color-mix(in srgb, var(--border) 70%, transparent); +} + +/* Numbered steps are the plan's spine: their numbers carry the accent. */ +[data-plan-card-body] .chat-markdown > ol > li::marker { + color: var(--info-foreground); + font-weight: 600; +} + +[data-plan-card-body] .chat-markdown > ol > li + li { + margin-top: 0.35rem; +} diff --git a/apps/web/src/fork/planCard/planCard.test.ts b/apps/web/src/fork/planCard/planCard.test.ts new file mode 100644 index 0000000000000..ce3aa57a8af61 --- /dev/null +++ b/apps/web/src/fork/planCard/planCard.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { resolveWidePlanBox } from "./planCardLayout"; +import { isHtmlPlan, planHtmlTitle, splitPlanSegments } from "./planContent"; +import { planHtmlDocument, planHtmlFrameHeight } from "./planHtml"; +import { planCardTitle, summarizePlan } from "./planSummary"; + +const PAGE = + "\nRollout & risks

x

"; + +describe("splitPlanSegments", () => { + it("keeps a Markdown plan as one Markdown part", () => { + const plan = "# Ship it\n\n1. Build\n2. Test"; + expect(splitPlanSegments(plan)).toEqual([{ kind: "markdown", text: plan }]); + }); + + it("renders a whole-document plan as one page titled from the document", () => { + expect(splitPlanSegments(PAGE)).toEqual([ + { kind: "html", html: PAGE, title: "Rollout & risks" }, + ]); + expect(planCardTitle(PAGE, "Proposed plan")).toBe("Rollout & risks"); + }); + + it("treats a balanced root element as an HTML plan, but not an inline example", () => { + expect(isHtmlPlan('

Steps

')).toBe(true); + expect(isHtmlPlan("
example
\n\nThen change the parser.")).toBe(false); + }); + + it("splits html-render fences out of Markdown and leaves plain html fences as code", () => { + const plan = [ + "# Plan", + "", + "```html-render", + "

Chart

", + "```", + "", + "```html", + "

markup the plan proposes

", + "```", + ].join("\n"); + expect(splitPlanSegments(plan)).toEqual([ + { kind: "markdown", text: "# Plan\n" }, + { kind: "html", html: "

Chart

", title: "Chart" }, + { + kind: "markdown", + text: "\n```html\n

markup the plan proposes

\n```", + }, + ]); + }); + + it("lifts a raw HTML document out of a Markdown plan up to its closing tag", () => { + const plan = `# Title\n\nIntro.\n\n${PAGE}\n\nAfter.`; + const segments = splitPlanSegments(plan); + expect(segments.map((segment) => segment.kind)).toEqual(["markdown", "html", "markdown"]); + expect(segments[1]).toMatchObject({ kind: "html", html: PAGE }); + }); + + it("does not lift a document that sits inside a code fence", () => { + const plan = `Example:\n\n\`\`\`\n${PAGE}\n\`\`\``; + expect(splitPlanSegments(plan)).toEqual([{ kind: "markdown", text: plan }]); + }); + + it("falls back to the given title for an untitled page", () => { + expect(planHtmlTitle("

no title

")).toBeNull(); + expect(splitPlanSegments("```html-render\n

x

\n```", "Plan")).toEqual([ + { kind: "html", html: "

x

", title: "Plan" }, + ]); + }); +}); + +describe("planHtmlDocument", () => { + it("adds the chat theme bootstrap, then a transparent page background the page can override", () => { + const document = planHtmlDocument( + "", + ); + const theme = document.indexOf('"; + +/** The page as the frame loads it: the chat's theme bootstrap, then the plan's base style. */ +export function planHtmlDocument(html: string): string { + const document = injectHtmlRenderBootstrap(html); + const theme = document.indexOf('" }, + }); + const open = document.indexOf('", open)); + expect(document).not.toContain("--bad name"); + }); + + it("keys pages by their markup and remembers their last height", () => { + expect(planHtmlKey("

a

")).toBe(planHtmlKey("

a

")); + expect(planHtmlKey("

a

")).not.toBe(planHtmlKey("

b

")); + const key = planHtmlKey("

remembered

"); + expect(readPlanHtmlHeight(key)).toBeUndefined(); + rememberPlanHtmlHeight(key, 640); + expect(readPlanHtmlHeight(key)).toBe(640); + }); + it("fits the frame to the page within sane bounds", () => { expect(planHtmlFrameHeight(undefined)).toBe(320); expect(planHtmlFrameHeight(10)).toBe(80); @@ -126,6 +153,25 @@ describe("resolveWidePlanBox", () => { ).toEqual({ offset: -272, width: 1280 }); }); + it("starts after the minimap on the left so it keeps taking its own clicks", () => { + expect( + resolveWidePlanBox({ + lane: { left: 432, right: 1168 }, + area, + padding: 20, + leftObstacles: [64], + }), + ).toEqual({ offset: -272, width: 1280 }); + expect( + resolveWidePlanBox({ + lane: { left: 232, right: 968 }, + area: { left: 0, right: 1200 }, + padding: 20, + leftObstacles: [64], + }), + ).toEqual({ offset: -148, width: 1096 }); + }); + it("has nothing to offer when the column already fills the area", () => { expect(resolveWidePlanBox({ lane: { left: 20, right: 1580 }, area, padding: 20 })).toBeNull(); }); diff --git a/apps/web/src/fork/planCard/planCardLayout.ts b/apps/web/src/fork/planCard/planCardLayout.ts index 5664fdba13693..dd5c7a6217bc9 100644 --- a/apps/web/src/fork/planCard/planCardLayout.ts +++ b/apps/web/src/fork/planCard/planCardLayout.ts @@ -22,13 +22,23 @@ export function resolveWidePlanBox(input: { readonly area: { readonly left: number; readonly right: number }; /** The conversation's side padding. */ readonly padding: number; - /** Left edges of floating things the card must not slide under. */ + /** Left edges of floating things on the right the card must not slide under. */ readonly obstacles?: ReadonlyArray; + /** Right edges of floating things on the left, such as the timeline minimap. */ + readonly leftObstacles?: ReadonlyArray; readonly maxWidth?: number; }): WidePlanBox | null { const { lane, area, padding } = input; const laneWidth = lane.right - lane.left; - const bandLeft = Math.min(lane.left, area.left + padding); + const bandLeft = Math.min( + lane.left, + Math.max( + area.left + padding, + ...(input.leftObstacles ?? []) + .filter((right) => right <= lane.left) + .map((right) => right + padding), + ), + ); // Floating things keep the same gap from the card as the area's edge does. const blockers = (input.obstacles ?? []) .filter((left) => left >= lane.right) diff --git a/apps/web/src/fork/planCard/planHtml.ts b/apps/web/src/fork/planCard/planHtml.ts index 97807958bb2f9..e9f999fba4c97 100644 --- a/apps/web/src/fork/planCard/planHtml.ts +++ b/apps/web/src/fork/planCard/planHtml.ts @@ -3,7 +3,11 @@ * * Pure, so it is testable without the React frame that shows it. */ -import { HTML_RENDER_MIN_HEIGHT, injectHtmlRenderBootstrap } from "@t3tools/shared/htmlRender"; +import { + HTML_RENDER_MIN_HEIGHT, + injectHtmlRenderBootstrap, + type HtmlRenderTheme, +} from "@t3tools/shared/htmlRender"; // A plan page can be long. The cap only stops a page with viewport-based // heights from growing its frame without end. @@ -13,15 +17,37 @@ const PLAN_HTML_INITIAL_HEIGHT = 320; // The chat's base style paints the thread background; a plan page sits on the // card instead, so it starts transparent. The page's own CSS still wins. const PLAN_PAGE_STYLE = ""; +const THEME_STYLE_OPEN = '", open); + const scriptEnd = document.indexOf("", open); + if (close === -1 || scriptEnd === -1) return document; const at = scriptEnd + "".length; - return document.slice(0, at) + PLAN_PAGE_STYLE + document.slice(at); + return ( + document.slice(0, close) + + (theme ? themeRule(theme) : "") + + document.slice(close, at) + + PLAN_PAGE_STYLE + + document.slice(at) + ); } export function planHtmlFrameHeight(contentHeight: number | undefined): number { @@ -30,3 +56,30 @@ export function planHtmlFrameHeight(contentHeight: number | undefined): number { Math.max(HTML_RENDER_MIN_HEIGHT, Math.round(contentHeight ?? PLAN_HTML_INITIAL_HEIGHT)), ); } + +// Plan rows are virtualised, so a frame remounts as it scrolls back into view. +// Starting it at the height it last reported keeps the list from jumping. +const MEASURED_HEIGHT_LIMIT = 200; +const measuredHeights = new Map(); + +/** A short, stable key for one page's markup. */ +export function planHtmlKey(html: string): string { + let hash = 0x811c9dc5; + for (let index = 0; index < html.length; index += 1) { + hash = Math.imul(hash ^ html.charCodeAt(index), 0x01000193); + } + return `${html.length}:${(hash >>> 0).toString(36)}`; +} + +export function readPlanHtmlHeight(key: string): number | undefined { + return measuredHeights.get(key); +} + +export function rememberPlanHtmlHeight(key: string, height: number): void { + measuredHeights.delete(key); + measuredHeights.set(key, height); + if (measuredHeights.size > MEASURED_HEIGHT_LIMIT) { + const oldest = measuredHeights.keys().next().value; + if (oldest !== undefined) measuredHeights.delete(oldest); + } +} diff --git a/docs/user/plan-review.md b/docs/user/plan-review.md index 4a6e5aaf3f7fd..dc1423174c956 100644 --- a/docs/user/plan-review.md +++ b/docs/user/plan-review.md @@ -29,10 +29,10 @@ reading time. **Expand plan** shows a long plan in full. ## Opening a plan -By default a plan opens for review as soon as it is ready. Turn that off with -**Settings → BK Add-ons → Open a ready plan automatically**; the plan then waits -behind the floating **Open the plan in preview** button above the composer, -which appears once an agent has proposed a plan that has not been implemented yet. +By default a plan opens for review as soon as it is ready. Closing the review +keeps that plan closed; it stays readable as its card in the conversation. Turn +automatic opening off with **Settings → BK Add-ons → Open a ready plan +automatically**; ready plans then stay as cards. The panel opens beside the chat as a tab, so the conversation stays visible. Use the maximise control in the panel header for a full-width read. diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 67641cc018aaf..2de5c503c09e0 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -492,12 +492,12 @@ export const ClientSettingsSchema = Schema.Struct({ // old keys, so everyone, including prior beta opt-outs, resets to the new // default sidebar. legacySidebarEnabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), - // T3-CUSTOM(expbkt3): native plan review. On by default; turning it off hides - // the Preview entry points. + // T3-CUSTOM(expbkt3): native plan review. On by default; turning it off leaves + // plans as cards in the conversation. nativePlanReviewEnabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), // T3-CUSTOM(expbkt3): a reviewable plan takes over the transcript as soon as - // it lands, so a plan gate cannot be missed. Off leaves the Preview button as - // the only way in. Requires nativePlanReviewEnabled. + // it lands, so a plan gate cannot be missed. Off leaves a ready plan as its + // conversation card. Requires nativePlanReviewEnabled. planReviewAutoOpenEnabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), // T3-CUSTOM(expbkt3): agent-rendered UI surfaces in the chat transcript. While // off, a `t3_show_ui` call stays an ordinary collapsed tool row. From e8ee65f5ba2d75c5fae89d08819d32449ecdf23e Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 18:40:25 +0000 Subject: [PATCH 11/18] fix(server,web): questions left by a failed run can be answered or dismissed When a run fails while the agent waits on a question (the provider stream closes, an event fails to ingest), the run ends and its provider session is detached, but the question stays pending with a live capability. Submit then failed with "Provider session ... was not found", dismiss failed with "This question needs an answer", and Stop was gone. A request whose session is detached or whose run has ended is now handled in message mode: an answer resolves it and reaches the agent as the next message, a dismissal closes it, and an approval closes without a reply. The composer marks such a question dismissible. Multi-select answers join their choices when sent as a message. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 21 +- .../orphanedRuntimeRequest.expbkt3.test.ts | 252 ++++++++++++++++++ .../orphanedRuntimeRequest.expbkt3.ts | 81 ++++++ apps/web/src/components/ChatView.tsx | 7 +- apps/web/src/fork/orphanedUserInputs.test.ts | 47 ++++ apps/web/src/fork/orphanedUserInputs.ts | 46 ++++ docs/operations/expbkt3-customizations.md | 1 + docs/user/t3-mcp-control.md | 3 + 8 files changed, 454 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts create mode 100644 apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts create mode 100644 apps/web/src/fork/orphanedUserInputs.test.ts create mode 100644 apps/web/src/fork/orphanedUserInputs.ts diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 3d765d75a819a..527e7ab30c729 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -148,6 +148,7 @@ import { planThreadDeletion } from "./ThreadDeletion.ts"; // T3-CUSTOM(expbkt3): durable fork command decisions and projection metadata. import { forkAsyncInputChange, updatePendingAsyncUserInputIds } from "./forkAsyncInput.expbkt3.ts"; import { findForkAsyncRequest, forkAsyncAnswer } from "./forkAsyncResponse.expbkt3.ts"; +import { messageAnswerText, respondableRuntimeRequest } from "./orphanedRuntimeRequest.expbkt3.ts"; import * as Result from "effect/Result"; import { legacyActivityItem } from "./ForkLegacyHistory.expbkt3.ts"; import { isForkThreadCommand, planForkThreadCommand } from "./forkCommands.expbkt3.ts"; @@ -7364,7 +7365,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio .pipe( Effect.mapError(() => new OrchestratorProjectionError({ threadId: command.threadId })), ); - const runtimeRequest = context.request; + // T3-CUSTOM(expbkt3): a request its provider outlived is answered in message mode. + const runtimeRequest = yield* respondableRuntimeRequest({ + projectionStore, + threadId: command.threadId, + context, + }).pipe( + Effect.mapError(() => new OrchestratorProjectionError({ threadId: command.threadId })), + ); if (runtimeRequest === undefined) { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -7487,6 +7495,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio } if (runtimeRequest.responseCapability.type === "message") { if (resolvedNodeStatus === "cancelled") return; + // T3-CUSTOM(expbkt3): an approval its provider outlived closes without a reply. + if (runtimeRequest.kind !== "user_input") return; if (approvalTurnItem?.type !== "user_input_request") { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -7496,7 +7506,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio } const replies: string[] = []; for (const question of approvalTurnItem.questions) { - const answer = command.answers?.[question.id]; + // T3-CUSTOM(expbkt3): a multi-select answer lists its choices. + const answer = messageAnswerText(command.answers?.[question.id]); if (typeof answer !== "string" || answer.trim().length === 0) { if (question.required === false) continue; return yield* new OrchestratorDispatchError({ @@ -7590,9 +7601,13 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio Effect.gen(function* () { // T3-CUSTOM(expbkt3): dismissing a fork question does not interrupt its provider. if (yield* dispatchForkAsyncResponse(command, events, effects)) return; + // T3-CUSTOM(expbkt3): a question its provider outlived can be dismissed. const request = yield* projectionStore - .getRuntimeRequest(command.threadId, command.requestId) + .getRuntimeResponseContext(command.threadId, command.requestId) .pipe( + Effect.flatMap((context) => + respondableRuntimeRequest({ projectionStore, threadId: command.threadId, context }), + ), Effect.mapError(() => new OrchestratorProjectionError({ threadId: command.threadId })), ); if (request === undefined || request.status !== "pending") { diff --git a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts new file mode 100644 index 0000000000000..b16164f7f74ba --- /dev/null +++ b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts @@ -0,0 +1,252 @@ +import { assert, describe, it } from "@effect/vitest"; +import { + CommandId, + EventId, + NodeId, + ProjectId, + ProviderDriverKind, + ProviderInstanceId, + ProviderSessionId, + RuntimeRequestId, + ThreadId, + TurnItemId, + type OrchestrationV2RuntimeRequest, +} from "@t3tools/contracts"; +import * as DateTime from "effect/DateTime"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; + +import * as SqlitePersistence from "../persistence/Sqlite.ts"; +import { CodexProviderCapabilitiesV2 } from "./Adapters/CodexAdapterV2.ts"; +import * as Orchestrator from "./Orchestrator.ts"; +import { isOrphanedRuntimeRequest, messageAnswerText } from "./orphanedRuntimeRequest.expbkt3.ts"; +import * as ProjectionStore from "./ProjectionStore.ts"; +import type { ProviderAdapterV2Shape } from "./ProviderAdapter.ts"; +import * as ProviderAdapterRegistry from "./ProviderAdapterRegistry.ts"; +import * as ProviderReplayHarness from "./testkit/ProviderReplayHarness.ts"; + +const instanceId = ProviderInstanceId.make("codex"); +const modelSelection = { instanceId, model: "gpt-5.1-codex" }; +const adapter = { + instanceId, + driver: ProviderDriverKind.make("codex"), + getCapabilities: () => Effect.succeed(CodexProviderCapabilitiesV2), + planSelectionTransition: () => Effect.succeed({ type: "apply_on_next_turn" as const }), + openSession: () => Effect.die("No provider process needed for orphaned requests"), +} as ProviderAdapterV2Shape; +const layerDatabase = SqlitePersistence.layerMemory; +const layerTest = Layer.mergeAll( + layerDatabase, + ProjectionStore.layer.pipe(Layer.provide(layerDatabase)), + ProviderReplayHarness.layerWithRegistry( + { name: "orphaned-requests" }, + ProviderAdapterRegistry.layerFromAdapters([adapter]), + { databaseLayer: layerDatabase, runEffectWorker: false }, + ), +); + +const liveRequest = (status: OrchestrationV2RuntimeRequest["status"] = "pending") => + ({ + id: RuntimeRequestId.make("request"), + nodeId: NodeId.make("node"), + providerTurnId: null, + nativeRequestRef: null, + kind: "user_input", + status, + responseCapability: { type: "live", providerSessionId: ProviderSessionId.make("session") }, + createdAt: DateTime.makeUnsafe(0), + resolvedAt: null, + }) satisfies OrchestrationV2RuntimeRequest; + +describe("isOrphanedRuntimeRequest", () => { + it("is orphaned once its session is detached or its run has ended", () => { + const request = liveRequest(); + assert.isTrue(isOrphanedRuntimeRequest({ request, sessionBound: false, runStatus: "running" })); + assert.isTrue(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: "failed" })); + assert.isFalse(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: "running" })); + assert.isFalse(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: undefined })); + }); + + it("leaves answered and message-mode requests alone", () => { + assert.isFalse( + isOrphanedRuntimeRequest({ + request: liveRequest("resolved"), + sessionBound: false, + runStatus: "failed", + }), + ); + assert.isFalse( + isOrphanedRuntimeRequest({ + request: { ...liveRequest(), responseCapability: { type: "message" } }, + sessionBound: false, + runStatus: "failed", + }), + ); + }); +}); + +describe("messageAnswerText", () => { + it("lists a multi-select answer's choices and keeps a text answer", () => { + assert.equal(messageAnswerText([" Later ", "", "Now"]), "Later, Now"); + assert.isUndefined(messageAnswerText([])); + assert.equal(messageAnswerText("Later"), "Later"); + }); +}); + +it.effect("answers, dismisses and closes requests whose provider session is gone", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadId = ThreadId.make("thread:orphaned-requests"); + const now = yield* DateTime.now; + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make("create-orphaned"), + threadId, + projectId: ProjectId.make("project:orphaned-requests"), + title: "Orphaned", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + createdBy: "user", + creationSource: "web", + }); + // The session the requests name was never bound to the thread, as after a + // failed run detaches it. + const goneSession = ProviderSessionId.make("session:gone"); + let ordinal = 0; + const seed = (name: string, kind: OrchestrationV2RuntimeRequest["kind"]) => + Effect.gen(function* () { + const requestId = RuntimeRequestId.make(`request:${name}`); + const nodeId = NodeId.make(`node:${name}`); + yield* projections.apply({ + id: EventId.make(`request:${name}`), + type: "runtime-request.updated", + threadId, + occurredAt: now, + payload: { + id: requestId, + nodeId, + providerTurnId: null, + nativeRequestRef: null, + kind, + status: "pending", + responseCapability: { type: "live", providerSessionId: goneSession }, + createdAt: now, + resolvedAt: null, + }, + }); + yield* projections.apply({ + id: EventId.make(`node:${name}`), + type: "node.updated", + threadId, + occurredAt: now, + payload: { + id: nodeId, + threadId, + runId: null, + parentNodeId: null, + rootNodeId: nodeId, + kind: kind === "user_input" ? "user_input_request" : "approval_request", + status: "waiting", + countsForRun: false, + providerThreadId: null, + providerTurnId: null, + nativeItemRef: null, + runtimeRequestId: requestId, + checkpointScopeId: null, + startedAt: now, + completedAt: null, + }, + }); + if (kind === "user_input") { + yield* projections.apply({ + id: EventId.make(`item:${name}`), + type: "turn-item.updated", + threadId, + occurredAt: now, + payload: { + id: TurnItemId.make(`item:${name}`), + threadId, + runId: null, + nodeId, + providerThreadId: null, + providerTurnId: null, + nativeItemRef: null, + parentItemId: null, + ordinal: ++ordinal, + status: "waiting", + title: null, + startedAt: now, + completedAt: null, + updatedAt: now, + type: "user_input_request", + requestId, + questions: [ + { + id: "next", + header: "TEC-1657", + question: "What should happen next?", + options: [ + { label: "Later", description: "Change nothing now." }, + { label: "Now", description: "Start now." }, + ], + }, + ], + }, + }); + } + return { requestId, nodeId }; + }); + + // An answer is recorded and reaches the agent as the next message. + const answered = yield* seed("answered", "user_input"); + yield* orchestrator.dispatch({ + type: "runtime-request.respond", + commandId: CommandId.make("respond-orphaned"), + threadId, + requestId: answered.requestId, + answers: { next: "Later" }, + }); + const answeredRequest = yield* projections.getRuntimeRequest(threadId, answered.requestId); + assert.equal(answeredRequest?.status, "resolved"); + assert.equal(answeredRequest?.responseCapability.type, "message"); + const { messages } = yield* projections.getThreadRecords(threadId, ["messages"]); + assert.isTrue( + messages.some( + (message) => message.role === "user" && message.text === "What should happen next?\nLater", + ), + ); + + // A dismissal closes the question without a reply. + const dismissed = yield* seed("dismissed", "user_input"); + yield* orchestrator.dispatch({ + type: "thread.user-input.dismiss", + commandId: CommandId.make("dismiss-orphaned"), + threadId, + requestId: dismissed.requestId, + }); + const dismissedContext = yield* projections.getRuntimeResponseContext( + threadId, + dismissed.requestId, + ); + assert.equal(dismissedContext.request?.status, "resolved"); + assert.equal(dismissedContext.node?.status, "cancelled"); + + // An approval closes; no provider can act on its decision. + const approval = yield* seed("approval", "command"); + yield* orchestrator.dispatch({ + type: "runtime-request.respond", + commandId: CommandId.make("approve-orphaned"), + threadId, + requestId: approval.requestId, + decision: "accept", + }); + assert.equal( + (yield* projections.getRuntimeRequest(threadId, approval.requestId))?.status, + "resolved", + ); + }).pipe(Effect.provide(layerTest)), +); diff --git a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts new file mode 100644 index 0000000000000..b1f793a72b0d0 --- /dev/null +++ b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts @@ -0,0 +1,81 @@ +// T3-CUSTOM(expbkt3): a question or approval that outlived the provider that asked it. +/** + * A provider question (Claude's AskUserQuestion) or approval is answered live: + * the answer goes back to the callback the provider is blocked on. When the run + * fails instead (the provider stream closes, an event fails to ingest), the run + * ends and its provider session is detached, but the request stays `pending` + * with a `live` capability that nothing can reach. Answering it then fails + * ("Provider session … was not found"), dismissing it fails ("This question + * needs an answer"), and Stop is gone because the run already ended. + * + * Such a request is handled the way a message-mode question is: an answer is + * recorded and reaches the agent as the next message, a dismissal closes it, and + * an approval closes without a reply, since no provider can act on it now. + */ +import type { + OrchestrationV2Run, + OrchestrationV2RuntimeRequest, + ThreadId, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; + +import type { + ProjectionRuntimeResponseContext, + ProjectionStoreV2Shape, +} from "./ProjectionStore.ts"; + +/** Run states after which no turn of the run waits for an answer. */ +const ENDED_RUN_STATUSES: ReadonlySet = new Set([ + "completed", + "failed", + "interrupted", + "cancelled", +]); + +/** Whether a pending live request can no longer reach the provider that asked it. */ +export function isOrphanedRuntimeRequest(input: { + readonly request: OrchestrationV2RuntimeRequest; + readonly sessionBound: boolean; + readonly runStatus: OrchestrationV2Run["status"] | undefined; +}): boolean { + return ( + input.request.status === "pending" && + input.request.responseCapability.type === "live" && + (!input.sessionBound || + (input.runStatus !== undefined && ENDED_RUN_STATUSES.has(input.runStatus))) + ); +} + +/** + * The request as a response should treat it: unchanged while its provider can + * still answer, else in message mode. The run is read only when the session is + * still bound, which is the uncommon case. + */ +export const respondableRuntimeRequest = (input: { + readonly projectionStore: Pick; + readonly threadId: ThreadId; + readonly context: ProjectionRuntimeResponseContext; +}) => + Effect.gen(function* () { + const { request, node, session } = input.context; + if (request === undefined || request.status !== "pending") return request; + if (request.responseCapability.type !== "live") return request; + let runStatus: OrchestrationV2Run["status"] | undefined; + const runId = node?.runId ?? null; + if (session !== undefined && runId !== null) { + const { runs } = yield* input.projectionStore.getThreadRecords(input.threadId, ["runs"]); + runStatus = runs.find((run) => run.id === runId)?.status; + } + return isOrphanedRuntimeRequest({ request, sessionBound: session !== undefined, runStatus }) + ? { ...request, responseCapability: { type: "message" as const } } + : request; + }); + +/** One question's answer as message text; a multi-select answer lists its choices. */ +export function messageAnswerText(answer: unknown): unknown { + if (!Array.isArray(answer)) return answer; + const choices = answer.filter( + (choice): choice is string => typeof choice === "string" && choice.trim().length > 0, + ); + return choices.length === 0 ? undefined : choices.map((choice) => choice.trim()).join(", "); +} diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 63a8f5b56ef0b..97fdc82e19084 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -15,6 +15,7 @@ import { useRightPanelMaximizedPreference } from "../rightPanelLayoutPreference" import { useCurrentUserId } from "../state/identity"; import { useDurableThreadOutbox } from "../fork/useDurableThreadOutbox"; import { DurableOutboxRecovery } from "../fork/DurableOutboxRecovery"; +import { releaseEndedRunUserInputs } from "../fork/orphanedUserInputs"; import { resolveThreadCreationProfile, threadCreationForkFields, @@ -3336,7 +3337,11 @@ export default function ChatView(props: ChatViewProps) { () => serverProjection === null ? { approvals: [], userInputs: [] } - : derivePendingThreadRequests(serverProjection), + : // T3-CUSTOM(expbkt3): a question whose run ended can be dismissed. + releaseEndedRunUserInputs( + derivePendingThreadRequests(serverProjection), + serverProjection, + ), [serverProjection], ); const pendingApprovals = useMemo( diff --git a/apps/web/src/fork/orphanedUserInputs.test.ts b/apps/web/src/fork/orphanedUserInputs.test.ts new file mode 100644 index 0000000000000..4ec2c66751710 --- /dev/null +++ b/apps/web/src/fork/orphanedUserInputs.test.ts @@ -0,0 +1,47 @@ +import type { PendingThreadRequests } from "@t3tools/client-runtime/state/thread-requests"; +import type { OrchestrationV2ThreadProjection, RuntimeRequestId } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { releaseEndedRunUserInputs } from "./orphanedUserInputs"; + +type Projection = Pick; + +const pending = (requestId: string): PendingThreadRequests => ({ + approvals: [], + userInputs: [ + { + requestId: requestId as RuntimeRequestId, + createdAt: "2026-10-15T15:28:34.029Z", + questions: [], + responseCapability: "live", + dismissible: false, + }, + ], +}); + +const projection = (runStatus: string): Projection => + ({ + runs: [{ id: "run:9", status: runStatus }], + turnItems: [{ type: "user_input_request", requestId: "request", runId: "run:9" }], + }) as unknown as Projection; + +describe("releaseEndedRunUserInputs", () => { + it("makes a question whose run failed answerable by message and dismissible", () => { + const [input] = releaseEndedRunUserInputs(pending("request"), projection("failed")).userInputs; + expect(input).toMatchObject({ + responseCapability: "message", + responseMode: "message", + dismissible: true, + }); + }); + + it("leaves a question alone while its run is still going", () => { + const requests = pending("request"); + expect(releaseEndedRunUserInputs(requests, projection("running"))).toBe(requests); + }); + + it("leaves a question from another run alone", () => { + const requests = pending("other"); + expect(releaseEndedRunUserInputs(requests, projection("failed"))).toBe(requests); + }); +}); diff --git a/apps/web/src/fork/orphanedUserInputs.ts b/apps/web/src/fork/orphanedUserInputs.ts new file mode 100644 index 0000000000000..7adb2e33a47e8 --- /dev/null +++ b/apps/web/src/fork/orphanedUserInputs.ts @@ -0,0 +1,46 @@ +/** + * T3-CUSTOM(expbkt3): a question whose run already ended can be dismissed. + * + * When a run fails while the agent waits on a question, the question stays + * pending with a live capability that no provider can answer. The server + * answers such a question in message mode (`orphanedRuntimeRequest.expbkt3.ts` + * in apps/server); this lets the composer offer Dismiss for it too, instead + * of only a Submit that the dead turn never receives. + */ +import type { PendingThreadRequests } from "@t3tools/client-runtime/state/thread-requests"; +import type { OrchestrationV2ThreadProjection } from "@t3tools/contracts"; + +const ENDED_RUN_STATUSES: ReadonlySet = new Set([ + "completed", + "failed", + "interrupted", + "cancelled", +]); + +export function releaseEndedRunUserInputs( + pending: PendingThreadRequests, + projection: Pick, +): PendingThreadRequests { + if (!pending.userInputs.some((input) => input.responseCapability === "live")) return pending; + const endedRunIds = new Set( + projection.runs.filter((run) => ENDED_RUN_STATUSES.has(run.status)).map((run) => run.id), + ); + if (endedRunIds.size === 0) return pending; + let changed = false; + const userInputs = pending.userInputs.map((input) => { + if (input.responseCapability !== "live") return input; + const item = projection.turnItems.findLast( + (candidate) => + candidate.type === "user_input_request" && candidate.requestId === input.requestId, + ); + if (item?.runId == null || !endedRunIds.has(item.runId)) return input; + changed = true; + return { + ...input, + responseCapability: "message" as const, + responseMode: "message" as const, + dismissible: true, + }; + }); + return changed ? { ...pending, userInputs } : pending; +} diff --git a/docs/operations/expbkt3-customizations.md b/docs/operations/expbkt3-customizations.md index 8bdec1e99aa08..9f3b776e221a1 100644 --- a/docs/operations/expbkt3-customizations.md +++ b/docs/operations/expbkt3-customizations.md @@ -180,6 +180,7 @@ are the identity and credential-actor restart predicate in | Chat comments | `packages/contracts/src/threadComments.ts`, `apps/server/src/threadcomments/`, `packages/client-runtime/src/state/threadComments.ts`, `apps/web/src/fork/threadComments/`, `apps/web/src/state/threadComments.ts`, `docs/user/chat-comments.md` | fork RPC methods in `rpcFork.ts` + one subscription tag in `client-runtime/src/rpc/client.ts`, `threadComments` capability, `chatCommentsEnabled` setting, `comments` right-panel kind (store/tabs), `ChatView.tsx` panel branch + banner + empty-send gate, `ChatComposer.tsx` send state + placeholder, `AssistantSelectionToolbar.tsx` group + `extraActions`, `MessagesTimeline.tsx` wrap, `ProviderCommandReactor.ts` seam | | Native plan review | `apps/server/src/planreview/`, `persistence/PlanReviewDocuments.ts`, migration 1009, `packages/shared/src/planReview.ts`, `packages/contracts/src/planReview.ts`, `apps/web/src/components/planreview/`, `apps/web/src/fork/planReviewSurface.tsx` | fork RPC group + scopes + handlers, one `ws.ts` dep, right-panel store/tabs, `ChatView.tsx` branch, Experiments settings toggles, nullable comment range in `reviewCommentContext.ts` + `reviewCommentSelection.ts` + `nativeReviewDiffAdapter.ts`, plan-comment card in `MessagesTimeline.tsx` + `ThreadFeed.tsx` | | Plan cards | `apps/web/src/fork/planCard/` (header, width preference, HTML parts, citation and comment source), `apps/server/src/orchestration-v2/forkAgentPlanInstructions.expbkt3.ts` (plan-mode `html-render` hint) | `ProposedPlanCard.tsx` imports (`ChatMarkdown` alias, folded preview, citation expand) + header element, `MessagesTimeline.tsx` `PlanCardRow` wrap, `useAssistantCitationTarget.ts` source predicates, `RunExecutionService.ts` interaction-mode argument | +| Orphaned questions | `apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts`, `apps/web/src/fork/orphanedUserInputs.ts` | `Orchestrator.ts` respond and dismiss read the request through `respondableRuntimeRequest`, one approval line and one multi-select line in the message branch; `ChatView.tsx` wraps `derivePendingThreadRequests` | | Session identity | `apps/server/src/identity/SessionIdentityEnvironment.ts`, `apps/server/src/provider/claudeSessionIdentity.expbkt3.ts` | `ProviderCommandReactor.ts` execution-options seam, `ProviderService.ts` adapter-spawn seam, `identityEnvironment` on `ProviderSessionExecutionOptions`, conditional scrub in `SourceControlExecutionEnvironment.ts`, `server.ts` layer, `ClaudeAdapter.ts` system-prompt append + `UserPromptSubmit` hook seams | | Agent views in chat | `apps/server/src/agentui/`, `persistence/AgentUiRenders.ts`, migration 1022, `packages/contracts/src/agentUi.ts`, `packages/client-runtime/src/state/agentUi.ts`, `apps/web/src/fork/agentUiSurface.tsx`, `apps/web/src/state/agentUi.ts` | `t3_show_ui` in the MCP control toolkit, fork RPC group + scopes + handlers, one `ws.ts` dep, one `server.ts` layer, marked handle passthrough in `ActivityPayloadProjection.ts`, `agentUi` field + read in `session-logic.ts`, one import and early return in `MessagesTimeline.tsx`, Experiments toggle + `settingsSearch.ts` entry | | User presence | `apps/server/src/presence/` (rules, tracker, message query, `presenceHttp.expbkt3.ts`, `presenceEnvironment.expbkt3.ts`, `backgroundPolicyRedaction.expbkt3.ts`), `t3_user_presence` tool, web + mobile `useThreadPresenceScope` | `userPresence` capability, one route + layer in `server.ts`, `BK_T3_PRESENCE_URL` beside the bearer in `ClaudeAdapter.ts` + `CodexAdapter.ts`, lease redaction at both background-policy RPCs in `ws.ts`, exported `retainBackgroundScope` + one `ChatView.tsx` hook call, `retainMobileBackgroundScope` + one `ThreadRouteScreen.tsx` hook call | diff --git a/docs/user/t3-mcp-control.md b/docs/user/t3-mcp-control.md index 12e98e06aed3c..7297ee150f437 100644 --- a/docs/user/t3-mcp-control.md +++ b/docs/user/t3-mcp-control.md @@ -167,6 +167,9 @@ question pauses the agent until someone answers it. A non-blocking question, such as a Codex async question, did not stop the agent: the answer reaches it as a new message, in its running turn or as a new turn, and `t3_answer` with `dismiss: true` closes the question without an answer. +A blocking question whose run already ended, for example after a provider +error, works the same way: the answer starts a new turn, `dismiss: true` closes +it, and an approval left open by such a run closes when answered. `t3_send_message` refuses only while a blocking question or an approval is open. Pass the `requestId` that `t3_inbox` shows when a session has more than one open question, and with every `clientRequestId`, so a retry cannot answer a From 2a6abc88e04fdbad6397c0be905bfedd37129636 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 18:47:17 +0000 Subject: [PATCH 12/18] chore(web): mark the plan review lookup as a fork edit Co-Authored-By: Claude Opus 5.5 --- apps/web/src/components/ChatView.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 97fdc82e19084..98c9f9dad4a99 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -3496,6 +3496,7 @@ export default function ChatView(props: ChatViewProps) { } return findLatestProposedPlan(serverProjection, activeLatestRun?.runId ?? null); }, [activeLatestRun?.runId, latestRunSettled, serverProjection]); + // T3-CUSTOM(expbkt3): the plan review takeover reads the thread's open review document. const planReviewDocumentId = useOpenPlanReviewDocumentId( activeThreadRef?.environmentId ?? null, activeThreadRef?.threadId ?? null, From 56e13066dcd6e4c8fba30e13707c9aeca3859abe Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Thu, 15 Oct 2026 18:56:39 +0000 Subject: [PATCH 13/18] fix(server,web): review fixes for orphaned questions - A completed run no longer counts as ended: its session stays live and a background subagent can still wait on an approval. Failed, interrupted and cancelled runs do. - Read only the request's own run, and take it from the timeline item when the node has none. - MCP lists a question whose run failed or stopped as non-blocking, so t3_answer can dismiss it and t3_send_message no longer refuses. Co-Authored-By: Claude Opus 5.5 --- .../src/mcp/toolkits/agent/pending.test.ts | 35 ++++++++++++++++++- apps/server/src/mcp/toolkits/agent/pending.ts | 21 +++++++++-- .../orphanedRuntimeRequest.expbkt3.test.ts | 4 +++ .../orphanedRuntimeRequest.expbkt3.ts | 28 ++++++++++----- apps/web/src/fork/orphanedUserInputs.test.ts | 3 +- apps/web/src/fork/orphanedUserInputs.ts | 8 ++--- docs/user/t3-mcp-control.md | 6 ++-- 7 files changed, 83 insertions(+), 22 deletions(-) diff --git a/apps/server/src/mcp/toolkits/agent/pending.test.ts b/apps/server/src/mcp/toolkits/agent/pending.test.ts index 08eada90b61ac..6763db5d10261 100644 --- a/apps/server/src/mcp/toolkits/agent/pending.test.ts +++ b/apps/server/src/mcp/toolkits/agent/pending.test.ts @@ -57,13 +57,14 @@ const shell = (fields: Partial>): SessionShell => const callerWith = (records: { readonly runtimeRequests: ReadonlyArray; readonly turnItems: ReadonlyArray; + readonly runs?: ReadonlyArray; }) => { const reads: Array = []; const caller = { threads: { getThreadRecords: (_threadId: unknown, _fields: unknown, filter: unknown) => { reads.push(filter); - return Effect.succeed(records); + return Effect.succeed({ runs: [], ...records }); }, }, } as unknown as Caller; @@ -149,6 +150,38 @@ describe("loadPendingDetail", () => { }), ); + it.effect("lets a question whose run failed be dismissed and answered by message", () => + Effect.gen(function* () { + const { caller, reads } = callerWith({ + runtimeRequests: [ + runtimeRequest({ + id: BLOCKING_ID, + responseCapability: { type: "live", providerSessionId: "provider-session-1" }, + }), + ], + turnItems: [questionItem(BLOCKING_ID, "Blocking?")], + runs: [{ id: "run-1", status: "failed" }], + }); + const detail = yield* loadPendingDetail( + caller, + shell({ + pendingRuntimeRequest: { + id: BLOCKING_ID, + kind: "user_input", + createdAt: DateTime.makeUnsafe(ASKED), + }, + }), + { includePlan: false }, + ); + expect(reads).toContainEqual({ runIds: ["run-1"] }); + expect(detail.question).toMatchObject({ + requestId: BLOCKING_ID, + async: true, + textOnly: true, + }); + }), + ); + it.effect("keeps an approval's details while a non-blocking question is open", () => Effect.gen(function* () { const approvalId = diff --git a/apps/server/src/mcp/toolkits/agent/pending.ts b/apps/server/src/mcp/toolkits/agent/pending.ts index 7e70242cffdcf..8f8d9aa703a93 100644 --- a/apps/server/src/mcp/toolkits/agent/pending.ts +++ b/apps/server/src/mcp/toolkits/agent/pending.ts @@ -24,6 +24,7 @@ import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; import { findForkAsyncRequest } from "../../../orchestration-v2/forkAsyncResponse.expbkt3.ts"; +import { runEndedWithoutAnswer } from "../../../orchestration-v2/orphanedRuntimeRequest.expbkt3.ts"; import type { Caller } from "../../threadAccess.ts"; import type { QuestionView } from "./answers.ts"; import { shortRef, toIso, type SessionShell } from "./view.ts"; @@ -147,6 +148,13 @@ const approvalSubject = (caller: Caller, shell: SessionShell, runId: RunId | nul type TurnItemType = OrchestrationV2TurnItem["type"]; +/** Whether the run that asked a live question has failed or stopped, which the server answers by message. */ +const runEndedFor = (caller: Caller, shell: SessionShell, runId: RunId) => + caller.threads.getThreadRecords(shell.id, ["runs"], { runIds: [runId] }).pipe( + Effect.map(({ runs }) => runEndedWithoutAnswer(runs.find((run) => run.id === runId)?.status)), + Effect.orElseSucceed(() => false), + ); + /** * Reads the details for the waits the shell reports. `includePlan` also reads * the newest proposed plan, which only a plan review needs. @@ -196,11 +204,20 @@ export const loadPendingDetail = Effect.fn("agentToolkit.loadPendingDetail")(fun if (secretItem?.type === "secret_request") { secret = { label: secretItem.label, reason: secretItem.reason }; } else if (item?.type === "user_input_request") { + // A question whose run failed or stopped no longer blocks the agent: the + // server sends its answer as a new turn and lets it be dismissed. + const orphaned = + runtime?.responseCapability.type === "live" && + item.runId !== null && + (yield* runEndedFor(caller, shell, item.runId)); question = { requestId: request.id, questions: item.questions, - textOnly: runtime?.responseCapability.type === "message" || item.responseMode === "message", - async: false, + textOnly: + orphaned || + runtime?.responseCapability.type === "message" || + item.responseMode === "message", + async: orphaned, askedAt: toIso(request.createdAt), }; } else if (item?.type === "approval_request") { diff --git a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts index b16164f7f74ba..f75a375f1cb2c 100644 --- a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts +++ b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.test.ts @@ -65,6 +65,10 @@ describe("isOrphanedRuntimeRequest", () => { assert.isTrue(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: "failed" })); assert.isFalse(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: "running" })); assert.isFalse(isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: undefined })); + // A completed run's session stays live for its background subagents. + assert.isFalse( + isOrphanedRuntimeRequest({ request, sessionBound: true, runStatus: "completed" }), + ); }); it("leaves answered and message-mode requests alone", () => { diff --git a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts index b1f793a72b0d0..54d000be55419 100644 --- a/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts +++ b/apps/server/src/orchestration-v2/orphanedRuntimeRequest.expbkt3.ts @@ -24,14 +24,22 @@ import type { ProjectionStoreV2Shape, } from "./ProjectionStore.ts"; -/** Run states after which no turn of the run waits for an answer. */ +/** + * Run states after which no turn of the run waits for an answer. A completed + * run is left out: its session stays live, and a background subagent of it + * can still be waiting on an approval. + */ const ENDED_RUN_STATUSES: ReadonlySet = new Set([ - "completed", "failed", "interrupted", "cancelled", ]); +/** Whether a run ended without its provider waiting for answers any longer. */ +export function runEndedWithoutAnswer(status: OrchestrationV2Run["status"] | undefined): boolean { + return status !== undefined && ENDED_RUN_STATUSES.has(status); +} + /** Whether a pending live request can no longer reach the provider that asked it. */ export function isOrphanedRuntimeRequest(input: { readonly request: OrchestrationV2RuntimeRequest; @@ -41,15 +49,14 @@ export function isOrphanedRuntimeRequest(input: { return ( input.request.status === "pending" && input.request.responseCapability.type === "live" && - (!input.sessionBound || - (input.runStatus !== undefined && ENDED_RUN_STATUSES.has(input.runStatus))) + (!input.sessionBound || runEndedWithoutAnswer(input.runStatus)) ); } /** * The request as a response should treat it: unchanged while its provider can - * still answer, else in message mode. The run is read only when the session is - * still bound, which is the uncommon case. + * still answer, else in message mode. Only the request's own run is read, and + * only while its session is still bound. */ export const respondableRuntimeRequest = (input: { readonly projectionStore: Pick; @@ -57,13 +64,16 @@ export const respondableRuntimeRequest = (input: { readonly context: ProjectionRuntimeResponseContext; }) => Effect.gen(function* () { - const { request, node, session } = input.context; + const { request, node, item, session } = input.context; if (request === undefined || request.status !== "pending") return request; if (request.responseCapability.type !== "live") return request; let runStatus: OrchestrationV2Run["status"] | undefined; - const runId = node?.runId ?? null; + // The node names the run; older requests carry it only on their timeline item. + const runId = node?.runId ?? item?.runId ?? null; if (session !== undefined && runId !== null) { - const { runs } = yield* input.projectionStore.getThreadRecords(input.threadId, ["runs"]); + const { runs } = yield* input.projectionStore.getThreadRecords(input.threadId, ["runs"], { + runIds: [runId], + }); runStatus = runs.find((run) => run.id === runId)?.status; } return isOrphanedRuntimeRequest({ request, sessionBound: session !== undefined, runStatus }) diff --git a/apps/web/src/fork/orphanedUserInputs.test.ts b/apps/web/src/fork/orphanedUserInputs.test.ts index 4ec2c66751710..369d23e6dd6c8 100644 --- a/apps/web/src/fork/orphanedUserInputs.test.ts +++ b/apps/web/src/fork/orphanedUserInputs.test.ts @@ -35,9 +35,10 @@ describe("releaseEndedRunUserInputs", () => { }); }); - it("leaves a question alone while its run is still going", () => { + it("leaves a question alone while its run is still going or has completed", () => { const requests = pending("request"); expect(releaseEndedRunUserInputs(requests, projection("running"))).toBe(requests); + expect(releaseEndedRunUserInputs(requests, projection("completed"))).toBe(requests); }); it("leaves a question from another run alone", () => { diff --git a/apps/web/src/fork/orphanedUserInputs.ts b/apps/web/src/fork/orphanedUserInputs.ts index 7adb2e33a47e8..3e110c8da7d4a 100644 --- a/apps/web/src/fork/orphanedUserInputs.ts +++ b/apps/web/src/fork/orphanedUserInputs.ts @@ -10,12 +10,8 @@ import type { PendingThreadRequests } from "@t3tools/client-runtime/state/thread-requests"; import type { OrchestrationV2ThreadProjection } from "@t3tools/contracts"; -const ENDED_RUN_STATUSES: ReadonlySet = new Set([ - "completed", - "failed", - "interrupted", - "cancelled", -]); +// The server's rule (`runEndedWithoutAnswer`): a completed run's session stays live. +const ENDED_RUN_STATUSES: ReadonlySet = new Set(["failed", "interrupted", "cancelled"]); export function releaseEndedRunUserInputs( pending: PendingThreadRequests, diff --git a/docs/user/t3-mcp-control.md b/docs/user/t3-mcp-control.md index 7297ee150f437..c1a547c0eea92 100644 --- a/docs/user/t3-mcp-control.md +++ b/docs/user/t3-mcp-control.md @@ -167,9 +167,9 @@ question pauses the agent until someone answers it. A non-blocking question, such as a Codex async question, did not stop the agent: the answer reaches it as a new message, in its running turn or as a new turn, and `t3_answer` with `dismiss: true` closes the question without an answer. -A blocking question whose run already ended, for example after a provider -error, works the same way: the answer starts a new turn, `dismiss: true` closes -it, and an approval left open by such a run closes when answered. +A blocking question whose run failed or was stopped, for example after a +provider error, works the same way: the answer starts a new turn, `dismiss: +true` closes it, and an approval left open by such a run closes when answered. `t3_send_message` refuses only while a blocking question or an approval is open. Pass the `requestId` that `t3_inbox` shows when a session has more than one open question, and with every `clientRequestId`, so a retry cannot answer a From 50ed9eff8633d8ed90eaffba28bd05b04ac5fcf0 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Fri, 16 Oct 2026 13:24:00 +0000 Subject: [PATCH 14/18] fix(server): Claude accounts revive waiting threads after resets and spread load - A failed hard-limit move no longer leaves the rejection in the duplicate filter, so a new turn's rejection moves the thread (2026-10-16: a thread stayed on `sam` for 20 min while `tushar` had room). - Limit notices expire at their reset; the "all accounts are at their limit" notice is derived live from a durable revive queue (migration 1053). - A thread no account can take is parked and continued in waves as accounts reset (revive pass after every status poll); the 5-minute reset wait is now a queue entry, so a restart no longer loses it. - Auto threads move off an account over its fair share at turn boundaries, at most once an hour. - New claudeAccounts.leaveQueue RPC and a "Leave queue" item in the picker. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/auth/rpcForkScopes.ts | 1 + .../ClaudeAccountsService.test.ts | 488 ++++++++++++- .../claudeAccounts/ClaudeAccountsService.ts | 646 +++++++++++++++--- .../ClaudeAutoswitchClient.test.ts | 31 + .../claudeAccounts/ClaudeAutoswitchClient.ts | 18 + .../applyClaudeAccountProfile.ts | 3 + .../Migrations.frozenIds.expbkt3.test.ts | 2 + apps/server/src/persistence/Migrations.ts | 4 + .../Migrations/055_OrchestrationV2.test.ts | 2 + .../1053_ThreadClaudeAccountWait.test.ts | 105 +++ .../1053_ThreadClaudeAccountWait.ts | 31 + .../src/persistence/ThreadClaudeAccount.ts | 130 +++- .../reconcileV2PreviewMigration.test.ts | 2 + .../src/wsForkHandlers.claudeAccounts.test.ts | 42 +- apps/server/src/wsForkHandlers.ts | 9 + .../claudeAccounts/ClaudeAccountPicker.tsx | 19 +- apps/web/src/fork/claudeAccounts/hooks.ts | 13 +- .../web/src/fork/claudeAccounts/model.test.ts | 29 + apps/web/src/fork/claudeAccounts/model.ts | 15 + apps/web/src/fork/claudeAccounts/state.ts | 10 + docs/operations/claude-account-switching.md | 25 +- docs/operations/expbkt3-customizations.md | 3 +- packages/contracts/src/claudeAccounts.ts | 27 +- packages/contracts/src/rpcFork.ts | 9 + 24 files changed, 1550 insertions(+), 114 deletions(-) create mode 100644 apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.test.ts create mode 100644 apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.ts diff --git a/apps/server/src/auth/rpcForkScopes.ts b/apps/server/src/auth/rpcForkScopes.ts index 791e0b576b058..2519a525eabb4 100644 --- a/apps/server/src/auth/rpcForkScopes.ts +++ b/apps/server/src/auth/rpcForkScopes.ts @@ -71,6 +71,7 @@ export const FORK_RPC_REQUIRED_SCOPES = { [WS_FORK_METHODS.subscribeClaudeAccounts]: AuthOrchestrationReadScope, [WS_FORK_METHODS.subscribeThreadClaudeAccount]: AuthOrchestrationReadScope, [WS_FORK_METHODS.claudeAccountsSetThreadMode]: AuthOrchestrationOperateScope, + [WS_FORK_METHODS.claudeAccountsLeaveQueue]: AuthOrchestrationOperateScope, // T3-CUSTOM(expbkt3): Claude account access per user. Listing is a read and // replacing an allow list is operate; both handlers also require an admin. [WS_FORK_METHODS.claudeAccountsAccessList]: AuthOrchestrationReadScope, diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts index 4017e80b1ebd7..58c6cb71b8854 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts @@ -1371,9 +1371,14 @@ describe("ClaudeAccountsService hard limits", () => { "tushar resets at 00:02 UTC — this thread continues on it at 00:03 UTC.", ); assert.equal(waiting.resolvedProfile, "tushar"); + assert.equal(waiting.wait?.expectedProfile, "tushar"); assert.equal(harness.placeCalls.length, placementsBefore); + // The wait is a queue entry: the status poll after the reset continues it. + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 0); yield* TestClock.adjust("3 minutes"); + yield* service.refreshStatus(); yield* takeUntil(seen, (view) => view.notice === undefined); assert.equal(harness.dispatched.length, 1); const { command, actorUserId } = harness.dispatched[0]!; @@ -1384,9 +1389,12 @@ describe("ClaudeAccountsService hard limits", () => { const resumed = yield* service.resolveForSession(threadId); assert.equal(resumed?.profile, "tushar"); assert.equal(resumed?.reason, "sticky"); + assert.equal(harness.placeCalls.length, placementsBefore); // Claude still refuses with the old reset: no second wait, the thread moves. + const started = yield* watchThreadInto(service, threadId); yield* PubSub.publish(harness.providerEvents, providerEvent("turn.started", threadId)); + yield* Queue.take(started.seen); yield* service.handleHardLimit({ threadId, rateLimitType: "five_hour", @@ -1441,20 +1449,29 @@ describe("ClaudeAccountsService hard limits", () => { }), ); - scenario("drops the wait when the thread's account mode changes", {}, (harness, service) => - Effect.gen(function* () { - yield* service.refreshStatus(); - const threadId = ThreadId.make("thread-reset-wait-mode"); - yield* service.resolveForSession(threadId); - yield* service.handleHardLimit({ - threadId, - rateLimitType: "five_hour", - resetAt: resetInTwoMinutes, - }); - yield* service.setThreadMode({ threadId, mode: { kind: "profile", profile: "agent" } }); - yield* TestClock.adjust("10 minutes"); - assert.equal(harness.dispatched.length, 0); - }), + scenario( + "drops the wait when the thread's account mode changes, continuing on the chosen account now", + {}, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-reset-wait-mode"); + yield* service.resolveForSession(threadId); + yield* service.handleHardLimit({ + threadId, + rateLimitType: "five_hour", + resetAt: resetInTwoMinutes, + }); + const view = yield* service.setThreadMode({ + threadId, + mode: { kind: "profile", profile: "agent" }, + }); + assert.equal(view.wait, undefined); + assert.equal(harness.dispatched.length, 1); + yield* TestClock.adjust("10 minutes"); + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 1); + }), ); scenario("moves instead when the reset is more than five minutes away", {}, (harness, service) => @@ -2037,3 +2054,446 @@ describe("ClaudeAccountsService access per user: owners, probing, hard limits, r }), ); }); + +/** + * A switcher whose `--place` the test opens and closes: no room until + * `state.open` names accounts, then the open account with the fewest pending + * placements. With no room it lists when agent, then barsha, recover. + */ +function gatedPlace() { + const state: { open: ReadonlyArray } = { open: [] }; + const place = (input: PlaceInput): SwitcherPlaceResult => { + let chosen: string | undefined; + for (const name of state.open) { + if (input.avoid.includes(name)) continue; + if (chosen === undefined || (input.pending[name] ?? 0) < (input.pending[chosen] ?? 0)) { + chosen = name; + } + } + if (chosen === undefined) { + return { + status: "no_eligible_profile", + chosen: null, + dir: null, + reason: "every profile is over its limit", + recovers_at: "1970-01-01T00:30:00Z", + recoveries: [ + { profile: "agent", recovers_at: "1970-01-01T00:30:00Z" }, + { profile: "barsha", recovers_at: "1970-01-01T00:45:00Z" }, + ], + }; + } + return { status: "placed", chosen, dir: null, reason: "fewest sessions" }; + }; + return { state, place }; +} + +/** Domain events are handled in order on one fiber: a deleted, watched thread marks them done. */ +const drainDomainEvents = (harness: Harness, service: ClaudeAccountsService["Service"]) => + Effect.gen(function* () { + const drainId = ThreadId.make(`thread-drain-${(eventCounter += 1)}`); + const drain = yield* watchThreadInto(service, drainId); + yield* PubSub.publish(harness.domainEvents, { + type: "thread.deleted", + payload: { threadId: drainId, deletedAt: "1970-01-01T00:00:00.000Z" }, + } as unknown as OrchestrationEvent); + yield* Queue.take(drain.seen); + }); + +const sessionSet = ( + threadId: ThreadId, + status: "ready" | "running", + activeTurnId: string | null, +): OrchestrationEvent => + ({ + type: "thread.session-set", + payload: { threadId, session: orchestrationSession(threadId, status, activeTurnId) }, + }) as unknown as OrchestrationEvent; + +/** Marks a turn as running on the thread, as the settled projection would. */ +const markTurnRunning = ( + harness: Harness, + service: ClaudeAccountsService["Service"], + threadId: ThreadId, +) => + Effect.gen(function* () { + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "running", "turn-1")); + yield* drainDomainEvents(harness, service); + }); + +const fiveHourAt = (threadId: ThreadId, resetAt: number) => ({ + threadId, + rateLimitType: "five_hour", + resetAt, +}); + +describe("ClaudeAccountsService revive queue", () => { + const farReset = 1_790_000_000; + + { + const gate = gatedPlace(); + scenario( + "handles a new turn's rejection after a failed move, even with the same reset", + { place: gate.place }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-same-reset"); + gate.state.open = ["tushar"]; + assert.equal((yield* service.resolveForSession(threadId))?.profile, "tushar"); + gate.state.open = []; + + assert.isTrue(yield* service.handleHardLimit(fiveHourAt(threadId, farReset))); + assert.deepEqual(harness.stoppedThreads, [threadId]); + assert.equal(harness.dispatched.length, 0); + assert.equal((yield* service.getThread(threadId)).wait?.position, 1); + // Copies that arrive while it waits are consumed quietly. + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + assert.equal(harness.hardLimitCalls.length, 1); + + // The person writes: the message takes the thread out of the queue. + const { seen } = yield* watchThreadInto(service, threadId); + yield* PubSub.publish(harness.domainEvents, { + type: "thread.turn-start-requested", + payload: { threadId }, + } as unknown as OrchestrationEvent); + yield* takeUntil(seen, (view) => view.wait === undefined); + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 0); + + // That turn is refused with the same reset: it is a new rejection, and agent has room. + assert.isTrue(yield* service.handleHardLimit(fiveHourAt(threadId, farReset))); + assert.equal(harness.hardLimitCalls.length, 2); + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "agent"); + assert.equal(harness.dispatched.length, 1); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "parks Auto threads no account can take and revives them when one has room", + { place: gate.place }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const active = ThreadId.make("thread-queue-active"); + const idle = ThreadId.make("thread-queue-idle"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(active); + yield* service.resolveForSession(idle); + gate.state.open = []; + yield* markTurnRunning(harness, service, active); + + yield* service.handleHardLimit(fiveHourAt(active, farReset)); + yield* service.handleHardLimit(fiveHourAt(idle, farReset)); + assert.includeMembers(harness.stoppedThreads, [active, idle]); + assert.equal(harness.dispatched.length, 0); + + const activeView = yield* service.getThread(active); + assert.deepEqual(activeView.wait && { ...activeView.wait, queuedAt: "" }, { + position: 1, + queuedAt: "", + expectedAt: "1970-01-01T00:30:00.000Z", + expectedProfile: "agent", + wasActive: true, + }); + assert.equal( + activeView.notice, + "All Claude accounts are at their limit. This thread is number 1 in the queue and continues on agent at about 00:30 UTC.", + ); + const idleView = yield* service.getThread(idle); + assert.equal(idleView.wait?.position, 2); + assert.equal( + idleView.notice, + "All Claude accounts are at their limit. This thread is number 2 in the queue and can continue on agent at about 00:30 UTC when you send a message.", + ); + + // Still no room: both keep waiting. + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 0); + assert.equal((yield* service.getThread(active)).wait?.position, 1); + + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + const revivedActive = yield* service.getThread(active); + const revivedIdle = yield* service.getThread(idle); + assert.equal(revivedActive.wait, undefined); + assert.equal(revivedActive.notice, undefined); + assert.equal(revivedActive.resolvedProfile, "agent"); + assert.equal(revivedIdle.wait, undefined); + assert.equal(revivedIdle.resolvedProfile, "agent"); + // Only the thread that was mid-turn is continued. + assert.equal(harness.dispatched.length, 1); + const { command, actorUserId } = harness.dispatched[0]!; + assert.equal(actorUserId, owner); + assert.equal(command.type, "thread.turn.start"); + if (command.type === "thread.turn.start") { + assert.equal(command.threadId, active); + assert.equal(command.message.text, "Continue where you left off."); + } + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "spreads a wave of waiting threads over the accounts that free up", + { + place: gate.place, + status: (dir) => + switcherStatus([ + switcherProfile(dir, "tushar"), + switcherProfile(dir, "agent", { rank: 2, elected: false }), + switcherProfile(dir, "barsha", { rank: 3, elected: false }), + switcherProfile(dir, "carol", { rank: 4, elected: false }), + ]), + }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threads = [1, 2, 3].map((index) => ThreadId.make(`thread-wave-${index}`)); + gate.state.open = ["tushar"]; + for (const threadId of threads) yield* service.resolveForSession(threadId); + gate.state.open = []; + for (const threadId of threads) { + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + } + for (const [index, threadId] of threads.entries()) { + assert.equal((yield* service.getThread(threadId)).wait?.position, index + 1); + } + + gate.state.open = ["agent", "barsha", "carol"]; + yield* service.refreshStatus(); + const placed = new Set(); + for (const threadId of threads) { + const view = yield* service.getThread(threadId); + assert.equal(view.wait, undefined); + placed.add(view.resolvedProfile ?? ""); + } + assert.deepEqual([...placed].toSorted(), ["agent", "barsha", "carol"]); + assert.equal(harness.dispatched.length, 3); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "takes a thread out of the queue without moving it", + { place: gate.place }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-leave"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + assert.isDefined((yield* service.getThread(threadId)).wait); + + const left = yield* service.leaveQueue(threadId); + assert.equal(left.wait, undefined); + assert.equal(left.notice, undefined); + assert.equal(left.resolvedProfile, "tushar"); + // Leaving twice is harmless. + yield* service.leaveQueue(threadId); + + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 0); + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "tushar"); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "fails a start with the account expected to have room first, and does not queue it", + { place: gate.place }, + (_harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-spawn"); + const result = yield* service.resolveForSession(threadId).pipe(Effect.result); + assert.equal(result._tag, "Failure"); + if (result._tag === "Failure") { + assert.equal( + result.failure.detail, + "All Claude accounts are at their limit — agent is expected to have room at 00:30 UTC.", + ); + } + assert.equal((yield* service.getThread(threadId)).wait, undefined); + }), + ); + } + + it.effect("keeps the queue across a restart and continues the thread there", () => + Effect.gen(function* () { + const repository = yield* Layer.build( + Layer.mergeAll(ThreadClaudeAccountRepo.layer, MigrationsLive).pipe( + Layer.provide(SqlitePersistenceMemory), + ), + ); + const gate = gatedPlace(); + const harness = yield* makeHarness({ + place: gate.place, + threadRepository: Layer.succeedContext(repository), + }); + const threadId = ThreadId.make("thread-queue-restart"); + + yield* Effect.gen(function* () { + const service = yield* ClaudeAccountsService; + yield* service.refreshStatus(); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + assert.equal((yield* service.getThread(threadId)).wait?.position, 1); + }).pipe(Effect.provide(harness.layer)); + + // A new server over the same table. + yield* Effect.gen(function* () { + const service = yield* ClaudeAccountsService; + const view = yield* service.getThread(threadId); + assert.equal(view.wait?.position, 1); + assert.equal(view.wait?.wasActive, true); + assert.equal(harness.dispatched.length, 0); + + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + const revived = yield* service.getThread(threadId); + assert.equal(revived.wait, undefined); + assert.equal(revived.resolvedProfile, "agent"); + assert.equal(harness.dispatched.length, 1); + const { command } = harness.dispatched[0]!; + assert.equal(command.type === "thread.turn.start" ? command.threadId : null, threadId); + }).pipe(Effect.provide(harness.layer)); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + ); +}); + +describe("ClaudeAccountsService notices", () => { + scenario("drops a notice tied to a reset once that time passes", {}, (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-notice-expiry"); + yield* service.setThreadMode({ threadId, mode: { kind: "profile", profile: "agent" } }); + yield* service.resolveForSession(threadId); + // Ten minutes after the test clock's epoch start. + yield* service.handleHardLimit(fiveHourAt(threadId, 600)); + assert.equal( + (yield* service.getThread(threadId)).notice, + "agent is at its usage limit until 00:10 UTC — switch to Auto or another account.", + ); + const { seen } = yield* watchThreadInto(service, threadId); + + yield* TestClock.adjust("11 minutes"); + yield* service.refreshStatus(); + yield* takeUntil(seen, (view) => view.notice === undefined); + assert.equal((yield* service.getThread(threadId)).notice, undefined); + assert.deepEqual(harness.stoppedThreads, []); + }), + ); +}); + +describe("ClaudeAccountsService rebalancing at turn boundaries", () => { + const sharedStatus = (dir: DirFor) => ({ + ...switcherStatus([ + switcherProfile(dir, "tushar", { sessions: 5, fair_share: 2, load: 5 }), + switcherProfile(dir, "agent", { rank: 2, elected: false, fair_share: 2, load: 0 }), + switcherProfile(dir, "barsha", { rank: 3, elected: false, fair_share: 2, load: 0 }), + ]), + place_rule: "space-share-v2", + }); + + { + const gate = gatedPlace(); + scenario( + "moves an idle Auto thread off an over-loaded account once, not again within the hour", + { place: gate.place, status: sharedStatus }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-rebalance"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = ["agent"]; + yield* Ref.set(harness.sessions, [liveSession(threadId, "ready")]); + const { seen } = yield* watchThreadInto(service, threadId); + + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* takeUntil(seen, (view) => view.resolvedProfile === "agent"); + yield* drainDomainEvents(harness, service); + assert.deepEqual(harness.stoppedThreads, [threadId]); + assert.include(harness.placeCalls.at(-1)!.avoid, "tushar"); + // Nothing asked the thread to continue: its next message spawns on agent. + assert.equal(harness.dispatched.length, 0); + assert.equal((yield* service.resolveForSession(threadId))?.reason, "sticky"); + + // agent is over its share now too, but the thread moved less than an hour ago. + yield* Ref.update(harness.status, (current) => ({ + ...current, + profiles: current.profiles.map((profile) => + profile.name === "agent" ? { ...profile, sessions: 6 } : profile, + ), + })); + yield* service.refreshStatus(); + gate.state.open = ["barsha"]; + const placements = harness.placeCalls.length; + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* drainDomainEvents(harness, service); + assert.equal(harness.placeCalls.length, placements); + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "agent"); + + yield* TestClock.adjust("61 minutes"); + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* takeUntil(seen, (view) => view.resolvedProfile === "barsha"); + yield* drainDomainEvents(harness, service); + assert.deepEqual(harness.stoppedThreads, [threadId, threadId]); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "never moves a pinned thread, and leaves a thread with a turn running alone", + { place: gate.place, status: sharedStatus }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const pinned = ThreadId.make("thread-rebalance-pinned"); + const busy = ThreadId.make("thread-rebalance-busy"); + yield* service.setThreadMode({ + threadId: pinned, + mode: { kind: "profile", profile: "tushar" }, + }); + yield* service.resolveForSession(pinned); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(busy); + gate.state.open = ["agent"]; + yield* Ref.set(harness.sessions, [ + liveSession(pinned, "ready"), + liveSession(busy, "running"), + ]); + const placements = harness.placeCalls.length; + + yield* PubSub.publish(harness.domainEvents, sessionSet(pinned, "ready", null)); + yield* PubSub.publish(harness.domainEvents, sessionSet(busy, "ready", null)); + yield* drainDomainEvents(harness, service); + assert.equal(harness.placeCalls.length, placements); + assert.deepEqual(harness.stoppedThreads, []); + assert.equal((yield* service.getThread(pinned)).resolvedProfile, "tushar"); + assert.equal((yield* service.getThread(busy)).resolvedProfile, "tushar"); + }), + ); + } +}); diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts index 84be8780fdebc..7a829c6551c3a 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts @@ -19,6 +19,14 @@ * affected Auto thread. Pinned threads get a notice instead. An Auto * thread whose window resets within five minutes keeps its account: the * session stops and the thread continues there once the window resets; + * - the **revive queue**: an Auto thread that no account can take stops and + * waits in `thread_claude_account` (so a restart keeps it). After every + * status poll the waiting threads are placed again in queue order, and one + * that was mid-turn gets the continue turn; + * - **rebalancing**: when an idle Auto thread's turn ends on an account that + * carries two or more sessions above its fair share (or can no longer take + * new ones), it moves to where the switcher would place it now, at most once + * an hour per thread; * - **access per user**: an account with no users assigned is open to * everyone; once an admin assigns users, only they may use it. Placement * avoids the accounts the thread's owner may not use, a pin to one fails, @@ -51,7 +59,6 @@ import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; -import * as FiberMap from "effect/FiberMap"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -97,6 +104,10 @@ const PENDING_PLACEMENT_WINDOW_MS = 90_000; /** How long an exhausted mark lasts when Claude did not say when the window resets. */ const DEFAULT_EXHAUSTED_MS = 60 * 60 * 1000; const MAX_HANDLED_CONDITIONS = 256; +/** A thread is moved off an over-loaded account at most this often. */ +export const REBALANCE_COOLDOWN_MS = 60 * 60 * 1000; +/** Sessions above the account's fair share before a finished turn moves its thread. */ +const REBALANCE_EXCESS = 2; /** A limit that resets within this long is waited out on the same account instead of moving. */ export const RESET_WAIT_THRESHOLD_MS = 5 * 60 * 1000; /** Margin after the reset so the status poll sees the account free before the thread continues. */ @@ -121,7 +132,11 @@ const sameThreadAccount = (left: ThreadClaudeAccount, right: ThreadClaudeAccount left.resolvedProfile === right.resolvedProfile && left.resolvedAt === right.resolvedAt && left.notice === right.notice && - left.pendingRestart === right.pendingRestart; + left.pendingRestart === right.pendingRestart && + left.wait?.position === right.wait?.position && + left.wait?.expectedAt === right.wait?.expectedAt && + left.wait?.expectedProfile === right.wait?.expectedProfile && + left.wait?.wasActive === right.wait?.wasActive; /** Published after each status read: watched threads re-read their observed account. */ const LIVE_SESSIONS_CHANGE = "\u0000live-sessions"; @@ -177,6 +192,8 @@ export interface ClaudeAccountsServiceShape { /** Re-reads `--status --json` now; the poller calls this on its own schedule. */ readonly refreshStatus: () => Effect.Effect; readonly markExhausted: (profile: string, resetsAt: string | undefined) => Effect.Effect; + /** Takes the thread out of the revive queue without moving it. */ + readonly leaveQueue: (threadId: ThreadId) => Effect.Effect; /** Places the thread again, avoiding the given accounts, and persists the result. */ readonly reassign: ( threadId: ThreadId, @@ -202,6 +219,27 @@ interface ExhaustedMark { readonly resetsAt: string | undefined; } +/** A composer notice; one tied to a time disappears at that time. */ +interface Notice { + readonly text: string; + readonly untilMs?: number; +} + +/** A thread in the revive queue: the stored row plus what only memory knows. */ +interface WaitEntry { + readonly queuedAt: string; + readonly wasActive: boolean; + readonly preferProfile: string | null; + readonly notBefore: string | null; + /** The account a model-scoped limit refused; the revive avoids it. Memory only. */ + readonly avoid?: string; + /** Whether the owner may use only some accounts, for the notice's wording. */ + readonly restricted: boolean; + /** When the first account the owner may use is expected to have room. */ + readonly expectedAt?: string; + readonly expectedProfile?: string; +} + const nowIso = Effect.map(DateTime.now, DateTime.formatIso); function clampPercent(value: number): number { @@ -314,7 +352,7 @@ export const make = Effect.gen(function* () { const fiveHourHistory: FiveHourHistory = new Map(); // Per-thread state the row does not carry. const threadProfiles = new Map(); - const notices = new Map(); + const notices = new Map(); const pendingRestart = new Set(); /** Threads with a turn in flight, as seen on the provider event bus. */ const activeTurns = new Set(); @@ -326,12 +364,13 @@ export const make = Effect.gen(function* () { const movedAwaitingTurn = new Set(); const handledConditions = new Set(); /** - * Auto threads waiting out a window that resets soon, each with the timer - * that continues it. A thread leaves the set just before its continue turn - * is sent, so that turn's own start does not cancel anything. + * The revive queue, mirrored from `thread_claude_account` and read back at + * start. A thread leaves it just before its continue turn is sent, so that + * turn's own start does not cancel anything. */ - const waitingForReset = new Set(); - const resetWaits = yield* FiberMap.make(); + const waits = new Map(); + /** When each thread was last moved by a rebalance. */ + const lastRebalancedAt = new Map(); /** * Live Claude session id -> the account it runs on, from each account's * `sessions/.json` (the CLI writes one at start and never removes it). @@ -344,6 +383,21 @@ export const make = Effect.gen(function* () { const announce = (key: string) => PubSub.publish(changes, key).pipe(Effect.ignore); const announceSnapshot = announce(SNAPSHOT_CHANGE); const placementLock = yield* Semaphore.make(1); + /** One revive pass at a time; a pass started meanwhile runs after it. */ + const reviveLock = yield* Semaphore.make(1); + const layerScope = yield* Effect.scope; + + const storedWaits = yield* repository.listWaiting().pipe( + Effect.catchCause((cause) => + Effect.as( + Effect.logWarning("claude.account.queue-load-failed", { cause }), + [] as ReadonlyArray, + ), + ), + ); + for (const row of storedWaits) { + if (row.wait !== null) waits.set(row.threadId, { ...row.wait, restricted: false }); + } const readSettings: Effect.Effect = settingsService.getSettings.pipe( @@ -745,6 +799,17 @@ export const make = Effect.gen(function* () { ); }); + /** Drops notices whose time has passed and tells their threads. */ + const expireNotices = Effect.gen(function* () { + const now = yield* Clock.currentTimeMillis; + for (const [threadId, notice] of notices) { + if (notice.untilMs !== undefined && notice.untilMs <= now) { + notices.delete(threadId); + yield* announce(threadId); + } + } + }); + const refreshStatus: ClaudeAccountsServiceShape["refreshStatus"] = () => Effect.gen(function* () { const input = yield* rankInput; @@ -764,6 +829,9 @@ export const make = Effect.gen(function* () { liveSessionProfiles = yield* indexLiveSessions(outcome.value); yield* announce(LIVE_SESSIONS_CHANGE); } + yield* expireNotices; + // Accounts may have room now: continue the threads waiting for one. + yield* revivePass; }); // Poll only while the setting is on; a disabled feature costs nothing but @@ -795,8 +863,79 @@ export const make = Effect.gen(function* () { // ---------------------------------------------------------------- threads - const threadView = (row: ThreadClaudeAccountRow): ThreadClaudeAccount => { - const notice = notices.get(row.threadId); + const setNotice = (threadId: ThreadId, text: string, until?: string) => { + const untilMs = until === undefined ? Number.NaN : Date.parse(until); + notices.set(threadId, Number.isNaN(untilMs) ? { text } : { text, untilMs }); + }; + + const isDue = (entry: WaitEntry, nowMs: number) => + entry.notBefore !== null && Date.parse(entry.notBefore) <= nowMs; + + /** The revive order: waits whose reset has passed, then interrupted turns, then the oldest. */ + const queueOrder = (nowMs: number): ReadonlyArray => + [...waits].toSorted(([, left], [, right]) => { + const byDue = Number(isDue(right, nowMs)) - Number(isDue(left, nowMs)); + if (byDue !== 0) return byDue; + const byActive = Number(right.wasActive) - Number(left.wasActive); + if (byActive !== 0) return byActive; + return left.queuedAt.localeCompare(right.queuedAt); + }); + + /** The queue state a waiting thread shows, with the notice derived from it. */ + const waitView = (threadId: ThreadId, entry: WaitEntry, nowMs: number) => { + const position = queueOrder(nowMs).findIndex(([id]) => id === threadId) + 1; + const notBeforeMs = entry.notBefore === null ? Number.NaN : Date.parse(entry.notBefore); + // A short reset wait still ahead: the thread continues on its own account. + if (entry.preferProfile !== null && entry.notBefore !== null && notBeforeMs > nowMs) { + return { + value: { + position, + queuedAt: entry.queuedAt, + expectedAt: entry.notBefore, + expectedProfile: entry.preferProfile, + wasActive: entry.wasActive, + }, + notice: `${entry.preferProfile} resets at ${formatResetClock( + isoFromUnixSeconds((notBeforeMs - RESET_WAIT_BUFFER_MS) / 1000), + )} — this thread continues on it at ${formatResetClock(entry.notBefore)}.`, + }; + } + const expectedAt = entry.expectedAt ?? entry.notBefore ?? undefined; + const expectedProfile = entry.expectedProfile ?? entry.preferProfile ?? undefined; + const limitLine = entry.restricted + ? "All Claude accounts you can use are at their limit." + : "All Claude accounts are at their limit."; + const target = expectedProfile === undefined ? "" : ` on ${expectedProfile}`; + const when = + expectedAt === undefined + ? " once an account has room" + : ` at about ${formatResetClock(expectedAt)}`; + const next = entry.wasActive + ? `continues${target}${when}` + : `can continue${target}${when} when you send a message`; + return { + value: { + position, + queuedAt: entry.queuedAt, + ...(expectedAt !== undefined ? { expectedAt } : {}), + ...(expectedProfile !== undefined ? { expectedProfile } : {}), + wasActive: entry.wasActive, + }, + notice: `${limitLine} This thread is number ${position} in the queue and ${next}.`, + }; + }; + + const threadView = (row: ThreadClaudeAccountRow, nowMs: number): ThreadClaudeAccount => { + const entry = waits.get(row.threadId); + const wait = entry === undefined ? undefined : waitView(row.threadId, entry, nowMs); + const stored = notices.get(row.threadId); + // A notice tied to a time never outlives it. + const live = + stored !== undefined && (stored.untilMs === undefined || stored.untilMs > nowMs) + ? stored.text + : undefined; + if (stored !== undefined && live === undefined) notices.delete(row.threadId); + const notice = wait?.notice ?? live; return { threadId: row.threadId, mode: row.mode, @@ -804,6 +943,7 @@ export const make = Effect.gen(function* () { ...(row.resolvedAt ? { resolvedAt: row.resolvedAt } : {}), ...(notice ? { notice } : {}), ...(pendingRestart.has(row.threadId) ? { pendingRestart: true } : {}), + ...(wait ? { wait: wait.value } : {}), }; }; @@ -821,13 +961,14 @@ export const make = Effect.gen(function* () { const getThread: ClaudeAccountsServiceShape["getThread"] = (threadId) => Effect.gen(function* () { const row = yield* repository.get(threadId).pipe(Effect.mapError(internal("getThread"))); - if (row.resolvedProfile || row.mode.kind !== "auto") return threadView(row); + const nowMs = yield* Clock.currentTimeMillis; + if (row.resolvedProfile || row.mode.kind !== "auto") return threadView(row, nowMs); // A session that started before placement (on the shared link) has no // resolved account yet: show the one its live process really uses. const observed = yield* observedProfile(threadId); - if (observed === undefined) return threadView(row); + if (observed === undefined) return threadView(row, nowMs); threadProfiles.set(threadId, observed); - return threadView({ ...row, resolvedProfile: observed }); + return threadView({ ...row, resolvedProfile: observed }, nowMs); }); const persistResolved = (threadId: ThreadId, profile: string) => @@ -901,10 +1042,24 @@ export const make = Effect.gen(function* () { }); }); + interface PlaceOptions { + /** Why the placement runs, for the log. */ + readonly purpose: "spawn" | "reassign" | "revive" | "rebalance"; + /** + * Accept only a real `placed` answer: no `fallback` account above its trip + * line and no degraded active link. A revive or a rebalance onto either + * would only fail again, so it waits for a real answer instead. + */ + readonly requirePlaced?: boolean; + /** Counts the answer as a pending placement (the default); a probe passes false. */ + readonly commit?: boolean; + } + /** Asks the switcher for an account. Must run under `placementLock`. */ const placeWith = ( threadId: ThreadId, avoid: ReadonlyArray, + options: PlaceOptions, ): Effect.Effect => Effect.gen(function* () { const pending = yield* pendingCounts; @@ -931,7 +1086,14 @@ export const make = Effect.gen(function* () { threadId, kind: outcome.kind, detail: outcome.detail, + purpose: options.purpose, }); + if (options.requirePlaced) { + return yield* new ClaudeAccountPolicyError({ + threadId, + detail: "Claude account placement is unavailable right now — try again shortly.", + }); + } if (outcome.kind === "unavailable") { const fallback = yield* currentProfileFromActiveLink; if (fallback !== undefined && !avoidSet.has(fallback.profile)) return fallback; @@ -950,9 +1112,35 @@ export const make = Effect.gen(function* () { chosen: result.chosen, }); } - if (result.status === "no_eligible_profile" || chosen === null) { + if ( + result.status === "no_eligible_profile" || + chosen === null || + (options.requirePlaced === true && result.status !== "placed") + ) { + yield* Effect.logInfo("claude.account.place-none", { + threadId, + status: result.status, + recoversAt: result.recovers_at ?? null, + recoveries: result.recoveries ?? [], + reason: result.reason ?? null, + purpose: options.purpose, + }); if (mayUseNone(restriction)) return yield* noAccountPolicyError(threadId); - const recoversAt = isoOrUndefined(result.recovers_at) ?? firstRecovery(disallowed); + // The first account this owner may use that is expected to have room. + // A switcher that lists recoveries speaks for every account; an older + // one only gives the first reset, which may be an account off limits. + let first: { readonly profile: string; readonly at: string } | undefined; + for (const recovery of result.recoveries ?? []) { + const at = isoOrUndefined(recovery.recovers_at); + if (at !== undefined && !disallowed.has(recovery.profile)) { + first = { profile: recovery.profile, at }; + break; + } + } + const recoversAt = + first?.at ?? + (result.recoveries ? undefined : isoOrUndefined(result.recovers_at)) ?? + firstRecovery(disallowed); // Nothing the owner may use is waiting on a reset, and whatever else // is left is off limits: that is "no access", not "at its limit". The // access list decides this; the host list, when known, only narrows it. @@ -971,13 +1159,19 @@ export const make = Effect.gen(function* () { return yield* new ClaudeAccountPolicyError({ threadId, detail: - recoversAt === undefined - ? `${limitLine}.` - : `${limitLine} — first resets at ${formatResetClock(recoversAt)}.`, + first !== undefined + ? `${limitLine} — ${first.profile} is expected to have room at ${formatResetClock(first.at)}.` + : recoversAt === undefined + ? `${limitLine}.` + : `${limitLine} — first resets at ${formatResetClock(recoversAt)}.`, + ...(recoversAt !== undefined ? { expectedAt: recoversAt } : {}), + ...(first !== undefined ? { expectedProfile: first.profile } : {}), }); } - const atMs = yield* Clock.currentTimeMillis; - pendingPlacements.push({ profile: chosen, atMs }); + if (options.commit !== false) { + const atMs = yield* Clock.currentTimeMillis; + pendingPlacements.push({ profile: chosen, atMs }); + } return { profile: chosen, dir: result.dir ? result.dir : profileDir(chosen), @@ -986,10 +1180,14 @@ export const make = Effect.gen(function* () { } as const; }); - const placeAndPersist = (threadId: ThreadId, avoid: ReadonlyArray) => + const placeAndPersist = ( + threadId: ThreadId, + avoid: ReadonlyArray, + purpose: PlaceOptions["purpose"], + ) => Semaphore.withPermit(placementLock)( Effect.gen(function* () { - const resolution = yield* placeWith(threadId, avoid); + const resolution = yield* placeWith(threadId, avoid, { purpose }); if (resolution !== undefined) { yield* persistResolved(threadId, resolution.profile).pipe( Effect.catchCause((cause) => @@ -1074,7 +1272,11 @@ export const make = Effect.gen(function* () { reason: "sticky", } as const; } - return yield* placeAndPersist(threadId, row.resolvedProfile ? [row.resolvedProfile] : []); + return yield* placeAndPersist( + threadId, + row.resolvedProfile ? [row.resolvedProfile] : [], + "spawn", + ); }); /** @@ -1097,7 +1299,7 @@ export const make = Effect.gen(function* () { ); const reassign: ClaudeAccountsServiceShape["reassign"] = (threadId, input) => - placeAndPersist(threadId, input.avoid); + placeAndPersist(threadId, input.avoid, "reassign"); const markExhausted: ClaudeAccountsServiceShape["markExhausted"] = (profile, resetsAt) => Effect.gen(function* () { @@ -1156,7 +1358,7 @@ export const make = Effect.gen(function* () { }); /** Restarts the thread's provider session so its next turn spawns on the new account. */ - const restartForNewAccount = (threadId: ThreadId) => + const restartForNewAccount = (threadId: ThreadId, why = "mode-changed") => Effect.gen(function* () { const sessions = yield* providers.listSessions(); const live = sessions.find( @@ -1168,7 +1370,7 @@ export const make = Effect.gen(function* () { yield* Effect.logInfo("claude.account.restart-deferred", { threadId }); return; } - yield* stopSessionQuietly(threadId, "mode-changed"); + yield* stopSessionQuietly(threadId, why); }); /** @@ -1188,15 +1390,94 @@ export const make = Effect.gen(function* () { yield* announce(threadId); }); - /** Drops a pending wait-for-reset; `true` when there was one. */ - const cancelResetWait = (threadId: ThreadId) => + // ------------------------------------------------------------ revive queue + + /** Forgets the rejections already handled for a thread, so the next one is judged afresh. */ + const forgetConditions = (threadId: ThreadId) => { + const prefix = `${threadId}\u0000`; + for (const key of handledConditions) { + if (key.startsWith(prefix)) handledConditions.delete(key); + } + }; + + /** Tells every waiting thread to re-read its place in the queue. */ + const announceQueue = Effect.suspend(() => + Effect.forEach([...waits.keys()], announce, { discard: true }), + ); + + /** + * Puts the thread in the revive queue (keeping its place when it is already + * there). `false` when the row could not be written: the caller falls back. + */ + const park = ( + threadId: ThreadId, + input: Omit & { + readonly preferProfile?: string; + readonly notBefore?: string; + }, + ) => + Effect.gen(function* () { + const queuedAt = waits.get(threadId)?.queuedAt ?? (yield* nowIso); + const entry: WaitEntry = { + ...input, + queuedAt, + preferProfile: input.preferProfile ?? null, + notBefore: input.notBefore ?? null, + }; + yield* repository.setWait({ + threadId, + wait: { + queuedAt, + wasActive: entry.wasActive, + preferProfile: entry.preferProfile, + notBefore: entry.notBefore, + }, + }); + waits.set(threadId, entry); + notices.delete(threadId); + yield* Effect.logInfo("claude.account.queued", { + threadId, + wasActive: entry.wasActive, + preferProfile: entry.preferProfile, + notBefore: entry.notBefore, + expectedAt: entry.expectedAt, + expectedProfile: entry.expectedProfile, + }); + yield* announceQueue; + return true; + }).pipe( + Effect.catchCause((cause) => + Effect.as(Effect.logWarning("claude.account.queue-write-failed", { threadId, cause }), false), + ), + ); + + /** Takes the thread out of the revive queue; `true` when it was waiting. */ + const dequeue = (threadId: ThreadId, why: string) => Effect.gen(function* () { - if (!waitingForReset.delete(threadId)) return false; - yield* FiberMap.remove(resetWaits, threadId); - yield* Effect.logInfo("claude.account.reset-wait-cancelled", { threadId }); + if (!waits.delete(threadId)) return false; + forgetConditions(threadId); + yield* Effect.logInfo("claude.account.dequeued", { threadId, why }); + yield* announceQueue; + yield* repository.clearWait(threadId); return true; }); + /** `dequeue` for event handlers: a failed write is logged, never raised. */ + const dequeueQuietly = (threadId: ThreadId, why: string) => + dequeue(threadId, why).pipe( + Effect.catchCause((cause) => + Effect.as(Effect.logWarning("claude.account.queue-clear-failed", { threadId, cause }), true), + ), + ); + + const leaveQueue: ClaudeAccountsServiceShape["leaveQueue"] = (threadId) => + Effect.gen(function* () { + if (yield* dequeue(threadId, "left-queue").pipe(Effect.mapError(internal("leaveQueue")))) { + yield* announce(threadId); + } + return yield* getThread(threadId); + }); + const setThreadMode: ClaudeAccountsServiceShape["setThreadMode"] = ({ threadId, mode, @@ -1238,7 +1519,9 @@ export const make = Effect.gen(function* () { yield* repository .setMode({ threadId, mode, updatedAt }) .pipe(Effect.mapError(internal("setThreadMode"))); - yield* cancelResetWait(threadId); + const waiting = waits.get(threadId); + yield* dequeue(threadId, "mode-changed").pipe(Effect.mapError(internal("setThreadMode"))); + forgetConditions(threadId); notices.delete(threadId); movedAwaitingTurn.delete(threadId); yield* pruneExhausted; @@ -1250,7 +1533,7 @@ export const make = Effect.gen(function* () { (!mayUse(access, owner, row.resolvedProfile) || !(yield* usableAndPresent(row.resolvedProfile))); if (mode.kind === "profile" && knownProfile(mode.profile)?.auth?.state === "logged_out") { - notices.set( + setNotice( threadId, `Claude account ${mode.profile} is logged out — sign it in on the Claude Accounts page or switch this thread to Auto.`, ); @@ -1260,6 +1543,9 @@ export const make = Effect.gen(function* () { } else { pendingRestart.delete(threadId); } + // Choosing an account for a thread that stopped mid-turn to wait is + // "continue now on that account": its session already stopped. + if (waiting?.wasActive) yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); yield* announce(threadId); return yield* getThread(threadId); }); @@ -1318,6 +1604,8 @@ export const make = Effect.gen(function* () { const handleHardLimit: ClaudeAccountsServiceShape["handleHardLimit"] = (condition) => Effect.gen(function* () { + // Read first: the turn this rejection ends may finish while this runs. + const wasActive = activeTurns.has(condition.threadId); const settings = yield* readSettings; if (!settings.enabled) return false; const { threadId, rateLimitType } = condition; @@ -1352,10 +1640,18 @@ export const make = Effect.gen(function* () { yield* Effect.logInfo("claude.account.hard-limit-stale", { threadId, rateLimitType }); return true; } + // A waiting thread's session is stopped; its next turn takes it out of + // the queue first. A rejection now is a late copy from before the wait. + if (waits.has(threadId)) { + yield* Effect.logInfo("claude.account.hard-limit-queued", { threadId, rateLimitType }); + return true; + } // Repeated copies of one rejection against one account are consumed // once. Accounts often share a reset instant (5-hour windows are // hour-aligned), so the account is part of the key. Without a reset time // nothing identifies the rejection, so every copy is judged on its own. + // The key stays only while the thread runs where this left it: a thread + // that waits forgets it, so a new turn's rejection is handled again. const conditionKey = `${threadId}\u0000${profile}\u0000${rateLimitType}\u0000${resetsAt}`; if (resetsAt !== undefined && !rememberCondition(conditionKey)) { return true; @@ -1373,6 +1669,7 @@ export const make = Effect.gen(function* () { rateLimitType, resetsAt, accountWide, + wasActive, election: election.kind === "ok" ? election.value.status : election.kind, }); // Only an account-wide window takes the account out of rotation; a @@ -1380,11 +1677,12 @@ export const make = Effect.gen(function* () { if (accountWide) yield* markExhausted(profile, resetsAt); if (row.mode.kind === "profile") { - notices.set( + setNotice( threadId, accountWide ? `${profile} is at its usage limit until ${formatResetClock(resetsAt)} — switch to Auto or another account.` : `${profile} has no ${scope} allowance left until ${formatResetClock(resetsAt)} — switch the model or account.`, + resetsAt, ); yield* announce(threadId); return true; @@ -1392,64 +1690,245 @@ export const make = Effect.gen(function* () { // A window that resets within the threshold is cheaper to wait out than // to leave: the thread keeps its account and its cache, and continues - // there once the window resets. The timer lives in memory, so a restart - // in between leaves the notice and a stopped thread. + // there once the window resets. The wait is a queue entry, so it + // survives a restart; it always continues, as part of the stopped turn. const nowMs = yield* Clock.currentTimeMillis; const resetMs = resetsAt === undefined ? Number.NaN : Date.parse(resetsAt); // A reset already in the past while Claude still refuses is not worth // another wait: the thread moves instead. if (resetMs > nowMs && resetMs - nowMs <= RESET_WAIT_THRESHOLD_MS) { - const continueAtMs = resetMs + RESET_WAIT_BUFFER_MS; - const continueAt = isoFromUnixSeconds(continueAtMs / 1000); - notices.set( - threadId, - `${profile} resets at ${formatResetClock(resetsAt)} — this thread continues on it at ${formatResetClock(continueAt)}.`, - ); - waitingForReset.add(threadId); - yield* stopSessionQuietly(threadId, "hard-limit-reset-wait"); - yield* FiberMap.run( - resetWaits, - threadId, - Effect.gen(function* () { - yield* Effect.sleep(Duration.millis(continueAtMs - nowMs)); - if (!waitingForReset.delete(threadId)) return; - // The continue asks the same account again, so a fresh rejection - // carrying the same reset is real, not a copy of this one. - handledConditions.delete(conditionKey); - notices.delete(threadId); - yield* pruneExhausted; - yield* Effect.logInfo("claude.account.reset-wait-continue", { threadId, profile }); - yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); - yield* announce(threadId); - }), - ); - yield* Effect.logInfo("claude.account.reset-wait", { threadId, profile, continueAt }); - yield* announce(threadId); - return true; + const continueAt = isoFromUnixSeconds((resetMs + RESET_WAIT_BUFFER_MS) / 1000); + const parked = yield* park(threadId, { + wasActive: true, + restricted: false, + preferProfile: profile, + ...(continueAt !== undefined ? { notBefore: continueAt } : {}), + }); + if (parked) { + handledConditions.delete(conditionKey); + yield* stopSessionQuietly(threadId, "hard-limit-reset-wait"); + yield* Effect.logInfo("claude.account.reset-wait", { threadId, profile, continueAt }); + return true; + } } const moved = yield* reassign(threadId, { avoid: [profile] }).pipe(Effect.result); - if (moved._tag === "Failure") { - notices.set(threadId, moved.failure.detail); + if (moved._tag === "Success" && moved.success !== undefined) { + notices.delete(threadId); + movedAwaitingTurn.add(threadId); + yield* stopSessionQuietly(threadId, "hard-limit"); + yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); yield* announce(threadId); return true; } - if (moved.success === undefined) { - notices.set( + + // No account can take the thread now. It stops and waits in the queue; + // the revive pass continues it once an account has room. + handledConditions.delete(conditionKey); + const failure = moved._tag === "Failure" ? moved.failure : undefined; + const restriction = yield* disallowedFor(threadId).pipe( + Effect.orElseSucceed(() => UNRESTRICTED), + ); + const parked = yield* park(threadId, { + wasActive, + restricted: isRestricted(restriction), + // A model-scoped limit leaves the account in rotation for others. + ...(accountWide ? {} : { avoid: profile }), + ...(failure?.expectedAt !== undefined ? { expectedAt: failure.expectedAt } : {}), + ...(failure?.expectedProfile !== undefined + ? { expectedProfile: failure.expectedProfile } + : {}), + }); + if (!parked) { + setNotice( threadId, - `${profile} is at its usage limit until ${formatResetClock(resetsAt)} and no other account could be chosen.`, + failure?.detail ?? + `${profile} is at its usage limit until ${formatResetClock(resetsAt)} and no other account could be chosen.`, + failure?.expectedAt ?? resetsAt, ); yield* announce(threadId); return true; } - notices.delete(threadId); - movedAwaitingTurn.add(threadId); - yield* stopSessionQuietly(threadId, "hard-limit"); - yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); - yield* announce(threadId); + yield* stopSessionQuietly(threadId, "hard-limit-queued"); + yield* revivePass.pipe(Effect.forkIn(layerScope)); return true; }); + /** + * Places one waiting thread if an account has room, under the placement + * lock. Answers whether it left the queue and should get the continue turn, + * which the caller sends outside the lock. + */ + const reviveOne = (threadId: ThreadId, nowMs: number, anyRoom: boolean) => + Semaphore.withPermit(placementLock)( + Effect.gen(function* () { + const entry = waits.get(threadId); + if (entry === undefined) return undefined; + // Still waiting out a reset on its own account. + if (entry.notBefore !== null && Date.parse(entry.notBefore) > nowMs) return undefined; + const row = yield* repository.get(threadId); + if (row.mode.kind !== "auto") { + yield* dequeue(threadId, "pinned"); + return { continueTurn: false } as const; + } + const restriction = yield* disallowedFor(threadId); + const prefer = entry.preferProfile; + let profile: string | undefined; + if ( + prefer !== null && + !restriction.disallowed.has(prefer) && + (yield* usableAndPresent(prefer)) + ) { + pendingPlacements.push({ profile: prefer, atMs: nowMs }); + profile = prefer; + } else { + if (!anyRoom) return undefined; + const placed = yield* placeWith(threadId, entry.avoid ? [entry.avoid] : [], { + purpose: "revive", + requirePlaced: true, + }).pipe(Effect.result); + if (placed._tag === "Failure") { + const { expectedAt, expectedProfile } = placed.failure; + const next: WaitEntry = { + queuedAt: entry.queuedAt, + wasActive: entry.wasActive, + preferProfile: entry.preferProfile, + notBefore: entry.notBefore, + restricted: isRestricted(restriction), + ...(entry.avoid !== undefined ? { avoid: entry.avoid } : {}), + ...(expectedAt !== undefined ? { expectedAt } : {}), + ...(expectedProfile !== undefined ? { expectedProfile } : {}), + }; + waits.set(threadId, next); + if ( + next.expectedAt !== entry.expectedAt || + next.expectedProfile !== entry.expectedProfile || + next.restricted !== entry.restricted + ) { + yield* announce(threadId); + } + return undefined; + } + profile = placed.success?.profile; + } + if (profile === undefined) return undefined; + yield* persistResolved(threadId, profile); + yield* dequeue(threadId, "revived"); + // Like a move: a rejection before the next turn starts is a stale copy. + movedAwaitingTurn.add(threadId); + yield* Effect.logInfo("claude.account.revived", { + threadId, + profile, + wasActive: entry.wasActive, + }); + return { continueTurn: entry.wasActive } as const; + }), + ); + + /** + * Places the waiting threads again, in queue order. Each placement counts as + * pending for the next, so a wave of resets spreads over the accounts. A + * thread that still finds no room stays queued; the next one is tried anyway + * (owners may use different accounts). + */ + const revivePass: Effect.Effect = Semaphore.withPermit(reviveLock)( + Effect.gen(function* () { + if (waits.size === 0) return; + if (!(yield* readSettings).enabled) return; + yield* pruneExhausted; + const nowMs = yield* Clock.currentTimeMillis; + // While the last status shows no account that may take a new session, + // asking the switcher per waiting thread only spends processes. + const anyRoom = + lastStatus === undefined || + lastStatus.profiles.some( + (profile) => + profile.eligible === true && + profile.auth?.state !== "logged_out" && + !exhausted.has(profile.name), + ); + for (const [threadId] of queueOrder(nowMs)) { + const outcome = yield* reviveOne(threadId, nowMs, anyRoom).pipe( + Effect.catchCause((cause) => + Effect.as( + Effect.logWarning("claude.account.revive-failed", { threadId, cause }), + undefined, + ), + ), + ); + if (outcome === undefined) continue; + if (outcome.continueTurn) yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); + yield* announce(threadId); + } + }), + ); + + // --------------------------------------------------------------- rebalance + + /** + * At a turn boundary, moves an idle Auto thread off an account that carries + * `REBALANCE_EXCESS` or more sessions above its fair share, or that can no + * longer take new sessions, to where the switcher would place it now. The + * thread loses its prompt cache once, so this runs at most once an hour per + * thread. Never fails: on any trouble the thread stays where it is. + */ + const maybeRebalance = (threadId: ThreadId, settled?: OrchestrationSession) => + Effect.gen(function* () { + if (waits.has(threadId) || pendingRestart.has(threadId)) return; + if (movedAwaitingTurn.has(threadId)) return; + const current = threadProfiles.get(threadId); + if (current === undefined) return; + const known = knownProfile(current); + // Only a switcher that reports fair shares spreads load; with an older one this never runs. + if (known === undefined || known.fair_share === undefined) return; + const sessions = typeof known.sessions === "number" ? known.sessions : 0; + const overShare = + typeof known.fair_share === "number" && sessions - known.fair_share >= REBALANCE_EXCESS; + const notPlaceable = known.fair_share === null || known.guard_ok === false; + if (!overShare && !notPlaceable) return; + const nowMs = yield* Clock.currentTimeMillis; + const last = lastRebalancedAt.get(threadId); + if (last !== undefined && nowMs - last < REBALANCE_COOLDOWN_MS) return; + if (!(yield* readSettings).enabled) return; + const row = yield* repository.get(threadId); + if (row.mode.kind !== "auto" || row.resolvedProfile !== current) return; + const live = (yield* providers.listSessions()).some( + (session) => session.threadId === threadId && session.status !== "closed", + ); + if (!live || !(yield* threadIsIdle(threadId, settled))) return; + + const target = yield* Semaphore.withPermit(placementLock)( + Effect.gen(function* () { + // A probe: nothing is persisted or counted unless the thread moves. + const probe = yield* placeWith(threadId, [current], { + purpose: "rebalance", + requirePlaced: true, + commit: false, + }).pipe(Effect.option); + if (Option.isNone(probe) || probe.value === undefined) return undefined; + const to = probe.value.profile; + if (to === current) return undefined; + pendingPlacements.push({ profile: to, atMs: yield* Clock.currentTimeMillis }); + lastRebalancedAt.set(threadId, nowMs); + yield* persistResolved(threadId, to); + return to; + }), + ); + if (target === undefined) return; + yield* Effect.logInfo("claude.account.rebalance", { + threadId, + from: current, + to: target, + reason: overShare ? "over-fair-share" : "not-placeable", + }); + yield* restartForNewAccount(threadId, "rebalance"); + yield* announceSnapshot; + }).pipe( + Effect.catchCause((cause) => + Effect.logWarning("claude.account.rebalance-failed", { threadId, cause }), + ), + ); + // ------------------------------------------------------------------ events const overlayFromEvent = (event: ProviderRuntimeEvent) => @@ -1485,8 +1964,7 @@ export const make = Effect.gen(function* () { case "turn.started": { activeTurns.add(event.threadId); // Someone sent a message during the wait: their turn replaces the continue. - if (yield* cancelResetWait(event.threadId)) { - notices.delete(event.threadId); + if (yield* dequeueQuietly(event.threadId, "turn-started")) { yield* announce(event.threadId); } // The first turn on the new account: from here a rejection is real. @@ -1517,8 +1995,7 @@ export const make = Effect.gen(function* () { // A message accepted during a wait replaces the continue, even before // its turn reaches the provider. if (event.type === "thread.turn-start-requested") { - if (yield* cancelResetWait(event.payload.threadId)) { - notices.delete(event.payload.threadId); + if (yield* dequeueQuietly(event.payload.threadId, "message-sent")) { yield* announce(event.payload.threadId); } return; @@ -1533,19 +2010,21 @@ export const make = Effect.gen(function* () { } else if (session.status !== "starting") { activeTurns.delete(event.payload.threadId); } - if ( - pendingRestart.has(event.payload.threadId) && + const settled = session.activeTurnId === null && session.status !== "running" && - session.status !== "starting" - ) { + session.status !== "starting"; + if (settled && pendingRestart.has(event.payload.threadId)) { yield* completePendingRestart(event.payload.threadId, session); + } else if (settled) { + yield* maybeRebalance(event.payload.threadId, session); } return; } if (event.type !== "thread.deleted") return; const threadId = event.payload.threadId; - yield* cancelResetWait(threadId); + yield* dequeueQuietly(threadId, "thread-deleted"); + lastRebalancedAt.delete(threadId); threadProfiles.delete(threadId); notices.delete(threadId); pendingRestart.delete(threadId); @@ -1600,7 +2079,7 @@ export const make = Effect.gen(function* () { if (profile === null) continue; const owner = yield* threadOwner(threadId); if (mayUse(access, owner, profile)) continue; - if (row.mode.kind === "profile") notices.set(threadId, noAccessDetail(profile)); + if (row.mode.kind === "profile") setNotice(threadId, noAccessDetail(profile)); yield* Effect.logInfo("claude.account.access-revoked", { threadId, profile }); yield* restartForNewAccount(threadId); yield* announce(threadId); @@ -1696,6 +2175,7 @@ export const make = Effect.gen(function* () { resolveForSession, refreshStatus, markExhausted, + leaveQueue, reassign, handleHardLimit, }); diff --git a/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.test.ts b/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.test.ts index 478eaeae969cc..5c93723631d89 100644 --- a/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.test.ts +++ b/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.test.ts @@ -149,6 +149,37 @@ describe("ClaudeAutoswitchClient", () => { }), ); + it.effect("decodes the space-share-v2 fields and keeps older answers without them", () => + Effect.gen(function* () { + const answers = [ + '{"status":"no_eligible_profile","chosen":null,"recovers_at":"2026-10-16T12:30:00Z","recoveries":[{"profile":"tushar","recovers_at":"2026-10-16T12:30:00Z"},{"profile":"agent","recovers_at":"2026-10-16T12:31:00Z"}],"snapshot":{"place_rule":"space-share-v2","profiles":[{"name":"sam","sessions":11,"fair_share":6,"load":12},{"name":"tushar","sessions":3,"fair_share":null,"load":3}]}}', + '{"status":"no_eligible_profile","chosen":null,"recovers_at":null}', + ]; + const { layer } = harness(() => output({ stdout: answers.shift() ?? "" })); + const [current, older] = yield* Effect.gen(function* () { + const client = yield* ClaudeAutoswitchClient; + const first = yield* client.place({ pending: {}, avoid: [] }); + const second = yield* client.place({ pending: {}, avoid: [] }); + return [first, second] as const; + }).pipe(Effect.provide(layer)); + + assert.equal(current.kind, "ok"); + if (current.kind === "ok") { + assert.deepEqual(current.value.recoveries, [ + { profile: "tushar", recovers_at: "2026-10-16T12:30:00Z" }, + { profile: "agent", recovers_at: "2026-10-16T12:31:00Z" }, + ]); + const profiles = current.value.snapshot?.profiles ?? []; + assert.equal(current.value.snapshot?.place_rule, "space-share-v2"); + assert.equal(profiles[0]?.fair_share, 6); + assert.equal(profiles[0]?.load, 12); + assert.equal(profiles[1]?.fair_share, null); + } + assert.equal(older.kind, "ok"); + if (older.kind === "ok") assert.equal(older.value.recoveries, undefined); + }), + ); + it.effect( "passes the limit type and profile to --hard-limit and honours the configured path", () => diff --git a/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.ts b/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.ts index 6413f158ee961..496be8d613259 100644 --- a/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.ts +++ b/apps/server/src/claudeAccounts/ClaudeAutoswitchClient.ts @@ -68,6 +68,10 @@ export const SwitcherProfile = Schema.Struct({ place_rank: Schema.optionalKey(NullableNumber), place_score: Schema.optionalKey(NullableNumber), guard_ok: Schema.optionalKey(Schema.NullOr(Schema.Boolean)), + // `space-share-v2`: the share of live sessions this account should carry + // (null when it is not placeable now) and its live plus pending sessions. + fair_share: Schema.optionalKey(NullableNumber), + load: Schema.optionalKey(NullableNumber), }); export type SwitcherProfile = typeof SwitcherProfile.Type; @@ -91,6 +95,20 @@ export const SwitcherPlaceResult = Schema.Struct({ dir: Schema.optionalKey(NullableString), reason: Schema.optionalKey(NullableString), recovers_at: Schema.optionalKey(NullableString), + /** + * On `no_eligible_profile`, when each account is expected to have room, + * earliest first (`recovers_at` is the first). Absent from older switchers. + */ + recoveries: Schema.optionalKey( + Schema.NullOr( + Schema.Array( + Schema.Struct({ + profile: Schema.String, + recovers_at: Schema.String, + }), + ), + ), + ), snapshot: Schema.optionalKey(Schema.NullOr(SwitcherStatus)), }); export type SwitcherPlaceResult = typeof SwitcherPlaceResult.Type; diff --git a/apps/server/src/claudeAccounts/applyClaudeAccountProfile.ts b/apps/server/src/claudeAccounts/applyClaudeAccountProfile.ts index 1a19e2e72896c..69701b120814c 100644 --- a/apps/server/src/claudeAccounts/applyClaudeAccountProfile.ts +++ b/apps/server/src/claudeAccounts/applyClaudeAccountProfile.ts @@ -23,6 +23,9 @@ export class ClaudeAccountPolicyError extends Schema.TaggedError [id, name] as const); diff --git a/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts b/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts index 6cc9ebd641a6d..8d22eeb1b0c80 100644 --- a/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts +++ b/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts @@ -44,6 +44,7 @@ layer("055_OrchestrationV2", (it) => { [1050, "ThreadSnapshotWindowIndexes"], [1051, "ScheduledTaskCustomGroups"], // T3-CUSTOM(expbkt3): scheduled task custom groups. [1052, "ScheduledTaskForkOptions"], // T3-CUSTOM(expbkt3): scheduled task options. + [1053, "ThreadClaudeAccountWait"], // T3-CUSTOM(expbkt3): Claude account revive queue. ]); assert.deepStrictEqual(yield* runMigrations(), []); @@ -80,6 +81,7 @@ layer("055_OrchestrationV2", (it) => { { migration_id: 1050, name: "ThreadSnapshotWindowIndexes" }, { migration_id: 1051, name: "ScheduledTaskCustomGroups" }, // T3-CUSTOM(expbkt3) { migration_id: 1052, name: "ScheduledTaskForkOptions" }, // T3-CUSTOM(expbkt3) + { migration_id: 1053, name: "ThreadClaudeAccountWait" }, // T3-CUSTOM(expbkt3) ]); // T3-CUSTOM(expbkt3): verify callback destinations and durable deliveries after the full upgrade. diff --git a/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.test.ts b/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.test.ts new file mode 100644 index 0000000000000..d5f23ab9256b3 --- /dev/null +++ b/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.test.ts @@ -0,0 +1,105 @@ +// T3-CUSTOM(expbkt3): the Claude account revive queue columns and the +// repository methods that write and read them. +import { assert, it } from "@effect/vitest"; +import { ThreadId } from "@t3tools/contracts"; +import * as NodeSqliteClient from "@t3tools/shared/nodeSqliteClient"; +import * as Effect from "effect/Effect"; +import * as SqlClient from "effect/sql/SqlClient"; + +import { runMigrations } from "../Migrations.ts"; +import * as ThreadClaudeAccountRepo from "../ThreadClaudeAccount.ts"; + +const columnNames = Effect.flatMap( + SqlClient.SqlClient, + (sql) => sql<{ readonly name: string }>`PRAGMA table_info(thread_claude_account)`, +).pipe(Effect.map((columns) => columns.map((column) => column.name))); + +it.layer(NodeSqliteClient.layer({ filename: ":memory:" }))("1053_ThreadClaudeAccountWait", (it) => { + it.effect("adds the wait columns once, keeping existing rows as not waiting", () => + Effect.gen(function* () { + yield* runMigrations({ toMigrationInclusive: 1052 }); + const sql = yield* SqlClient.SqlClient; + yield* sql` + INSERT INTO thread_claude_account (thread_id, mode_json, resolved_profile, resolved_at, updated_at) + VALUES ('thread-old', '{"kind":"auto"}', 'sam', '2026-10-16T12:00:00.000Z', '2026-10-16T12:00:00.000Z') + `; + yield* runMigrations({ toMigrationInclusive: 1053 }); + yield* runMigrations({ toMigrationInclusive: 1053 }); + const names = yield* columnNames; + for (const expected of [ + "wait_queued_at", + "wait_was_active", + "wait_prefer_profile", + "wait_not_before", + ]) { + assert.strictEqual(names.filter((name) => name === expected).length, 1); + } + + const repository = yield* ThreadClaudeAccountRepo.make; + const old = yield* repository.get(ThreadId.make("thread-old")); + assert.strictEqual(old.resolvedProfile, "sam"); + assert.strictEqual(old.wait, null); + assert.deepStrictEqual(yield* repository.listWaiting(), []); + }), + ); + + it.effect("writes, lists and clears a wait without touching the mode or account", () => + Effect.gen(function* () { + yield* runMigrations(); + const repository = yield* ThreadClaudeAccountRepo.make; + const queued = ThreadId.make("thread-queued"); + const later = ThreadId.make("thread-queued-later"); + const draft = ThreadId.make("thread-queued-draft"); + yield* repository.setResolved({ + threadId: queued, + profile: "sam", + resolvedAt: "2026-10-16T12:19:02.000Z", + }); + yield* repository.setWait({ + threadId: later, + wait: { + queuedAt: "2026-10-16T12:20:00.000Z", + wasActive: false, + preferProfile: null, + notBefore: null, + }, + }); + yield* repository.setWait({ + threadId: queued, + wait: { + queuedAt: "2026-10-16T12:19:02.000Z", + wasActive: true, + preferProfile: "sam", + notBefore: "2026-10-16T12:21:00.000Z", + }, + }); + + const waiting = yield* repository.listWaiting(); + assert.deepStrictEqual( + waiting.map((row) => row.threadId), + [queued, later], + ); + assert.deepStrictEqual(waiting[0]!.wait, { + queuedAt: "2026-10-16T12:19:02.000Z", + wasActive: true, + preferProfile: "sam", + notBefore: "2026-10-16T12:21:00.000Z", + }); + assert.strictEqual(waiting[0]!.resolvedProfile, "sam"); + assert.strictEqual(waiting[1]!.wait?.wasActive, false); + + yield* repository.clearWait(queued); + // Clearing a thread with no row is a no-op. + yield* repository.clearWait(draft); + const cleared = yield* repository.get(queued); + assert.strictEqual(cleared.wait, null); + assert.strictEqual(cleared.resolvedProfile, "sam"); + assert.deepStrictEqual(cleared.mode, { kind: "auto" }); + assert.deepStrictEqual( + (yield* repository.listWaiting()).map((row) => row.threadId), + [later], + ); + assert.strictEqual((yield* repository.get(draft)).wait, null); + }), + ); +}); diff --git a/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.ts b/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.ts new file mode 100644 index 0000000000000..219ed4c2fa028 --- /dev/null +++ b/apps/server/src/persistence/Migrations/1053_ThreadClaudeAccountWait.ts @@ -0,0 +1,31 @@ +// T3-CUSTOM(expbkt3): the durable revive queue for Claude account placement. +// +// A thread that no account can take waits in `thread_claude_account` until +// the status poll finds room. All four columns are null for a thread that is +// not waiting: +// +// - `wait_queued_at`: when it joined the queue (oldest goes first); +// - `wait_was_active`: 1 when a turn was in flight, so the revive sends the +// continue turn; 0 when it wakes only once someone writes; +// - `wait_prefer_profile`: the account a short reset wait continues on; +// - `wait_not_before`: the time that wait ends. +import * as Effect from "effect/Effect"; +import * as SqlClient from "effect/sql/SqlClient"; + +export default Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + const columns = yield* sql<{ readonly name: string }>`PRAGMA table_info(thread_claude_account)`; + const present = new Set(columns.map((column) => column.name)); + if (!present.has("wait_queued_at")) { + yield* sql`ALTER TABLE thread_claude_account ADD COLUMN wait_queued_at TEXT`; + } + if (!present.has("wait_was_active")) { + yield* sql`ALTER TABLE thread_claude_account ADD COLUMN wait_was_active INTEGER`; + } + if (!present.has("wait_prefer_profile")) { + yield* sql`ALTER TABLE thread_claude_account ADD COLUMN wait_prefer_profile TEXT`; + } + if (!present.has("wait_not_before")) { + yield* sql`ALTER TABLE thread_claude_account ADD COLUMN wait_not_before TEXT`; + } +}); diff --git a/apps/server/src/persistence/ThreadClaudeAccount.ts b/apps/server/src/persistence/ThreadClaudeAccount.ts index f9a0f99a4b88b..e0859fc87e97d 100644 --- a/apps/server/src/persistence/ThreadClaudeAccount.ts +++ b/apps/server/src/persistence/ThreadClaudeAccount.ts @@ -1,9 +1,10 @@ /** * T3-CUSTOM(expbkt3): persistence for the Claude account a thread runs on. * - * One row per thread, rewritten whole: the mode the user chose (`auto` or a - * pinned profile) plus the account the thread last resolved to. An absent row - * means `auto` with nothing resolved yet. + * One row per thread: the mode the user chose (`auto` or a pinned profile), + * the account the thread last resolved to, and its place in the revive queue + * when no account could take it. An absent row means `auto` with nothing + * resolved and no wait. */ import { CLAUDE_ACCOUNT_MODE_AUTO, ClaudeAccountMode, ThreadId } from "@t3tools/contracts"; import * as Context from "effect/Context"; @@ -22,12 +23,23 @@ import { export type ThreadClaudeAccountRepositoryError = ProjectionRepositoryError; +/** A thread waiting in the revive queue (migration 1053). */ +export interface ThreadClaudeAccountWait { + readonly queuedAt: string; + /** A turn was in flight when it stopped, so the revive sends the continue turn. */ + readonly wasActive: boolean; + /** A short reset wait: continue on this account once `notBefore` passes. */ + readonly preferProfile: string | null; + readonly notBefore: string | null; +} + export interface ThreadClaudeAccountRow { readonly threadId: ThreadId; readonly mode: ClaudeAccountMode; readonly resolvedProfile: string | null; readonly resolvedAt: string | null; readonly updatedAt: string; + readonly wait: ThreadClaudeAccountWait | null; } const DbRow = Schema.Struct({ @@ -36,6 +48,27 @@ const DbRow = Schema.Struct({ resolvedProfile: Schema.NullOr(Schema.String), resolvedAt: Schema.NullOr(Schema.String), updatedAt: Schema.String, + waitQueuedAt: Schema.NullOr(Schema.String), + waitWasActive: Schema.NullOr(Schema.Number), + waitPreferProfile: Schema.NullOr(Schema.String), + waitNotBefore: Schema.NullOr(Schema.String), +}); + +const fromDbRow = (row: typeof DbRow.Type): ThreadClaudeAccountRow => ({ + threadId: row.threadId, + mode: row.mode, + resolvedProfile: row.resolvedProfile, + resolvedAt: row.resolvedAt, + updatedAt: row.updatedAt, + wait: + row.waitQueuedAt === null + ? null + : { + queuedAt: row.waitQueuedAt, + wasActive: row.waitWasActive === 1, + preferProfile: row.waitPreferProfile, + notBefore: row.waitNotBefore, + }, }); export class ThreadClaudeAccountRepository extends Context.Service< @@ -57,6 +90,20 @@ export class ThreadClaudeAccountRepository extends Context.Service< readonly profile: string; readonly resolvedAt: string; }) => Effect.Effect; + /** Puts the thread in the revive queue (or replaces its entry); mode and account are kept. */ + readonly setWait: (input: { + readonly threadId: ThreadId; + readonly wait: ThreadClaudeAccountWait; + }) => Effect.Effect; + /** Takes the thread out of the revive queue; a missing row is a no-op. */ + readonly clearWait: ( + threadId: ThreadId, + ) => Effect.Effect; + /** Every thread in the revive queue, oldest first. */ + readonly listWaiting: () => Effect.Effect< + ReadonlyArray, + ThreadClaudeAccountRepositoryError + >; /** Every thread whose last session ran on the given account. */ readonly listByProfile: ( profile: string, @@ -81,6 +128,7 @@ export const defaultRow = (threadId: ThreadId): ThreadClaudeAccountRow => ({ resolvedProfile: null, resolvedAt: null, updatedAt: "1970-01-01T00:00:00.000Z", + wait: null, }); export const make = Effect.gen(function* () { @@ -91,7 +139,11 @@ export const make = Effect.gen(function* () { mode_json AS "mode", resolved_profile AS "resolvedProfile", resolved_at AS "resolvedAt", - updated_at AS "updatedAt" + updated_at AS "updatedAt", + wait_queued_at AS "waitQueuedAt", + wait_was_active AS "waitWasActive", + wait_prefer_profile AS "waitPreferProfile", + wait_not_before AS "waitNotBefore" `; const getRow = SqlSchema.findOneOption({ @@ -112,6 +164,16 @@ export const make = Effect.gen(function* () { `, }); + const listWaitingRows = SqlSchema.findAll({ + Request: Schema.Void, + Result: DbRow, + execute: () => sql` + SELECT ${columns} FROM thread_claude_account + WHERE wait_queued_at IS NOT NULL + ORDER BY wait_queued_at ASC + `, + }); + const setModeRow = SqlSchema.void({ Request: Schema.Struct({ threadId: ThreadId, @@ -141,6 +203,44 @@ export const make = Effect.gen(function* () { `, }); + const setWaitRow = SqlSchema.void({ + Request: Schema.Struct({ + threadId: ThreadId, + queuedAt: Schema.String, + // SQLite has no boolean; node:sqlite refuses one as a parameter. + wasActive: Schema.Number, + preferProfile: Schema.NullOr(Schema.String), + notBefore: Schema.NullOr(Schema.String), + }), + execute: ({ threadId, queuedAt, wasActive, preferProfile, notBefore }) => sql` + INSERT INTO thread_claude_account ( + thread_id, mode_json, resolved_profile, resolved_at, updated_at, + wait_queued_at, wait_was_active, wait_prefer_profile, wait_not_before + ) + VALUES ( + ${threadId}, ${JSON.stringify(CLAUDE_ACCOUNT_MODE_AUTO)}, NULL, NULL, ${queuedAt}, + ${queuedAt}, ${wasActive}, ${preferProfile}, ${notBefore} + ) + ON CONFLICT(thread_id) DO UPDATE SET + wait_queued_at = ${queuedAt}, + wait_was_active = ${wasActive}, + wait_prefer_profile = ${preferProfile}, + wait_not_before = ${notBefore} + `, + }); + + const clearWaitRow = SqlSchema.void({ + Request: Schema.Struct({ threadId: ThreadId }), + execute: ({ threadId }) => sql` + UPDATE thread_claude_account SET + wait_queued_at = NULL, + wait_was_active = NULL, + wait_prefer_profile = NULL, + wait_not_before = NULL + WHERE thread_id = ${threadId} + `, + }); + const deleteRow = SqlSchema.void({ Request: Schema.Struct({ threadId: ThreadId }), execute: ({ threadId }) => sql` @@ -151,7 +251,7 @@ export const make = Effect.gen(function* () { return ThreadClaudeAccountRepository.of({ get: (threadId) => getRow({ threadId }).pipe( - Effect.map(Option.getOrElse(() => defaultRow(threadId))), + Effect.map(Option.match({ onNone: () => defaultRow(threadId), onSome: fromDbRow })), Effect.mapError(mapError("ThreadClaudeAccount.get")), ), setMode: ({ threadId, mode, updatedAt }) => @@ -160,8 +260,26 @@ export const make = Effect.gen(function* () { ), setResolved: (input) => setResolvedRow(input).pipe(Effect.mapError(mapError("ThreadClaudeAccount.setResolved"))), + setWait: ({ threadId, wait }) => + setWaitRow({ + threadId, + queuedAt: wait.queuedAt, + wasActive: wait.wasActive ? 1 : 0, + preferProfile: wait.preferProfile, + notBefore: wait.notBefore, + }).pipe(Effect.mapError(mapError("ThreadClaudeAccount.setWait"))), + clearWait: (threadId) => + clearWaitRow({ threadId }).pipe(Effect.mapError(mapError("ThreadClaudeAccount.clearWait"))), + listWaiting: () => + listWaitingRows(undefined).pipe( + Effect.map((rows) => rows.map(fromDbRow)), + Effect.mapError(mapError("ThreadClaudeAccount.listWaiting")), + ), listByProfile: (profile) => - listRows({ profile }).pipe(Effect.mapError(mapError("ThreadClaudeAccount.listByProfile"))), + listRows({ profile }).pipe( + Effect.map((rows) => rows.map(fromDbRow)), + Effect.mapError(mapError("ThreadClaudeAccount.listByProfile")), + ), delete: (threadId) => deleteRow({ threadId }).pipe(Effect.mapError(mapError("ThreadClaudeAccount.delete"))), }); diff --git a/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts b/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts index 3823f8466abe7..fa0b710df8f94 100644 --- a/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts +++ b/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts @@ -20,6 +20,7 @@ const pendingV2Migrations = [ [1050, "ThreadSnapshotWindowIndexes"], [1051, "ScheduledTaskCustomGroups"], [1052, "ScheduledTaskForkOptions"], + [1053, "ThreadClaudeAccountWait"], ] as const; describe("fork V2 ledger upgrade", () => { @@ -65,6 +66,7 @@ describe("fork V2 ledger upgrade", () => { pendingV2Migrations[7], pendingV2Migrations[8], pendingV2Migrations[9], + pendingV2Migrations[10], ]); assert.deepStrictEqual(yield* runMigrations(), []); assert.deepStrictEqual(yield* sql`SELECT * FROM orchestration_v2_legacy_imports`, imports); diff --git a/apps/server/src/wsForkHandlers.claudeAccounts.test.ts b/apps/server/src/wsForkHandlers.claudeAccounts.test.ts index 17ba9ee0cf70b..9373f4fe2bc4c 100644 --- a/apps/server/src/wsForkHandlers.claudeAccounts.test.ts +++ b/apps/server/src/wsForkHandlers.claudeAccounts.test.ts @@ -1,8 +1,15 @@ /** - * T3-CUSTOM(expbkt3): the admin gate on Claude account access RPCs, and the - * per-connection identity the account handlers hand the service. + * T3-CUSTOM(expbkt3): the admin gate on Claude account access RPCs, the + * per-connection identity the account handlers hand the service, and the + * thread access check in front of leaving the revive queue. */ -import { ClaudeAccountsError, ThreadId, UserId, WS_FORK_METHODS } from "@t3tools/contracts"; +import { + ClaudeAccountsError, + OrchestrationGetSnapshotError, + ThreadId, + UserId, + WS_FORK_METHODS, +} from "@t3tools/contracts"; import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Stream from "effect/Stream"; @@ -17,6 +24,7 @@ function makeHandlers( actorUserId: UserId | null, actorIsAdmin: boolean, directory: ReadonlyArray | null = [member], + threadAccess: "draft" | "allowed" | "denied" = "draft", ) { const calls: Array<{ readonly method: string; readonly input: unknown }> = []; const entries = { entries: [{ profile: "agent", userIds: [member] }] }; @@ -36,6 +44,11 @@ function makeHandlers( calls.push({ method: "setThreadMode", input }); return { threadId: ThreadId.make("thread-1"), mode: { kind: "auto" } } as const; }), + leaveQueue: (threadId: unknown) => + Effect.sync(() => { + calls.push({ method: "leaveQueue", input: threadId }); + return { threadId: ThreadId.make("thread-1"), mode: { kind: "auto" } } as const; + }), watchSnapshot: (viewer: unknown) => { calls.push({ method: "watchSnapshot", input: viewer }); return Stream.empty; @@ -63,8 +76,13 @@ function makeHandlers( observeRpcEffect: (_method, effect) => effect, observeRpcStream: (_method, stream) => stream, projectionSnapshotQuery: { - getThreadShellById: () => Effect.succeedNone, + getThreadShellById: () => + threadAccess === "draft" ? Effect.succeedNone : Effect.succeedSome({}), } as unknown as ForkWsHandlerDeps["projectionSnapshotQuery"], + requireThreadAccess: () => + threadAccess === "denied" + ? Effect.fail(new OrchestrationGetSnapshotError({ message: "Thread not found." })) + : Effect.void, } satisfies Partial; return { handlers: makeForkWsHandlers(deps as unknown as ForkWsHandlerDeps), calls }; } @@ -157,4 +175,20 @@ describe("Claude account access websocket handlers", () => { ]); }), ); + + it.effect("takes a thread out of the queue only after the thread access check", () => + Effect.gen(function* () { + const threadId = ThreadId.make("thread-queued"); + const allowed = makeHandlers(member, false, [member], "allowed"); + yield* allowed.handlers[WS_FORK_METHODS.claudeAccountsLeaveQueue]({ threadId }); + expect(allowed.calls).toEqual([{ method: "leaveQueue", input: threadId }]); + + const denied = makeHandlers(outsider, false, [member], "denied"); + const error = yield* denied.handlers[WS_FORK_METHODS.claudeAccountsLeaveQueue]({ + threadId, + }).pipe(Effect.flip); + expect(error._tag === "ClaudeAccountsError" ? error.reason : error._tag).toBe("not-found"); + expect(denied.calls).toEqual([]); + }), + ); }); diff --git a/apps/server/src/wsForkHandlers.ts b/apps/server/src/wsForkHandlers.ts index 0342257b424e7..107a24f1a42ad 100644 --- a/apps/server/src/wsForkHandlers.ts +++ b/apps/server/src/wsForkHandlers.ts @@ -298,6 +298,14 @@ export const makeForkWsHandlers = ({ ), { "rpc.aggregate": "claude-accounts" }, ), + [WS_FORK_METHODS.claudeAccountsLeaveQueue]: (input) => + observeRpcEffect( + WS_FORK_METHODS.claudeAccountsLeaveQueue, + guardClaudeAccountsThread("leaveQueue", input.threadId).pipe( + Effect.andThen(claudeAccounts.leaveQueue(input.threadId)), + ), + { "rpc.aggregate": "claude-accounts" }, + ), [WS_FORK_METHODS.subscribeClaudeAccounts]: () => observeRpcStream( WS_FORK_METHODS.subscribeClaudeAccounts, @@ -333,6 +341,7 @@ export const makeForkWsHandlers = ({ ForkWsHandlers, | typeof WS_FORK_METHODS.claudeAccountsGetThread | typeof WS_FORK_METHODS.claudeAccountsSetThreadMode + | typeof WS_FORK_METHODS.claudeAccountsLeaveQueue | typeof WS_FORK_METHODS.subscribeClaudeAccounts | typeof WS_FORK_METHODS.subscribeThreadClaudeAccount | typeof WS_FORK_METHODS.claudeAccountsAccessList diff --git a/apps/web/src/fork/claudeAccounts/ClaudeAccountPicker.tsx b/apps/web/src/fork/claudeAccounts/ClaudeAccountPicker.tsx index f7b3ce01b1c11..5c2ad209f8682 100644 --- a/apps/web/src/fork/claudeAccounts/ClaudeAccountPicker.tsx +++ b/apps/web/src/fork/claudeAccounts/ClaudeAccountPicker.tsx @@ -9,6 +9,8 @@ * Claude Accounts dashboard. Rows follow Auto's order for the next new * session (#1 = next pick). The chosen mode carries the theme's primary * colour; under Auto, the account this thread runs on gets a lighter tint. + * A thread waiting in the server's revive queue shows its place and a way + * out; picking an account continues it there at once. */ import { CLAUDE_ACCOUNT_MODE_AUTO, @@ -28,6 +30,7 @@ import { Badge } from "../../components/ui/badge"; import { Button } from "../../components/ui/button"; import { Menu, + MenuItem, MenuPopup, MenuRadioGroup, MenuRadioItem, @@ -50,8 +53,11 @@ import { AUTO_ACCOUNT_LABEL, autoRowDetail, isAutoMode, + LEAVE_QUEUE_LABEL, modeEquals, PENDING_RESTART_HINT, + QUEUE_PICK_HINT, + queueLabel, spaceLine, SWITCH_RESTARTS_SESSION_HINT, switchRestartsSession, @@ -344,7 +350,7 @@ function ClaudeAccountPickerMenu(props: { readonly hidden: boolean; }) { const { snapshot } = props; - const { account, setMode } = useThreadClaudeAccount(props.threadRef, true); + const { account, setMode, leaveQueue } = useThreadClaudeAccount(props.threadRef, true); const [open, setOpen] = useComposerMenuState(props.hidden); const [now, setNow] = useState(() => Date.now()); const [hoveredValue, setHoveredValue] = useState(null); @@ -356,7 +362,8 @@ function ClaudeAccountPickerMenu(props: { }, [open]); const view = triggerView(snapshot, account); - const tooltip = triggerTooltip(view); + const queued = queueLabel(account); + const tooltip = queued === null ? triggerTooltip(view) : `${triggerTooltip(view)} · ${queued}`; const mode = account?.mode ?? CLAUDE_ACCOUNT_MODE_AUTO; const rows = accountRows(snapshot, account, now); const auto = isAutoMode(account); @@ -406,6 +413,9 @@ function ClaudeAccountPickerMenu(props: { {account?.notice ? ( {account.notice} ) : null} + {account?.wait ? ( + {QUEUE_PICK_HINT} + ) : null} {account?.pendingRestart ? ( {PENDING_RESTART_HINT} ) : null} @@ -413,6 +423,11 @@ function ClaudeAccountPickerMenu(props: { {unavailable} ) : null} + {account?.wait ? ( + void leaveQueue()}> + {LEAVE_QUEUE_LABEL} + + ) : null} { diff --git a/apps/web/src/fork/claudeAccounts/hooks.ts b/apps/web/src/fork/claudeAccounts/hooks.ts index 2d92150b5f342..ec2391a20e06f 100644 --- a/apps/web/src/fork/claudeAccounts/hooks.ts +++ b/apps/web/src/fork/claudeAccounts/hooks.ts @@ -148,6 +148,8 @@ export interface ThreadClaudeAccountState { /** Null until anything is known, which reads as Auto. */ readonly account: ThreadClaudeAccount | null; readonly setMode: (mode: ClaudeAccountMode) => Promise; + /** Takes the thread out of the revive queue; the thread stream then shows it. */ + readonly leaveQueue: () => Promise; } /** The thread's account choice and its setter. `enabled: false` mounts no streams. */ @@ -174,6 +176,7 @@ export function useThreadClaudeAccount( key === null ? undefined : state.choices[key], ); const write = useSetThreadClaudeAccountMode(); + const leave = useAtomCommand(claudeAccountsEnvironment.leaveQueue); const isServerThread = target !== null; // A draft pick the server refused before the thread existed. @@ -191,6 +194,14 @@ export function useThreadClaudeAccount( [isServerThread, server, threadRef, write], ); + const leaveQueue = useCallback(async () => { + if (threadRef === null || !isServerThread) return; + await leave({ + environmentId: threadRef.environmentId, + input: { threadId: threadRef.threadId }, + }); + }, [isServerThread, leave, threadRef]); + let account = server; if ( threadRef !== null && @@ -200,7 +211,7 @@ export function useThreadClaudeAccount( const base = local.result ?? server; account = { ...base, threadId: threadRef.threadId, mode: local.mode }; } - return { account, setMode }; + return { account, setMode, leaveQueue }; } export interface ClaudeAccountAccessState { diff --git a/apps/web/src/fork/claudeAccounts/model.test.ts b/apps/web/src/fork/claudeAccounts/model.test.ts index c7c620c1308ea..7618a82b2f874 100644 --- a/apps/web/src/fork/claudeAccounts/model.test.ts +++ b/apps/web/src/fork/claudeAccounts/model.test.ts @@ -12,6 +12,7 @@ import { autoRowDetail, nextPick, orderAccounts, + queueLabel, recoversIn, resetsIn, ringTone, @@ -373,3 +374,31 @@ describe("ringTone", () => { expect(ringTone(100)).toBe("bad"); }); }); + +describe("queueLabel", () => { + it("names the thread's place in the revive queue, and nothing when it is not waiting", () => { + expect(queueLabel(null)).toBeNull(); + expect(queueLabel(thread({ resolvedProfile: "sam" }))).toBeNull(); + expect( + queueLabel( + thread({ + resolvedProfile: "sam", + wait: { + position: 2, + queuedAt: "2026-10-02T07:55:00.000Z", + expectedAt: "2026-10-02T08:31:00.000Z", + expectedProfile: "agent", + wasActive: true, + }, + }), + ), + ).toBe("Waiting for an account · number 2 in the queue"); + expect( + queueLabel( + thread({ + wait: { position: 1, queuedAt: "2026-10-02T07:55:00.000Z", wasActive: false }, + }), + ), + ).toBe("No account free · number 1 in the queue"); + }); +}); diff --git a/apps/web/src/fork/claudeAccounts/model.ts b/apps/web/src/fork/claudeAccounts/model.ts index 6ee591c06ec21..9ce037003a9aa 100644 --- a/apps/web/src/fork/claudeAccounts/model.ts +++ b/apps/web/src/fork/claudeAccounts/model.ts @@ -22,6 +22,9 @@ export const AUTO_ACCOUNT_RULE = "new sessions go to the account with the most r export const SWITCH_RESTARTS_SESSION_HINT = "Switching restarts this thread's session on the next message"; export const PENDING_RESTART_HINT = "Applies after this turn"; +export const LEAVE_QUEUE_LABEL = "Leave queue"; +/** Under the queue notice: picking an account is the way to continue at once. */ +export const QUEUE_PICK_HINT = "Pick an account below to continue on it now."; /** Bar bands, as on the dashboard: only the trip lines are policy. */ export const WARN_USED_PERCENT = 60; @@ -425,6 +428,18 @@ export function autoRowDetail( : `Would pick #1 ${next} by space to reset`; } +/** + * A short label for a thread waiting in the server's revive queue, for the + * trigger's tooltip; null when it is not waiting. The full sentence is the + * thread's notice. + */ +export function queueLabel(thread: ThreadClaudeAccount | null): string | null { + const wait = thread?.wait; + if (wait === undefined) return null; + const place = `number ${wait.position} in the queue`; + return wait.wasActive ? `Waiting for an account · ${place}` : `No account free · ${place}`; +} + export function isAutoMode(thread: ThreadClaudeAccount | null): boolean { return thread === null || thread.mode.kind === "auto"; } diff --git a/apps/web/src/fork/claudeAccounts/state.ts b/apps/web/src/fork/claudeAccounts/state.ts index 2a752b5253605..b0b0127681005 100644 --- a/apps/web/src/fork/claudeAccounts/state.ts +++ b/apps/web/src/fork/claudeAccounts/state.ts @@ -44,6 +44,16 @@ export const claudeAccountsEnvironment = { key: ({ environmentId, input }) => `${environmentId}:${input.threadId}`, }, }), + /** Takes a waiting thread out of the server's revive queue without moving it. */ + leaveQueue: createEnvironmentRpcCommand(connectionAtomRuntime, { + label: "environment-data:claude-accounts:leave-queue", + tag: WS_METHODS.claudeAccountsLeaveQueue, + scheduler, + concurrency: { + mode: "serial", + key: ({ environmentId, input }) => `${environmentId}:${input.threadId}`, + }, + }), /** Every account's allow list (admins only). */ access: createEnvironmentRpcQueryAtomFamily(connectionAtomRuntime, { label: "environment-data:claude-accounts:access", diff --git a/docs/operations/claude-account-switching.md b/docs/operations/claude-account-switching.md index fa90043fc1650..0602ab9f11bfb 100644 --- a/docs/operations/claude-account-switching.md +++ b/docs/operations/claude-account-switching.md @@ -213,6 +213,25 @@ How it works when the flag is on: rejection arriving before the first turn starts on the new account is a stale copy from the stopped session; a rejection from the new account, even with the same reset instant, moves the thread again. +- **Revive queue.** When no account can take the Auto thread, its session stops and the + thread waits in `thread_claude_account` (migration `1053_ThreadClaudeAccountWait`), so a + restart keeps it. A limit that resets within five minutes is the same kind of entry, + pinned to the account it hit and due one minute after the reset. After every status + poll the server places the waiting threads again, in order: due reset waits, then + threads that were mid-turn, then the oldest. Only a real `placed` answer counts, and + each placement counts as pending for the next, so a wave of resets spreads. A thread + that was mid-turn gets the continue turn; an idle one only gets its account. The + composer shows the thread's place and when the first account it may use should have + room (from the switcher's `recoveries`). A message to the thread, a mode change or + `claudeAccounts.leaveQueue` takes it out; picking an account while it waited mid-turn + continues it there at once. A message still fails at start when nothing has room, with + the account expected to free up first. +- **Rebalance.** When an idle Auto thread's turn ends on an account that carries two or + more sessions above its `fair_share`, or that the switcher reports as not placeable, + the server asks `--place` where the thread would go now (avoiding its account). On a + `placed` answer it moves the thread and restarts its idle session, at most once an hour + per thread (the thread loses its prompt cache once). Pinned threads never move. An + older switcher without `fair_share` disables this. - **Hard limit, pinned thread.** Nothing moves. The thread gets the notice " is at its usage limit until HH:MM UTC — switch to Auto or another account." (account-wide) or " has no allowance left until HH:MM UTC — switch the model or account." @@ -236,8 +255,10 @@ account `~/.claude-active` resolves to (realpath; `default` for `~/.claude`), st per thread. A switcher crash or 10 s timeout logs a warning and leaves the spawn environment unchanged, except for an owner restricted by account access (below), who is never left on `~/.claude-active` blindly. When no account has headroom the start fails -with "All Claude accounts are at their limit — first resets at HH:MM UTC." (without the -time when the switcher gives none). The binary path is the setting, else `~/.local/bin/claude-autoswitch` +with "All Claude accounts are at their limit — is expected to have room at +HH:MM UTC." (an older switcher without `recoveries`: "— first resets at HH:MM UTC."; +without a time when the switcher gives none). Each such answer logs +`claude.account.place-none`. The binary path is the setting, else `~/.local/bin/claude-autoswitch` (a leading `~` expands to the server user's home, so macOS hosts work too); `T3_CLAUDE_AUTOSWITCH_BIN` overrides it for tests. The hard-limit rotation runs the same resolved binary. Calls use an argv array, never a shell diff --git a/docs/operations/expbkt3-customizations.md b/docs/operations/expbkt3-customizations.md index 9f3b776e221a1..ed4306352d648 100644 --- a/docs/operations/expbkt3-customizations.md +++ b/docs/operations/expbkt3-customizations.md @@ -39,7 +39,8 @@ the async question indicator; a read-only environment badge; the BK desktop and mobile builds; the deploy, CI and marker tooling; Claude account profiles per thread (`experimental.claudeAccountProfiles`: Auto placement through `claude-autoswitch --place`, per-thread pinning, sticky `CLAUDE_CONFIG_DIR` per spawn, hard-limit move with a -continue turn; [claude-account-switching.md](./claude-account-switching.md#per-thread-account-placement)). +continue turn, a durable revive queue when no account has room, turn-boundary +rebalancing; [claude-account-switching.md](./claude-account-switching.md#per-thread-account-placement)). **Dropped in favour of upstream's own behaviour:** diff --git a/packages/contracts/src/claudeAccounts.ts b/packages/contracts/src/claudeAccounts.ts index d16e62818f3fc..68f2c426dede6 100644 --- a/packages/contracts/src/claudeAccounts.ts +++ b/packages/contracts/src/claudeAccounts.ts @@ -15,7 +15,14 @@ import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; -import { IsoDateTime, NonNegativeInt, ThreadId, TrimmedString, UserId } from "./baseSchemas.ts"; +import { + IsoDateTime, + NonNegativeInt, + PositiveInt, + ThreadId, + TrimmedString, + UserId, +} from "./baseSchemas.ts"; export const ClaudeAccountProfileName = Schema.String.check( Schema.isNonEmpty(), @@ -110,6 +117,22 @@ export const ClaudeAccountsSnapshot = Schema.Struct({ }); export type ClaudeAccountsSnapshot = typeof ClaudeAccountsSnapshot.Type; +/** + * A thread no account could take, waiting in the server's revive queue. The + * server continues it on the first account with room, in queue order. + */ +export const ThreadClaudeAccountWait = Schema.Struct({ + /** 1-based place in the order the server revives waiting threads. */ + position: PositiveInt, + queuedAt: IsoDateTime, + /** When the first account this thread may use is expected to have room. */ + expectedAt: Schema.optionalKey(IsoDateTime), + expectedProfile: Schema.optionalKey(ClaudeAccountProfileName), + /** A turn was running when it stopped: the revive continues it on its own. */ + wasActive: Schema.Boolean, +}); +export type ThreadClaudeAccountWait = typeof ThreadClaudeAccountWait.Type; + export const ThreadClaudeAccount = Schema.Struct({ threadId: ThreadId, mode: ClaudeAccountMode, @@ -120,6 +143,8 @@ export const ThreadClaudeAccount = Schema.Struct({ notice: Schema.optionalKey(Schema.String), /** True when a mode change waits for the running turn to finish. */ pendingRestart: Schema.optionalKey(Schema.Boolean), + /** Present while the thread waits in the revive queue. */ + wait: Schema.optionalKey(ThreadClaudeAccountWait), }); export type ThreadClaudeAccount = typeof ThreadClaudeAccount.Type; diff --git a/packages/contracts/src/rpcFork.ts b/packages/contracts/src/rpcFork.ts index f278eb29bd61c..63a143bdea1dc 100644 --- a/packages/contracts/src/rpcFork.ts +++ b/packages/contracts/src/rpcFork.ts @@ -151,6 +151,7 @@ export const WS_FORK_METHODS = { // T3-CUSTOM(expbkt3): Claude account profiles per thread. claudeAccountsGetThread: "claudeAccounts.getThread", claudeAccountsSetThreadMode: "claudeAccounts.setThreadMode", + claudeAccountsLeaveQueue: "claudeAccounts.leaveQueue", subscribeClaudeAccounts: "subscribeClaudeAccounts", subscribeThreadClaudeAccount: "subscribeThreadClaudeAccount", // T3-CUSTOM(expbkt3): Claude account access per user (admin only). @@ -502,6 +503,13 @@ export const WsClaudeAccountsSetThreadModeRpc = Rpc.make( }, ); +/** Takes a waiting thread out of the revive queue without moving it. */ +export const WsClaudeAccountsLeaveQueueRpc = Rpc.make(WS_FORK_METHODS.claudeAccountsLeaveQueue, { + payload: ClaudeAccountsThreadInput, + success: ThreadClaudeAccount, + error: claudeAccountsError, +}); + export const WsSubscribeClaudeAccountsRpc = Rpc.make(WS_FORK_METHODS.subscribeClaudeAccounts, { payload: Schema.Struct({}), success: ClaudeAccountsSnapshot, @@ -628,6 +636,7 @@ export const FORK_WS_RPCS = [ // T3-CUSTOM(expbkt3): Claude account profiles per thread. WsClaudeAccountsGetThreadRpc, WsClaudeAccountsSetThreadModeRpc, + WsClaudeAccountsLeaveQueueRpc, WsSubscribeClaudeAccountsRpc, WsSubscribeThreadClaudeAccountRpc, // T3-CUSTOM(expbkt3): Claude account access per user. From bbe817cc2d7456adb6eeef12a9a573819c9a5e7a Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Fri, 16 Oct 2026 13:37:40 +0000 Subject: [PATCH 15/18] fix(server): review fixes for the Claude account revive queue and rebalance - A revive checks it still owns the queue entry before it moves or continues the thread, so Leave queue / a user message during placement wins. - The duplicate filter forgets a condition after a successful move and when the first turn on the new account starts. - Rebalance runs only at a real turn end, pre-checks the last status before asking the switcher, backs off 5 min after a probe finds nowhere better, moves at most one thread per source account per status read, and writes only the row so the old session keeps its own attribution. - Parking no longer forks a revive pass; archived threads leave the queue; a passed reset time is no longer shown as expected. - Test counts for the new RPC (237) and migration 1053; formatting. Co-Authored-By: Claude Opus 5.5 --- .../ClaudeAccountsService.test.ts | 236 ++++++++++++++++-- .../claudeAccounts/ClaudeAccountsService.ts | 166 +++++++++--- .../src/mcp/toolkits/webUi/catalog.test.ts | 2 +- .../mcp/toolkits/webUi/registration.test.ts | 4 +- .../Migrations/055_OrchestrationV2.test.ts | 5 +- docs/operations/claude-account-switching.md | 9 +- 6 files changed, 362 insertions(+), 60 deletions(-) diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts index 58c6cb71b8854..0524afa95c9d2 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts @@ -18,7 +18,9 @@ import { ThreadId, UserId, } from "@t3tools/contracts"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -155,6 +157,8 @@ interface HarnessOptions { readonly placeFailed?: boolean; /** `--place` dies with a defect, as a bug in the client would. */ readonly placeDefect?: boolean; + /** Runs inside every `--place` call before it answers, e.g. to hold it in flight. */ + readonly beforePlace?: () => Effect.Effect; /** Replaces the thread-account repository, e.g. with one whose reads fail. */ readonly threadRepository?: Layer.Layer; } @@ -228,15 +232,21 @@ const makeHarness = (options: HarnessOptions = {}) => place: (input) => Effect.sync(() => { placeCalls.push({ pending: { ...input.pending }, avoid: input.avoid }); - if (options.placeUnavailable) { - return { kind: "unavailable", detail: "old switcher" } as const; - } - if (options.placeFailed) { - return { kind: "failed", detail: "timed out after 10s" } as const; - } - if (options.placeDefect) throw new Error("place exploded"); - return { kind: "ok", value: place(input) } as const; - }), + }).pipe( + Effect.andThen(options.beforePlace?.() ?? Effect.void), + Effect.andThen( + Effect.sync(() => { + if (options.placeUnavailable) { + return { kind: "unavailable", detail: "old switcher" } as const; + } + if (options.placeFailed) { + return { kind: "failed", detail: "timed out after 10s" } as const; + } + if (options.placeDefect) throw new Error("place exploded"); + return { kind: "ok", value: place(input) } as const; + }), + ), + ), hardLimit: (input) => Effect.sync(() => { hardLimitCalls.push(input); @@ -2059,9 +2069,19 @@ describe("ClaudeAccountsService access per user: owners, probing, hard limits, r * A switcher whose `--place` the test opens and closes: no room until * `state.open` names accounts, then the open account with the fewest pending * placements. With no room it lists when agent, then barsha, recover. + * `state.hold` keeps every `--place` in flight until it is released. */ function gatedPlace() { - const state: { open: ReadonlyArray } = { open: [] }; + const state: { + open: ReadonlyArray; + hold?: { readonly entered: Deferred.Deferred; readonly release: Deferred.Deferred }; + } = { open: [] }; + const beforePlace = (): Effect.Effect => { + const hold = state.hold; + return hold === undefined + ? Effect.void + : Effect.andThen(Deferred.succeed(hold.entered, undefined), Deferred.await(hold.release)); + }; const place = (input: PlaceInput): SwitcherPlaceResult => { let chosen: string | undefined; for (const name of state.open) { @@ -2085,7 +2105,7 @@ function gatedPlace() { } return { status: "placed", chosen, dir: null, reason: "fewest sessions" }; }; - return { state, place }; + return { state, place, beforePlace }; } /** Domain events are handled in order on one fiber: a deleted, watched thread marks them done. */ @@ -2121,6 +2141,13 @@ const markTurnRunning = ( yield* drainDomainEvents(harness, service); }); +/** A turn runs and settles on the thread, as the projection reports a turn boundary. */ +const endTurn = (harness: Harness, threadId: ThreadId) => + Effect.gen(function* () { + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "running", "turn-1")); + yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + }); + const fiveHourAt = (threadId: ThreadId, resetAt: number) => ({ threadId, rateLimitType: "five_hour", @@ -2334,6 +2361,115 @@ describe("ClaudeAccountsService revive queue", () => { ); } + { + const gate = gatedPlace(); + scenario( + "does not revive a thread that left the queue while its placement was in flight", + { place: gate.place, beforePlace: gate.beforePlace }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-race"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + assert.isDefined((yield* service.getThread(threadId)).wait); + + const entered = yield* Deferred.make(); + const release = yield* Deferred.make(); + gate.state.open = ["agent"]; + gate.state.hold = { entered, release }; + const pass = yield* service.refreshStatus().pipe(Effect.forkScoped); + // The switcher is answering for the thread; the person leaves the queue now. + yield* Deferred.await(entered); + const left = yield* service.leaveQueue(threadId); + assert.equal(left.wait, undefined); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(pass); + + assert.equal(harness.dispatched.length, 0); + const view = yield* service.getThread(threadId); + assert.equal(view.wait, undefined); + assert.equal(view.resolvedProfile, "tushar"); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "takes an archived thread out of the queue so it is not continued", + { place: gate.place }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-archived"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + const { seen } = yield* watchThreadInto(service, threadId); + + yield* PubSub.publish(harness.domainEvents, { + type: "thread.archived", + payload: { + threadId, + archivedAt: "1970-01-01T00:00:00.000Z", + updatedAt: "1970-01-01T00:00:00.000Z", + }, + } as unknown as OrchestrationEvent); + yield* takeUntil(seen, (view) => view.wait === undefined); + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + assert.equal(harness.dispatched.length, 0); + // The account row stays for an unarchive. + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "tushar"); + }), + ); + } + + scenario( + "handles a model-scoped rejection again once the thread is back on that account", + {}, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-scoped-return"); + assert.equal((yield* service.resolveForSession(threadId))?.profile, "tushar"); + const opus = { threadId, rateLimitType: "seven_day_opus", resetAt: farReset }; + + yield* service.handleHardLimit(opus); + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "agent"); + // A burst of copies of that one rejection moves the thread once. + yield* service.handleHardLimit(opus); + yield* service.handleHardLimit(opus); + assert.equal(harness.hardLimitCalls.length, 1); + assert.equal(harness.dispatched.length, 1); + + // The first turn on agent; later the thread is placed back on tushar, + // which a model-scoped limit left in rotation. + const { seen } = yield* watchThreadInto(service, threadId); + yield* PubSub.publish(harness.providerEvents, providerEvent("turn.started", threadId)); + yield* Queue.take(seen); + assert.equal( + (yield* service.reassign(threadId, { avoid: ["agent", "barsha"] }))?.profile, + "tushar", + ); + + // Claude refuses Opus on tushar again, same reset: a real rejection, handled. + assert.isTrue(yield* service.handleHardLimit(opus)); + assert.deepEqual(harness.hardLimitCalls, [ + { type: "seven_day_opus", profile: "tushar" }, + { type: "seven_day_opus", profile: "tushar" }, + ]); + assert.notEqual((yield* service.getThread(threadId)).resolvedProfile, "tushar"); + assert.equal(harness.dispatched.length, 2); + }), + ); + it.effect("keeps the queue across a restart and continues the thread there", () => Effect.gen(function* () { const repository = yield* Layer.build( @@ -2429,7 +2565,7 @@ describe("ClaudeAccountsService rebalancing at turn boundaries", () => { yield* Ref.set(harness.sessions, [liveSession(threadId, "ready")]); const { seen } = yield* watchThreadInto(service, threadId); - yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* endTurn(harness, threadId); yield* takeUntil(seen, (view) => view.resolvedProfile === "agent"); yield* drainDomainEvents(harness, service); assert.deepEqual(harness.stoppedThreads, [threadId]); @@ -2448,13 +2584,13 @@ describe("ClaudeAccountsService rebalancing at turn boundaries", () => { yield* service.refreshStatus(); gate.state.open = ["barsha"]; const placements = harness.placeCalls.length; - yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* endTurn(harness, threadId); yield* drainDomainEvents(harness, service); assert.equal(harness.placeCalls.length, placements); assert.equal((yield* service.getThread(threadId)).resolvedProfile, "agent"); yield* TestClock.adjust("61 minutes"); - yield* PubSub.publish(harness.domainEvents, sessionSet(threadId, "ready", null)); + yield* endTurn(harness, threadId); yield* takeUntil(seen, (view) => view.resolvedProfile === "barsha"); yield* drainDomainEvents(harness, service); assert.deepEqual(harness.stoppedThreads, [threadId, threadId]); @@ -2486,8 +2622,8 @@ describe("ClaudeAccountsService rebalancing at turn boundaries", () => { ]); const placements = harness.placeCalls.length; - yield* PubSub.publish(harness.domainEvents, sessionSet(pinned, "ready", null)); - yield* PubSub.publish(harness.domainEvents, sessionSet(busy, "ready", null)); + yield* endTurn(harness, pinned); + yield* endTurn(harness, busy); yield* drainDomainEvents(harness, service); assert.equal(harness.placeCalls.length, placements); assert.deepEqual(harness.stoppedThreads, []); @@ -2496,4 +2632,72 @@ describe("ClaudeAccountsService rebalancing at turn boundaries", () => { }), ); } + + { + const gate = gatedPlace(); + scenario( + "does not ask the switcher again within five minutes after a probe found nowhere better", + { place: gate.place, status: sharedStatus }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-rebalance-backoff"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* Ref.set(harness.sessions, [liveSession(threadId, "ready")]); + const before = harness.placeCalls.length; + + yield* endTurn(harness, threadId); + yield* drainDomainEvents(harness, service); + assert.equal(harness.placeCalls.length, before + 1); + assert.deepEqual(harness.stoppedThreads, []); + + yield* endTurn(harness, threadId); + yield* drainDomainEvents(harness, service); + assert.equal(harness.placeCalls.length, before + 1); + + yield* TestClock.adjust("6 minutes"); + yield* endTurn(harness, threadId); + yield* drainDomainEvents(harness, service); + assert.equal(harness.placeCalls.length, before + 2); + assert.equal((yield* service.getThread(threadId)).resolvedProfile, "tushar"); + }), + ); + } + + { + const gate = gatedPlace(); + scenario( + "moves one thread per status read off an over-loaded account, whose count is stale until then", + { place: gate.place, status: sharedStatus }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const first = ThreadId.make("thread-rebalance-first"); + const second = ThreadId.make("thread-rebalance-second"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(first); + yield* service.resolveForSession(second); + gate.state.open = ["agent", "barsha"]; + yield* Ref.set(harness.sessions, [ + liveSession(first, "ready"), + liveSession(second, "ready"), + ]); + + yield* endTurn(harness, first); + yield* endTurn(harness, second); + yield* drainDomainEvents(harness, service); + assert.deepEqual(harness.stoppedThreads, [first]); + assert.notEqual((yield* service.getThread(first)).resolvedProfile, "tushar"); + assert.equal((yield* service.getThread(second)).resolvedProfile, "tushar"); + + // The next status read counts the move; tushar is still over its share. + yield* service.refreshStatus(); + yield* endTurn(harness, second); + yield* drainDomainEvents(harness, service); + assert.deepEqual(harness.stoppedThreads, [first, second]); + }), + ); + } }); diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts index 7a829c6551c3a..49937d9429792 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts @@ -108,6 +108,8 @@ const MAX_HANDLED_CONDITIONS = 256; export const REBALANCE_COOLDOWN_MS = 60 * 60 * 1000; /** Sessions above the account's fair share before a finished turn moves its thread. */ const REBALANCE_EXCESS = 2; +/** After a rebalance probe finds nowhere better, the thread is not probed again for this long. */ +export const REBALANCE_PROBE_BACKOFF_MS = 5 * 60 * 1000; /** A limit that resets within this long is waited out on the same account instead of moving. */ export const RESET_WAIT_THRESHOLD_MS = 5 * 60 * 1000; /** Margin after the reset so the status poll sees the account free before the thread continues. */ @@ -193,7 +195,9 @@ export interface ClaudeAccountsServiceShape { readonly refreshStatus: () => Effect.Effect; readonly markExhausted: (profile: string, resetsAt: string | undefined) => Effect.Effect; /** Takes the thread out of the revive queue without moving it. */ - readonly leaveQueue: (threadId: ThreadId) => Effect.Effect; + readonly leaveQueue: ( + threadId: ThreadId, + ) => Effect.Effect; /** Places the thread again, avoiding the given accounts, and persists the result. */ readonly reassign: ( threadId: ThreadId, @@ -371,6 +375,16 @@ export const make = Effect.gen(function* () { const waits = new Map(); /** When each thread was last moved by a rebalance. */ const lastRebalancedAt = new Map(); + /** When a rebalance probe last found nowhere better for each thread. */ + const lastFailedProbeAt = new Map(); + /** + * Accounts a rebalance moved a thread off since the last status read. Their + * session counts are stale until the next read, so each gives up at most + * one thread per read. + */ + const rebalancedFrom = new Set(); + /** Threads whose settled projection last showed a running turn. */ + const projectedRunning = new Set(); /** * Live Claude session id -> the account it runs on, from each account's * `sessions/.json` (the CLI writes one at start and never removes it). @@ -385,16 +399,17 @@ export const make = Effect.gen(function* () { const placementLock = yield* Semaphore.make(1); /** One revive pass at a time; a pass started meanwhile runs after it. */ const reviveLock = yield* Semaphore.make(1); - const layerScope = yield* Effect.scope; - const storedWaits = yield* repository.listWaiting().pipe( - Effect.catchCause((cause) => - Effect.as( - Effect.logWarning("claude.account.queue-load-failed", { cause }), - [] as ReadonlyArray, + const storedWaits = yield* repository + .listWaiting() + .pipe( + Effect.catchCause((cause) => + Effect.as( + Effect.logWarning("claude.account.queue-load-failed", { cause }), + [] as ReadonlyArray, + ), ), - ), - ); + ); for (const row of storedWaits) { if (row.wait !== null) waits.set(row.threadId, { ...row.wait, restricted: false }); } @@ -768,6 +783,8 @@ export const make = Effect.gen(function* () { lastStatus = status; lastStatusAtMs = yield* Clock.currentTimeMillis; unavailableReason = undefined; + // Fresh session counts include every rebalance made before this read. + rebalancedFrom.clear(); const generatedMs = typeof status.generated_ms === "number" ? status.generated_ms : lastStatusAtMs; for (const profile of status.profiles) { @@ -900,8 +917,10 @@ export const make = Effect.gen(function* () { )} — this thread continues on it at ${formatResetClock(entry.notBefore)}.`, }; } - const expectedAt = entry.expectedAt ?? entry.notBefore ?? undefined; - const expectedProfile = entry.expectedProfile ?? entry.preferProfile ?? undefined; + // A reset wait whose time has passed says nothing about when room comes: + // only the last placement's answer does. + const expectedAt = entry.expectedAt; + const expectedProfile = entry.expectedProfile; const limitLine = entry.restricted ? "All Claude accounts you can use are at their limit." : "All Claude accounts are at their limit."; @@ -1257,8 +1276,10 @@ export const make = Effect.gen(function* () { // Sticky: an account the thread already ran on is kept until a hard // limit, a sign-out, an exhausted mark or its removal takes it out. - // Nearing a limit never moves an existing thread; only new placements - // avoid it. Losing access to the account re-places it too. + // Nearing a limit never moves a running thread here; only new + // placements avoid it, and the turn-boundary rebalance may move an idle + // thread off an over-loaded account (it writes the row this reads). + // Losing access to the account re-places it too. if ( row.resolvedProfile !== null && !disallowed.has(row.resolvedProfile) && @@ -1447,7 +1468,10 @@ export const make = Effect.gen(function* () { return true; }).pipe( Effect.catchCause((cause) => - Effect.as(Effect.logWarning("claude.account.queue-write-failed", { threadId, cause }), false), + Effect.as( + Effect.logWarning("claude.account.queue-write-failed", { threadId, cause }), + false, + ), ), ); @@ -1466,7 +1490,10 @@ export const make = Effect.gen(function* () { const dequeueQuietly = (threadId: ThreadId, why: string) => dequeue(threadId, why).pipe( Effect.catchCause((cause) => - Effect.as(Effect.logWarning("claude.account.queue-clear-failed", { threadId, cause }), true), + Effect.as( + Effect.logWarning("claude.account.queue-clear-failed", { threadId, cause }), + true, + ), ), ); @@ -1714,6 +1741,10 @@ export const make = Effect.gen(function* () { const moved = yield* reassign(threadId, { avoid: [profile] }).pipe(Effect.result); if (moved._tag === "Success" && moved.success !== undefined) { + // Copies of this rejection are caught as stale until the first turn + // starts on the new account; past that, the key would only swallow a + // real rejection if the thread came back to this account. + handledConditions.delete(conditionKey); notices.delete(threadId); movedAwaitingTurn.add(threadId); yield* stopSessionQuietly(threadId, "hard-limit"); @@ -1723,8 +1754,8 @@ export const make = Effect.gen(function* () { } // No account can take the thread now. It stops and waits in the queue; - // the revive pass continues it once an account has room. - handledConditions.delete(conditionKey); + // the revive pass after the next status poll continues it once an + // account has room. const failure = moved._tag === "Failure" ? moved.failure : undefined; const restriction = yield* disallowedFor(threadId).pipe( Effect.orElseSucceed(() => UNRESTRICTED), @@ -1749,8 +1780,9 @@ export const make = Effect.gen(function* () { yield* announce(threadId); return true; } + // While it waits, copies are caught by the queue check above. + handledConditions.delete(conditionKey); yield* stopSessionQuietly(threadId, "hard-limit-queued"); - yield* revivePass.pipe(Effect.forkIn(layerScope)); return true; }); @@ -1779,13 +1811,14 @@ export const make = Effect.gen(function* () { !restriction.disallowed.has(prefer) && (yield* usableAndPresent(prefer)) ) { - pendingPlacements.push({ profile: prefer, atMs: nowMs }); profile = prefer; } else { if (!anyRoom) return undefined; + // Counted as pending only once the thread is known to take it, below. const placed = yield* placeWith(threadId, entry.avoid ? [entry.avoid] : [], { purpose: "revive", requirePlaced: true, + commit: false, }).pipe(Effect.result); if (placed._tag === "Failure") { const { expectedAt, expectedProfile } = placed.failure; @@ -1812,8 +1845,16 @@ export const make = Effect.gen(function* () { profile = placed.success?.profile; } if (profile === undefined) return undefined; - yield* persistResolved(threadId, profile); - yield* dequeue(threadId, "revived"); + // The reads and the switcher call above yield: a message, a mode + // change or "Leave queue" may have taken the thread out meanwhile. + // `dequeue` removes it before its first yield, so only one side wins. + if (!(yield* dequeueQuietly(threadId, "revived"))) return undefined; + pendingPlacements.push({ profile, atMs: yield* Clock.currentTimeMillis }); + yield* persistResolved(threadId, profile).pipe( + Effect.catchCause((cause) => + Effect.logWarning("claude.account.persist-failed", { threadId, cause }), + ), + ); // Like a move: a rejection before the next turn starts is a stale copy. movedAwaitingTurn.add(threadId); yield* Effect.logInfo("claude.account.revived", { @@ -1870,25 +1911,32 @@ export const make = Effect.gen(function* () { * `REBALANCE_EXCESS` or more sessions above its fair share, or that can no * longer take new sessions, to where the switcher would place it now. The * thread loses its prompt cache once, so this runs at most once an hour per - * thread. Never fails: on any trouble the thread stays where it is. + * thread, and an account gives up at most one thread per status read (its + * session count is stale until the next one). The switcher is asked only + * when the last status shows another account with room the owner may use, + * and not again for five minutes after it found nothing better. Never + * fails: on any trouble the thread stays where it is. */ const maybeRebalance = (threadId: ThreadId, settled?: OrchestrationSession) => Effect.gen(function* () { if (waits.has(threadId) || pendingRestart.has(threadId)) return; if (movedAwaitingTurn.has(threadId)) return; const current = threadProfiles.get(threadId); - if (current === undefined) return; - const known = knownProfile(current); + if (current === undefined || rebalancedFrom.has(current)) return; + const status = lastStatus; + const known = status?.profiles.find((profile) => profile.name === current); // Only a switcher that reports fair shares spreads load; with an older one this never runs. - if (known === undefined || known.fair_share === undefined) return; + if (status === undefined || known === undefined || known.fair_share === undefined) return; const sessions = typeof known.sessions === "number" ? known.sessions : 0; const overShare = typeof known.fair_share === "number" && sessions - known.fair_share >= REBALANCE_EXCESS; const notPlaceable = known.fair_share === null || known.guard_ok === false; if (!overShare && !notPlaceable) return; const nowMs = yield* Clock.currentTimeMillis; - const last = lastRebalancedAt.get(threadId); - if (last !== undefined && nowMs - last < REBALANCE_COOLDOWN_MS) return; + const lastMove = lastRebalancedAt.get(threadId); + if (lastMove !== undefined && nowMs - lastMove < REBALANCE_COOLDOWN_MS) return; + const lastProbe = lastFailedProbeAt.get(threadId); + if (lastProbe !== undefined && nowMs - lastProbe < REBALANCE_PROBE_BACKOFF_MS) return; if (!(yield* readSettings).enabled) return; const row = yield* repository.get(threadId); if (row.mode.kind !== "auto" || row.resolvedProfile !== current) return; @@ -1896,21 +1944,47 @@ export const make = Effect.gen(function* () { (session) => session.threadId === threadId && session.status !== "closed", ); if (!live || !(yield* threadIsIdle(threadId, settled))) return; + // Without a subprocess: is there anywhere better at all? + yield* pruneExhausted; + const { disallowed } = yield* disallowedFor(threadId); + const pending = yield* pendingCounts; + const roomElsewhere = status.profiles.some( + (profile) => + profile.name !== current && + !disallowed.has(profile.name) && + !exhausted.has(profile.name) && + profile.auth?.state !== "logged_out" && + typeof profile.fair_share === "number" && + profile.fair_share > + (typeof profile.sessions === "number" ? profile.sessions : 0) + + (pending[profile.name] ?? 0), + ); + if (!roomElsewhere) return; const target = yield* Semaphore.withPermit(placementLock)( Effect.gen(function* () { - // A probe: nothing is persisted or counted unless the thread moves. + if (rebalancedFrom.has(current)) return undefined; + // A probe: nothing is written or counted unless the thread moves. const probe = yield* placeWith(threadId, [current], { purpose: "rebalance", requirePlaced: true, commit: false, }).pipe(Effect.option); - if (Option.isNone(probe) || probe.value === undefined) return undefined; - const to = probe.value.profile; - if (to === current) return undefined; + const to = + Option.isSome(probe) && probe.value !== undefined ? probe.value.profile : undefined; + if (to === undefined || to === current) { + lastFailedProbeAt.set(threadId, nowMs); + return undefined; + } + // The probe took time: a turn may have started since. + if (!(yield* threadIsIdle(threadId, settled))) return undefined; pendingPlacements.push({ profile: to, atMs: yield* Clock.currentTimeMillis }); lastRebalancedAt.set(threadId, nowMs); - yield* persistResolved(threadId, to); + lastFailedProbeAt.delete(threadId); + rebalancedFrom.add(current); + // Only the row: the thread still runs on `current` until its session + // stops, and the sticky branch adopts the row at the next spawn. + yield* repository.setResolved({ threadId, profile: to, resolvedAt: yield* nowIso }); return to; }), ); @@ -1922,6 +1996,7 @@ export const make = Effect.gen(function* () { reason: overShare ? "over-fair-share" : "not-placeable", }); yield* restartForNewAccount(threadId, "rebalance"); + yield* announce(threadId); yield* announceSnapshot; }).pipe( Effect.catchCause((cause) => @@ -1968,7 +2043,10 @@ export const make = Effect.gen(function* () { yield* announce(event.threadId); } // The first turn on the new account: from here a rejection is real. - if (movedAwaitingTurn.delete(event.threadId)) yield* announce(event.threadId); + if (movedAwaitingTurn.delete(event.threadId)) { + forgetConditions(event.threadId); + yield* announce(event.threadId); + } return; } case "turn.completed": @@ -2002,11 +2080,15 @@ export const make = Effect.gen(function* () { } if (event.type === "thread.session-set") { const session = event.payload.session; + const running = session.status === "running" && session.activeTurnId !== null; // T3-CUSTOM(expbkt3): native run/session projections drive account lifecycle. - if (session.status === "running" && session.activeTurnId !== null) { + if (running) { activeTurns.add(event.payload.threadId); - if (movedAwaitingTurn.delete(event.payload.threadId)) + projectedRunning.add(event.payload.threadId); + if (movedAwaitingTurn.delete(event.payload.threadId)) { + forgetConditions(event.payload.threadId); yield* announce(event.payload.threadId); + } } else if (session.status !== "starting") { activeTurns.delete(event.payload.threadId); } @@ -2014,17 +2096,29 @@ export const make = Effect.gen(function* () { session.activeTurnId === null && session.status !== "running" && session.status !== "starting"; + // Only the settle that ends a turn this projection saw running is a + // turn boundary; the many other session updates are not. + const turnEnded = settled && projectedRunning.delete(event.payload.threadId); if (settled && pendingRestart.has(event.payload.threadId)) { yield* completePendingRestart(event.payload.threadId, session); - } else if (settled) { + } else if (turnEnded) { yield* maybeRebalance(event.payload.threadId, session); } return; } + if (event.type === "thread.archived") { + // An archived thread is not continued; its account row stays for an unarchive. + if (yield* dequeueQuietly(event.payload.threadId, "thread-archived")) { + yield* announce(event.payload.threadId); + } + return; + } if (event.type !== "thread.deleted") return; const threadId = event.payload.threadId; yield* dequeueQuietly(threadId, "thread-deleted"); lastRebalancedAt.delete(threadId); + lastFailedProbeAt.delete(threadId); + projectedRunning.delete(threadId); threadProfiles.delete(threadId); notices.delete(threadId); pendingRestart.delete(threadId); diff --git a/apps/server/src/mcp/toolkits/webUi/catalog.test.ts b/apps/server/src/mcp/toolkits/webUi/catalog.test.ts index 53fa79edd0944..59910b6736479 100644 --- a/apps/server/src/mcp/toolkits/webUi/catalog.test.ts +++ b/apps/server/src/mcp/toolkits/webUi/catalog.test.ts @@ -54,7 +54,7 @@ const invocation = ( it("generates one unique virtual tool and complete schemas for every web RPC", () => { // T3-CUSTOM(expbkt3): the catalog includes native V2 methods and every retained fork RPC. - expect(WEB_UI_VIRTUAL_TOOL_COUNT).toBe(236); + expect(WEB_UI_VIRTUAL_TOOL_COUNT).toBe(237); expect(WEB_UI_STREAM_TOOL_COUNT).toBe(31); expect(WEB_UI_VIRTUAL_TOOL_COUNT).toBe(WsRpcGroup.requests.size); expect(new Set(WEB_UI_VIRTUAL_TOOLS.map((tool) => tool.name)).size).toBe( diff --git a/apps/server/src/mcp/toolkits/webUi/registration.test.ts b/apps/server/src/mcp/toolkits/webUi/registration.test.ts index 5230767cd41a0..966942d216c19 100644 --- a/apps/server/src/mcp/toolkits/webUi/registration.test.ts +++ b/apps/server/src/mcp/toolkits/webUi/registration.test.ts @@ -81,9 +81,9 @@ it.effect("registers four compact tools while listing the complete virtual surfa expect(listed.structuredContent).toMatchObject({ ok: true, // T3-CUSTOM(expbkt3): registration exposes native V2 methods and all retained fork RPCs. - rpcCount: 236, + rpcCount: 237, streamCount: 31, - matchedCount: 236, + matchedCount: 237, }); const schema = yield* withInvocation( diff --git a/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts b/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts index 8d22eeb1b0c80..d51c0e7395a7c 100644 --- a/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts +++ b/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts @@ -19,8 +19,9 @@ layer("055_OrchestrationV2", (it) => { // upstream's scheduled-task webhook migrations (57-58) at 1046-1047, the // review comment send-once mark at 1048 and upstream's MCP app context and // snapshot indexes (59-60) at 1049-1050, scheduled task custom - // groups at 1051 and scheduled task options at 1052. - ...Array.from({ length: 53 }, (_, index) => 1000 + index), + // groups at 1051, scheduled task options at 1052 and the Claude account + // revive queue at 1053. + ...Array.from({ length: 54 }, (_, index) => 1000 + index), ], ); }), diff --git a/docs/operations/claude-account-switching.md b/docs/operations/claude-account-switching.md index 0602ab9f11bfb..0fd36f92299fa 100644 --- a/docs/operations/claude-account-switching.md +++ b/docs/operations/claude-account-switching.md @@ -223,15 +223,18 @@ How it works when the flag is on: that was mid-turn gets the continue turn; an idle one only gets its account. The composer shows the thread's place and when the first account it may use should have room (from the switcher's `recoveries`). A message to the thread, a mode change or - `claudeAccounts.leaveQueue` takes it out; picking an account while it waited mid-turn + `claudeAccounts.leaveQueue` takes it out, and so does archiving it; picking an account while it waited mid-turn continues it there at once. A message still fails at start when nothing has room, with the account expected to free up first. - **Rebalance.** When an idle Auto thread's turn ends on an account that carries two or more sessions above its `fair_share`, or that the switcher reports as not placeable, the server asks `--place` where the thread would go now (avoiding its account). On a `placed` answer it moves the thread and restarts its idle session, at most once an hour - per thread (the thread loses its prompt cache once). Pinned threads never move. An - older switcher without `fair_share` disables this. + per thread (the thread loses its prompt cache once). It asks only when the projection + saw the turn run and settle, when the last status shows another account with room + under its `fair_share` that the owner may use, and not again for five minutes after a + probe found nothing better. An account gives up at most one thread per status read. + Pinned threads never move. An older switcher without `fair_share` disables this. - **Hard limit, pinned thread.** Nothing moves. The thread gets the notice " is at its usage limit until HH:MM UTC — switch to Auto or another account." (account-wide) or " has no allowance left until HH:MM UTC — switch the model or account." From 4ca0a20b4c453aeaa1df220e5255c44c3a8b256c Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Fri, 16 Oct 2026 14:42:55 +0000 Subject: [PATCH 16/18] fix(server): revive skips the continue when a turn already started; every turn start forgets stale limit keys Co-Authored-By: Claude Opus 5.5 --- .../claudeAccounts/ClaudeAccountsService.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts index 49937d9429792..1a84dbb552db5 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts @@ -783,7 +783,10 @@ export const make = Effect.gen(function* () { lastStatus = status; lastStatusAtMs = yield* Clock.currentTimeMillis; unavailableReason = undefined; - // Fresh session counts include every rebalance made before this read. + // One rebalance per source account per status read. A moved session + // whose restart was deferred is still counted in this read, so a second + // thread can leave the account one read early; the slack of 2 over the + // fair share absorbs that. rebalancedFrom.clear(); const generatedMs = typeof status.generated_ms === "number" ? status.generated_ms : lastStatusAtMs; @@ -1898,7 +1901,11 @@ export const make = Effect.gen(function* () { ), ); if (outcome === undefined) continue; - if (outcome.continueTurn) yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); + // A message accepted while this pass waited on a lock is already the + // next turn; a continue on top of it would be a second one. + if (outcome.continueTurn && (yield* threadIsIdle(threadId))) { + yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); + } yield* announce(threadId); } }), @@ -1976,7 +1983,9 @@ export const make = Effect.gen(function* () { lastFailedProbeAt.set(threadId, nowMs); return undefined; } - // The probe took time: a turn may have started since. + // The probe took time: a turn may have reached the provider since. A + // message the shell accepted but has not sent yet is caught later: + // `restartForNewAccount` re-reads the shell and defers the restart. if (!(yield* threadIsIdle(threadId, settled))) return undefined; pendingPlacements.push({ profile: to, atMs: yield* Clock.currentTimeMillis }); lastRebalancedAt.set(threadId, nowMs); @@ -2042,9 +2051,12 @@ export const make = Effect.gen(function* () { if (yield* dequeueQuietly(event.threadId, "turn-started")) { yield* announce(event.threadId); } + // Copies of one rejection all arrive before the next turn starts, so a + // new turn forgets them; a key kept after a failed park would + // otherwise swallow every later rejection with the same reset. + forgetConditions(event.threadId); // The first turn on the new account: from here a rejection is real. if (movedAwaitingTurn.delete(event.threadId)) { - forgetConditions(event.threadId); yield* announce(event.threadId); } return; From 34d6ad4b5e414e14175d940d610e7ba73cc3f098 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Fri, 16 Oct 2026 14:51:45 +0000 Subject: [PATCH 17/18] fix(server): a revive skips its continue only when the shell shows a turn in flight The provider-level idle check also counted the failed turn that parked the thread, so a mid-turn thread never got its continue. Test for the race. Co-Authored-By: Claude Opus 5.5 --- .../ClaudeAccountsService.test.ts | 28 +++++++++++++++++++ .../claudeAccounts/ClaudeAccountsService.ts | 20 +++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts index 0524afa95c9d2..6a59576d32be2 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.test.ts @@ -2265,6 +2265,34 @@ describe("ClaudeAccountsService revive queue", () => { ); } + { + const gate = gatedPlace(); + scenario( + "revives without a continue when the shell already shows the next turn", + { place: gate.place }, + (harness, service) => + Effect.gen(function* () { + yield* service.refreshStatus(); + const threadId = ThreadId.make("thread-queue-raced"); + gate.state.open = ["tushar"]; + yield* service.resolveForSession(threadId); + gate.state.open = []; + yield* markTurnRunning(harness, service, threadId); + yield* service.handleHardLimit(fiveHourAt(threadId, farReset)); + assert.equal((yield* service.getThread(threadId)).wait?.wasActive, true); + + // The person's message was accepted before the revive pass ran. + yield* Ref.set(harness.shellSession, orchestrationSession(threadId, "running", "turn-2")); + gate.state.open = ["agent"]; + yield* service.refreshStatus(); + const view = yield* service.getThread(threadId); + assert.equal(view.wait, undefined); + assert.equal(view.resolvedProfile, "agent"); + assert.equal(harness.dispatched.length, 0); + }), + ); + } + { const gate = gatedPlace(); scenario( diff --git a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts index 1a84dbb552db5..78c11b3fad3aa 100644 --- a/apps/server/src/claudeAccounts/ClaudeAccountsService.ts +++ b/apps/server/src/claudeAccounts/ClaudeAccountsService.ts @@ -1589,6 +1589,21 @@ export const make = Effect.gen(function* () { */ const CONTINUE_TEXT = "Continue where you left off."; + /** Whether the thread's shell shows a turn accepted or running. */ + const shellTurnInFlight = (threadId: ThreadId) => + query.getThreadShellById(threadId).pipe( + Effect.map((shell) => { + const session = Option.isSome(shell) ? shell.value.session : null; + return ( + session !== null && + (session.activeTurnId !== null || + session.status === "running" || + session.status === "starting") + ); + }), + Effect.orElseSucceed(() => false), + ); + const dispatchContinueTurn = (threadId: ThreadId, text: string) => Effect.gen(function* () { const shell = yield* query.getThreadShellById(threadId); @@ -1902,8 +1917,9 @@ export const make = Effect.gen(function* () { ); if (outcome === undefined) continue; // A message accepted while this pass waited on a lock is already the - // next turn; a continue on top of it would be a second one. - if (outcome.continueTurn && (yield* threadIsIdle(threadId))) { + // next turn; a continue on top of it would be a second one. The shell + // marks a turn active as soon as its command is accepted. + if (outcome.continueTurn && !(yield* shellTurnInFlight(threadId))) { yield* dispatchContinueTurn(threadId, CONTINUE_TEXT); } yield* announce(threadId); From 5afbacbb0126e76e659213b74db18d59cc5ab609 Mon Sep 17 00:00:00 2001 From: bk-agent-01 Date: Mon, 19 Oct 2026 10:49:08 +0000 Subject: [PATCH 18/18] fix(server): stop the pull-request sweep and plan-flag reads from freezing bkt3 The minute pull-request sweep checked all 1,100 unsettled threads on bkt3. Each check starts 6-7 git processes and every start forks the 1.4 GB server on the event loop, so a sweep ran 50-95 s of every two minutes with the main thread saturated (55% of samples in uv_spawn). Replies took 504 ms median during a sweep against 2 ms outside it, and desktop connections hit their 15 s setup limit. A thread with no run or message for a day is now checked once every 30 passes; its own events still queue an immediate check. The agent toolkit's plan-flag correction read a run's turn items with an `IN (...) OR (...)` filter, so SQLite walked every item of the thread in ordinal order: 0.3-2.6 s per cold 30,000-item thread, measured as a 2.5 s event-loop stall at 10:32 UTC. Run filters now read through the (thread_id, run_id) index: 4 ms median on the same cold threads. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/ProjectionStore.ts | 17 +- .../ThreadPullRequestService.ts | 15 ++ .../threadPullRequestSweep.expbkt3.test.ts | 196 ++++++++++++++++++ .../threadPullRequestSweep.expbkt3.ts | 57 +++++ 4 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.test.ts create mode 100644 apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.ts diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index ccd482284336c..682b7d64e5d12 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -2728,18 +2728,31 @@ export const layer: Layer.Layer = return yield* new ProjectionStoreThreadNotFoundError({ threadId }); } + // T3-CUSTOM(expbkt3): a run filter reads only that run's items through the + // (thread_id, run_id) index. In ordinal order SQLite walked every item of the + // thread instead, which took 0.3-2.6 s on a cold 30,000-item thread and froze + // the server (agent toolkit plan-flag checks run it for up to 40 sessions). + const turnItemRunIdsWithoutNull = + filter?.turnItemRunIds === undefined || filter.turnItemRunIds.includes(null) + ? undefined + : filter.turnItemRunIds.filter((id): id is RunId => id !== null); + const turnItemRunIndex = + filter?.turnItemRunId !== undefined || turnItemRunIdsWithoutNull !== undefined + ? sql`INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx` + : sql``; + const boundedTurnItemRows = fields !== undefined && !fields.includes("turnItems") ? [] : window === undefined ? yield* sql` SELECT payload_json - FROM orchestration_v2_projection_turn_items + FROM orchestration_v2_projection_turn_items ${turnItemRunIndex} /* T3-CUSTOM(expbkt3): run filters use the run index. */ WHERE thread_id = ${threadId} ${filter?.turnItemTypes === undefined ? sql`` : sql`AND type IN (SELECT value FROM json_each(${encodeIdList(filter.turnItemTypes)}))`} ${filter?.turnItemStatuses === undefined ? sql`` : sql`AND status IN (SELECT value FROM json_each(${encodeIdList(filter.turnItemStatuses)}))`} ${filter?.turnItemRunId === undefined ? sql`` : sql`AND run_id = ${filter.turnItemRunId}`} - ${filter?.turnItemRunIds === undefined ? sql`` : sql`AND (run_id IN (SELECT value FROM json_each(${encodeIdList(filter.turnItemRunIds.filter((id): id is RunId => id !== null))})) OR (${filter.turnItemRunIds.includes(null) ? 1 : 0} = 1 AND run_id IS NULL))`} + ${filter?.turnItemRunIds === undefined ? sql`` : turnItemRunIdsWithoutNull !== undefined ? sql`AND run_id IN (SELECT value FROM json_each(${encodeIdList(turnItemRunIdsWithoutNull)}))` : sql`AND (run_id IN (SELECT value FROM json_each(${encodeIdList(filter.turnItemRunIds.filter((id): id is RunId => id !== null))})) OR (${filter.turnItemRunIds.includes(null) ? 1 : 0} = 1 AND run_id IS NULL))`} /* T3-CUSTOM(expbkt3): an OR here defeats the run index. */ ORDER BY ordinal ASC, turn_item_id ASC ` : yield* sql` diff --git a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts index cd88d93977768..a1dabb267b2f8 100644 --- a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts +++ b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts @@ -12,6 +12,8 @@ import { } from "@t3tools/contracts"; import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker"; import * as Cause from "effect/Cause"; +// T3-CUSTOM(expbkt3): the idle-thread rotation reads the clock. +import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; @@ -28,6 +30,8 @@ import * as PullRequestService from "../pullRequest/PullRequestService.ts"; import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; import { forkParked } from "../serverActivation.ts"; import * as Orchestrator from "./Orchestrator.ts"; +// T3-CUSTOM(expbkt3): idle threads ride a slow rotation in the periodic sweep. +import { includeInPeriodicSweep } from "./threadPullRequestSweep.expbkt3.ts"; class ThreadPullRequestServiceV2 extends Context.Service< ThreadPullRequestServiceV2, @@ -106,6 +110,8 @@ export const make = Effect.gen(function* () { const crypto = yield* Crypto.Crypto; const fileSystem = yield* FileSystem.FileSystem; const pendingBackfill = new Map(); + // T3-CUSTOM(expbkt3): counts all-thread passes for the idle-thread rotation. + let periodicPass = 0; const finishBackfill = (threads: ReadonlyArray>) => { for (const thread of threads) pendingBackfill.delete(thread.id); @@ -167,8 +173,17 @@ export const make = Effect.gen(function* () { for (const threadId of checkedIds) { if (!visibleThreadIds.has(threadId)) pendingBackfill.delete(threadId); } + // T3-CUSTOM(expbkt3): BEGIN — an all-thread pass checks idle threads on a rotation. + const nowMs = yield* Clock.currentTimeMillis; + const pass = periodicPass; + if (request.threadId === null) periodicPass += 1; + // T3-CUSTOM(expbkt3): END const threads = threadSnapshot.threads.filter( (thread) => + // T3-CUSTOM(expbkt3): a one-thread check and backfill are never rotated. + (request.threadId !== null || + pendingBackfill.has(thread.id) || + includeInPeriodicSweep(thread, nowMs, pass)) && thread.archivedAt === null && ((thread.settledOverride !== "settled" && thread.settledAt === null) || request.threadId !== null || diff --git a/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.test.ts b/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.test.ts new file mode 100644 index 0000000000000..cd5fda6ed57d2 --- /dev/null +++ b/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.test.ts @@ -0,0 +1,196 @@ +import { + ProjectId, + ProviderInstanceId, + ThreadId, + type OrchestrationProjectShell, + type OrchestrationV2ThreadShell, +} from "@t3tools/contracts"; +import { describe, expect, it } from "@effect/vitest"; +import * as Crypto from "effect/Crypto"; +import * as DateTime from "effect/DateTime"; +import * as Deferred from "effect/Deferred"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; +import * as Queue from "effect/Queue"; +import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; + +import * as GitManager from "../git/GitManager.ts"; +import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; +import * as PullRequestService from "../pullRequest/PullRequestService.ts"; +import * as ServerActivation from "../serverActivation.ts"; +import * as Orchestrator from "./Orchestrator.ts"; +import * as ProjectStore from "./ProjectStore.ts"; +import * as ThreadPullRequestService from "./ThreadPullRequestService.ts"; +import { + IDLE_SWEEP_EVERY_PASSES, + idleSweepSlot, + includeInPeriodicSweep, + isIdleForSweep, +} from "./threadPullRequestSweep.expbkt3.ts"; + +const NOW = DateTime.makeUnsafe("2026-10-19T10:00:00.000Z"); +const NOW_MS = DateTime.toEpochMillis(NOW); +const hoursAgo = (hours: number) => DateTime.makeUnsafe(NOW_MS - hours * 60 * 60 * 1000); + +const threadShell = ( + id: string, + overrides: Partial = {}, +): OrchestrationV2ThreadShell => { + const threadId = ThreadId.make(id); + return { + id: threadId, + projectId: ProjectId.make(`project-${id}`), + title: id, + providerInstanceId: ProviderInstanceId.make("codex"), + modelSelection: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-5.4" }, + runtimeMode: "full-access", + interactionMode: "default", + branch: `feature/${id}`, + worktreePath: null, + activeProviderThreadId: null, + lineage: { rootThreadId: threadId, parentThreadId: null, relationshipToParent: null }, + forkedFrom: null, + createdBy: "user", + creationSource: "web", + activeRunId: null, + latestRunId: null, + status: "idle", + pendingRuntimeRequest: null, + latestVisibleMessage: null, + latestUserMessageAt: null, + hasActionableProposedPlan: false, + itemCount: 0, + visibleItemCount: 0, + createdAt: hoursAgo(24 * 30), + updatedAt: NOW, + archivedAt: null, + settledOverride: null, + settledAt: null, + lastVisitedAt: null, + deletedAt: null, + ...overrides, + }; +}; + +describe("periodic pull-request sweep rotation", () => { + it("treats a thread as idle only after a day with no run, message or live work", () => { + expect(isIdleForSweep(threadShell("old", { latestRunCompletedAt: hoursAgo(25) }), NOW_MS)).toBe( + true, + ); + expect( + isIdleForSweep(threadShell("recent", { latestRunCompletedAt: hoursAgo(2) }), NOW_MS), + ).toBe(false); + expect( + isIdleForSweep(threadShell("messaged", { latestUserMessageAt: hoursAgo(1) }), NOW_MS), + ).toBe(false); + expect(isIdleForSweep(threadShell("new", { createdAt: hoursAgo(3) }), NOW_MS)).toBe(false); + expect( + isIdleForSweep( + threadShell("waiting", { + latestRunCompletedAt: hoursAgo(48), + activityRunStatus: "waiting", + }), + NOW_MS, + ), + ).toBe(false); + }); + + it("checks every idle thread exactly once per rotation and active threads on every pass", () => { + const idle = Array.from({ length: 200 }, (_, index) => + threadShell(`idle-${index}`, { latestRunCompletedAt: hoursAgo(72) }), + ); + const active = threadShell("active", { latestRunCompletedAt: hoursAgo(1) }); + const checks = new Map(); + for (let pass = 0; pass < IDLE_SWEEP_EVERY_PASSES; pass += 1) { + for (const thread of [...idle, active]) { + if (includeInPeriodicSweep(thread, NOW_MS, pass)) { + checks.set(thread.id, (checks.get(thread.id) ?? 0) + 1); + } + } + } + expect(idle.every((thread) => checks.get(thread.id) === 1)).toBe(true); + expect(checks.get(active.id)).toBe(IDLE_SWEEP_EVERY_PASSES); + // The slots spread the idle threads instead of bunching them on one pass. + const perSlot = new Set(idle.map((thread) => idleSweepSlot(thread.id))); + expect(perSlot.size).toBeGreaterThan(IDLE_SWEEP_EVERY_PASSES / 2); + }); + + it.effect( + "the service looks up an idle thread once per rotation and an active one every pass", + () => + Effect.scoped( + Effect.gen(function* () { + const idle = threadShell("idle", { latestRunCompletedAt: hoursAgo(72) }); + const active = threadShell("active", { latestRunCompletedAt: hoursAgo(1) }); + const project = (thread: OrchestrationV2ThreadShell): OrchestrationProjectShell => ({ + id: thread.projectId, + title: thread.title, + workspaceRoot: `/workspace/${thread.id}`, + defaultModelSelection: null, + scripts: [], + repositoryIdentity: null, + createdAt: "2026-08-01T00:00:00.000Z", + updatedAt: "2026-08-01T00:00:00.000Z", + }); + const activation = yield* Deferred.make(); + const passes = yield* Queue.unbounded(); + const lookups = new Map(); + const layerDependencies = Layer.mergeAll( + Layer.mock(Orchestrator.OrchestratorV2)({ + streamDomainEvents: Stream.never, + getShellSnapshot: () => + Queue.offer(passes, undefined).pipe( + Effect.as({ + schemaVersion: 2, + snapshotSequence: 1, + threads: [idle, active], + archivedThreads: [], + }), + ), + }), + Layer.mock(ProjectStore.ProjectStoreV2)({ + listShells: () => Effect.succeed([project(idle), project(active)]), + }), + Layer.mock(GitManager.GitManager)({}), + Layer.mock(PullRequestService.PullRequestService)({}), + // No repository identity: the group ends right after this lookup. + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ + resolve: (root) => + Effect.sync(() => { + lookups.set(root, (lookups.get(root) ?? 0) + 1); + return null; + }), + }), + Layer.succeed(ServerActivation.ServerActivation, Deferred.await(activation)), + Layer.succeed( + Crypto.Crypto, + Crypto.make({ + randomBytes: (size) => new Uint8Array(size).fill(1), + digest: (_algorithm, data) => Effect.succeed(data), + }), + ), + FileSystem.layerNoop({}), + ); + + yield* Effect.gen(function* () { + yield* TestClock.setTime(NOW_MS); + const service = yield* ThreadPullRequestService.make; + yield* service.start(); + yield* Deferred.succeed(activation, undefined); + // The startup pass, then one pass a minute. + yield* Queue.take(passes); + yield* service.drain; + for (let pass = 1; pass < IDLE_SWEEP_EVERY_PASSES; pass += 1) { + yield* TestClock.adjust("1 minute"); + yield* Queue.take(passes); + yield* service.drain; + } + expect(lookups.get("/workspace/active")).toBe(IDLE_SWEEP_EVERY_PASSES); + expect(lookups.get("/workspace/idle")).toBe(1); + }).pipe(Effect.provide(layerDependencies)); + }), + ), + ); +}); diff --git a/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.ts b/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.ts new file mode 100644 index 0000000000000..b2014a0f10c30 --- /dev/null +++ b/apps/server/src/orchestration-v2/threadPullRequestSweep.expbkt3.ts @@ -0,0 +1,57 @@ +/** + * T3-CUSTOM(expbkt3): which threads a periodic pull-request sweep checks. + * + * Each check starts several git processes, and every start forks the whole + * server process on the event loop. With 1,100 unsettled threads the sweep ran + * for 50-95 s of every two minutes and kept the loop saturated, so clients + * timed out while connecting. A thread with no run or message for a day is + * now checked on a rotation, once every IDLE_SWEEP_EVERY_PASSES all-thread + * passes. Its own events (a run ending, a checkpoint, an unsettle) still queue + * an immediate one-thread check, so activity is never waited on. + */ +import type { OrchestrationV2ThreadShell } from "@t3tools/contracts"; +import * as DateTime from "effect/DateTime"; + +export const IDLE_SWEEP_AFTER_MS = 24 * 60 * 60 * 1000; +export const IDLE_SWEEP_EVERY_PASSES = 30; + +type SweepThread = Pick< + OrchestrationV2ThreadShell, + | "id" + | "activeRunId" + | "activityRunStatus" + | "createdAt" + | "latestUserMessageAt" + | "latestRunRequestedAt" + | "latestRunStartedAt" + | "latestRunCompletedAt" +>; + +const latestActivityMs = (thread: SweepThread) => + Math.max( + ...[ + thread.createdAt, + thread.latestUserMessageAt, + thread.latestRunRequestedAt, + thread.latestRunStartedAt, + thread.latestRunCompletedAt, + ].map((at) => (at == null ? Number.NEGATIVE_INFINITY : DateTime.toEpochMillis(at))), + ); + +export const isIdleForSweep = (thread: SweepThread, nowMs: number) => + thread.activeRunId === null && + thread.activityRunStatus == null && + nowMs - latestActivityMs(thread) >= IDLE_SWEEP_AFTER_MS; + +/** The pass, modulo IDLE_SWEEP_EVERY_PASSES, on which an idle thread is checked. */ +export const idleSweepSlot = (threadId: string) => { + let hash = 0; + for (let index = 0; index < threadId.length; index += 1) { + hash = (Math.imul(hash, 31) + threadId.charCodeAt(index)) >>> 0; + } + return hash % IDLE_SWEEP_EVERY_PASSES; +}; + +/** Whether all-thread pass number `pass` checks this thread. */ +export const includeInPeriodicSweep = (thread: SweepThread, nowMs: number, pass: number) => + !isIdleForSweep(thread, nowMs) || idleSweepSlot(thread.id) === pass % IDLE_SWEEP_EVERY_PASSES;