[ac-pplx]

What this changes

The network pool tells node-ctl that node status changed by calling node_state_sync::mark_node_status_dirty, a free function. On a Tokio runtime it spawns a detached task that dials node-ctl's fixed socket and retries with backoff, so nothing could substitute it. Every unit test that changed pool state on a runtime reached for the host's node-ctl socket.

After this change, the pool owns the destination as a trait object, Arc<dyn NodeStatusDirtySink>. Production passes a sink that calls the same function; tests pass one that records marks in the process.

The seam and its production wiring

	Before	After
Who decides where marks go	nobody: 29 call sites call crate::node_state_sync::mark_node_status_dirty()	NetworkPool.node_status_dirty, set at construction; the 29 sites call self/pool.mark_node_status_dirty(), which forwards to it
Production value	(the free function)	NodeCtlNodeStatusDirtySink, built in ServiceSettings::from_env, moved into the service constructor and passed to NetworkPool::open
What production runs per mark	mark_node_status_dirty()	NodeCtlNodeStatusDirtySink::mark_node_status_dirty → the same, now private, mark_node_status_dirty()
firewall-reply-fixture example (the other NetworkPool::open caller)	(the free function)	passes NodeCtlNodeStatusDirtySink
NetworkPool::open_in_memory_for_test, so service_for_unit_tests	(the free function)	a RecordingNodeStatusDirtySink

The production behavior is the same:

• Same function. The production sink is a unit struct whose one method calls the function the call sites used to call, with its body unchanged: the same process-wide coalescing flag, detached task, node-ctl client and socket, 1 s → 300 s retry schedule and log lines. Off a runtime it is still the same debug-logged no-op.
• Same positions. Every call site keeps its position. Marks fire on the same paths, in the same order, under the same locks (for example set_draining still marks while holding the store lock).
• Same startup. ServiceSettings::from_env builds the unit struct before the pool opens. That allocates an Arc and nothing else: no I/O, no log line.

There is no change to wire formats, schema, metrics, configuration or log lines.

NetworkPool::open is public, and the example calls it, so slot_pool re-exports NodeStatusDirtySink and NodeCtlNodeStatusDirtySink.

Call-site inventory

Every production path from node-mgr to node-ctl in pplx-space-node-mgr goes through the dirty mark. That was true before this change and still is.

File	Function (sites)
slot_pool.rs	update_config (1), allocate_pooled_slot (2), claim_ephemeral_slot_for_setup (2), mark_ephemeral_slot_leased (1), persist_ephemeral_setup_failure_state (1), mark_ephemeral_for_cleanup (1), release (2), set_draining (1), repair (1)
slot_pool/db.rs	apply_startup_lease_updates (1)
slot_pool/scheduler.rs	recover_abandoned_work (1)
slot_pool/reset.rs	reset_slot (9)
slot_pool/worker.rs	run_validation_pump (1), claim_missing_for_create (1), claim_ready_for_destroy (1), create_slot (1), destroy_slot_with (1), reprobe_startup_leasing_with (1)

Before: these 29 sites called crate::node_state_sync::mark_node_status_dirty(). After: the same 29 sites call the pool's sink, and rg 'node_state_sync::mark_node_status_dirty' finds nothing.

NodeCtlNodeStatusDirtySink is constructed only in ServiceSettings::from_env and the example. The node-ctl client (pplx_space_node_ctl_client) is called only from node_state_sync.rs.

Tests

NetworkPool::open_in_memory_for_test, and so service_for_unit_tests, now records marks instead of notifying node-ctl. open_in_memory_with_node_status_sink_for_test gives a test the recorder to assert on. It takes Arc<RecordingNodeStatusDirtySink> and coerces it internally, rather than any sink, so a test pool cannot be handed NodeCtlNodeStatusDirtySink. RecordingNodeStatusDirtySink lives in node_state_sync::tests, beside the trait.

The exact inventory. The #298 tripwire panicked at a test's first host contact, so its lists were lower bounds. This one records every contact and lets the test continue. It logs each call to the production dirty mark, each detached node-ctl dial it spawns, each dial attempt, each node-ctl client connect, and each read of a fixed run-directory pid link. It ran over the whole package's tests at the base commit and at this branch:

Contact	Base commit	This branch
Tests calling the production dirty mark	76 (494 calls)	0
… on a Tokio runtime, spawning a node-ctl dial	45	0
… reaching the node-ctl client's connect	43 (54 attempts)	0
Tests reading fixed run-directory pid links	14 (92 reads)	14 (92 reads), unchanged by design

The 9 ignored tests, run separately with --run-ignored only, made no contact either.

At the base commit, the 45 tests that spawned a dial were:

• service::runtime::tests: allocate_grants_a_lease_then_returns_it_without_new_admission; allocate_refusal_grants_no_lease (3 cases); release_refusal_keeps_the_lease (4); stop_process_forwards_an_owned_process_to_the_supervisor (5); stop_process_refuses_without_signalling (5); stop_process_reports_an_already_gone_process_as_not_stopped (2)
• service::execution_fence::tests: execution_preflight_waits_for_cleanup_and_revalidates (4); execution_preflight_rejects_legacy_leases_without_releasing_them; stopped_unbound_snapshot_reaches_filesystem_without_allocating
• service::launch_prepare::tests: cancelled_preparation_retains_exclusion_capacity_and_disk_accounting; preparation_rechecks_lease_after_waiting_for_cleanup_ownership
• service::cleanup_inspect::tests: every_failed_stop_preserves_the_active_lease_and_reservation; cleanup_does_not_stop_a_lease_replaced_while_it_waited_for_the_gate
• service::tests: concurrent_new_lease_admission_allows_only_one_reserved_cap; startup_rpc_admission_shares_work_and_waits_for_cleanup
• service::network_pool_metrics::shutdown_stops_a_bounded_pass_of_wedged_probes
• 12 slot_pool::tests: cancelled_ephemeral_waiter_retains_ownership_until_durable_completion, ephemeral_completion_storage_failure_retains_and_retries_cleanup_intent, fill_failure_waits_for_launched_setup_and_repair_work_before_retry, reset_retry_accounting_db_failure_schedules_retry_without_quarantine, reset_slot_keeps_dirty_when_retained_cache_is_still_owned_by_lease_uid, reset_slot_quarantines_after_cleanup_retry_exhaustion, retirement_progresses_while_drained_without_rebuilding_capacity, startup_health_recovery_does_not_reclaim_fresh_allocations, startup_health_recovery_retries_uncertainty_without_releasing_ownership, startup_leasing_reprobe_retries_while_draining_and_stops_after_release, startup_reconcile_preserves_interrupted_destroy_cleanup_obligation, unrelated_fill_wakeups_do_not_multiply_pending_retry_tasks

All of them now record their marks in the process. Their assertions are unchanged except where noted below.

Tests that assert the mark

1. slot_pool::node_status_tests::node_status_changes_mark_node_ctl_dirty (new module, wired by a #[cfg(test)] mod line).
   • A table over allocation, release and configuration update. Each runs on a Tokio runtime, the path that used to dial node-ctl.
   • Seven rows, each checking its effect on the pool (the lease exists or doesn't, the slot is dirty or still ready, the configuration did or didn't change):
     • applied: allocation of a ready slot; release of a leased sandbox; a publishable configuration update;
     • refused: allocation while the pool drains (stops at the drain check); allocation with no ready slot (reaches the pooled store path and is refused ResourceExhausted); release of a sandbox with no lease; an update whose ready-slot target is above max_slots.
   • An applied change must mark node status; a refused one, which leaves status as it was, must not.
2. service::runtime::tests::allocate_grants_a_lease_then_returns_it_without_new_admission (converted; it dialed node-ctl at the base commit).
   • The Node fixture now owns its pool's recorder. Its NoReadySlot refusal case swaps in a fresh pool through the same recorder.
   • The test clears the recorder just before the grant, then requires the granted lease to mark node status, and the idempotent retry, which returns the existing lease, not to.
3. service::tests::allocate_test_lease no longer claims its lease on a separate thread to keep allocation off the runtime and its notification from dialing node-ctl. It claims on the caller's thread, and the mark goes to the recorder. Its callers (cleanup_inspect, launch_prepare and snapshots tests) are unchanged. The CleanupNode fixture comment that described the workaround now says the pool records node-status marks, and node-ctl notification is gone from the host-cleanup list in claim_contention_tests.

Mutation proofs

Each mutation ran with the tripwire build, which recorded 0 contacts on every regression path.

Mutation	Result
M1 Allocation success path drops its mark	allocation fails ("Allocate changed node status without marking it"); allocate_grants_… fails ("a granted lease must reach node-ctl's node status")
M2 Release drops its mark	release fails
M3 update_config drops its mark	config_update fails
M4 release marks on every call	release_without_lease fails (1 ≠ 0)
M5 claim_allocation marks on every call	allocation_while_draining and allocation_without_ready_slot fail; allocate_grants_… fails on the retry ("returning the existing lease changes no node status", 1 ≠ 0)
M6 The pooled no-ready-slot refusal marks even with no stale lease to reset	only allocation_without_ready_slot fails (1 ≠ 0); allocation_while_draining passes, because it stops at the drain check first
S1 Seam: the in-memory pool ignores the injected recorder and installs its own	allocation, release, config_update and allocate_grants_… fail
S2 Seam: the Node fixture keeps the default pool, whose recorder it does not hold	allocate_grants_… fails

The seam mutations replace the sink with another in-process recorder, never the production sink, so their regression paths cannot dial node-ctl either.

Call sites left unrouted

• Fixed run-directory pid links (RUN_DIR) are business logic and stay unrouted. The 14 tests that read them, identical at the base commit and on this branch:
  • service::cleanup_inspect::tests::cleanup_does_not_stop_a_lease_replaced_while_it_waited_for_the_gate
  • service::cleanup_inspect::tests::every_failed_stop_preserves_the_active_lease_and_reservation
  • service::execution_fence::tests::execution_authority_survives_slot_release_and_rejects_stale_mutations
  • service::execution_fence::tests::stopped_unbound_snapshot_reaches_filesystem_without_allocating
  • service::tests::service_tests::inspect_sandbox_remains_available_when_diagnostics_are_saturated
  • slot_pool::tests::slot_pool_tests::dynamic_claim_keeps_live_lease_when_pod_namespace_mismatches
  • slot_pool::tests::slot_pool_tests::dynamic_claim_retires_dead_lease_when_pod_namespace_disappeared
  • slot_pool::tests::slot_pool_tests::reset_retry_accounting_db_failure_schedules_retry_without_quarantine
  • slot_pool::tests::slot_pool_tests::reset_slot_keeps_dirty_when_retained_cache_is_still_owned_by_lease_uid
  • slot_pool::tests::slot_pool_tests::reset_slot_quarantines_after_cleanup_retry_exhaustion
  • slot_pool::tests::slot_pool_tests::runtime_reconcile_dirties_interrupted_leasing_slot
  • slot_pool::tests::slot_pool_tests::runtime_reconcile_keeps_dead_lease_uid_reserved_while_allowing_new_lease
  • slot_pool::tests::slot_pool_tests::startup_health_recovery_retries_uncertainty_without_releasing_ownership
  • slot_pool::tests::slot_pool_tests::startup_leasing_reprobe_retries_while_draining_and_stops_after_release
• pplx-space-node-mgr-cli (crates/apps/node/mgr/cli) dials node-ctl through pplx_space_node_api_client::connect_node_ctl to warn when inbound reconciliation would override a setting. That is a separate operator binary with no unit test reaching it, outside this package and this seam.
• Inbound node-ctl calls. SPACE_NODECTL in service.rs and network_helpers.rs authorizes node-ctl calling into node-manager. It is not a node-mgr → node-ctl path.
• The production constructor still opens the host network pool (NetworkPool::open), so unit tests cannot build it. The Kind runs below prove the production wiring.

Host safety

• New and converted tests use in-memory slot pools, in-process recorders and the existing in-process supervisor stub. None reaches node-ctl, fixed host paths, firewall or netns state, or processes it does not own.
• The tripwire runs above show that, including on every mutation's regression path.
• Neither the new table nor allocate_grants_… is among the run-directory readers.

Overlaps with open PRs

• node-mgr: validate LoadSnapshot requests before staging host state #269, sbx-mgr: characterize stop, delete and create core RPC paths (tests only) #271, api: characterize Kubernetes substrate suspend, pooled preparation and restore hydration (tests only) #273, docs: localized AGENTS.md for sbx-mgr, node-mgr and the api crate #275, api: release a Kubernetes create's claimed execution on every early exit #277 and tests: keep three tests off host state they don't own #278 are merged.
• sbx-mgr: keep quarantined rows through inventory and bound recovery that can never succeed #270 (sbx-mgr) is open and shares no files.
• Open PRs touching node-mgr also share no files with this change:
  • [api] stop retrying restore pods whose snapshot the host CPU cannot load #221 edits service/snapshots.rs and service/snapshots/tests.rs;
  • Add an opt-in Firecracker balloon device per template #212 edits service/snapshots.rs;
  • credmgr: scope volume DEKs to the node running the sandbox #128 edits service/encrypted_volumes.rs and its tests.
• tests: stop tests depending on root, sudo or fixed host paths #317 is open and edits node-mgr Cargo.toml and the encrypted_volumes, gateway_isolation and permission_tree test files. It shares no file with this PR.
• allocate_test_lease, which snapshots/tests.rs calls, keeps its signature and behavior.

Validation

Head e0ceccdb1efac5d73c622c0d6db7c0e604314745.

The Kind rows below ran at 3c14d4427d621f1cb9e50ba37fbf34f2159ec324 and were not re-run for this head. Since then the change is test code and comments, plus one production line: dropping NodeCtlNodeStatusDirtySink's unused Default derive. No production call path, wiring or behavior differs. Every other row ran at this head.

Check	Result
cargo fmt --all -- --check	pass
cargo clippy --workspace --locked --all-targets --all-features -- -D warnings	pass, 0 diagnostics
rustfmt --edition 2024 --check scripts/generate-env-contract.rs, generate-env-contract.sh --check, check-rust-error-policy.sh	pass
scripts/source_guards/*.py	env_test_hook_boundary, node_selector_extras_smoke and observability_vendor_boundary pass. thin_service_mains exits 0 but reports crates/apps/vol/mgr/svc/src/main.rs, which this PR does not touch (pre-existing)
check_no_inline_rust_test_modules.py	flags two crates/libs/observability files this PR does not touch (pre-existing); nothing in node-mgr
pre-commit (Gitleaks)	pass on the commit and on --all-files
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-node-mgr --retries 0 ×3	465 passed, 9 ignored skipped, on each of three consecutive runs. The Postgres setup script ran each time, with 0 dependency-skip markers. .config/nextest.toml scopes Redis to pplx-space-api; node-mgr has no Redis-backed tests
Ignored tests (--run-ignored only, at 3c14d44; untouched since)	8 pass (opt-in tests that return early without their environment). clone_tree::tests::private_clone_blocks_foreign_uid_grafts_and_shared_clone_is_held fails its "run in a disposable root container" assertion, as designed when not root; it is in the nextest privileged group. 0 host contacts
Record-and-continue tripwire	base: 76 tests call the production dirty mark, 43 reach node-ctl's connect; branch: 0 and 0
Mutation proofs (8)	each fails its intended tests; 0 host contacts on every regression path
Kind (at 3c14d44): fresh cluster	built from scratch on the current harness: no Kind cluster, node containers or ~/.kind-data existed beforehand
Kind: binary provenance	space-nodemgr-svc sha256 74b08de2…357e3c is identical in the staged artifact, in space-rust-build-kind:local and in all four node-manager pods (two sandbox workers, two builders). The artifact build's synced crates/, proto/, Cargo.toml and Cargo.lock are identical to this checkout, and the binary contains NodeCtlNodeStatusDirtySink. The pods kept 0 restarts and the same digest through the typed run
Kind: Tilt	42 resources ok and 4 not applicable, none pending or in progress, before and after the typed smoke
smoke-kind-e2e.sh (bootstrapping the fresh cluster, BOOTSTRAP_TIMEOUT=1800, DATA_PLANE_URL set, as CI does)	PASS 7/7
smoke-kind-typed.sh all, ECW pass (SPACE_SMOKE_KIND_COLD_RECEIPTS and the admin token file set, as CI does)	77 passed, 0 warned, 0 failed, 0 inconclusive, including ecw.cold_create_environment and local.ecw_template_build_from_oci_boots
smoke-kind-typed.sh all, native pass	23 passed, 0 warned, 0 failed, 0 inconclusive; native_contract.command_replay_credentials IGNORED by design
Production dirty mark in Kind	0 failed to mark node-status facet dirty retries in either sandbox node-manager across the run
CHANGE-INDUCED failures	none; no rerun was needed

🤖 Generated with Claude Code

[the-gigi-pplx]

Review: 2 findings

Low

1. crates/libs/node/mgr/server/src/slot_pool/node_status_tests.rs:1: the module doc says every lease or configuration change must mark node status, but the table covers three paths.
   I removed the mark from set_draining and from all nine reset_slot sites in slot_pool/reset.rs at the same time. The whole package's lib suite still passed (465 passed, 0 failed). Dropping the allocation mark did fail case_1_allocation and allocate_grants_…, so the table isn't vacuous for the paths it covers. The 29 production sites are a mechanical 1:1 swap, so nothing regresses here. Still, the seam's value is in catching a future dropped mark, and the reset, drain and worker paths aren't guarded yet. Either add a drain row and a reset row (the recorder makes both cheap), or narrow the doc to the three paths tested.

2. crates/libs/node/mgr/server/src/node_state_sync.rs:17-18: "marks made while it is in flight are absorbed by it" overstates the coalescing.
   The flag clears only after node-ctl's MarkFacetDirty returns (node_state_sync.rs:47). Node-ctl records the dirty ref and queues SyncDirty before it replies (node/ctl/server/src/lib.rs:840-858). So a mark can arrive after node-ctl's sync has already read status but before the sender clears the flag. That mark is dropped, not absorbed, and only node-ctl's periodic reconciliation picks it up. This behavior predates the PR, so the code is fine. Only the new sentence is wrong. Something like "marks made while it is in flight are dropped; node-ctl's periodic reconciliation covers a change that lands after its sync read status" would be accurate.

What I checked: all 29 call sites map 1:1 between base and head (12 in slot_pool.rs, 1 in db.rs, 9 in reset.rs, 1 in scheduler.rs, 6 in worker.rs). The free function's body is unchanged and now private. NodeCtlNodeStatusDirtySink is built only in config.rs:84 and the example. open_in_memory_for_test installs the recorder, and the _with_node_status_sink_ variant's parameter type rules out the production sink. The Node fixture's swapped pool is the only Arc<NetworkPool> the service holds.

Verified against e0ceccdb1efac5d73c622c0d6db7c0e604314745. What I ran: the touched test modules (94 passed), the full pplx-space-node-mgr lib suite under three mutations (allocation mark dropped: 2 intended failures; drain + reset marks dropped: 465 passed), and the PR's own tests at the head. I didn't run Kind.

[the-gigi-pplx]

Approved. 2 non-blocking findings in the review comment above.