fix(minvmd,minimald): subnet validation, attached_count lifecycle, tokio::fs - #537

sequenceDiagram
    participant Caller
    participant GvproxySwitch
    participant PtaskAttachment
    participant GvproxyProcess

    Caller->>GvproxySwitch: attach_ptask()
    GvproxySwitch->>GvproxySwitch: attached_count.fetch_add(1)
    GvproxySwitch-->>Caller: PtaskAttachment { pid, stopping, attached_count }

    Note over Caller: ptask runs...

    Caller->>GvproxySwitch: detach_ptask(attachment)
    GvproxySwitch->>PtaskAttachment: drop(attachment)
    PtaskAttachment->>PtaskAttachment: attached_count.fetch_sub(1)
    alt count reaches 0
        PtaskAttachment->>PtaskAttachment: stopping.swap(true)
        PtaskAttachment->>GvproxyProcess: kill(pid, SIGTERM)
    end

_{Comment @coderabbitai help to get the list of available commands.}

Revise claim for head 579fc38.

Auto-revise 1 of 3.

Commit pushed: d265c06

Generated by sdd-execute (opus tier)

Commit pushed: 786ea6f

Generated by sdd-execute (opus tier)

Addressed the three open review threads in commit 8af0123 (one push to this branch):

• /32 test-coverage gap (r3457174617) — added subnet_new_rejects_prefix_32, asserting SwitchSubnet::new(.., 32) returns Err(SwitchSubnetError::InvalidPrefix(32)). For /32 span = 1, so host(0) is the network address and every host() call returns None — the exact pathology the validation guards. Kept it as its own test (rather than folding into subnet_new_rejects_prefix_above_32) since /32 is in-range, not above it, matching the _31 naming.
• attached_count underflow (r3457174606) — added debug_assert!(prev > 0, ..) after the fetch_sub in PtaskAttachment::drop, so an accidental extra drop is caught in debug builds instead of silently wrapping to u32::MAX and skipping the SIGTERM guard.
• detach_ptask receiver / doc (r3457174614) — relaxed detach_ptask to &self (it touches no switch state, so a shared borrow suffices) and reworded the PtaskAttachment doc to state that every drop path logs the detach, detach_ptask being just a named way to drop at a chosen point.

The HIGH PID-reuse SIGTERM guard (r3457043606) was already addressed on this branch (the prev == 1 && !self.stopping.swap(true, Ordering::AcqRel) claim at net.rs:392), so no change was needed there.

Pre-PR gate (scoped to the changed crate): cargo fmt -p minvmd -- --check clean, cargo test -p minvmd → 62 passed (incl. the new test), cargo clippy -p minvmd --all-targets -- -D warnings clean.

Generated by sdd-execute (opus tier) for issue #537 · ◷

Commit pushed: d67089b

Generated by sdd-execute (opus tier)

Addressed the teardown-signalling review feedback in a follow-up commit (crates/minvmd/src/net.rs):

• Independent-crash residual gap (this thread, r3457263802) — documented the recycled-PID window the swap guard cannot close: an independent gvproxy crash never sets stopping, so the last PtaskAttachment::Drop still signals self.pid after the supervisor has reaped the child. The comment now states this explicitly rather than implying completeness.
• GvproxySwitch::Drop unconditional SIGKILL (r3457452845) — gated the SIGKILL behind !self.stopping.swap(true, AcqRel), mirroring the SIGTERM guard on the last attachment drop, so an already-claimed teardown no longer escalates to a possibly-recycled PID.
• #[must_use] on PtaskAttachment (r3457452841) — added, since dropping it decrements the attached count and may terminate the switch.
• Stale test comment (r3457452849) — corrected the last_ptask_detach_terminates_switch comment to reflect that the guarded Drop now sees stopping set and skips its SIGKILL.

Gate: cargo fmt -p minvmd -- --check, cargo clippy -p minvmd --all-targets -- -D warnings, and cargo test -p minvmd (62 passed) all green.

Generated by sdd-execute (opus tier) for issue #537 · ◷

Auto-resolve conflict 1 of 3.

/revise resolve the merge conflict with main: merge main into this branch (or rebase onto it) and resolve the conflicts, then push to this PR branch. The conflict appeared after sibling PR #547 merged; most collisions are append-only unions in shared files (the crate manifest, the lib module / pub use list) — take the union of both sides.

Auto-resolve conflict 2 of 3.

/revise resolve the merge conflict with main: merge main into this branch (or rebase onto it) and resolve the conflicts, then push to this PR branch. The conflict appeared after sibling PR #546 merged; most collisions are append-only unions in shared files (the crate manifest, the lib module / pub use list) — take the union of both sides.

Revise claim for head f3372ab.

Auto-revise 2 of 3.

sdd-validate findings — implementation boundary

PR: fix(minvmd,minimald): subnet validation, attached_count lifecycle, tokio::fs
Head SHA: f3372ab
Boundary resolved: Implementation (changed files: crates/minvmd/src/net.rs, crates/minimald/src/net/mod.rs, crates/minvmd/Cargo.toml, Cargo.lock)
Gate set applied: Implementation gates (proof artifacts, files in scope, credentials)

Gate 1 — Proof artifacts re-executed and passing

Info — Infrastructure limit; deferred to consumer CI

Both proof artifacts require index.crates.io egress to resolve/compile dependencies. The firewall blocks that domain in this agent container:

error: failed to get `clap` as a dependency of package `args v0.0.1`
  ...unable to update registry `crates-io`
  [56] CONNECT tunnel failed, response 403

• Proof 1: cargo test -p minvmd -p minimald — cannot execute (registry 403). Consumer CI (expected: test job in ci.yml covering cargo test) is the declared gate. The PR description confirms new tests subnet_new_rejects_zero_prefix, subnet_new_rejects_prefix_31, subnet_new_rejects_prefix_32, subnet_new_rejects_prefix_above_32, subnet_new_accepts_valid_prefixes, and last_ptask_detach_terminates_switch fail on base and pass after this PR.
• Proof 2: cargo clippy -p minvmd -p minimald --all-targets -- -D warnings — cannot execute (same registry 403). Consumer CI clippy job is the declared gate.

Note: the check-runs and commit-status APIs both return 403 for this integration token (Resource not accessible by integration), so consumer CI pass/fail cannot be confirmed programmatically. The PR's mergeable_state: blocked is ambiguous (may reflect missing required reviewer approval rather than failing CI). A human should confirm CI is green before merge.

Gate 2 — Changed files within task scope

Warning — crates/minvmd/Cargo.toml not listed in files in scope
crates/minvmd/Cargo.toml:12 — adds thiserror.workspace = true. Required because crates/minvmd/src/net.rs introduces SwitchSubnetError using #[derive(thiserror::Error)]. Not a protected path; mechanically necessary for the implementation.

Warning — Cargo.lock not listed in files in scope
Cargo.lock:3423–3430 — adds thiserror 2.0.18 as a dependency of the minvmd package. Automatic lockfile update consequent to the Cargo.toml change above. Not a protected path.

All other changed files (crates/minvmd/src/net.rs, crates/minimald/src/net/mod.rs) are within the declared scope.

Gate 3 — No real credentials in the diff

Clean. No secrets, tokens, or credentials found in the diff.

Result

No Blocker findings. Two Warnings (build artifact files outside explicit scope, both necessary consequences of the thiserror dependency and neither a protected path). Two Info findings (proof artifacts deferred to consumer CI due to infrastructure limits).

Lifecycle: Issue #526 already carries sdd:review from a prior clean pass — no lifecycle move performed.

sdd-validate · implementation boundary · pass 6 of 6 on this PR

network:
  allowed:
    - defaults
    - "index.crates.io"

Generated by sdd-validate for issue #537 · ◷