test(flow): cover the heartbeat recovery paths that no test executed

tests/Unit/Service/Flow/FlowHeartbeatRecoveryTest.php

@@ -511,4 +511,49 @@ public function testOnlyTheNodeWhoseTaskEndedRecovers(): void {
 		$this->assertArrayNotHasKey('askA', $kept, 'a node that answered has nothing left to remember');
 		$this->assertSame($taskB, (string)($kept['askB']['taskUuid'] ?? ''), 'the waiting sibling keeps its own task');
 	}//end testOnlyTheNodeWhoseTaskEndedRecovers()
+
+	/**
+	 * 🔴 THE SYMMETRIC CASE, PINNED: a task completed while the run was NOT yet
+	 * suspended. `signal()` refuses any run that is not `suspended`, so that
+	 * completion's wake is simply LOST — there is no queue for it, and nothing
+	 * retries it. The design decided this needs no new mechanism, and this test
+	 * is what makes that decision falsifiable: the lost wake must cost latency
+	 * only, because the node parks on a NON-NULL heartbeat and the next wake
+	 * re-reads the task.
+	 *
+	 * Were the heartbeat ever allowed to be null here, this run would be
+	 * unreachable forever — `findDue()` never returns a run with a null
+	 * `resume_at` — which is exactly the trap `UserTaskNode` documents.
+	 *
+	 * @return void
+	 *
+	 * @spec openspec/changes/flow-heartbeat-recovery/specs/flow-heartbeat-recovery/spec.md#requirement-a-heartbeat-wake-re-reads-the-awaited-task-and-applies-a-terminal-outcome
+	 */
+	public function testACompletionThatRacedTheSuspensionIsRecoveredByTheHeartbeat(): void {
+		$run = $this->suspendedOnBothTasks();
+		$slots = ($run->getContext()['resumeState'] ?? []);
+		$taskA = (string)$slots['askA']['taskUuid'];
+
+		// THE RACE: the task completes while the run is still mid-walk. The
+		// completion listener calls signal(), which refuses a non-suspended
+		// run — so the wake is lost and nothing queues a retry of it.
+		$this->complete(uuid: $taskA, completedBy: 'bob');
+		$run->setStatus(FlowRun::STATUS_RUNNING);
+		$this->assertNull(
+			$this->service->signal($run, []),
+			'a run that is not suspended refuses the signal, so the completion wake is lost'
+		);
+
+		// The walk finishes and the run parks — on a heartbeat that is NEVER
+		// null, which is the only reason the lost wake is recoverable at all.
+		$run->setStatus(FlowRun::STATUS_SUSPENDED);
+		$this->assertNotNull($run->getResumeAt(), 'a task-waiting run must park on a clock, never on a signal alone');
+
+		// The next heartbeat re-reads the task and applies the outcome.
+		$run = $this->service->execute($run, $this->flow(), new HeartbeatSubject());
+
+		$this->assertSame([$taskA], $this->recovered, 'the raced completion is recovered, and audited as a recovery');
+		$this->assertSame(['askA', 'askB'], $this->created, 'recovery never creates a task');
+	}//end testACompletionThatRacedTheSuspensionIsRecoveredByTheHeartbeat()
+
 }//end class

tests/Unit/Service/Flow/FlowTaskBridgeTest.php

@@ -332,4 +332,103 @@ public function testTheBagSeparatesADecisionFromAnEnding(): void {
 		$this->assertFalse($bag['rejected']);
 		$this->assertSame(Task::STATE_TERMINATED, $bag['outcome'], 'an ending with no outcome reports its state');
 	}//end testTheBagSeparatesADecisionFromAnEnding()
