Skip to content

ck, daemon: say when a module is running something other than what its config asks for (#108) - #120

Open
iceteaSA wants to merge 1 commit into
cortexkit:masterfrom
iceteaSA:feat/pending-reload-verdict
Open

iceteaSA wants to merge 1 commit into
cortexkit:masterfrom
iceteaSA:feat/pending-reload-verdict

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

Approved issue: #108

What changes

  • Rescan preview no longer says "would restart". It says would change (pending reload), the tense-shift of the apply label changed-pending-reload. The unreachable "restarted" arm is deleted. After an apply that deferred anything, ck prints one line saying the restart is deferred and names ck module restart <id>.
  • SupervisorEntry.pending_reload: Option<PendingReloadVerdict> (subc-control 0.20.0, additive, serde default). The daemon derives it in supervisor.list from two facts it already has, and stores nothing new:
    • path: SupervisorObservedProcess.spawned_from vs the currently configured program. Differing means the config asks for a different program than the one running.
    • image: the existing RunningImageAgreement. Mismatch means the file under this program changed since spawn (a replace at a stable path).
  • Three-valued throughout. A stopped module, or any 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". A None from an older daemon also renders as unknown. ck module list gets a compact marker (pending (path), pending (image), pending (path+image), unknown, nothing), and ck module status explains each signal separately: they call for different actions (fix the config or respawn, versus investigate a swapped file).
  • Forward compatibility. The path enum has an Unknown decode fallback, and the image side reuses RunningImageAgreement's existing one, so an older client doesn't fail the whole supervisor.list decode on a newer variant. Every rendered daemon- or module-supplied string goes through ck's terminal-safe escaping.

Cost

It reuses ExecutableIdentityProbe and 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.provenance already pays the same cost. I left provenance.rs untouched 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 unknown for 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 is unknown, with the path signal still shown in ck module status. Your CI's first run on this PR confirmed that on a real Windows host: three ck_cli snapshots 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 the nothing-vs-unknown diff, 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

  • Tests on the pure derivation and on the renderer, each shown red by breaking its seam:
    • path mismatch;
    • image mismatch;
    • each Unavailable reason stays unknown under collapse in both directions (to "nothing" and to "pending"), including a future reason;
    • path pending while the image is unavailable, so neither side hides the other;
    • an old daemon's response decodes to None, which renders as unknown;
    • the preview label pairs with the apply label and has no "restart".
  • New golden supervisor_entry_reload_verdict_states.json covers every state. No existing golden changed.
  • Cross-family review (grok-4.5): APPROVE after one fix round. It found that the unsupported-platform collapse wasn't mutation-locked; its two mutations now go red.
  • At 00af6f41, rebased on 2f1c884f:
    • fmt ✓ · clippy --all-targets -D warnings ✓ · check-wire-crate-versions.sh ✓ (13 crates) · TS client 167 pass / 0 fail;
    • workspace tests: 2 failed targets, both ck-bus NATS tests that need a nats-server binary this host doesn't have;
    • ck_cli 54/0 · subc-daemon lib 400/0.
  • Rebase note: master independently took subc-control 0.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 versioned subc-control requirements moved to 0.20. The one code conflict, in ck.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_refused and three fleet_lint tests 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 as None). No behaviour change to rescan itself. You said you'd send the fleet notice at merge.

@cortexkit-ci

cortexkit-ci Bot commented Sep 23, 2026

Copy link
Copy Markdown

Waiting for design-approved on #108.

This PR needs a linked issue with the design-approved label before it can be reviewed or merged. Link the approved issue with Approved issue: #<issue> (or Refs #<issue>) in the description. A maintainer will apply the design-approved label on the issue, or trivial on the pull request when there is genuinely no design to agree. The issue stays open until the change ships; maintainers take care of it then.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant