Conversation
|
Waiting for This PR needs a linked issue with the |
…s config asks for (cortexkit#108) Derive independent path and running-image signals from the current launch spec and the existing executable identity probe. Unknown stays distinct from agreement, including stopped processes and responses from older daemons. Relabel the rescan preview to match deferred apply; apply still stores the spec without restarting. A cold supervisor.list hashes each module image serially under the probe’s digest-cache lock (same cost as supervisor.provenance); warm calls re-stat only. CONSUMER-IMPACT: subc-control 0.20 adds `SupervisorEntry.pending_reload` (serde default; old daemons decode as None). No behaviour change to rescan.
iceteaSA
force-pushed
the
feat/pending-reload-verdict
branch
from
September 24, 2026 07:36
089d815 to
00af6f4
Compare
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.
Linked issue
Approved issue: #108
What changes
would change (pending reload), the tense-shift of the apply labelchanged-pending-reload. The unreachable"restarted"arm is deleted. After an apply that deferred anything,ckprints one line saying the restart is deferred and namesck module restart <id>.SupervisorEntry.pending_reload: Option<PendingReloadVerdict>(subc-control 0.20.0, additive, serde default). The daemon derives it insupervisor.listfrom two facts it already has, and stores nothing new:SupervisorObservedProcess.spawned_fromvs the currently configured program. Differing means the config asks for a different program than the one running.RunningImageAgreement.Mismatchmeans the file under this program changed since spawn (a replace at a stable path).Unavailable { reason }(not running, unreadable path, unconfirmed identity, unsupported platform), is rendered as cannot tell, with the reason. It is never "nothing pending" and never "pending". ANonefrom an older daemon also renders as unknown.ck module listgets a compact marker (pending (path),pending (image),pending (path+image),unknown,nothing), andck module statusexplains each signal separately: they call for different actions (fix the config or respawn, versus investigate a swapped file).Unknowndecode fallback, and the image side reusesRunningImageAgreement's existing one, so an older client doesn't fail the wholesupervisor.listdecode on a newer variant. Every rendered daemon- or module-supplied string goes throughck's terminal-safe escaping.Cost
It reuses
ExecutableIdentityProbeand its digest cache, with no new hashing path. Warm calls re-stat only. A cold list (the first after a daemon restart or a binary swap) hashes each module image serially, because the probe holds its digest-cache mutex across hashing (provenance.rs:91-95).supervisor.provenancealready pays the same cost. I leftprovenance.rsuntouched on purpose: it's the PID-recycling / TOCTOU path from #60/#76, and changing its lock scope belongs in its own change. No supervisor, registry or state lock is held across the probe await: operands are snapshotted, the guard is released, then the probe runs.Windows reads
unknownfor every module (behaviour to confirm)Windows has no running-image probe, so the image signal there is always
Unavailable{unsupported_platform}, and by the three-valued rule every module's list marker isunknown, with the path signal still shown inck module status. Your CI's first run on this PR confirmed that on a real Windows host: threeck_clisnapshots had hard-coded the Linux rendering. The tests now split on capability, mirroring the probe's own cfg gate, with a positive assertion on each arm. I proved the Windows arm from Linux by inverting the condition: all three go red with thenothing-vs-unknowndiff, and pass when restored.It's correct under the rule, but it means the list column carries no per-module information on Windows. If you'd rather Windows show the path half (for example
nothing (image n/a)) because the image probe is permanently rather than transiently unavailable there, that's a small rendering change. I haven't made it, because it relaxes the rule you set in #108.What it doesn't close
The window is narrowed, not closed: a module can still be stale for reasons neither signal sees (a shared library swapped under an unchanged binary, for example).
Verification
Unavailablereason stays unknown under collapse in both directions (to "nothing" and to "pending"), including a future reason;None, which renders as unknown;supervisor_entry_reload_verdict_states.jsoncovers every state. No existing golden changed.00af6f41, rebased on2f1c884f:--all-targets -D warnings✓ ·check-wire-crate-versions.sh✓ (13 crates) · TS client 167 pass / 0 fail;ck-busNATS tests that need anats-serverbinary this host doesn't have;ck_cli54/0 · subc-daemon lib 400/0.subc-control0.19.0, the number this branch had chosen, and git merged the identical line without a conflict, so the bump would have vanished silently. Versions are set above master rather than to either side's: control 0.20.0, daemon 0.21.3, core 0.20.23, client-rs 0.18.5, ck-bus 0.1.8, with the three versionedsubc-controlrequirements moved to 0.20. The one code conflict, inck.rs, was both sides appending tests at the same place; I kept both.Unrelated, measured while checking a red I saw mid-build:
control::tests::route_open_refusal_names_the_check_that_refusedand threefleet_linttests fail on master under CPU load: 1 run in 25 of the daemon lib binary with 8 busy loops running alongside, 0 in 25 without. They pass alone, and on this branch 0 in 25. Not from this change; noted in case your CI hits them.CONSUMER-IMPACT: subc-control 0.20 adds
SupervisorEntry.pending_reload(serde default; old daemons decode asNone). No behaviour change to rescan itself. You said you'd send the fleet notice at merge.