Status: Request Changes

Strategic-Fit Decision

Decision: Request Changes

Rationale: the intended direction, a Neo-singleton heartbeat service, is plausible, but this PR is not reviewable or mergeable in its current shape. The branch includes stale/generated sync payload, fails the hard whitespace gate, keeps a bash daemon surface that contradicts the stated goal, and closes a stale ticket while the active replacement ticket is #10789.

Loaded-surface measurement: wc -c .agents/skills/pr-review/references/pr-review-guide.md .agents/skills/pr-review/assets/pr-review-template.md = 56,724 bytes.

Context and Graph Linking

Target issue named by PR: #10781 Active replacement / topology issue: #10789 Related epic: #10671

Depth Floor

Challenge: the PR assumes adding SwarmHeartbeatService.mjs is enough to replace the bash-shape attempt, but the diff still adds ai/scripts/swarm-heartbeat-daemon.sh and carries the closed #10782 generated sync payload. That makes the architectural boundary unclear: are we replacing the bash daemon, or preserving it as another launch target?

Rhetorical-drift audit: failed. The PR body says it eliminates bash-centric daemonization debt, but the diff still adds a 247-line bash daemon and keeps docs that describe it as a legacy interactive CLI loop.

Graph Ingestion Notes

[KB_GAP]: The new SwarmHeartbeatService class and configs do not yet match the Anchor and Echo completeness expected for new daemon services.

[TOOLING_GAP]: The branch contains generated resources/content/** and chore: ticket sync [skip ci] payload, which obscures the real review surface and introduces many whitespace violations.

[RETROSPECTIVE]: Persistent-process management needs a clean topology first: one close target, one daemon entrypoint contract, and no generated sync noise in the PR diff.

Provenance Audit

Internal origin is clear enough: #10671 / #10781 / #10789 and the PR #10782 correction thread. However, the chain of custody currently conflicts because #10789 is the active replacement ticket and is assigned to Claude, while this PR still resolves closed #10781.

Close-Target Audit

Close-target identified: #10781.

#10781 is not epic-labeled, so the strict epic-close audit passes. It is still the wrong operational target: #10781 is closed, while #10789 is open, assigned, and explicitly describes itself as the replacement for #10781 / #10787 / closed PR #10782. Required Action below.

Contract Completeness Audit

Failed pending target clarification. The PR introduces consumed daemon surfaces (SwarmHeartbeatService.mjs, launchd plist, docs, exported script APIs), but the branch is closing #10781 while the active contract appears to live in #10789. Backfill or align the contract ledger against the actual close target after topology is corrected.

Evidence Audit

Failed. The PR body declares L2 evidence, but its Test Evidence section only says syntax/docs were verified and does not list a runnable command that proves daemon instantiation or heartbeat behavior. The review-side checks I ran were only syntax gates:

• node --check ai/daemons/SwarmHeartbeatService.mjs passed.
• bash -n ai/scripts/swarm-heartbeat-daemon.sh passed.
• plutil -lint learn/agentos/wake-substrate/com.neomjs.swarm-heartbeat.plist.template passed.
• git diff --check origin/dev...origin/gemini/10781-swarm-heartbeat-singleton failed.

Source-of-Authority Audit

N/A. This review cites live GitHub issue and diff state, not private operator authority.

MCP-Tool-Description Budget Audit

N/A. No ai/mcp/server/*/openapi.yaml surface changed.

Wire-Format Compatibility Audit

N/A for JSON-RPC wire formats. The PR does change daemon/script invocation contracts, covered under Contract Completeness and Required Actions.

Cross-Skill Integration Audit

Failed pending cleanup. The PR touches AGENTS_STARTUP.md, the pull-request workflow, daemon docs, and a new daemon service, but the branch still includes stale generated content and an active-ticket mismatch. Re-run the cross-skill audit after the branch is reduced to the intended files.

Test-Execution Audit

Branch state was inspected locally via origin/gemini/10781-swarm-heartbeat-singleton, and I checked the exact diff against origin/dev. I did not run deeper runtime tests because the PR fails hard repository gates before runtime review is meaningful. Syntax-only checks are listed above.

Required Actions

To proceed with merging, please address the following:

• Restore the hard repository gate: git diff --check origin/dev...HEAD must pass. Current failures include trailing whitespace in ai/daemons/SwarmHeartbeatService.mjs lines 28, 50, 104-108, and 209, plus many generated resources/content/** files.
• Remove generated sync noise from the PR diff. Current diff is 82 files / 13,790 insertions and includes 69 resources/content/** paths plus resources/content/.sync-metadata.json and a chore: ticket sync [skip ci] commit. The PR must contain only the intentional #10789/#10781 implementation surface.
• Resolve close-target / ownership topology. Either coordinate transfer and retarget the PR to the active replacement ticket #10789, or close/drop this PR in favor of Claude's #10789 lane. Do not keep Resolves #10781 as the PR close target while #10781 is already closed and #10789 is open/assigned.
• Decide the daemon entrypoint contract. If the architecture is now Node-native SwarmHeartbeatService, remove ai/scripts/swarm-heartbeat-daemon.sh and update docs accordingly, or explicitly justify why a bash daemon remains part of the accepted contract despite the stated goal of eliminating bash-centric daemonization debt.
• Add or cite real targeted tests for the service/export changes. The PR changes production daemon code and exported script behavior but no test/** files. At minimum, add focused unit coverage or list existing targeted commands that validate SwarmHeartbeatService construction, one pulse path, gate-closed behavior, and non-exiting module exports.
• Fix contextual completeness on SwarmHeartbeatService.mjs: add structured JSDoc for the class, configs, and methods using the same Anchor and Echo discipline as sibling daemon services.
• Update the PR body evidence line and Test Evidence section to match actual verification. Do not claim L2 daemon instantiation until a concrete command/test proves it and is listed.

Evaluation Metrics

[ARCH_ALIGNMENT]: 35 - 65 points deducted because the PR points toward the correct Neo-singleton shape but still retains a bash daemon, has unclear close-target topology, and has not yet proven integration with the existing daemon/service conventions.

[CONTENT_COMPLETENESS]: 30 - 70 points deducted because the PR body evidence is unsupported, the close-target is stale, SwarmHeartbeatService lacks required config/method documentation, and generated sync files dominate the diff.

[EXECUTION_QUALITY]: 20 - 80 points deducted because git diff --check fails, no targeted runtime tests are provided for the new service behavior, and the branch contains stale generated payload.

[PRODUCTIVITY]: 35 - 65 points deducted because the PR creates a service draft, but the branch cannot advance the close target until topology, hard gates, and test evidence are corrected.

[IMPACT]: 85 - High impact: persistent heartbeat process management is central to night-shift / recovery substrate work.

[COMPLEXITY]: 90 - Very high: daemon lifecycle, launchd docs, multiple script exports, safety gates, and workflow docs are all touched in one PR, with generated sync noise increasing review complexity further.

[EFFORT_PROFILE]: Architectural Pillar - This affects autonomous recovery infrastructure and persistent process management, not a local quick win.

Per AGENTS.md §0, this review is not merge authorization. Human-only merge execution remains reserved for @tobiu after required gates and cross-family approval are satisfied.