From 84cb13412ebebd6533e8a3de1579c40ae001573e Mon Sep 17 00:00:00 2001 From: Hasan Shoaib <142208891+realhasanshoaib@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:21:47 +0400 Subject: [PATCH] fix(server): failed threads no longer scan every tool item for errors (#18628) --- .../orchestration-v2/ProjectionSettlement.test.ts | 2 +- .../src/orchestration-v2/ProjectionStore.test.ts | 9 +++++++++ .../src/orchestration-v2/ProjectionStore.ts | 2 -- apps/server/src/persistence/Migrations.ts | 2 ++ .../Migrations/055_OrchestrationV2.test.ts | 4 +++- .../Migrations/062_ThreadFailureLookupIndex.ts | 15 +++++++++++++++ .../reconcileV2PreviewMigration.test.ts | 2 ++ 7 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/persistence/Migrations/062_ThreadFailureLookupIndex.ts diff --git a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts index f626d0c385834605192736b3ef43c2c9adac5095..8dcb13ceda9e98fba4928bb9fc67fb123ef8fe92 100644 --- a/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionSettlement.test.ts @@ -520,7 +520,7 @@ it.effect("shell failure lookups stay on the thread's own turn items", () => // node_id, so a node_ordinal lookup walks the whole history once per thread. const itemLookups = plan.filter((row) => row.detail.startsWith("SEARCH item ")); assert.lengthOf(itemLookups, 2); - assert.isTrue(itemLookups.every((row) => row.detail.includes("turn_items_thread_run_idx"))); + assert.isTrue(itemLookups.every((row) => row.detail.includes("turn_items_failed_error_idx"))); // The pending secret request lookup is bounded the same way. const secretLookups = plan.filter((row) => row.detail.startsWith("SEARCH secret ")); assert.lengthOf(secretLookups, 1); diff --git a/apps/server/src/orchestration-v2/ProjectionStore.test.ts b/apps/server/src/orchestration-v2/ProjectionStore.test.ts index bda7b896e1876a0c420a8f7e552ca249ef484ce8..066ff7def15dda2529ea2a7da59291a94bbc4227 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.test.ts @@ -3426,6 +3426,15 @@ it.layer(layerTest)("ProjectionStoreV2", (it) => { ); assert.isAbove(sessionLookup, bindingLookup, marker); assert.include(plan[sessionLookup], "(provider_session_id=?)"); + const errorLookups = plan.filter( + (detail) => + /^SEARCH (?:item|error) /.test(detail) && !detail.includes("(turn_item_id=?)"), + ); + assert.lengthOf(errorLookups, marker.includes("blocking") ? 2 : 1, marker); + for (const errorLookup of errorLookups) { + assert.include(errorLookup, "turn_items_failed_error_idx"); + assert.include(errorLookup, "(thread_id=? AND run_id=? AND node_id=?)"); + } } }), ); diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 4f10a0512fd53506168fa972ce060426dc9c12f7..f3348438352703cb915ede612937c048ee776eeb 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -5214,7 +5214,6 @@ export const layer: Layer.Layer = ( SELECT item.payload_json FROM orchestration_v2_projection_turn_items item - INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx INNER JOIN orchestration_v2_projection_runs r ON r.run_id = item.run_id WHERE r.run_id = presented.run_id AND r.status = 'failed' @@ -5231,7 +5230,6 @@ export const layer: Layer.Layer = ( SELECT item.payload_json FROM orchestration_v2_projection_turn_items item - INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx WHERE item.thread_id = t.thread_id AND item.run_id = blocked.run_id AND item.type = 'error' AND item.status = 'failed' diff --git a/apps/server/src/persistence/Migrations.ts b/apps/server/src/persistence/Migrations.ts index 69c057cae666b06719744386f2a5298bd68d8e4e..1792046effbfc968022b7ec3c0b8cadd5dbfeb37 100644 --- a/apps/server/src/persistence/Migrations.ts +++ b/apps/server/src/persistence/Migrations.ts @@ -75,6 +75,7 @@ import Migration0058 from "./Migrations/058_WebhookRelayDeliveries.ts"; import Migration0059 from "./Migrations/059_McpAppModelContext.ts"; import Migration0060 from "./Migrations/060_ThreadSnapshotWindowIndexes.ts"; import Migration0061 from "./Migrations/061_PullRequestReviewGuides.ts"; +import Migration0062 from "./Migrations/062_ThreadFailureLookupIndex.ts"; /** * Migration loader with all migrations defined inline. @@ -150,6 +151,7 @@ export const migrationEntries = [ [59, "McpAppModelContext", Migration0059], [60, "ThreadSnapshotWindowIndexes", Migration0060], [61, "PullRequestReviewGuides", Migration0061], + [62, "ThreadFailureLookupIndex", Migration0062], ] as const; export const migrationManifest = migrationEntries.map(([id, name]) => [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 7d5c845cbf22aa8605cd24c96f8d38057af6dddc..237e8daf3748256601f1fca4db6a9d8e3f1a1330 100644 --- a/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts +++ b/apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts @@ -13,7 +13,7 @@ layer("055_OrchestrationV2", (it) => { Effect.sync(() => { assert.deepStrictEqual( migrationEntries.map(([id]) => id), - Array.from({ length: 61 }, (_, index) => index + 1), + Array.from({ length: 62 }, (_, index) => index + 1), ); }), ); @@ -33,6 +33,7 @@ layer("055_OrchestrationV2", (it) => { [59, "McpAppModelContext"], [60, "ThreadSnapshotWindowIndexes"], [61, "PullRequestReviewGuides"], + [62, "ThreadFailureLookupIndex"], ]); assert.deepStrictEqual(yield* runMigrations(), []); @@ -60,6 +61,7 @@ layer("055_OrchestrationV2", (it) => { { migration_id: 59, name: "McpAppModelContext" }, { migration_id: 60, name: "ThreadSnapshotWindowIndexes" }, { migration_id: 61, name: "PullRequestReviewGuides" }, + { migration_id: 62, name: "ThreadFailureLookupIndex" }, ]); const tables = yield* sql<{ readonly name: string }>` diff --git a/apps/server/src/persistence/Migrations/062_ThreadFailureLookupIndex.ts b/apps/server/src/persistence/Migrations/062_ThreadFailureLookupIndex.ts new file mode 100644 index 0000000000000000000000000000000000000000..8e9ea5c8c0522468a8fd68a71672710557597abc --- /dev/null +++ b/apps/server/src/persistence/Migrations/062_ThreadFailureLookupIndex.ts @@ -0,0 +1,15 @@ +import * as Effect from "effect/Effect"; +import * as SqlClient from "effect/sql/SqlClient"; + +export default Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + + // Shell and limit-recovery reads need one root error, not every item in a run. + yield* sql` + CREATE INDEX orchestration_v2_projection_turn_items_failed_error_idx + ON orchestration_v2_projection_turn_items( + thread_id, run_id, node_id, updated_at DESC, ordinal DESC, turn_item_id DESC + ) + WHERE type = 'error' AND status = 'failed' + `; +}); diff --git a/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts b/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts index 5881fdfda408b6c993c18fea6ac2f7fcebd7bf84..684520bce168abcbdaf929097d3f6a21bbc2bfca 100644 --- a/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts +++ b/apps/server/src/persistence/reconcileV2PreviewMigration.test.ts @@ -42,6 +42,7 @@ describe("V2 preview upgrade", () => { [59, "McpAppModelContext"], [60, "ThreadSnapshotWindowIndexes"], [61, "PullRequestReviewGuides"], + [62, "ThreadFailureLookupIndex"], ]); assert.deepStrictEqual(yield* runMigrations(), []); assert.deepStrictEqual(yield* sql`SELECT * FROM orchestration_v2_legacy_imports`, imports); @@ -126,6 +127,7 @@ describe("V2 preview upgrade", () => { [59, "McpAppModelContext"], [60, "ThreadSnapshotWindowIndexes"], [61, "PullRequestReviewGuides"], + [62, "ThreadFailureLookupIndex"], ]); }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))), ); -- 2.54.0