+
+	// ---- Heartbeat recovery -------------------------------------------------------
+
+	/**
+	 * 🔴 THE OTHER HALF OF THE REFUSAL TRAIL. The guarded signal seam records
+	 * that a completion was refused; without this entry the trail ends there
+	 * and a recovered answer reads as one that vanished. Attributed to the
+	 * task's COMPLETER, because the fact being recorded is that person's
+	 * answer arriving late by poll — not the cron job acting.
+	 *
+	 * Driven through the REAL bridge. Every other test of this behaviour mocks
+	 * FlowTaskBridge (the nodes are the unit there), so this method's body had
+	 * no execution coverage at all until this test.
+	 *
+	 * @return void
+	 *
+	 * @spec openspec/changes/flow-heartbeat-recovery/specs/flow-heartbeat-recovery/spec.md#requirement-a-heartbeat-recovered-delivery-is-recorded-on-the-tasks-audit
+	 */
+	public function testAHeartbeatRecoveryIsAuditedToTheTasksCompleter(): void {
+		$task = $this->terminalTask();
+		$task->setCompletedBy('bob');
+
+		$seen = [];
+		$this->tasks->expects($this->once())
+			->method('record')
+			->willReturnCallback(
+				function (string $uuid, string $action, ?string $actor, string $reason) use (&$seen, $task): Task {
+					$seen = ['uuid' => $uuid, 'action' => $action, 'actor' => $actor, 'reason' => $reason];
+
+					return $task;
+				}
+			);
+
+		$this->bridge->recordHeartbeatRecovery(task: $task);
+
+		$this->assertSame('t-1', $seen['uuid']);
+		$this->assertSame('heartbeat-recovered', $seen['action']);
+		$this->assertSame('bob', $seen['actor'], 'the recovery is the completer\'s answer arriving, not the worker\'s');
+		$this->assertStringContainsString('run-1', $seen['reason'], 'the reason names the run whose signal never arrived');
+	}//end testAHeartbeatRecoveryIsAuditedToTheTasksCompleter()
+
+	/**
+	 * 🔴 BEST-EFFORT, AND THAT IS THE POINT. The recovery itself is the node
+	 * applying the outcome; this entry only describes it. An audit write that
+	 * fails must therefore NOT propagate — letting it out would abort the walk
+	 * that was recovering the run and put the run straight back into the wedge
+	 * this whole change exists to remove.
+	 *
+	 * @return void
+	 *
+	 * @spec openspec/changes/flow-heartbeat-recovery/specs/flow-heartbeat-recovery/spec.md#requirement-a-heartbeat-recovered-delivery-is-recorded-on-the-tasks-audit
+	 */
+	public function testAFailedRecoveryAuditIsSwallowedSoTheRecoveredRunStands(): void {
+		$task = $this->terminalTask();
+		$task->setCompletedBy('bob');
+
+		$this->tasks->expects($this->once())
+			->method('record')
+			->willThrowException(new RuntimeException('the audit table is unavailable'));
+
+		$this->bridge->recordHeartbeatRecovery(task: $task);
+
+		// Reached only because nothing propagated: the recovery outlives its
+		// own audit failure.
+		$this->addToAssertionCount(1);
+	}//end testAFailedRecoveryAuditIsSwallowedSoTheRecoveredRunStands()
+
+	/**
+	 * A task that ended WITHOUT a completer — terminated or expired rather than
+	 * answered — still records its recovery, with no actor rather than an
+	 * invented one. `completedBy` is null on exactly those endings, and an
+	 * audit that guessed a name there would be worse than one that admits it
+	 * has none.
+	 *
+	 * @return void
+	 *
+	 * @spec openspec/changes/flow-heartbeat-recovery/specs/flow-heartbeat-recovery/spec.md#requirement-a-heartbeat-recovered-delivery-is-recorded-on-the-tasks-audit
+	 */
+	public function testARecoveredEndingWithNoCompleterRecordsNoActor(): void {
+		$task = $this->terminalTask();
+		$task->setState(Task::STATE_TERMINATED);
+		$task->setCompletedBy(null);
+
+		$actor = 'unset';
+		$this->tasks->expects($this->once())
+			->method('record')
+			->willReturnCallback(
+				function (string $uuid, string $action, ?string $seenActor, string $reason) use (&$actor, $task): Task {
+					$actor = $seenActor;
+
+					return $task;
+				}
+			);
+
+		$this->bridge->recordHeartbeatRecovery(task: $task);
+
+		$this->assertNull($actor, 'an ending nobody answered is recorded with no actor, never a guessed one');
+	}//end testARecoveredEndingWithNoCompleterRecordsNoActor()
+
 }//end class