Stop interrupted tasks from replaying rejected subtasks

scripts/check-task-lifecycle.ts

@@ -1,12 +1,13 @@
 import assert from "node:assert/strict"
 
-import type { HistoryItem } from "../packages/types/src/history"
+import type { HistoryItem, PendingTaskAction } from "../packages/types/src/history"
 
 import {
 	abandonDelegatedChild,
 	completeDelegatedChild,
 	delegateTaskToChild,
 	interruptDelegatedChild,
+	settleRejectedCreateSubtaskAction,
 } from "../src/core/task-persistence/taskLifecycle"
 
 const taskIds = ["parent", "child-a", "child-b"] as const
@@ -16,16 +17,26 @@ type ModelState = Record<TaskId, HistoryItem | undefined>
 interface Transition {
 	name: string
 	next: ModelState
+	delegation?: { parentId: TaskId }
+	completion?: { childId: TaskId; pendingActionId?: string }
+	settlement?: { taskId: TaskId; actionId: string }
 }
 
 interface TraceStep {
 	action: string
 	state: ModelState
 }
 
+interface WitnessContext {
+	prev: ModelState
+	next: ModelState
+	transition: Transition
+}
+
 const MAX_DEPTH = 12
 const MAX_STATES = 10_000
-const expectedActions = ["delegate", "interrupt", "complete", "abandon"] as const
+const actionIds = ["action-1", "action-2"] as const
+const expectedActions = ["delegate", "interrupt", "complete", "abandon", "stage", "settle-rejected"] as const
 const semanticLandmarks = {
 	"interrupted-child-redelegation": (state: ModelState) =>
 		state.parent?.status === "delegated" &&
@@ -37,6 +48,58 @@ const semanticLandmarks = {
 		state["child-a"]?.status === "delegated" &&
 		state["child-a"].awaitingChildId === "child-b",
 } satisfies Record<string, (state: ModelState) => boolean>
+const semanticWitnesses = {
+	"interrupted-pending-delegation-settled": ({ prev, next, transition }: WitnessContext) =>
+		transition.settlement !== undefined &&
+		prev[transition.settlement.taskId]?.status === "interrupted" &&
+		prev[transition.settlement.taskId]?.pendingAction?.actionId === transition.settlement.actionId &&
+		next[transition.settlement.taskId]?.pendingAction === undefined,
+	"successful-active-delegation-settlement": ({ prev, next, transition }: WitnessContext) =>
+		transition.delegation !== undefined &&
+		prev[transition.delegation.parentId]?.status === "active" &&
+		prev[transition.delegation.parentId]?.pendingAction?.kind === "create_subtask" &&
+		next[transition.delegation.parentId]?.pendingAction === undefined,
+	"completion-preserves-unrelated-pending-action": ({ prev, next, transition }: WitnessContext) => {
+		if (transition.completion === undefined) return false
+		const beforeAction = prev[transition.completion.childId]?.pendingAction
+		return (
+			beforeAction !== undefined &&
+			next[transition.completion.childId]?.status === "completed" &&
+			canonicalTask(next[transition.completion.childId]?.pendingAction) === canonicalTask(beforeAction)
+		)
+	},
+	"stale-settlement-rejected": ({ prev, next, transition }: WitnessContext) => {
+		if (transition.settlement === undefined) return false
+		const beforeAction = prev[transition.settlement.taskId]?.pendingAction
+		return (
+			beforeAction?.kind === "create_subtask" &&
+			beforeAction.actionId !== transition.settlement.actionId &&
+			next[transition.settlement.taskId]?.pendingAction?.actionId === beforeAction.actionId
+		)
+	},
+	"matching-completion-clears-pending-action": ({ prev, next, transition }: WitnessContext) => {
+		if (transition.completion?.pendingActionId === undefined) return false
+		const beforeAction = prev[transition.completion.childId]?.pendingAction
+		const after = next[transition.completion.childId]
+		return (
+			beforeAction?.kind === "create_subtask" &&
+			beforeAction.actionId === transition.completion.pendingActionId &&
+			after?.status === "completed" &&
+			after.pendingAction === undefined
+		)
+	},
+	"replacement-completion-preserves-pending-action": ({ prev, next, transition }: WitnessContext) => {
+		if (transition.completion?.pendingActionId === undefined) return false
+		const beforeAction = prev[transition.completion.childId]?.pendingAction
+		const after = next[transition.completion.childId]
+		return (
+			beforeAction?.kind === "create_subtask" &&
+			beforeAction.actionId !== transition.completion.pendingActionId &&
+			after?.status === "completed" &&
+			canonicalTask(after.pendingAction) === canonicalTask(beforeAction)
+		)
+	},
+} satisfies Record<string, (context: WitnessContext) => boolean>
 
 function task(id: TaskId, parentTaskId?: TaskId): HistoryItem {
 	return {
@@ -54,6 +117,17 @@ function task(id: TaskId, parentTaskId?: TaskId): HistoryItem {
 	}
 }
 
+function createSubtaskAction(actionId: (typeof actionIds)[number]): PendingTaskAction {
+	return {
+		kind: "create_subtask",
+		actionId,
+		approvalText: "{}",
+		mode: "code",
+		message: `message for ${actionId}`,
+		todos: [],
+	}
+}
+
 function initialState(): ModelState {
 	return { parent: task("parent"), "child-a": undefined, "child-b": undefined }
 }
@@ -70,18 +144,38 @@ function transitions(state: ModelState): Transition[] {
 		const parent = state[parentId]
 		if (!parent) continue
 
+		const awaitedStatus = parent.awaitingChildId ? state[parent.awaitingChildId as TaskId]?.status : undefined
+		const delegationValid =
+			parent.status === "active" || (parent.status === "delegated" && awaitedStatus === "interrupted")
+
 		for (const childId of taskIds) {
 			if (childId === parentId || state[childId]) continue
-			const awaitedStatus = parent.awaitingChildId ? state[parent.awaitingChildId as TaskId]?.status : undefined
-			if (parent.status !== "active" && !(parent.status === "delegated" && awaitedStatus === "interrupted")) {
-				continue
-			}
-			const delegated = delegateTaskToChild(parent, childId, awaitedStatus)
+			if (!delegationValid) continue
+			const delegated = { ...delegateTaskToChild(parent, childId, awaitedStatus), pendingAction: undefined }
 			result.push({
 				name: `delegate(${parentId}, ${childId})`,
 				next: replace(state, delegated, task(childId, parentId)),
+				delegation: { parentId },
 			})
 		}
+
+		for (const actionId of actionIds) {
+			if (parent.status === "active" || parent.status === "interrupted") {
+				result.push({
+					name: `stage(${parentId}, ${actionId})`,
+					next: replace(state, { ...parent, pendingAction: createSubtaskAction(actionId) }),
+				})
+			}
+
+			const pending = parent.pendingAction
+			if (!delegationValid && parent.status !== "completed" && pending?.kind === "create_subtask") {
+				result.push({
+					name: `settle-rejected(${parentId}, ${actionId})`,
+					next: replace(state, settleRejectedCreateSubtaskAction(parent, actionId)),
+					settlement: { taskId: parentId, actionId },
+				})
+			}
+		}
 	}
 
 	for (const childId of taskIds) {
@@ -104,7 +198,19 @@ function transitions(state: ModelState): Transition[] {
 			result.push({
 				name: `complete(${childId})`,
 				next: replace(state, completed.parent, completed.child),
+				completion: { childId },
 			})
+			for (const actionId of actionIds) {
+				const completedChild: HistoryItem =
+					child.pendingAction?.actionId === actionId
+						? { ...completed.child, pendingAction: undefined }
+						: completed.child
+				result.push({
+					name: `complete(${childId}, ${actionId})`,
+					next: replace(state, completed.parent, completedChild),
+					completion: { childId, pendingActionId: actionId },
+				})
+			}
 		}
 
 		if (parent.status === "delegated" && parent.awaitingChildId === child.id && child.status === "interrupted") {
@@ -189,10 +295,49 @@ function checkTransitionInvariants(previous: ModelState, transition: Transition)
 			violations.push(`${id}: completed task changed after ${transition.name}`)
 		}
 	}
+
+	const settlement = transition.settlement
+	if (settlement) {
+		const before = previous[settlement.taskId]
+		const after = transition.next[settlement.taskId]
+		const beforeAction = before?.pendingAction
+		const afterAction = after?.pendingAction
+
+		if (afterAction && canonicalTask(afterAction) !== canonicalTask(beforeAction)) {
+			violations.push(`${settlement.taskId}: settlement after ${transition.name} modified a replacement action`)
+		}
+		if (
+			!afterAction &&
+			beforeAction &&
+			!(beforeAction.kind === "create_subtask" && beforeAction.actionId === settlement.actionId)
+		) {
+			violations.push(`${settlement.taskId}: settlement after ${transition.name} cleared a non-matching action`)
+		}
+		const beforeRest = { ...before, pendingAction: undefined }
+		const afterRest = { ...after, pendingAction: undefined }
+		if (canonicalTask(beforeRest) !== canonicalTask(afterRest)) {
+			violations.push(
+				`${settlement.taskId}: settlement after ${transition.name} changed status, lineage, or accounting`,
+			)
+		}
+	}
+
+	const completion = transition.completion
+	if (completion) {
+		const beforeAction = previous[completion.childId]?.pendingAction
+		const afterAction = transition.next[completion.childId]?.pendingAction
+
+		if (afterAction && canonicalTask(afterAction) !== canonicalTask(beforeAction)) {
+			violations.push(`${completion.childId}: completion after ${transition.name} replaced a pending action`)
+		}
+		if (!afterAction && beforeAction && beforeAction.actionId !== completion.pendingActionId) {
+			violations.push(`${completion.childId}: completion after ${transition.name} cleared a non-matching action`)
+		}
+	}
 	return violations
 }
 
-function canonicalTask(value: HistoryItem | undefined): string {
+function canonicalTask(value: unknown): string {
 	return JSON.stringify(value ?? null)
 }
 
@@ -204,6 +349,7 @@ function runModelCheck(): number {
 	const visited = new Set([canonical(start)])
 	const reachedActions = new Set<string>()
 	const reachedLandmarks = new Set<string>()
+	const reachedWitnesses = new Set<string>()
 	const frontier: ModelState[] = []
 
 	for (let index = 0; index < queue.length; index++) {
@@ -220,6 +366,9 @@ function runModelCheck(): number {
 
 		for (const transition of transitions(node.state)) {
 			reachedActions.add(transition.name.slice(0, transition.name.indexOf("(")))
+			for (const [name, matches] of Object.entries(semanticWitnesses)) {
+				if (matches({ prev: node.state, next: transition.next, transition })) reachedWitnesses.add(name)
+			}
 			const transitionViolations = checkTransitionInvariants(node.state, transition)
 			const trace = [...node.trace, { action: transition.name, state: transition.next }]
 			if (transitionViolations.length) {
@@ -244,6 +393,10 @@ function runModelCheck(): number {
 	if (missingLandmarks.length) {
 		throw new Error(`Task lifecycle model has unreachable semantic landmarks: ${missingLandmarks.join(", ")}`)
 	}
+	const missingWitnesses = Object.keys(semanticWitnesses).filter((name) => !reachedWitnesses.has(name))
+	if (missingWitnesses.length) {
+		throw new Error(`Task lifecycle model has unreachable semantic witnesses: ${missingWitnesses.join(", ")}`)
+	}
 	const unexploredSuccessor = frontier
 		.flatMap((state) => transitions(state))
 		.find((transition) => !visited.has(canonical(transition.next)))
@@ -278,10 +431,29 @@ function runRepresentativeScenarios(): void {
 	const interruptedCompletion = completeDelegatedChild(delegated, interruptedA, "resumed result")
 	assert.equal(interruptedCompletion.child.status, "completed")
 	assert.equal(interruptedCompletion.parent.status, "active")
+
+	const rejectedParent: HistoryItem = {
+		...parent,
+		status: "interrupted",
+		pendingAction: createSubtaskAction("action-1"),
+	}
+	assert.throws(() => delegateTaskToChild(rejectedParent, childA.id), /Invalid task status transition/)
+	const settled = settleRejectedCreateSubtaskAction(rejectedParent, "action-1")
+	assert.equal(settled.status, "interrupted")
+	assert.equal(settled.pendingAction, undefined)
+	assert.equal(settled.childIds, rejectedParent.childIds)
+
+	const replacement = { ...rejectedParent, pendingAction: createSubtaskAction("action-2") }
+	assert.equal(settleRejectedCreateSubtaskAction(replacement, "action-1"), replacement)
+	assert.equal(
+		settleRejectedCreateSubtaskAction({ ...rejectedParent, status: "completed" }, "action-1").pendingAction
+			?.actionId,
+		"action-1",
+	)
 }
 
 runRepresentativeScenarios()
 const checkedStates = runModelCheck()
 console.log(
-	`Task lifecycle model check passed: ${checkedStates} reachable states, ${expectedActions.length}/${expectedActions.length} actions reachable, ${Object.keys(semanticLandmarks).length}/${Object.keys(semanticLandmarks).length} landmarks reached, depth <= ${MAX_DEPTH}, ${taskIds.length} task slots`,
+	`Task lifecycle model check passed: ${checkedStates} reachable states, ${expectedActions.length}/${expectedActions.length} actions reachable, ${Object.keys(semanticLandmarks).length}/${Object.keys(semanticLandmarks).length} landmarks reached, ${Object.keys(semanticWitnesses).length}/${Object.keys(semanticWitnesses).length} semantic witnesses reached, depth <= ${MAX_DEPTH}, ${taskIds.length} task slots`,
 )