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

The changed-files coverage ratchet on #3358 was right, and it was pointing
at something real rather than at a percentage. Of the 32 statements that
change added, 30 were the body of
`FlowTaskBridge::recordHeartbeatRecovery()` -- and every test that exercised
the recovery MOCKED FlowTaskBridge, because in those tests the nodes are the
unit. So the method that writes the recovery's audit trail had no execution
coverage at all: the audit entry, its attribution, and the catch that makes
it best-effort were asserted nowhere.

That matters more than the percentage does. The guarded signal seam records
a refusal; this entry is the other half of that trail, and a silent
regression in it would make a recovered answer read as one that vanished.

Three tests through the REAL bridge, in the suite that already builds one:

- the entry is recorded as `heartbeat-recovered`, attributed to the task's
  completedBy, with a reason naming the run whose signal never arrived;
- an audit write that THROWS is swallowed, because the recovery is the node
  applying the outcome and letting the failure out would abort the very walk
  that was un-wedging the run;
- an ending nobody answered (terminated, expired -- `completedBy` is null on
  exactly those) records no actor rather than a guessed one.

And one test for the symmetric case the change documented but left unpinned:
a completion that RACED the suspension. `signal()` refuses a run that is not
suspended, so that wake is lost with nothing to retry it; the test asserts
the refusal, asserts the run parks on a non-null heartbeat, and asserts the
next wake recovers it. That is the whole basis for deciding the race needs
no new mechanism, and it is now falsifiable.

Every one of the four was checked by mutation -- breaking the action name,
the attribution, the catch, or the recovery call itself turns each red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: rubenvdlinde

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