diff --git a/.changeset/fn-8972-mission-recompute-blocked-guard.md b/.changeset/fn-8972-mission-recompute-blocked-guard.md new file mode 100644 index 0000000000..6bf136c1ea --- /dev/null +++ b/.changeset/fn-8972-mission-recompute-blocked-guard.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Paused missions no longer un-pause when hierarchy status rolls up. +category: fix +dev: Guards shouldApplyRecomputedStatus, store recompute helpers, and terminal-task reconcile milestone writes. diff --git a/docs/missions.md b/docs/missions.md index e295a600c3..0691751158 100644 --- a/docs/missions.md +++ b/docs/missions.md @@ -381,6 +381,8 @@ If validation cannot run (unexpected loop state, duplicate trigger, blocked vali Mission `status` and `autopilotEnabled` transitions are atomically written with a mission activity event. The event records stable actor type/id, optional display name, source, and before/after values; unchanged values create no transition event. Dashboard controls identify an operator, tools identify an agent when they expose a sensitive mutation, and autonomous engine paths identify the system/autopilot. +Automatic hierarchy rollup, including terminal-task delivery reconciliation, owns only `planning`, `active`, and `complete` for missions and milestones. It never rewrites intentional `blocked` or `archived` status during hierarchy churn; a blocked mission stays blocked even after all milestones complete. Those statuses change only through resume, an explicit status write, or the mission clear-blocked path. + ## `autopilotEnabled` vs `autoAdvance` - **`autopilotEnabled`**: primary control for autopilot behavior — enables background monitoring, orchestration, and automatic slice activation when a slice completes. Also triggers auto-planning (converting features to tasks) when a slice is activated. diff --git a/packages/core/src/__tests__/mission-status-recompute-guard.test.ts b/packages/core/src/__tests__/mission-status-recompute-guard.test.ts new file mode 100644 index 0000000000..eb9433d9b4 --- /dev/null +++ b/packages/core/src/__tests__/mission-status-recompute-guard.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Database } from "../db/db.js"; +import { MissionStore } from "../missions/mission-store.js"; +import { + MILESTONE_STATUSES, + MISSION_STATUSES, + ROLLUP_OWNED_MILESTONE_STATUSES, + ROLLUP_OWNED_MISSION_STATUSES, + SLICE_STATUSES, + shouldApplyRecomputedStatus, +} from "../missions/mission-types.js"; + +describe("shouldApplyRecomputedStatus", () => { + it("skips identical statuses", () => { + expect(shouldApplyRecomputedStatus("active", "active", ROLLUP_OWNED_MISSION_STATUSES)).toBe(false); + }); + + it("updates each rollup-owned mission and milestone status when the computed value differs", () => { + for (const current of ROLLUP_OWNED_MISSION_STATUSES) { + for (const computed of ROLLUP_OWNED_MISSION_STATUSES) { + expect(shouldApplyRecomputedStatus(current, computed, ROLLUP_OWNED_MISSION_STATUSES)) + .toBe(current !== computed); + } + } + for (const current of ROLLUP_OWNED_MILESTONE_STATUSES) { + for (const computed of ROLLUP_OWNED_MILESTONE_STATUSES) { + expect(shouldApplyRecomputedStatus(current, computed, ROLLUP_OWNED_MILESTONE_STATUSES)) + .toBe(current !== computed); + } + } + }); + + it("preserves mission statuses outside automatic rollup ownership", () => { + for (const current of MISSION_STATUSES.filter((status) => !ROLLUP_OWNED_MISSION_STATUSES.includes(status))) { + for (const computed of ROLLUP_OWNED_MISSION_STATUSES) { + expect(shouldApplyRecomputedStatus(current, computed, ROLLUP_OWNED_MISSION_STATUSES)).toBe(false); + } + } + }); + + it("preserves milestone statuses outside automatic rollup ownership", () => { + for (const current of MILESTONE_STATUSES.filter((status) => !ROLLUP_OWNED_MILESTONE_STATUSES.includes(status))) { + for (const computed of ROLLUP_OWNED_MILESTONE_STATUSES) { + expect(shouldApplyRecomputedStatus(current, computed, ROLLUP_OWNED_MILESTONE_STATUSES)).toBe(false); + } + } + }); + + it("keeps slice rollup fully owned", () => { + expect(SLICE_STATUSES).toEqual(["pending", "active", "complete"]); + }); +}); + +describe("MissionStore synchronous rollup guard", () => { + const createStore = () => new MissionStore("/tmp/fusion-mission-store-test", { + prepare: vi.fn().mockReturnValue({ get: vi.fn().mockReturnValue(undefined) }), + bumpLastModified: vi.fn(), + } as unknown as Database); + + it("preserves blocked and archived mission intent while updating rollup-owned rows", () => { + const store = createStore(); + const updateMission = vi.spyOn(store, "updateMission").mockImplementation((id, updates) => ({ id, ...updates } as never)); + vi.spyOn(store, "computeMissionStatus").mockReturnValue("active"); + vi.spyOn(store, "getMission").mockReturnValue({ id: "M-1", status: "blocked" } as never); + (store as any).recomputeMissionStatus("M-1"); + vi.spyOn(store, "getMission").mockReturnValue({ id: "M-2", status: "archived" } as never); + (store as any).recomputeMissionStatus("M-2"); + vi.spyOn(store, "getMission").mockReturnValue({ id: "M-3", status: "planning" } as never); + (store as any).recomputeMissionStatus("M-3"); + expect(updateMission).toHaveBeenCalledTimes(1); + expect(updateMission).toHaveBeenCalledWith("M-3", { status: "active" }); + }); + + it("preserves blocked milestones while updating rollup-owned rows", () => { + const store = createStore(); + const updateMilestone = vi.spyOn(store, "updateMilestone").mockImplementation((id, updates) => ({ id, ...updates } as never)); + vi.spyOn(store, "computeMilestoneStatus").mockReturnValue("complete"); + vi.spyOn(store, "getMilestone").mockReturnValue({ id: "MS-1", status: "blocked" } as never); + (store as any).recomputeMilestoneStatus("MS-1"); + vi.spyOn(store, "getMilestone").mockReturnValue({ id: "MS-2", status: "active" } as never); + (store as any).recomputeMilestoneStatus("MS-2"); + expect(updateMilestone).toHaveBeenCalledTimes(1); + expect(updateMilestone).toHaveBeenCalledWith("MS-2", { status: "complete" }); + }); +}); diff --git a/packages/core/src/__tests__/postgres/mission-store.pg.test.ts b/packages/core/src/__tests__/postgres/mission-store.pg.test.ts index 2e1c237531..37739c645c 100644 --- a/packages/core/src/__tests__/postgres/mission-store.pg.test.ts +++ b/packages/core/src/__tests__/postgres/mission-store.pg.test.ts @@ -1501,4 +1501,109 @@ pgTest("MissionStore (PostgreSQL backend mode)", () => { expect(triaged.taskId).toMatch(/^ERR-\d+$/); }); + + describe("automatic status rollup ownership", () => { + it("preserves blocked and archived missions through hierarchy churn", async () => { + const m = missions(); + const createProtectedHierarchy = async (status: "blocked" | "archived") => { + const mission = await m.createMission({ title: `Protected ${status}` }); + const milestone = await m.addMilestone(mission.id, { title: "MS" }); + const slice = await m.addSlice(milestone.id, { title: "SL" }); + await m.updateMission(mission.id, { status }); + return { mission, milestone, slice }; + }; + const renamed = await createProtectedHierarchy("blocked"); + const eventCount = (await m.getMissionEvents(renamed.mission.id)).events.length; + await m.updateMilestone(renamed.milestone.id, { title: "renamed" }); + expect(await m.getMission(renamed.mission.id)).toMatchObject({ status: "blocked" }); + expect((await m.getMissionEvents(renamed.mission.id)).events).toHaveLength(eventCount); + const sliced = await createProtectedHierarchy("blocked"); + await m.updateSlice(sliced.slice.id, { title: "edited" }); + await m.deleteSlice(sliced.slice.id); + expect(await m.getMission(sliced.mission.id)).toMatchObject({ status: "blocked" }); + const deleted = await createProtectedHierarchy("blocked"); + await m.deleteMilestone(deleted.milestone.id); + expect(await m.getMission(deleted.mission.id)).toMatchObject({ status: "blocked" }); + const archived = await createProtectedHierarchy("archived"); + await m.updateMilestone(archived.milestone.id, { title: "churn" }); + expect(await m.getMission(archived.mission.id)).toMatchObject({ status: "archived" }); + }); + + /* + FNXC:MissionStatusRollup 2026-08-11-04:41: + The recompute transaction locks the mission before calculating its derived status. Hold that + calculation while an explicit blocked write queues behind the row lock; releasing the rollup + must leave the later operator write intact instead of replaying a stale derived status. + */ + it("does not overwrite an explicit blocked mission queued during a rollup", async () => { + const m = missions(); + const mission = await m.createMission({ title: "Concurrent protected mission" }); + const milestone = await m.addMilestone(mission.id, { title: "MS" }); + const originalCompute = (m as any).computeMissionStatusWithHandle.bind(m); + let enteredCompute!: () => void; + let releaseCompute!: () => void; + const computeEntered = new Promise((resolve) => { enteredCompute = resolve; }); + const computeRelease = new Promise((resolve) => { releaseCompute = resolve; }); + (m as any).computeMissionStatusWithHandle = async (...args: unknown[]) => { + enteredCompute(); + await computeRelease; + return originalCompute(...args); + }; + try { + const rollup = m.updateMilestone(milestone.id, { title: "churn" }); + await computeEntered; + const explicitBlock = m.updateMission(mission.id, { status: "blocked" }); + releaseCompute(); + await Promise.all([rollup, explicitBlock]); + } finally { + (m as any).computeMissionStatusWithHandle = originalCompute; + } + expect(await m.getMission(mission.id)).toMatchObject({ status: "blocked" }); + }); + + it("does not overwrite an explicit blocked milestone queued during a rollup", async () => { + const m = missions(); + const mission = await m.createMission({ title: "Concurrent protected milestone" }); + const milestone = await m.addMilestone(mission.id, { title: "MS" }); + const slice = await m.addSlice(milestone.id, { title: "SL" }); + const originalCompute = (m as any).computeMilestoneStatusWithHandle.bind(m); + let enteredCompute!: () => void; + let releaseCompute!: () => void; + const computeEntered = new Promise((resolve) => { enteredCompute = resolve; }); + const computeRelease = new Promise((resolve) => { releaseCompute = resolve; }); + (m as any).computeMilestoneStatusWithHandle = async (...args: unknown[]) => { + enteredCompute(); + await computeRelease; + return originalCompute(...args); + }; + try { + const rollup = m.updateSlice(slice.id, { title: "churn" }); + await computeEntered; + const explicitBlock = m.updateMilestone(milestone.id, { status: "blocked" }); + releaseCompute(); + await Promise.all([rollup, explicitBlock]); + } finally { + (m as any).computeMilestoneStatusWithHandle = originalCompute; + } + expect(await m.getMilestone(milestone.id)).toMatchObject({ status: "blocked" }); + }); + + it("preserves blocked milestones during terminal-task reconcile", async () => { + const m = missions(); + const mission = await m.createMission({ title: "Protected reconcile" }); + const milestone = await m.addMilestone(mission.id, { title: "MS" }); + const slice = await m.addSlice(milestone.id, { title: "SL" }); + const feature = await m.addFeature(slice.id, { title: "Delivered" }); + const task = await h.store().createTask({ description: "done", column: "done" }); + await m.updateMilestone(milestone.id, { status: "blocked" }); + const milestoneUpdated = vi.fn(); + m.on("milestone:updated", milestoneUpdated); + await m.reconcileFeatureDoneWithTerminalTask(feature.id, task.id); + expect(await m.getFeature(feature.id)).toMatchObject({ status: "done", taskId: task.id }); + expect(await m.getSlice(slice.id)).toMatchObject({ status: "complete" }); + expect(await m.getMilestone(milestone.id)).toMatchObject({ status: "blocked" }); + expect(milestoneUpdated).not.toHaveBeenCalled(); + }); + }); + }); diff --git a/packages/core/src/async-stores/async-mission-store.ts b/packages/core/src/async-stores/async-mission-store.ts index ee6f5c6f92..b578969ad7 100644 --- a/packages/core/src/async-stores/async-mission-store.ts +++ b/packages/core/src/async-stores/async-mission-store.ts @@ -14,7 +14,7 @@ import { EventEmitter } from "node:events"; import { and, desc, eq, inArray, notInArray, sql } from "drizzle-orm"; import * as schema from "../postgres/schema/index.js"; import type { AsyncDataLayer } from "../postgres/data-layer.js"; -import { boundMissionEventReason, classifyMissionResumeBlockers, FEATURE_LOOP_REPAIR_TRANSITIONS, buildMissionStatusEventMetadata, featureValidationRepairEligibility, FEATURE_LOOP_TRANSITIONS, normalizeMissionAssertionType, normalizeMissionTransitionActorForEvent, renderValidationCause, selectNextSerialMissionSlice, VALIDATION_INFLIGHT_STALE_MAX_AGE_MS } from "../missions/mission-types.js"; +import { boundMissionEventReason, classifyMissionResumeBlockers, FEATURE_LOOP_REPAIR_TRANSITIONS, buildMissionStatusEventMetadata, featureValidationRepairEligibility, FEATURE_LOOP_TRANSITIONS, normalizeMissionAssertionType, normalizeMissionTransitionActorForEvent, renderValidationCause, ROLLUP_OWNED_MILESTONE_STATUSES, ROLLUP_OWNED_MISSION_STATUSES, selectNextSerialMissionSlice, shouldApplyRecomputedStatus, VALIDATION_INFLIGHT_STALE_MAX_AGE_MS } from "../missions/mission-types.js"; import type { Mission, Milestone, @@ -1414,9 +1414,17 @@ export class AsyncMissionStore extends EventEmitter { if (reconciledSlice !== slice) await updateSlice(tx, reconciledSlice); const milestoneStatus = await this.computeMilestoneStatusWithHandle(tx, milestone.id); - const reconciledMilestone = milestone.status === milestoneStatus - ? milestone - : { ...milestone, status: milestoneStatus, updatedAt: now }; + /* + FNXC:MissionStatusRollup 2026-08-11-04:27: + This automatic writer bypasses recomputeMilestoneStatus because it must persist within this + terminal-task transaction via updateMilestone(tx, ...). Dashboard mission-routes and engine + mission-state-reconcile call this path, so it independently applies the shared ownership rule. + */ + const reconciledMilestone = shouldApplyRecomputedStatus( + milestone.status, + milestoneStatus, + ROLLUP_OWNED_MILESTONE_STATUSES, + ) ? { ...milestone, status: milestoneStatus, updatedAt: now } : milestone; if (reconciledMilestone !== milestone) await updateMilestone(tx, reconciledMilestone); return { @@ -2933,16 +2941,55 @@ export class AsyncMissionStore extends EventEmitter { if (slice && slice.status !== newStatus) await this.updateSlice(sliceId, { status: newStatus }); } + /* + FNXC:MissionStatusRollup 2026-08-11-04:27: + updateMilestone, deleteMilestone, updateSlice, deleteSlice, slice admission, and the engine's + recomputeMissionStatusChain reach this cascade. It must not clear blocked/archived intent: even + an all-complete blocked mission stays blocked until an explicit clear or resume. Lock the row, + compute, and persist in one transaction so an explicit status write cannot race past the guard. + The direct atomic milestone write retains updateMilestone's normal mission cascade after commit. + The terminal-task transaction has a second milestone writer guarded with this same predicate below. + */ private async recomputeMilestoneStatus(milestoneId: string): Promise { - const newStatus = await this.computeMilestoneStatus(milestoneId); - const milestone = await getMilestone(this.db, milestoneId); - if (milestone && milestone.status !== newStatus) await this.updateMilestone(milestoneId, { status: newStatus }); + const updated = await this.layer.transactionImmediate(async (tx) => { + await tx.select().from(schema.project.milestones).where(eq(schema.project.milestones.id, milestoneId)).for("update"); + const milestone = await getMilestone(tx, milestoneId); + if (!milestone) return undefined; + const newStatus = await this.computeMilestoneStatusWithHandle(tx, milestoneId); + if (!shouldApplyRecomputedStatus(milestone.status, newStatus, ROLLUP_OWNED_MILESTONE_STATUSES)) return undefined; + const updated = { ...milestone, status: newStatus, updatedAt: new Date().toISOString() }; + await updateMilestone(tx, updated); + return updated; + }); + if (!updated) return; + this.emit("milestone:updated", updated); + await this.recomputeMissionStatus(updated.missionId); } private async recomputeMissionStatus(missionId: string): Promise { - const newStatus = await this.computeMissionStatus(missionId); - const mission = await getMission(this.db, missionId); - if (mission && mission.status !== newStatus) await this.updateMission(missionId, { status: newStatus }); + const outcome = await this.layer.transactionImmediate(async (tx) => { + await tx.select().from(schema.project.missions).where(eq(schema.project.missions.id, missionId)).for("update"); + const mission = await getMission(tx, missionId); + if (!mission) return undefined; + const newStatus = await this.computeMissionStatusWithHandle(tx, missionId); + if (!shouldApplyRecomputedStatus(mission.status, newStatus, ROLLUP_OWNED_MISSION_STATUSES)) return undefined; + const updated = { ...mission, status: newStatus, updatedAt: new Date().toISOString() }; + await updateMission(tx, updated); + const event: MissionEvent = { + id: this.generateId("ME"), missionId, eventType: "mission_status_changed", + description: `Mission status changed from ${mission.status} to ${updated.status}`, + metadata: buildMissionStatusEventMetadata({ + entity: "mission", field: "status", from: mission.status, to: updated.status, ids: {}, + actor: { type: "system", id: "mission-store", displayName: "Mission store", source: "mission-store" }, + }), + timestamp: new Date().toISOString(), seq: (await getMaxEventSeq(tx)) + 1, + }; + await insertMissionEvent(tx, event); + return { updated, event }; + }); + if (!outcome) return; + this.emit("mission:updated", outcome.updated); + this.emit("mission:event", outcome.event); } /* diff --git a/packages/core/src/missions/mission-store.ts b/packages/core/src/missions/mission-store.ts index 7dbd0dfda7..b96cb3f467 100644 --- a/packages/core/src/missions/mission-store.ts +++ b/packages/core/src/missions/mission-store.ts @@ -17,7 +17,7 @@ const severityAuditLog = createLogger("core-mission-store"); import { EventEmitter } from "node:events"; import type { Database } from "../db/db.js"; import { fromJson, toJson, toJsonNullable } from "../db/db.js"; -import { FEATURE_LOOP_TRANSITIONS, normalizeMissionAssertionOrigin, normalizeMissionAssertionScope, normalizeMissionAssertionType, renderValidationCause, selectNextSerialMissionSlice, VALIDATION_INFLIGHT_STALE_MAX_AGE_MS } from "./mission-types.js"; +import { FEATURE_LOOP_TRANSITIONS, normalizeMissionAssertionOrigin, normalizeMissionAssertionScope, normalizeMissionAssertionType, renderValidationCause, ROLLUP_OWNED_MILESTONE_STATUSES, ROLLUP_OWNED_MISSION_STATUSES, selectNextSerialMissionSlice, shouldApplyRecomputedStatus, VALIDATION_INFLIGHT_STALE_MAX_AGE_MS } from "./mission-types.js"; import type { Goal, GoalStatus } from "../goals/goal-types.js"; import type { Mission, @@ -4607,6 +4607,12 @@ export class MissionStore extends EventEmitter { } } + /* + FNXC:MissionStatusRollup 2026-08-11-04:27: + FN-8962's audit found this synchronous backend is test-only, but it remains behaviorally aligned + with AsyncMissionStore. Its two recompute helpers are its complete rollup-writer set: grep confirms + it has no reconcileFeatureDoneWithTerminalTask equivalent, so protected intent cannot drift here. + */ /** * Recompute and update the milestone status. * Called automatically after slice changes. @@ -4615,7 +4621,7 @@ export class MissionStore extends EventEmitter { const newStatus = this.computeMilestoneStatus(milestoneId); const milestone = this.getMilestone(milestoneId); - if (milestone && milestone.status !== newStatus) { + if (milestone && shouldApplyRecomputedStatus(milestone.status, newStatus, ROLLUP_OWNED_MILESTONE_STATUSES)) { this.updateMilestone(milestoneId, { status: newStatus }); // Don't emit here - updateMilestone already emits and triggers mission recompute } @@ -4629,7 +4635,7 @@ export class MissionStore extends EventEmitter { const newStatus = this.computeMissionStatus(missionId); const mission = this.getMission(missionId); - if (mission && mission.status !== newStatus) { + if (mission && shouldApplyRecomputedStatus(mission.status, newStatus, ROLLUP_OWNED_MISSION_STATUSES)) { this.updateMission(missionId, { status: newStatus }); // Don't emit here - updateMission already emits } diff --git a/packages/core/src/missions/mission-types.ts b/packages/core/src/missions/mission-types.ts index b080063ddb..64396b19ad 100644 --- a/packages/core/src/missions/mission-types.ts +++ b/packages/core/src/missions/mission-types.ts @@ -18,6 +18,9 @@ import { redactSecrets } from "../secrets/redact-secrets.js"; export const MISSION_STATUSES = ["planning", "active", "blocked", "complete", "archived"] as const; export type MissionStatus = (typeof MISSION_STATUSES)[number]; +/** Statuses that hierarchy rollup can derive for missions. */ +export const ROLLUP_OWNED_MISSION_STATUSES = ["planning", "active", "complete"] as const satisfies readonly MissionStatus[]; + /** The persisted source that prevents a mission from resuming automatically. */ export type MissionBlockerSource = "feature-stop" | "lineage-stop" | "unspecified"; @@ -84,6 +87,24 @@ export function classifyMissionResumeBlockers(input: { export const MILESTONE_STATUSES = ["planning", "active", "blocked", "complete"] as const; export type MilestoneStatus = (typeof MILESTONE_STATUSES)[number]; +/** Statuses that hierarchy rollup can derive for milestones. */ +export const ROLLUP_OWNED_MILESTONE_STATUSES = ["planning", "active", "complete"] as const satisfies readonly MilestoneStatus[]; + +/* +FNXC:MissionStatusRollup 2026-08-11-04:27: +Every automatic rollup writer—the recompute helpers in both stores and AsyncMissionStore's +in-transaction terminal-task reconcile—may move a row only between statuses it can derive. +blocked/archived are operator or system intent from pause, stop, PATCH, and fn_mission_set_status; +only explicit resumeMission, clearMissionBlockedStatus, or autopilot completion clears them. +*/ +export function shouldApplyRecomputedStatus( + current: T, + computed: T, + rollupOwned: readonly T[], +): boolean { + return current !== computed && rollupOwned.includes(current); +} + /** Status values for a Slice (work unit) */ export const SLICE_STATUSES = ["pending", "active", "complete"] as const; export type SliceStatus = (typeof SLICE_STATUSES)[number];