fix(trusty-installer): probe trusty-mpm /health over HTTP instead of reporting unknown - #4935
Open
mac-duetto wants to merge 1 commit into
Open
fix(trusty-installer): probe trusty-mpm /health over HTTP instead of reporting unknown#4935mac-duetto wants to merge 1 commit into
mac-duetto wants to merge 1 commit into
Conversation
…reporting unknown `probe_member_health` returned `ProbeOutcome::Unprobeable` for any member whose `ManageStrategy` is `OwnVerb` — trusty-mpm alone — even though the daemon answers `GET http://127.0.0.1:7880/health`, its `http_addr` discovery file is present, and `probe_http::fixed_port_for` already maps `"trusty-mpm" => Some(7880)`. Probeability is a property of the daemon's HTTP transport, not of its lifecycle-management strategy. The two axes coincided while the probe was a `<binary> health --json` subprocess and diverged when #4246 moved it to HTTP `/health`; the match arm kept keying off the lifecycle enum. `OwnVerb` now takes the same transport as `Launchd`; only `ManageStrategy::None` (a non-daemon) stays `Unprobeable`. `ManageStrategy` itself is untouched — it still governs `tctl start|stop|restart` dispatch and `needs_kickstart`, which independently requires `Launchd`. That is why this is a one-arm change and why mpm stays `OwnVerb` in `stable_set`: a confirmed-down mpm now reaches `Refused` yet still cannot be handed to `launchctl kickstart -k` against the nonexistent `com.trusty.mpm` label. ACCEPTED POLICY CONSEQUENCE. mpm is `required: true`, and `down` — unlike `unknown` — fails `VerifyTailReport::build` and degrades `status`'s exit code. A user who has simply not started mpm now gets `tctl status` / `tctl stack health` / `tctl stack doctor` -> `degraded`, exit 2, and `tctl install` -> NOT VERIFIED. This is intended: mpm becomes consistent with its declared `required` flag rather than exempt from it, exactly as a stopped trusty-search already behaves. No escape hatch, env-var opt-out or `required: false` demotion was added. It also stops `tctl up` issuing a redundant `start` against a daemon already known to be serving. Tests: `own_verb_member_is_unknown` is replaced by `own_verb_member_is_probed_over_http` (the pin this issue removes, inverted rather than deleted), plus `own_verb_member_refused_when_nothing_listens`, `non_daemon_member_is_unprobeable`, and `required_mpm_reporting_down_fails_verification`. The `OwnVerb` row of `needs_kickstart_only_for_confirmed_down_launchd` is now load-bearing for a live code path and says so. `stable_set::mpm_uses_own_verb` and `manage_strategy_for_matches_the_stable_set` are unchanged and green — proof the lifecycle axis was not touched. Also corrects the doc comments and vmtest-harness sites that recorded the #4246 carve-out as a current invariant, and widens the harness oracle's `H_P` to accept `down` for a non-launchd member pre-start (the same ordering artefact it already accepts for a launchd member with no plist); a genuinely dead mpm still fails at `verify_daemon_liveness`, which probes it after `tctl start --json`. Closes #4925 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4925.
What changed
One match arm in
crates/trusty-installer/src/commands/probe.rs—probe_member_healthnow routes
ManageStrategy::OwnVerbthrough the same HTTP transport asLaunchd:ManageStrategywas overloaded to mean two things that stopped being the same thingin #4246: how a member is started/stopped, and whether its health is probeable. Those
coincided while the probe was
<binary> health --json, a subprocess contract. Once#4246 moved the transport to HTTP
/health, probeability became a property of thedaemon's transport rather than its supervision model — but the match arm still keyed it
off the lifecycle enum, leaving trusty-mpm (the only
OwnVerbmember) reported asunknowndespite answeringGET http://127.0.0.1:7880/healthwith 200.Everything else in the diff is doc comments in the eight files that cited mpm as the
live "unprobeable" example, tests, a
changelog.dfragment, and lockstep updates tovmtest-harness/lib/verify.shand the harness-contracts doc.stable_set.rsis not inthe diff at all — mpm keeps
OwnVerbfor lifecycle, which is what keepsneeds_kickstartunable to fire for it.
Accepted policy change
This is user-visible and was reviewed and accepted before implementation (see #4925).
A stopped trusty-mpm now yields
tctl status→down/degraded/ exit 2, andtctl install→ NOT VERIFIED, because mpm isrequired: true(stable_set.rs:181).This makes mpm consistent with its declared
requiredflag rather than exempt from it —the same behaviour trusty-search, trusty-memory and trusty-review already have. CI that
gates on
tctl statusexit codes may start failing where it previously passed.required: falsedemotion was explicitly not the chosen path. No env-var opt-out,no
probeablefield, no second probe surface, no--jsonadded totrusty-mpm health,no mpm special-case in any caller.
Verification
Built the patched binary and ran it against the live daemon, with the pre-fix binary
installed at
~/.cargo/bin/tctlas a same-host, same-moment control:tgacorrectly staysn/a— the non-daemon arm is intact.make checkfails identically on this branch and onorigin/main— the two Tauricrates
trusty-code-guiandtrusty-mpm-guiboth fail withThe frontendDist configuration is set to "ui/dist" but this path doesn't exist, becausecrates/*/ui/distis gitignored and needs a pnpm build. Neither crate appears in this diff. Reproduced on
origin/mainin a throwaway worktree, so the CI-equivalent gate (per.github/workflows/ci.yml:23-28) was run instead:cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warnings(3 GUI excludes)cargo test --workspace(same excludes)trusty-installerpackage suiteprobe.rsis 252 SLOC against the 500 cap (was 217).Tests
probe::tests::own_verb_member_is_unknown→own_verb_member_is_probed_over_http.Inverted rather than deleted, so the reversal is recorded where the carve-out was.
probe::tests::own_verb_member_refused_when_nothing_listens— drives a realprobe to
RefusedwithOwnVerband assertsneeds_kickstart(&outcome, OwnVerb) == false.This is the safety-critical pin: a confirmed-down mpm must never be eligible for
launchctl kickstart -k com.trusty.mpm, a label that does not exist.probe::tests::non_daemon_member_is_unprobeable— pins thatNonekeepsUnprobeable, so the widened arm cannot creep to cover every strategy.verify_tail::tests::required_mpm_reporting_down_fails_verification.stable_set::tests::mpm_uses_own_verbandmanage_strategy_for_matches_the_stable_setare unmodified and green — proof the lifecycle axis was untouched.
The one behavioural non-Rust edit
vmtest-harness/lib/verify.shwidens theH_Paccepted-health set forplist_installed == nullmembers by one word:Without it the harness would go red on every run: the oracle snapshots
tctl stack doctorbefore anything starts daemons, so post-fix mpm reads
downthere, anddownwaspreviously accepted only for
plist_installed == false.Net assertive power at that step is unchanged.
null⟺ daemon +OwnVerb⟺ trusty-mpmalone (
doctor.rs:117-123,stable_set.rs:133); aLaunchdmember with a missing plistgets
Some(false)and already accepteddown. Pre-change mpm readunknownunconditionally at that site, so it asserted nothing about mpm either way.
not_installedis still rejected. mpm liveness was, and remains, asserted exclusively by
verify_daemon_liveness, which runs aftertctl start --json, probes/healthdirectly,is unconditional in all three scenarios, and fails closed (
die 60).The scoping is clean — the
falseandtruebranches are untouched. The formal setnotation in
02-harness-contracts.mdwas updated in lockstep.Follow-ups, not in scope
a policy question this change does not resolve, even though it removes the only member
that produced
unknownin practice./healthenvelope carries noversionfield (probe_http.rs:164-165), so mpmreports
healthyand can never reportstale. Version-floor detection does not applyto it.
verify_stack_doctorcannot distinguish a serving mpm from a dead one,leaving
verify_daemon_livenessas the sole assertion of mpm liveness. Not a regression —it already was — but worth recording.
🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools