diff --git a/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts b/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts index 49075d3be..bb6ef862d 100644 --- a/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts +++ b/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -7019,6 +7020,76 @@ describe("aiOrchestratorService", () => { } }); + it("does not sync unlinked run steps onto mission steps already claimed by missionStepId", async () => { + const fixture = await createFixture(); + try { + const mission = fixture.missionService.create({ + prompt: "Avoid ambiguous stepKey sync collisions.", + laneId: fixture.laneId, + plannedSteps: [ + { + index: 0, + title: "Review change", + detail: "Primary review step", + kind: "review", + metadata: { stepType: "review", stepKey: "review" }, + }, + ], + }); + const missionStep = fixture.missionService.get(mission.id)?.steps[0]; + if (!missionStep) throw new Error("Expected mission step"); + + const started = fixture.orchestratorService.startRun({ + missionId: mission.id, + steps: [ + { + stepKey: "review", + title: missionStep.title, + stepIndex: 0, + dependencyStepKeys: [], + executorKind: "manual", + missionStepId: missionStep.id, + metadata: { stepType: "review" }, + }, + ], + }); + const runId = started.run.id; + fixture.missionService.update({ + missionId: mission.id, + status: "in_progress", + }); + + const spawnedStepId = randomUUID(); + const now = new Date().toISOString(); + fixture.db.run( + ` + insert into orchestrator_steps ( + id, + run_id, + project_id, + mission_step_id, + step_key, + title, + step_index, + status, + lane_id, + metadata_json, + created_at, + updated_at + ) values (?, ?, ?, null, 'review', 'Coordinator review worker', 1, 'failed', ?, '{}', ?, ?) + `, + [spawnedStepId, runId, fixture.projectId, fixture.laneId, now, now], + ); + + await fixture.aiOrchestratorService.syncMissionFromRun(runId, "coordinator_worker_failed"); + + const refreshed = fixture.missionService.get(mission.id); + expect(refreshed?.steps[0]?.status).not.toBe("failed"); + } finally { + fixture.dispose(); + } + }); + it("opens terminal failed-step interventions for each matched failed mission step", async () => { const fixture = await createFixture(); try { diff --git a/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts b/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts index 86e74d836..28d6d40e9 100644 --- a/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts +++ b/apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts @@ -258,6 +258,7 @@ import { getMaxCoordinatorRecoveries as getMaxCoordinatorRecoveriesCtx, dispatchOrchestratorHookCtx, maybeDispatchTeammateIdleHookCtx, + resolveMissionStepForRunStep, } from "./missionLifecycle"; import { resolveFirstPostPlanningPhaseKey } from "../missions/phaseEngine"; import type { HookDispatchDeps } from "./missionLifecycle"; @@ -6970,40 +6971,12 @@ Check all worker statuses and continue managing the mission from here. Read work return interventionId; }; - const stableString = (value: unknown): string | null => { - if (typeof value !== "string") return null; - const trimmed = value.trim(); - return trimmed.length > 0 ? trimmed : null; - }; - - const resolveMissionStepForRunStep = ( - mission: MissionDetail, - runStep: OrchestratorRunGraph["steps"][number], - missionStepById: Map = new Map(mission.steps.map((step) => [step.id, step])), - ): MissionDetail["steps"][number] | null => { - if (runStep.missionStepId) { - const byId = missionStepById.get(runStep.missionStepId); - if (byId) return byId; - } - const runStepId = stableString(runStep.id); - const runStepKey = stableString(runStep.stepKey); - return mission.steps.find((step) => { - const metadata = isRecord(step.metadata) ? step.metadata : null; - if (!metadata) return false; - const metadataOrchestratorStepId = stableString(metadata.orchestratorStepId); - if (runStepId && metadataOrchestratorStepId === runStepId) return true; - const metadataStepKey = stableString(metadata.stepKey); - return Boolean(runStepKey && metadataStepKey === runStepKey); - }) ?? null; - }; - const syncMissionStepsFromRun = (graph: OrchestratorRunGraph) => { const mission = missionService.get(graph.run.missionId); if (!mission) return; - const missionStepById = new Map(mission.steps.map((step) => [step.id, step])); for (const runStep of graph.steps) { - const missionStep = resolveMissionStepForRunStep(mission, runStep, missionStepById); + const missionStep = resolveMissionStepForRunStep(mission, runStep, graph); if (!missionStep) continue; const nextStatus = mapOrchestratorStepStatus(runStep.status); if (missionStep.status === nextStatus) continue; @@ -7055,7 +7028,6 @@ Check all worker statuses and continue managing the mission from here. Read work const mission = missionService.get(graph.run.missionId); if (!mission) return; - const missionStepById = new Map(mission.steps.map((step) => [step.id, step])); const hasMatchingOpenIntervention = ( failedRunStep: OrchestratorRunGraph["steps"][number] | null, failedMissionStep: MissionDetail["steps"][number] | null, @@ -7070,7 +7042,7 @@ Check all worker statuses and continue managing the mission from here. Read work .filter((step) => step.status === "failed") .map((runStep) => ({ runStep, - missionStep: resolveMissionStepForRunStep(mission, runStep, missionStepById), + missionStep: resolveMissionStepForRunStep(mission, runStep, graph), })) .find(({ runStep, missionStep }) => !hasMatchingOpenIntervention(runStep, missionStep)); const fallbackMissionStep = failedRunPair diff --git a/apps/desktop/src/main/services/orchestrator/missionLifecycle.ts b/apps/desktop/src/main/services/orchestrator/missionLifecycle.ts index ba0b40e27..787cdd6d9 100644 --- a/apps/desktop/src/main/services/orchestrator/missionLifecycle.ts +++ b/apps/desktop/src/main/services/orchestrator/missionLifecycle.ts @@ -33,6 +33,7 @@ import { } from "./orchestratorContext"; import type { OrchestratorRunGraph, + MissionDetail, MissionStatus, MissionStepStatus, OrchestratorWorkerRole, @@ -160,6 +161,62 @@ export const TERMINAL_PHASE_STEP_STATUSES = new Set([ ]); +function stableMissionLinkString(value: unknown): string | null { + if (typeof value !== "string") return null; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +/** + * Resolve the mission step that corresponds to an orchestrator run step. + * Avoids ambiguous stepKey matches when coordinator-spawned steps reuse a key + * already owned by a mission step linked via missionStepId on another run step. + */ +export function resolveMissionStepForRunStep( + mission: MissionDetail, + runStep: OrchestratorRunGraph["steps"][number], + graph?: Pick, +): MissionDetail["steps"][number] | null { + const missionStepById = new Map(mission.steps.map((step) => [step.id, step])); + if (runStep.missionStepId) { + const byId = missionStepById.get(runStep.missionStepId); + if (byId) return byId; + } + + const runStepId = stableMissionLinkString(runStep.id); + if (runStepId) { + const byOrchestratorId = mission.steps.find((step) => { + const metadata = isRecord(step.metadata) ? step.metadata : null; + if (!metadata) return false; + return stableMissionLinkString(metadata.orchestratorStepId) === runStepId; + }); + if (byOrchestratorId) return byOrchestratorId; + } + + const runStepKey = stableMissionLinkString(runStep.stepKey); + if (!runStepKey) return null; + + const claimedMissionStepIds = new Set( + (graph?.steps ?? []) + .map((step) => stableMissionLinkString(step.missionStepId)) + .filter((id): id is string => Boolean(id)), + ); + + const stepKeyMatches = mission.steps.filter((step) => { + const metadata = isRecord(step.metadata) ? step.metadata : null; + if (!metadata) return false; + const metadataStepKey = stableMissionLinkString(metadata.stepKey); + if (metadataStepKey !== runStepKey) return false; + if (claimedMissionStepIds.has(step.id) && runStep.missionStepId !== step.id) { + return false; + } + return true; + }); + + if (stepKeyMatches.length === 1) return stepKeyMatches[0]!; + return null; +} + /** * Sync mission steps from an orchestrator run graph. */ @@ -169,10 +226,7 @@ export function syncMissionStepsFromRun(ctx: OrchestratorContext, graph: Orchest if (!mission) return; for (const step of graph.steps) { - const missionStep = mission.steps.find((ms) => { - const msMeta = isRecord(ms.metadata) ? ms.metadata : {}; - return msMeta.orchestratorStepId === step.id || msMeta.stepKey === step.stepKey; - }); + const missionStep = resolveMissionStepForRunStep(mission, step, graph); if (missionStep) { const apply = () => { try {