[nwparker]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​40	$\color{#cf222e}{\Huge{\mathbf{−}}}$​11	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​29
Prod	0	0	0	0

Supersedes #15165 by @Tauri-EPO — carried over so it can run full CI (fork PRs only run track-community-pr in this repo). Original authorship preserved in the commit.

ELI5

A test claimed "a heartbeat that arrives after the job already finished is ignored." It proved that by checking the heartbeat column ended up empty — but the column starts empty and the test never put anything in it. So "the guard correctly ignored the late heartbeat" and "something wiped the column" looked identical to the test, and so did "recordHeartbeat does nothing at all."

This PR writes a real heartbeat first, so the assertion compares against a timestamp instead of against the row's starting state. A second test does the same for dispatches that end in failure rather than success — a gap that let a whole class of regression through unnoticed.

Test-only. No product code changes.

---

Background: what the straggler guard is

A dispatch is one attempt at running a task on a worker terminal. While it runs, the worker sends a heartbeat every 5 minutes; each one stamps dispatch_contexts.last_heartbeat_at via recordHeartbeat (src/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts:51-58):

// Why: only bump status='dispatched' — a zombie heartbeat from a finished dispatch would mask a hung retry from the stale detector (§5.3.4).
"UPDATE dispatch_contexts SET last_heartbeat_at = ? WHERE id = ? AND status = 'dispatched'"

That AND status = 'dispatched' is the straggler guard. A straggler is a heartbeat that arrives after the dispatch has already settled — completed (dispatch-completion.ts:9-16), or failed / circuit_broken (failDispatch, dispatch-completion.ts:76-105; the status is circuit_broken once failure_count reaches DISPATCH_CIRCUIT_BREAK_FAILURES = 3, dispatch-circuit-breaker.ts:2). Stragglers are normal, not exotic: a heartbeat is a message in flight, and the worker can settle while one is still in transit — more so over an SSH/federation relay, where the delivery leg is long.

The failure it protects against is on the read side. getStaleDispatches (dispatch-completion.ts:60-74) is what notices a wedged worker:

WHERE status = 'dispatched' AND dispatched_at IS NOT NULL
  AND julianday(dispatched_at) < julianday(?)
  AND (last_heartbeat_at IS NULL OR julianday(last_heartbeat_at) < julianday(?))

Its only caller is warnStaleDispatches (coordinator-task-dispatch.ts:17-26), which tells the coordinator worker <handle> on task <id> has not sent a heartbeat in ~10 min. The threshold is deliberately the documented cadence × 2 (coordinator-task-dispatch.ts:13-14), and it warns rather than auto-fails (:16). So last_heartbeat_at is the sole liveness signal, and the warning is the only thing that surfaces a hung worker to a human. Corrupt that column and a genuinely hung dispatch simply never gets reported — the task sits there looking alive, holding its worker slot, forever. There is no second detector behind it.

Why the DB-level guard has to exist even though callers check too. recordHeartbeat has three callers. Only one pre-checks status: lifecycle-reconciliation.ts:170, guarded by :150-157 (suppresses heartbeats for non-dispatched dispatches). The other two do not:

• db/federation/federation-relay-import.ts:92 — heartbeats imported from a federated relay; guarded only by !duplicate.
• db/legacy/legacy-lifecycle-operation.ts:141 — the legacy lifecycle path.

On those two paths the SQL status filter is the only thing standing between a relayed straggler and the liveness column. That is what the test in question is supposed to be guarding.

---

What Changed

src/main/runtime/orchestration/db-heartbeat-straggler-guard.test.ts	new, 40 lines — both halves of the invariant
src/main/runtime/orchestration/db.test.ts	the old straggler case removed (main:657-667)

The new file shares one fixture, seedHeartbeatedDispatch() (db-heartbeat-straggler-guard.test.ts:12-19), which records a heartbeat of '2026-05-03T00:00:00.000Z' while the row is still dispatched (:17) — that seed line is the entire point of the change. Two cases then settle the row and fire a straggler at '2026-05-04…':

• ignores a straggler heartbeat after the dispatch completed (:23-30) — the migrated case.
• ignores a straggler heartbeat after the dispatch failed (:32-39) — new.

recordHeartbeat updates last_heartbeat_at on dispatched rows stays in db.test.ts; nothing else moved. Test counts: db.test.ts 61 → 60, new file 2, so 62 total — one case relocated, one added.

The assertion, before and after

Before (db.test.ts on main, lines 657-667):

const ctx = d.createDispatchContext(task.id, 'term_a')
d.completeDispatch(ctx.id)                                  // last_heartbeat_at is still null here

d.recordHeartbeat(ctx.id, '2026-05-04T00:00:00.000Z')
const after = d.getDispatchContext(task.id)
expect(after?.last_heartbeat_at).toBeNull()

After (db-heartbeat-straggler-guard.test.ts:23-30):

const { d, dispatchId } = seedHeartbeatedDispatch()         // seeds '2026-05-03…' while dispatched
d.completeDispatch(dispatchId)

d.recordHeartbeat(dispatchId, '2026-05-04T00:00:00.000Z')

expect(d.getDispatchContextById(dispatchId)?.last_heartbeat_at).toBe('2026-05-03T00:00:00.000Z')

Why the old one was vacuous. last_heartbeat_at is NULL from row creation, and the old test never wrote to it, so toBeNull() was an assertion about the row's initial value. It therefore held for three mutually exclusive reasons that the test could not tell apart:

1. the status filter excluded the row — the thing it meant to assert;
2. completeDispatch cleared the column on its way through;
3. recordHeartbeat was inert for every row, guard or no guard.

Only (1) is correct behavior. (2) and (3) are product bugs that would have shipped with this test still green — and (2) is a bug in the exact column the stale detector reads. The new expected value is a timestamp, which separates them: '2026-05-03…' means the guard held, null means something cleared the column, '2026-05-04…' means the guard is gone.

One correction to the issue's framing. #15164 / #15165 describe the old assertion as one that "can never fail." That is overstated, and I checked rather than repeating it — see mutation C below, where removing the status filter outright does turn the old assertion red, because the straggler write leaves '2026-05-04…' where null was expected. The accurate claim is narrower: the old assertion could not distinguish the guard working from the column being cleared, and it had no coverage at all for settled-as-failed rows. The commit subject states that weaker, true claim.

Why the second test exists

The filter matches status = 'dispatched' exactly, so it also rejects stragglers on rows settled as failed or circuit_broken. Only the completed path was ever covered. That leaves a specific, very plausible regression uncaught: someone rewrites the guard as status != 'completed' — which reads like a faithful paraphrase of "ignore heartbeats after the dispatch finished" and is wrong, because failed and circuit_broken are also finished. Under that change the entire suite stayed green on main. It does not any more.

Why a new file rather than editing in place

db.test.ts on main measures 799 effective lines against the 800 ceiling for **/*.test.ts (.oxlintrc.json:157-160, skipBlankLines + skipComments). Adding the one-line seed in place would have put it at exactly 800 — legal, but with zero headroom, closing the file to any further test until #14889 splits it, and forcing the failDispatch sibling to live in an unrelated file whose name does not describe it. A max-lines disable is not an option (AGENTS.md hard rule).

Moving both cases out instead solves all three problems at once, and the numbers are in the right direction — see Known limitations for the re-measured figures.

---

Linked Issue

Fixes #15164

Visual Proof

N/A — test-only change. No product code, no rendered UI, no user-visible behavior.

Testing

macOS (Darwin 25.3.0), Node via repo toolchain. The contributor reports the original assertion green on Windows 11 / Node 24.19.0.

Baseline, both touched files:

npx vitest run --config config/vitest.config.ts \
  src/main/runtime/orchestration/db.test.ts \
  src/main/runtime/orchestration/db-heartbeat-straggler-guard.test.ts

 Test Files  2 passed (2)
      Tests  62 passed (62)

Regression-injection proof

A test that could not distinguish outcomes is only provable by making it fail, so I mutated the product code and ran the new assertion and the old one side by side against the same mutation. The old assertion was restored verbatim into a scratch file for the duration; all mutations and the scratch file were reverted afterwards, and git status is clean (the product file is untouched in this PR — the diff is 2 test files).

Mutation A — the realistic straggler bug. Guard widened to status != 'completed' in dispatch-completion.ts:55:

 ❯ db-heartbeat-straggler-guard.test.ts (2 tests | 1 failed)
   × ignores a straggler heartbeat after the dispatch failed

AssertionError: expected '2026-05-04T00:00:00.000Z' to be '2026-05-03T00:00:00.000Z'
Expected: "2026-05-03T00:00:00.000Z"
Received: "2026-05-04T00:00:00.000Z"

 Test Files  1 failed | 1 passed (2)
      Tests  1 failed | 2 passed (3)

The 1 passed test file is the old assertion — green on the bug. This is the coverage gap made concrete: a relayed straggler now overwrites the liveness stamp on a failed/circuit_broken row, and the pre-PR suite says everything is fine.

Mutation B — the confound. completeDispatch (:13) also sets last_heartbeat_at = NULL, the guard left intact:

 FAIL  db-heartbeat-straggler-guard.test.ts > ignores a straggler heartbeat after the dispatch completed
AssertionError: expected null to be '2026-05-03T00:00:00.000Z'

- Expected: "2026-05-03T00:00:00.000Z"
+ Received: null

 Test Files  1 failed | 1 passed (2)
      Tests  1 failed | 2 passed (3)

Again the old assertion passes — null was exactly what it expected. This is reason (2) above: the heartbeat history the stale detector depends on is being destroyed on completion, and the test written to guard that column is satisfied by the destruction.

Mutation C — honesty check. AND status = 'dispatched' removed entirely:

   × recordHeartbeat is a no-op for completed rows (straggler ignored)   [old assertion]
   × ignores a straggler heartbeat after the dispatch completed
   × ignores a straggler heartbeat after the dispatch failed

AssertionError: expected '2026-05-04T00:00:00.000Z' to be null            [old assertion]

 Test Files  2 failed (2)
      Tests  3 failed (3)

All three fail, old assertion included. That is why the "can never fail" framing above is corrected rather than repeated.

Post-mutation restore: git status clean, 62 passed (62) again.

Lint and format

npx oxlint <both test files>       exit 0, no findings
npx oxfmt --check <both test files>   All matched files use the correct format.

Not run locally: pnpm typecheck, full pnpm lint, pnpm build, full suite — CI covers them (these OOM on this machine).

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

Review

• Malicious-code scan of the contributor diff: reviewed the full patch (1 file, +5/-1, single commit 3d39601 by Tauri-EPO). No network calls, no process.env / fs / shell / eval / child_process, no dependency, lockfile, workflow, or installer changes, no embedded secrets. Enumerated every codepoint >127 in the patch — the only non-ASCII character was U+2014 em dash in a comment; no zero-width, bidi, or BOM characters (no Trojan-Source vector). The last_heartbeat_at = NULL injection described above was a local experiment and is not in the diff; confirmed against gh pr diff.
• Cross-platform (macOS / Linux / Windows): N/A in substance. The assertions compare explicit ISO literal strings round-tripped verbatim through SQLite TEXT. No wall clock, no timezone, no path separators, no line-ending sensitivity, no metaKey.
• SSH / remote execution boundary: N/A. No product code; nothing that reports on, stops, or lists remote work. The live / unverifiable / exited vocabulary is not touched. (The federation relay path is described above as motivation, not modified.)
• Remote wire compatibility: N/A. No RPC params, stream opcodes, published host content, DB schema, or persisted format changed. Both tests drive the existing public recordHeartbeat / completeDispatch / failDispatch methods against an in-memory DB, so there is no wire or migration surface.
• Folder workspaces / provider neutrality / git 2.25 baseline: not implicated — no git invocation, no provider-specific concept, no workspace-shape assumption.
• Performance: negligible — 4 extra in-memory SQLite statements across the suite.
• UI quality: N/A — no renderer code.
• Security risk: none. Test-only, :memory: database, no I/O.

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Deviations from #15165 as authored: the 3-line // Why: block was collapsed to one line per AGENTS.md, the commit subject was rewritten to the accurate claim (the old assertion was weak, not incapable of failing), the failDispatch sibling case was added, and both cases were relocated into db-heartbeat-straggler-guard.test.ts. The contributor's authorship is preserved on the original commit and carried as a Co-authored-by trailer on the relocation commit.

The assertion also switched from getDispatchContext(task.id) to getDispatchContextById(dispatchId). Minor, but deliberate: the former returns the newest row for the task (dispatch-context-store.ts:119-126, ORDER BY rowid DESC LIMIT 1), so it would follow a retry onto a different row; the latter pins the exact dispatch the straggler was addressed to (:128-134), which is what the invariant is about.

Known limitations / follow-ups

• db.test.ts remains large — but this PR frees budget rather than consuming it. Re-measured on both sides with the same method (append N synthetic code lines, read the count oxlint reports, subtract N):

  	effective lines	headroom to 800
  main	799	1
  this branch	790	10

  Boundary-checked rather than extrapolated: on this branch +10 synthetic lines lints clean and +11 errors with File has too many lines (801); on main +1 lints clean and +2 errors at 801. So merging this returns ~9 lines to a file that currently has one line of room left. test(orchestration): split dispatch-liveness tests out of db.test.ts #14889 (also @Tauri-EPO) splits the dispatch-liveness cases out and is the durable fix; it is not folded in here because that is a different, larger change, and this PR no longer depends on its merge order.

• circuit_broken has no straggler case of its own. failDispatch transitions to circuit_broken after DISPATCH_CIRCUIT_BREAK_FAILURES (3) consecutive failures, and the status = 'dispatched' filter excludes it for the same reason it excludes failed, but no test pins it. Driving a row to that status needs a multi-failure fixture that would outgrow this PR.

• The §5.3.4 reference in the recordHeartbeat comment does not resolve to anything in docs/ today. Left as-is (untouched product code), noted here for whoever maintains that comment.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 22fee24f-5908-420b-a4eb-377219a5e5a9

📥 Commits

Reviewing files that changed from the base of the PR and between 0553c88 and 870b695.

📒 Files selected for processing (2)

• src/main/runtime/orchestration/db-heartbeat-straggler-guard.test.ts
• src/main/runtime/orchestration/db.test.ts

💤 Files with no reviewable changes (1)

• src/main/runtime/orchestration/db.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

Added a Vitest suite that verifies late heartbeats remain ignored after dispatch completion and failure. The suite initializes and cleans up an in-memory OrchestrationDb. Removed the previous equivalent regression test from db.test.ts.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR fulfills issue [#15164] by seeding an active heartbeat and verifying that completion does not clear or overwrite it.
Out of Scope Changes check	✅ Passed	All changes are focused on related orchestration tests and test-file organization; no unrelated product changes are included.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
Title check	✅ Passed	The title clearly identifies the test change that distinguishes the heartbeat straggler guard from the row's initial null value.
Description check	✅ Passed	The description is complete, focused, and covers the change, rationale, linked issue, testing, review, limitations, and checklist.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Tauri-EPO]

Thanks for carrying this through, and for the correction on the commit subject — you are right, and I was overstating it. "It can never fail" describes injection C only; removing the AND status = 'dispatched' filter outright does make the old toBeNull() fail, because the straggler write leaves a real timestamp behind. Your subject says what the change actually closes, which is narrower and true.

The failDispatch sibling is the better catch. I read that filter as scoping to the live dispatch path and did not follow it through to failed / circuit_broken, where the same exact match on 'dispatched' lets a straggler past unchecked. You confirmed it the same way I confirmed the first case — widen the filter to status != 'completed', watch the suite go from green to red — so I can read that result directly rather than take it on trust.

Noted on the // Why: block: one line, not three.

I will close #15165 once this lands.