Skip to content

test(mcp,api): close verification gaps in daemon resilience tests - #279

Merged
StefanSteiner merged 4 commits into
tableau:mainfrom
StefanSteiner:test/daemon-resilience-coverage
Sep 6, 2026
Merged

test(mcp,api): close verification gaps in daemon resilience tests#279
StefanSteiner merged 4 commits into
tableau:mainfrom
StefanSteiner:test/daemon-resilience-coverage

Conversation

@StefanSteiner

Copy link
Copy Markdown
Contributor

Summary

Closes four verification gaps in the daemon subsystem found by retrospective adversarial review — tests that cannot fail even if the code they guard is reverted, or that were disabled everywhere despite naming only one platform. This is test-only / test-support work; no production behavior changes.

Per-task detail

1 — daemon resilience tests were disabled on every platform (#271)

#[ignore = "flaky on macOS CI — daemon startup exceeds 150s timeout"] is unconditional, so despite the reason naming macOS, Linux and Windows silently skipped all eight tests too, and no CI job passes --ignored. Changed each to #[cfg_attr(target_os = "macos", ignore = "...")].

Ran all eight locally with --ignored twice in a row: 8/8 pass, ~14–22s total, far under the 150s timeout that motivated the ignore — nothing here is actually broken, just slow-and-flaky specifically on macOS CI.

Also fixed a stale comment in health_listener_waits_for_command_after_accept that still described a 100ms accept-loop poll (now 5ms).

CI change needed (I cannot edit .github/workflows/): add a job that runs cargo test -p hyperdb-mcp -- --ignored with a generous timeout (poll interval is 5ms; the daemon-startup budget is 150s per test, so a Linux/Windows-only job needs real headroom — a 20–25 minute job timeout comfortably covers all eight sequentially even under CI contention). This belongs alongside the existing hyperdb-mcp test job in .github/workflows/ci.yml, scoped to ubuntu-latest and windows-latest runners only (skip macOS, where the #[cfg_attr] keeps them ignored for the stated reason).

2 — recovery_tests.rs false-pass race (#272)

The peer's try_lock probe only proves anything while the client is still blocked inside report_hyperd_error_to_daemon (200ms budget). If the client's call returns first, try_lock trivially succeeds regardless of production behavior — a silent false pass, not a flake.

Reproduced directly: inserting an artificial 300ms delay before the old probe logic made the test report ok while proving nothing. The same delay against the new code fails loudly:

first report probe is inconclusive: the loss worker's call returned before the peer could observe the engine mutex...
second report probe is inconclusive: the reinit worker's call returned before the peer could observe the engine mutex...

Fix: an AtomicBool set by each worker the instant its call returns, read by the peer before try_lock. Also deleted the comment claiming production still holds the engine mutex through Engine::new — true at v0.7.2, false today (ensure_engine uses a separate engine_initialization single-flight mutex).

3 — set_nonblocking test could not fail (#273)

health_listener_waits_for_command_after_accept passes trivially on Linux, where accepted sockets are already blocking regardless of the listener's mode — so reverting the one-line set_nonblocking(false) fix keeps Linux CI green. Extracted the accept step into accept_and_force_blocking (behavior-preserving refactor) and added a unit test that reads the accepted socket's real O_NONBLOCK flag via fcntl(F_GETFL).

Verified by reverting the fix: the new test fails on this machine with the flag observed set (flags=4 vs expected 0); restoring the fix makes it pass again. Also named the BSD/macOS-vs-Linux platform reason in the code comment, and added the missing hyperdb-mcp/CHANGELOG.md entry (merged into the existing ## [Unreleased]### Fixed heading, no duplicate heading).

4 — torn PID-file read (#275 item 2)

The child reports its hyperd PID with a non-atomic fs::write (File::create then write_all). wait_for_reported_pid already retried on NotFound but treated a successful read of empty/unparseable content as fatal. Now it retries past that window to the deadline.

Added wait_for_reported_pid_retries_past_a_torn_write, reproducing the exact non-atomic sequence with a stretched-out empty window. Verified by reverting the fix: fails with the exact reported symptom (invalid PID report "": cannot parse integer from empty string); restoring the fix passes again.

Verification

  • cargo fmt --all -- --check — exit 0
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — exit 0 (one needless_continue fixed along the way)
  • cargo test -p hyperdb-mcp611 passed, 0 failed, exit 0 (per-crate; not a workspace-wide total)
  • cargo test -p hyperdb-api --test process_tests12 passed, 0 failed, exit 0
  • The 8 previously-ignored daemon tests via --ignored --test-threads=18 passed, 0 failed, exit 0 (two consecutive runs)
  • Every hardened test proven by reverting the code it guards and observing the failure, then restoring (see per-task detail above and commit messages)

Not verified / out of scope

  • CI behavior on Linux/Windows runners themselves — only verified locally on macOS (where the un-ignored tests still run, since --ignored was passed explicitly). The actual CI job addition is described above but not implemented (blocked by the .github/workflows/ edit restriction on this environment).
  • Whether the eight tests are ever flaky on a real, loaded macOS CI runner (vs. this local machine) — I have no access to that environment; the existing #[ignore] on macOS is left in place on the strength of the original report rather than re-litigated.

…ows CI

Eight resilience integration tests in daemon_tests.rs — including
client_report_triggers_restart_after_kill,
hyperd_monitor_detects_killed_hyperd_and_restarts,
engine_recovers_after_hyperd_killed, and
daemon_mode_two_engines_share_same_hyperd — carried an unconditional
#[ignore], so Linux and Windows CI silently skipped them even though the
stated reason names macOS only. The subsystem whose entire purpose is
resilience had zero automated restart-after-crash coverage on any platform.

Switch each to #[cfg_attr(target_os = "macos", ignore = ...)] so the
existing reason and behavior are preserved on macOS while Linux and Windows
now run them. Verified locally (macOS, where the ignore still applies by
default): running them explicitly with --ignored passes reliably, 8/8, in
~14-22s total across two consecutive runs — nowhere near the 150s timeout
that motivated the ignore, so the ignore itself is a real per-platform
accommodation, not evidence the tests are broken.

Also corrects a stale comment in health_listener_waits_for_command_after_accept
that still described the listener's old 100ms accept-loop poll interval
(now 5ms).
slow_health_report_does_not_hold_engine_mutex asserted that the engine
mutex stays available while a REPORT_HYPERD_ERROR round-trip is pending,
using a peer-side try_lock probe as the proof. But the probe was only
meaningful while the calling worker was still blocked inside
report_hyperd_error_to_daemon (200ms budget). If the worker's call
returned before the peer reached the probe — e.g. a scheduler delay
pushing the peer past 200ms — the worker had already released everything,
so try_lock trivially succeeded and the assertion passed without
observing the property at all. Reproduced directly: inserting an
artificial 300ms delay before the old probe made the test report 'ok'
despite proving nothing; the same delay against the new code fails
loudly with an explicit 'inconclusive' message instead.

Fix: an AtomicBool set by each worker the instant its call returns, read
by the peer before try_lock. If the worker already finished, the
observation is marked probe_valid: false and the test fails with a clear
'rerun' message instead of silently passing.

Also deletes a comment claiming 'production is red here because
ensure_engine holds the engine mutex throughout Engine::new' — true at
v0.7.2, false today: ensure_engine takes a separate engine_initialization
single-flight mutex and drops the engine guard before Engine::new. Left
in place, it would send a future maintainer hunting a problem that
doesn't exist.
health_listener_waits_for_command_after_accept infers HealthListener's
set_nonblocking(false) fix (for accepted sockets inheriting the
listener's O_NONBLOCK on BSD-derived kernels — macOS included, not Linux)
from read timing. On Linux, accepted sockets are blocking regardless of
the listener's mode, so that test passes trivially there even with the
fix reverted; it can only ever catch a regression on the BSD kernels that
actually have the bug.

Extract the accept-and-force-blocking step into accept_and_force_blocking
(behavior-preserving refactor — same logic, now unit-testable) and add
accept_and_force_blocking_clears_nonblocking_flag, which reads the
accepted socket's real O_NONBLOCK flag via fcntl(F_GETFL) instead of
inferring it from behavior. Verified by reverting the one-line
set_nonblocking(false) fix: the new test fails on this machine with
O_NONBLOCK observed set (flags=4 vs expected 0); restoring the fix makes
it pass again.

Also names the BSD/macOS-vs-Linux platform reason in
accept_and_force_blocking's doc comment (previously boilerplate that a
future simplifier could plausibly delete), and adds the missing
CHANGELOG entry for the underlying fix, merged into the existing
Unreleased ### Fixed heading.
The child in callback_connection_shutdowns_hyperd_after_parent_kill
reports its hyperd PID via fs::write, which is File::create (truncate or
create) then write_all — not atomic. wait_for_reported_pid already
retried on NotFound, but treated a successful read of unparseable
content as fatal, so a parent poll landing in the create/write_all gap
made "".parse::<u32>() panic the whole test instead of retrying.

Treat an empty or unparseable read the same as 'not created yet' and
keep polling to the deadline, only surfacing an error if the file never
became parseable in time.

Adds wait_for_reported_pid_retries_past_a_torn_write, which reproduces
the exact non-atomic sequence (create, sleep, then write real content)
against a stretched-out empty window. Verified by reverting the fix: the
new test fails with the exact reported symptom (invalid PID report "":
cannot parse integer from empty string); restoring the fix makes it pass
again.
@StefanSteiner
StefanSteiner merged commit 56bc0d0 into tableau:main Sep 6, 2026
17 checks passed
StefanSteiner added a commit to StefanSteiner/hyper-api-rust that referenced this pull request Sep 6, 2026
Follow-up to the adversarial review of tableau#278.

The new tolerant `DaemonInfo` fallback in `discover()` yields a record where
the pre-change `Malformed` arm returned early, so control now reaches the
stale-cleanup `remove_file` it never used to reach. That lands squarely in
the scenario this PR exists to serve: an old client that fails a single
300 ms PING against a live *newer* daemon would delete that daemon's
discovery record, which the daemon only rewrites on `hyperd` restart —
hiding a healthy daemon from every client on the machine, not just the one
that could not parse it. It also contradicted the doctor's own operator
message, "The daemon discovery file was malformed; it was left unchanged."

`discover()` now tracks whether the record came from the tolerant fallback
and skips the cleanup in that case. A strictly parsed record on a dead port
is still stale-cleaned exactly as before; both halves are pinned by the new
`discover_preserves_a_leniently_parsed_stale_record` regression test.

Also in this change:

- The fallback no longer re-reads the file from disk.
  `RawDiscoveryRead::Malformed` and `Oversized` carry the bytes already
  read, removing both the redundant I/O and the window in which the strict
  and tolerant parses could observe different content.
- `discover()`'s `Oversized` arm is folded into the fallback instead of
  returning `None`. The legacy reader applies no size cap, so the arm is
  unreachable today; folding it in means adding a cap later cannot silently
  turn a large-but-valid record into an undiscoverable daemon — the exact
  defect the fallback exists to prevent.
- The changelog described a design the code does not have (legacy fields
  "always" extracted, the strict shape used only to pick a log message).
  Rewritten to match the implementation: strict parse first, tolerant
  fallback second, and the preserved-record behavior documented.
- `send_doctor_command` now classifies a peer that closes without writing as
  `UnexpectedEof`, matching the `send_command_with_timeout` fix. This changes
  one operator-visible doctor warning: a recorded candidate that accepts the
  connection and then answers nothing is reported as
  `daemon_discovery_candidate_unreachable` rather than
  `daemon_status_malformed`, which is what actually happened. Pinned by
  `real_doctor_reports_a_silent_peer_close_as_unreachable_not_malformed`.

Test polish carried over from the tableau#279 review:

- `process_tests`' doc comment credited the parent with writing the PID
  report; the child writes it and the parent polls.
- The child now publishes that PID atomically (temp file plus `fs::rename`),
  so a poll cannot read a truncated-yet-parseable prefix and silently accept
  a wrong PID. The retry loop stays as defense in depth.
- `recovery_tests` reads `probe_valid` after `try_lock` rather than before.
  The flag is monotonic false->true, so reading it later can only produce a
  false *inconclusive*, never a false pass; reading it first left a window
  for the worker to return in between and mark a vacuous probe valid.
StefanSteiner added a commit to StefanSteiner/hyper-api-rust that referenced this pull request Sep 6, 2026
Follow-up to the adversarial review of tableau#278.

The new tolerant `DaemonInfo` fallback in `discover()` yields a record where
the pre-change `Malformed` arm returned early, so control now reaches the
stale-cleanup `remove_file` it never used to reach. That lands squarely in
the scenario this PR exists to serve: an old client that fails a single
300 ms PING against a live *newer* daemon would delete that daemon's
discovery record, which the daemon only rewrites on `hyperd` restart —
hiding a healthy daemon from every client on the machine, not just the one
that could not parse it. It also contradicted the doctor's own operator
message, "The daemon discovery file was malformed; it was left unchanged."

`discover()` now tracks whether the record came from the tolerant fallback
and skips the cleanup in that case. A strictly parsed record on a dead port
is still stale-cleaned exactly as before; both halves are pinned by the new
`discover_preserves_a_leniently_parsed_stale_record` regression test.

Also in this change:

- The fallback no longer re-reads the file from disk.
  `RawDiscoveryRead::Malformed` and `Oversized` carry the bytes already
  read, removing both the redundant I/O and the window in which the strict
  and tolerant parses could observe different content. Those bytes are held
  in a `DiscoveryBytes` newtype whose `Debug` prints only a length:
  `RawDiscoveryRead` is `Debug`-formatted into diagnostics, and a derived
  `Debug` on a `Vec<u8>` would echo the whole unparsed file as a decimal
  byte list. `raw_discovery_read_is_non_mutating_and_distinguishes_io`
  already asserted that contents do not leak, but only as text, so it would
  not have caught that form; it now checks for the byte-list rendering too.
- `discover()`'s `Oversized` arm is folded into the fallback instead of
  returning `None`. The legacy reader applies no size cap, so the arm is
  unreachable today; folding it in means adding a cap later cannot silently
  turn a large-but-valid record into an undiscoverable daemon — the exact
  defect the fallback exists to prevent.
- The changelog described a design the code does not have (legacy fields
  "always" extracted, the strict shape used only to pick a log message).
  Rewritten to match the implementation: strict parse first, tolerant
  fallback second, and the preserved-record behavior documented.
- `send_doctor_command` now classifies a peer that closes without writing as
  `UnexpectedEof`, matching the `send_command_with_timeout` fix. This changes
  one operator-visible doctor warning: a recorded candidate that accepts the
  connection and then answers nothing is reported as
  `daemon_discovery_candidate_unreachable` rather than
  `daemon_status_malformed`, which is what actually happened. Pinned by
  `real_doctor_reports_a_silent_peer_close_as_unreachable_not_malformed`.

Test polish carried over from the tableau#279 review:

- `process_tests`' doc comment credited the parent with writing the PID
  report; the child writes it and the parent polls.
- The child now publishes that PID atomically (temp file plus `fs::rename`),
  so a poll cannot read a truncated-yet-parseable prefix and silently accept
  a wrong PID. The retry loop stays as defense in depth.
- `recovery_tests` reads `probe_valid` after `try_lock` rather than before.
  The flag is monotonic false->true, so reading it later can only produce a
  false *inconclusive*, never a false pass; reading it first left a window
  for the worker to return in between and mark a vacuous probe valid.
StefanSteiner added a commit to StefanSteiner/hyper-api-rust that referenced this pull request Sep 6, 2026
Follow-up to the adversarial review of tableau#278.

The new tolerant `DaemonInfo` fallback in `discover()` yields a record where
the pre-change `Malformed` arm returned early, so control now reaches the
stale-cleanup `remove_file` it never used to reach. That lands squarely in
the scenario this PR exists to serve: an old client that fails a single
300 ms PING against a live *newer* daemon would delete that daemon's
discovery record, which the daemon only rewrites on `hyperd` restart —
hiding a healthy daemon from every client on the machine, not just the one
that could not parse it. It also contradicted the doctor's own operator
message, "The daemon discovery file was malformed; it was left unchanged."

`discover()` now tracks whether the record came from the tolerant fallback
and skips the cleanup in that case. A strictly parsed record on a dead port
is still stale-cleaned exactly as before; both halves are pinned by the new
`discover_preserves_a_leniently_parsed_stale_record` regression test.

Also in this change:

- The fallback no longer re-reads the file from disk.
  `RawDiscoveryRead::Malformed` and `Oversized` carry the bytes already
  read, removing both the redundant I/O and the window in which the strict
  and tolerant parses could observe different content. Those bytes are held
  in a `DiscoveryBytes` newtype whose `Debug` prints only a length:
  `RawDiscoveryRead` is `Debug`-formatted into diagnostics, and a derived
  `Debug` on a `Vec<u8>` would echo the whole unparsed file as a decimal
  byte list. `raw_discovery_read_is_non_mutating_and_distinguishes_io`
  already asserted that contents do not leak, but only as text, so it would
  not have caught that form; it now checks for the byte-list rendering too.
- `discover()`'s `Oversized` arm is folded into the fallback instead of
  returning `None`. The legacy reader applies no size cap, so the arm is
  unreachable today; folding it in means adding a cap later cannot silently
  turn a large-but-valid record into an undiscoverable daemon — the exact
  defect the fallback exists to prevent.
- The changelog described a design the code does not have (legacy fields
  "always" extracted, the strict shape used only to pick a log message).
  Rewritten to match the implementation: strict parse first, tolerant
  fallback second, and the preserved-record behavior documented.
- `send_doctor_command` now classifies a peer that closes without writing as
  `UnexpectedEof`, matching the `send_command_with_timeout` fix. This changes
  one operator-visible doctor warning: a recorded candidate that accepts the
  connection and then answers nothing is reported as
  `daemon_discovery_candidate_unreachable` rather than
  `daemon_status_malformed`, which is what actually happened. Pinned by
  `real_doctor_reports_a_silent_peer_close_as_unreachable_not_malformed`.

Test polish carried over from the tableau#279 review:

- `process_tests`' doc comment credited the parent with writing the PID
  report; the child writes it and the parent polls.
- The child now publishes that PID atomically (temp file plus `fs::rename`),
  so a poll cannot read a truncated-yet-parseable prefix and silently accept
  a wrong PID. The retry loop stays as defense in depth.
- `recovery_tests` reads `probe_valid` after `try_lock` rather than before.
  The flag is monotonic false->true, so reading it later can only produce a
  false *inconclusive*, never a false pass; reading it first left a window
  for the worker to return in between and mark a vacuous probe valid.
StefanSteiner added a commit that referenced this pull request Sep 6, 2026
…TATUS (#286)

* fix(mcp): stop discarding daemon discovery forward compatibility

Four correctness defects in hyperdb-mcp/src/daemon/discovery.rs:

- discover() deserialized the whole DaemonRecord (including the nested
  identity object) and then discarded it via record.info().clone(),
  buying strictness that cost forward compatibility. A daemon.json with
  a retyped identity, a reshaped executable_path, or an identity
  missing executable_path made a live, healthy daemon undiscoverable,
  defeating wait_for_daemon, status_degraded, and Engine::is_running.
  discover() now always extracts the legacy DaemonInfo fields (which
  ignore unknown fields) and only uses the stricter DaemonRecord parse
  to choose a debug-log message. The doctor's separate bounded raw
  reader is untouched and keeps the strict contract, since it
  deliberately wants to know about a malformed identity block.
  Fixes #270.

- write_discovery_record deleted the existing daemon.json before
  renaming the temp file into place, on the false premise that
  std::fs::rename fails when the target exists on Windows. It doesn't:
  MoveFileExW (MOVEFILE_REPLACE_EXISTING) and the FileRenameInfoEx
  fallback both replace an existing target, matching Unix. The
  unnecessary delete opened a window where daemon.json didn't exist at
  all, observable by a concurrent discover() during try_restart_hyperd.
  The write is now a single rename, as the doc comment already claimed.

- The bounded raw discovery reader classified an oversized-but
  well-formed record as Malformed, sending users to fix JSON syntax
  that was never broken. RawDiscoveryRead gained an Oversized variant;
  the doctor now reports what actually happened.

- send_command_with_timeout returned Ok("") when a health-command peer
  closed the connection without writing anything, so daemon_stop
  printed "Daemon responded:" with nothing after it and exited 0. It
  now returns UnexpectedEof.

Fixes #270. Fixes 3 of 5 items in #275 (the atomic write, the
oversized-record misclassification, and the empty peer-close success).

* fix(mcp): preserve discovery records the tolerant parse could not verify

Follow-up to the adversarial review of #278.

The new tolerant `DaemonInfo` fallback in `discover()` yields a record where
the pre-change `Malformed` arm returned early, so control now reaches the
stale-cleanup `remove_file` it never used to reach. That lands squarely in
the scenario this PR exists to serve: an old client that fails a single
300 ms PING against a live *newer* daemon would delete that daemon's
discovery record, which the daemon only rewrites on `hyperd` restart —
hiding a healthy daemon from every client on the machine, not just the one
that could not parse it. It also contradicted the doctor's own operator
message, "The daemon discovery file was malformed; it was left unchanged."

`discover()` now tracks whether the record came from the tolerant fallback
and skips the cleanup in that case. A strictly parsed record on a dead port
is still stale-cleaned exactly as before; both halves are pinned by the new
`discover_preserves_a_leniently_parsed_stale_record` regression test.

Also in this change:

- The fallback no longer re-reads the file from disk.
  `RawDiscoveryRead::Malformed` and `Oversized` carry the bytes already
  read, removing both the redundant I/O and the window in which the strict
  and tolerant parses could observe different content. Those bytes are held
  in a `DiscoveryBytes` newtype whose `Debug` prints only a length:
  `RawDiscoveryRead` is `Debug`-formatted into diagnostics, and a derived
  `Debug` on a `Vec<u8>` would echo the whole unparsed file as a decimal
  byte list. `raw_discovery_read_is_non_mutating_and_distinguishes_io`
  already asserted that contents do not leak, but only as text, so it would
  not have caught that form; it now checks for the byte-list rendering too.
- `discover()`'s `Oversized` arm is folded into the fallback instead of
  returning `None`. The legacy reader applies no size cap, so the arm is
  unreachable today; folding it in means adding a cap later cannot silently
  turn a large-but-valid record into an undiscoverable daemon — the exact
  defect the fallback exists to prevent.
- The changelog described a design the code does not have (legacy fields
  "always" extracted, the strict shape used only to pick a log message).
  Rewritten to match the implementation: strict parse first, tolerant
  fallback second, and the preserved-record behavior documented.
- `send_doctor_command` now classifies a peer that closes without writing as
  `UnexpectedEof`, matching the `send_command_with_timeout` fix. This changes
  one operator-visible doctor warning: a recorded candidate that accepts the
  connection and then answers nothing is reported as
  `daemon_discovery_candidate_unreachable` rather than
  `daemon_status_malformed`, which is what actually happened. Pinned by
  `real_doctor_reports_a_silent_peer_close_as_unreachable_not_malformed`.

Test polish carried over from the #279 review:

- `process_tests`' doc comment credited the parent with writing the PID
  report; the child writes it and the parent polls.
- The child now publishes that PID atomically (temp file plus `fs::rename`),
  so a poll cannot read a truncated-yet-parseable prefix and silently accept
  a wrong PID. The retry loop stays as defense in depth.
- `recovery_tests` reads `probe_valid` after `try_lock` rather than before.
  The flag is monotonic false->true, so reading it later can only produce a
  false *inconclusive*, never a false pass; reading it first left a window
  for the worker to return in between and mark a vacuous probe valid.

* fix(mcp): publish a restarted hyperd endpoint to daemon.json before STATUS

`try_restart_hyperd` copied the new endpoint into the in-memory `DaemonInfo`
that the health port's STATUS command serves, released that lock, and only
then rewrote `daemon.json`. `discovery::discover()` — the path every client's
`Engine::new` takes — reads the file, so a client discovering inside that
window connected to the `hyperd` the daemon had just dropped.

The write failing was worse than the window: the error return happened with
the new endpoint already published to STATUS and the replacement
`HyperProcess` dropped on the way out, leaving STATUS durably naming a
`hyperd` the daemon itself had killed.

Persist the discovery file first and flip `DaemonInfo` second, both under one
`info_arc` lock, so observing the new endpoint through either channel implies
a live `hyperd` behind it and no STATUS reader can catch an endpoint the file
has not committed. `write_discovery_record` is already atomic (temp file +
`fs::rename`), so this is purely an ordering change.

Also fix the readiness gate the restart tests share, which is what actually
reddened CI — the ordering hazard above is real but was not the cause, since
the file and STATUS agreed in all 32 instrumented runs.
`wait_for_endpoint_change_or_recovery` returned as soon as the endpoint STATUS
reported was TCP-connectable, never comparing it against the pre-kill one
despite its name. For a few milliseconds after SIGKILL the doomed hyperd's
listening socket still completes inbound connections, and the callers poll
immediately after the kill while the 5s monitor tick has not fired — so the
gate returned on its first attempt carrying the pre-kill endpoint, and the
caller then raced the kernel tearing that socket down.

Every failing run finished in ~0.53s against ~5.47s for a passing one, which
is proof the failures land before any restart could have completed. The
renamed `wait_for_live_hyperd_after_kill` waits for the killed endpoint to
stop accepting connections first, then for STATUS's endpoint to be reachable.
It deliberately does not require the endpoint to *change*, since the OS is
free to hand the replacement the same port and a changed-endpoint gate would
then wait out its whole timeout on a healthy restart.

Measured on macOS via `--ignored`, 20 runs each, before -> after:

  engine_recovers_after_hyperd_killed                4 failures -> 0
  hyperd_monitor_detects_killed_hyperd_and_restarts   0 failures -> 0
  client_report_triggers_restart_after_kill           0 failures -> 0

The monitor test was not failing but was passing *vacuously*: 5 of its 20
baseline runs finished in ~0.46s instead of ~5.4s, having verified no restart
at all. After the fix all 20 take the full restart time.

Fixes #284
StefanSteiner added a commit that referenced this pull request Sep 7, 2026
… budget (#309)

* test: make changelog contract robust to rollover; bump restart budget

Fix A: smoke_demo_and_changelog_contract asserted KV/export/routing claims
only against the `## [Unreleased]` slice of the embedded mcp CHANGELOG.
PR #307 (the rc.3 changelog rollover) moved every bullet into
`## [1.0.0-rc.3]`, emptying Unreleased and failing the test. Because #307
was docs-only, CI's `paths-ignore: **/*.md` skipped the Rust suite, so the
break surfaced only on a later code PR. Assert instead against the current
release window (`## [Unreleased]` + the most-recent dated section) via a new
`current_release_window` helper, so a future rollover can't silently
rebreak it. The window is bounded to the newest dated section so a stale
token in an ancient entry can't satisfy a deleted-claim check.

Fix B: engine_recovers_after_hyperd_killed and
hyperd_monitor_detects_killed_hyperd_and_restarts time out on
ubuntu-latest under CI load — a 5s monitor tick plus cold hyperd spawn
against a 12s readiness budget. Replace the three magic `12`s with a named
RESTART_READINESS_BUDGET_SECS = 45 and document the derivation. Budget bump
only; the macOS-only ignore is unchanged, so Linux/Windows crash-recovery
coverage from #279/#286 stays. See #305.

* test: gate RESTART_READINESS_BUDGET_SECS to cfg(unix)

The constant's only use sites are the three `#[cfg(unix)]` restart tests
and their Unix-only helpers, so on Windows it compiled but was never
referenced and `clippy -D warnings` (windows-latest) rejected it as dead
code. Gate the definition with `#[cfg(unix)]` so it exists exactly where
it's used — the honest fix, not `#[allow(dead_code)]`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant