Skip to content

No test covers the Reaper LiveDaemonPID wiring in startEnginePlane — deleting it silently disables orphan reaping (fail-closed blind spot) #5935

Description

@cajasmota

Summary

The Reaper's LiveDaemonPID wiring in startEnginePlane
(internal/daemon/engineplane.go) is load-bearing but has no test coverage.
Because the sweep is fail-closed (#5933), deleting that line silently disables
orphan-watcher reaping forever — no error, no log, no failing test.

Mutation evidence

Delete the LiveDaemonPID: line from the ReaperConfig literal in
startEnginePlane, then:

go build ./...                                                              -> OK
go test ./internal/daemon/ -run 'Split|Engine|Serve|Reaper|Watch' -count=1  -> ok

Fully green with the wiring removed. Reproduced independently twice (by the
adversarial reviewer and by the implementer).

Why it could not be covered in #5933

  • startEnginePlane / RunEngine have no direct test coverage; the only harness
    that reaches them is the full daemon.Run path in phaseb_test.go.
  • Reaper.Start has a hardcoded ~30s startup delay before its first sweep, and a
    5-minute sweep interval.
  • ReaperConfig.Interval is never set by engineplane.go, so the interval is not
    test-injectable.

Any real end-to-end assertion would therefore take 30s+ — out of scope for a
bounded fix, and it would land as a slow test in the default suite.

What #5933 does and does not cover

TestReaper_sweepWatchers_LiveDaemonPIDFromPidfile enforces the reaper/watchreg
contract using the exact closure shape engineplane.go wires (real pidfile,
real ReadPIDFile parse path, match-survives / mismatch-reaped). It does not
detect removal of the wiring line itself, because it constructs its own
ReaperConfig rather than going through startEnginePlane.

Suggested fix

  1. Make ReaperConfig.Interval (and ideally the startup delay) test-injectable,
    and have engineplane.go pass an explicit value.
  2. Add a RunEngine/startEnginePlane-level test asserting the constructed
    Reaper resolves LiveDaemonPID from the daemon pidfile — gated opt-in like the
    existing GRAFEL_FSNOTIFY_TESTS pattern if it remains slow.

Alternatively, a cheap structural guard: have startEnginePlane fail loudly (or
log Warn at startup) if it ever builds a watcher-reaping ReaperConfig with a nil
LiveDaemonPID, converting a silent permanent degradation into an observable one.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1User-visible correctness, contributor-reported, or a gate that lets bugs ship

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions