authorgravatar for 142208891+realhasanshoaib@users.noreply.github.comHasan Shoaib <142208891+realhasanshoaib@users.noreply.github.com> 2026-10-14 00:21:47+04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-13 13:21:47-07:00
log84cb13412ebebd6533e8a3de1579c40ae001573e
treeda12d92875727dce27b0e1988b739cc26c222097
parentfb8d0fc19c27ff860bdefb7dcfe69eae546f837c
signature Signed by PGP key B5690EEEBB952194

fix(server): failed threads no longer scan every tool item for errors (#18628)

7 files changed, 32 insertions(+), 4 deletions(-)

apps/server/src/orchestration-v2/ProjectionSettlement.test.ts+1-1
...@@ -520,7 +520,7 @@ it.effect("shell failure lookups stay on the thread's own turn items", () =>...@@ -520,7 +520,7 @@ it.effect("shell failure lookups stay on the thread's own turn items", () =>
520 // node_id, so a node_ordinal lookup walks the whole history once per thread.520 // node_id, so a node_ordinal lookup walks the whole history once per thread.
521 const itemLookups = plan.filter((row) => row.detail.startsWith("SEARCH item "));521 const itemLookups = plan.filter((row) => row.detail.startsWith("SEARCH item "));
522 assert.lengthOf(itemLookups, 2);522 assert.lengthOf(itemLookups, 2);
523 assert.isTrue(itemLookups.every((row) => row.detail.includes("turn_items_thread_run_idx")));523 assert.isTrue(itemLookups.every((row) => row.detail.includes("turn_items_failed_error_idx")));
524 // The pending secret request lookup is bounded the same way.524 // The pending secret request lookup is bounded the same way.
525 const secretLookups = plan.filter((row) => row.detail.startsWith("SEARCH secret "));525 const secretLookups = plan.filter((row) => row.detail.startsWith("SEARCH secret "));
526 assert.lengthOf(secretLookups, 1);526 assert.lengthOf(secretLookups, 1);
apps/server/src/orchestration-v2/ProjectionStore.test.ts+9
...@@ -3426,6 +3426,15 @@ it.layer(layerTest)("ProjectionStoreV2", (it) => {...@@ -3426,6 +3426,15 @@ it.layer(layerTest)("ProjectionStoreV2", (it) => {
3426 );3426 );
3427 assert.isAbove(sessionLookup, bindingLookup, marker);3427 assert.isAbove(sessionLookup, bindingLookup, marker);
3428 assert.include(plan[sessionLookup], "(provider_session_id=?)");3428 assert.include(plan[sessionLookup], "(provider_session_id=?)");
3429 const errorLookups = plan.filter(
3430 (detail) =>
3431 /^SEARCH (?:item|error) /.test(detail) && !detail.includes("(turn_item_id=?)"),
3432 );
3433 assert.lengthOf(errorLookups, marker.includes("blocking") ? 2 : 1, marker);
3434 for (const errorLookup of errorLookups) {
3435 assert.include(errorLookup, "turn_items_failed_error_idx");
3436 assert.include(errorLookup, "(thread_id=? AND run_id=? AND node_id=?)");
3437 }
3429 }3438 }
3430 }),3439 }),
3431 );3440 );
apps/server/src/orchestration-v2/ProjectionStore.ts-2
...@@ -5214,7 +5214,6 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =...@@ -5214,7 +5214,6 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
5214 (5214 (
5215 SELECT item.payload_json5215 SELECT item.payload_json
5216 FROM orchestration_v2_projection_turn_items item5216 FROM orchestration_v2_projection_turn_items item
5217 INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx
5218 INNER JOIN orchestration_v2_projection_runs r ON r.run_id = item.run_id5217 INNER JOIN orchestration_v2_projection_runs r ON r.run_id = item.run_id
5219 WHERE r.run_id = presented.run_id5218 WHERE r.run_id = presented.run_id
5220 AND r.status = 'failed'5219 AND r.status = 'failed'
...@@ -5231,7 +5230,6 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =...@@ -5231,7 +5230,6 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
5231 (5230 (
5232 SELECT item.payload_json5231 SELECT item.payload_json
5233 FROM orchestration_v2_projection_turn_items item5232 FROM orchestration_v2_projection_turn_items item
5234 INDEXED BY orchestration_v2_projection_turn_items_thread_run_idx
5235 WHERE item.thread_id = t.thread_id5233 WHERE item.thread_id = t.thread_id
5236 AND item.run_id = blocked.run_id5234 AND item.run_id = blocked.run_id
5237 AND item.type = 'error' AND item.status = 'failed'5235 AND item.type = 'error' AND item.status = 'failed'
apps/server/src/persistence/Migrations.ts+2
...@@ -75,6 +75,7 @@ import Migration0058 from "./Migrations/058_WebhookRelayDeliveries.ts";...@@ -75,6 +75,7 @@ import Migration0058 from "./Migrations/058_WebhookRelayDeliveries.ts";
75import Migration0059 from "./Migrations/059_McpAppModelContext.ts";75import Migration0059 from "./Migrations/059_McpAppModelContext.ts";
76import Migration0060 from "./Migrations/060_ThreadSnapshotWindowIndexes.ts";76import Migration0060 from "./Migrations/060_ThreadSnapshotWindowIndexes.ts";
77import Migration0061 from "./Migrations/061_PullRequestReviewGuides.ts";77import Migration0061 from "./Migrations/061_PullRequestReviewGuides.ts";
78import Migration0062 from "./Migrations/062_ThreadFailureLookupIndex.ts";
7879
79/**80/**
80 * Migration loader with all migrations defined inline.81 * Migration loader with all migrations defined inline.
...@@ -150,6 +151,7 @@ export const migrationEntries = [...@@ -150,6 +151,7 @@ export const migrationEntries = [
150 [59, "McpAppModelContext", Migration0059],151 [59, "McpAppModelContext", Migration0059],
151 [60, "ThreadSnapshotWindowIndexes", Migration0060],152 [60, "ThreadSnapshotWindowIndexes", Migration0060],
152 [61, "PullRequestReviewGuides", Migration0061],153 [61, "PullRequestReviewGuides", Migration0061],
154 [62, "ThreadFailureLookupIndex", Migration0062],
153] as const;155] as const;
154156
155export const migrationManifest = migrationEntries.map(([id, name]) => [id, name] as const);157export const migrationManifest = migrationEntries.map(([id, name]) => [id, name] as const);
apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts+3-1
...@@ -13,7 +13,7 @@ layer("055_OrchestrationV2", (it) => {...@@ -13,7 +13,7 @@ layer("055_OrchestrationV2", (it) => {
13 Effect.sync(() => {13 Effect.sync(() => {
14 assert.deepStrictEqual(14 assert.deepStrictEqual(
15 migrationEntries.map(([id]) => id),15 migrationEntries.map(([id]) => id),
16 Array.from({ length: 61 }, (_, index) => index + 1),16 Array.from({ length: 62 }, (_, index) => index + 1),
17 );17 );
18 }),18 }),
19 );19 );
...@@ -33,6 +33,7 @@ layer("055_OrchestrationV2", (it) => {...@@ -33,6 +33,7 @@ layer("055_OrchestrationV2", (it) => {
33 [59, "McpAppModelContext"],33 [59, "McpAppModelContext"],
34 [60, "ThreadSnapshotWindowIndexes"],34 [60, "ThreadSnapshotWindowIndexes"],
35 [61, "PullRequestReviewGuides"],35 [61, "PullRequestReviewGuides"],
36 [62, "ThreadFailureLookupIndex"],
36 ]);37 ]);
37 assert.deepStrictEqual(yield* runMigrations(), []);38 assert.deepStrictEqual(yield* runMigrations(), []);
3839
...@@ -60,6 +61,7 @@ layer("055_OrchestrationV2", (it) => {...@@ -60,6 +61,7 @@ layer("055_OrchestrationV2", (it) => {
60 { migration_id: 59, name: "McpAppModelContext" },61 { migration_id: 59, name: "McpAppModelContext" },
61 { migration_id: 60, name: "ThreadSnapshotWindowIndexes" },62 { migration_id: 60, name: "ThreadSnapshotWindowIndexes" },
62 { migration_id: 61, name: "PullRequestReviewGuides" },63 { migration_id: 61, name: "PullRequestReviewGuides" },
64 { migration_id: 62, name: "ThreadFailureLookupIndex" },
63 ]);65 ]);
6466
65 const tables = yield* sql<{ readonly name: string }>`67 const tables = yield* sql<{ readonly name: string }>`
apps/server/src/persistence/Migrations/062_ThreadFailureLookupIndex.ts created+15
...@@ -0,0 +1,15 @@
1import * as Effect from "effect/Effect";
2import * as SqlClient from "effect/sql/SqlClient";
3
4export default Effect.gen(function* () {
5 const sql = yield* SqlClient.SqlClient;
6
7 // Shell and limit-recovery reads need one root error, not every item in a run.
8 yield* sql`
9 CREATE INDEX orchestration_v2_projection_turn_items_failed_error_idx
10 ON orchestration_v2_projection_turn_items(
11 thread_id, run_id, node_id, updated_at DESC, ordinal DESC, turn_item_id DESC
12 )
13 WHERE type = 'error' AND status = 'failed'
14 `;
15});
apps/server/src/persistence/reconcileV2PreviewMigration.test.ts+2
...@@ -42,6 +42,7 @@ describe("V2 preview upgrade", () => {...@@ -42,6 +42,7 @@ describe("V2 preview upgrade", () => {
42 [59, "McpAppModelContext"],42 [59, "McpAppModelContext"],
43 [60, "ThreadSnapshotWindowIndexes"],43 [60, "ThreadSnapshotWindowIndexes"],
44 [61, "PullRequestReviewGuides"],44 [61, "PullRequestReviewGuides"],
45 [62, "ThreadFailureLookupIndex"],
45 ]);46 ]);
46 assert.deepStrictEqual(yield* runMigrations(), []);47 assert.deepStrictEqual(yield* runMigrations(), []);
47 assert.deepStrictEqual(yield* sql`SELECT * FROM orchestration_v2_legacy_imports`, imports);48 assert.deepStrictEqual(yield* sql`SELECT * FROM orchestration_v2_legacy_imports`, imports);
...@@ -126,6 +127,7 @@ describe("V2 preview upgrade", () => {...@@ -126,6 +127,7 @@ describe("V2 preview upgrade", () => {
126 [59, "McpAppModelContext"],127 [59, "McpAppModelContext"],
127 [60, "ThreadSnapshotWindowIndexes"],128 [60, "ThreadSnapshotWindowIndexes"],
128 [61, "PullRequestReviewGuides"],129 [61, "PullRequestReviewGuides"],
130 [62, "ThreadFailureLookupIndex"],
129 ]);131 ]);
130 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),132 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
131 );133 );