fix(cli): allow bounded graceful shutdown budget

[branarakic]

Impact

This keeps the production shutdown hard-stop at 15 seconds by default, while allowing an operator to select a validated bounded budget between 5 seconds and 5 minutes for RPC-heavy processes that are already draining an in-flight chain callback. Invalid values fail startup instead of disabling the guard.

The RFC-64 M1 real-node adapter now waits for that bounded graceful exit before deciding whether cleanup passed. A forced or non-zero exit still fails the evidence gate and prevents a PASS artifact.

Before

sequenceDiagram
    participant Operator
    participant Daemon
    participant Chain as Chain callback
    Operator->>Daemon: POST /shutdown
    Daemon->>Chain: Wait for in-flight callback
    Note over Daemon: Fixed 15 second hard stop
    Daemon--xOperator: Forced exit 100
    Note over Operator: Valid canary cannot publish PASS

Loading

After

sequenceDiagram
    participant Operator
    participant Daemon
    participant Chain as Chain callback
    Operator->>Daemon: Start with optional bounded shutdown budget
    Daemon->>Daemon: Validate 5s to 5m or fail startup
    Operator->>Daemon: POST /shutdown
    Daemon->>Chain: Drain in-flight callback
    Chain-->>Daemon: Callback complete
    Daemon-->>Operator: Clean exit 0 within bound
    Note over Operator: Forced or non-zero exit still fails M1

Loading

Validation

• CLI shutdown suite: 36 passed
• RFC-64 M1 verifier and adapter suites: 79 passed
• M1 TypeScript project: clean
• git diff --check: clean

Scope

• Stacked on test(rfc64): add release-shaped M1 testnet adapter #2028.
• Does not change the default fleet timeout.
• Does not merge or deploy anything by itself.

[otReviewAgent]

🔴 Bug: Raised shutdown timeout can still be preempted by fixed shutdown watchers

What's wrong
The new env override makes the daemon's graceful shutdown budget configurable up to five minutes, but the surrounding process managers still use fixed shorter shutdown windows. That means an operator can choose a valid value and still have the process killed or reported failed while it is behaving according to the new contract, risking interrupted cleanup and misleading shutdown failures.

Example
Set DKG_SHUTDOWN_HARD_TIMEOUT_MS=300000 for a publisher whose agent.stop() takes four minutes after server.close(). The worker is within its configured shutdown budget, but the supervisor can start counting failed probes after the fixed 30s grace and SIGKILL it before the worker reaches its own forced-exit path; the testnet adapter similarly reports a stop timeout after 90s even though the daemon is allowed to keep draining.

Suggested direction
Derive supervisor shutdownGraceMs and adapter stop wait from the same resolved shutdown hard timeout, with probe/forced-cleanup slack and any operation-level cap applied only after ensuring it cannot undercut the daemon's configured budget.

For Agents
Coordinate all shutdown observers with the resolved hard timeout. Check packages/cli/src/daemon/shutdown.ts, packages/cli/src/daemon/supervisor-liveness.ts, packages/cli/src/cli-supervisor.ts, and the M1 adapter. Preserve the default 15s behavior, but prove that a high DKG_SHUTDOWN_HARD_TIMEOUT_MS cannot be preempted by supervisor SIGKILL or adapter timeout before the worker deadline plus cleanup slack.

The actual env-backed shutdown constant is not covered

What's wrong
The new tests verify the pure parser, but they do not prove that the daemon-exported timeout actually reads the environment variable during startup. That leaves the main changed behavior unprotected.

Example
A regression like export const SHUTDOWN_HARD_TIMEOUT_MS = DEFAULT_SHUTDOWN_HARD_TIMEOUT_MS would still pass these new resolver tests, but DKG_SHUTDOWN_HARD_TIMEOUT_MS=60000 would no longer affect the daemon's actual hard-stop guard.

Suggested direction
Cover the module-load path, not just the helper, so the operator-facing env override is verified end to end.

For Agents
Add an isolated-module test in packages/cli/test/shutdown-timeout.test.ts that sets process.env.DKG_SHUTDOWN_HARD_TIMEOUT_MS before importing ../src/daemon/shutdown.js, then asserts the exported SHUTDOWN_HARD_TIMEOUT_MS uses the override. Add a matching invalid-env import test if startup failure is intended behavior.

[branarakic]

Addressed in ebfa064. The shutdown parser is now pure and receives an explicit value. The daemon resolves the worker environment once at the lifecycle startup boundary; the supervisor derives a grace window from that same child environment; and the M1 adapter derives a named exit-observer budget from each role environment and rejects an operation timeout that would undercut it. Defaults remain 15s daemon / 30s supervisor-adapter, while a 60s worker budget produces 66s observers. Added explicit daemon-env, supervisor-env, 15s/60s/300s grace, adapter above/below-cap tests. Validation: 73 targeted CLI tests and 80 M1 tests pass.

[otReviewAgent]

🟡 Issue: Avoid leaving a live-looking timeout constant after making the timeout configurable

What's wrong
The new configurability splits the concept of “shutdown hard timeout” into a default value and a resolved runtime value, but the old exported name still looks like the canonical timeout. That weakens the type/API boundary and makes future maintenance harder because readers have to know by convention that the most obvious constant is no longer the value production should use.

Example
A future call site that imports SHUTDOWN_HARD_TIMEOUT_MS to size a watcher, test harness, or operation budget will silently get 15s even when DKG_SHUTDOWN_HARD_TIMEOUT_MS=60000 is configured. The PR already had to replace one such direct use in lifecycle.ts, so leaving the old name as a normal export preserves the same footgun.

Suggested direction
Collapse the API around one clear boundary: DEFAULT_SHUTDOWN_HARD_TIMEOUT_MS for the fleet default and resolveShutdownHardTimeoutMs(...) for runtime behavior. Do not keep an exported SHUTDOWN_HARD_TIMEOUT_MS name that reads like the effective timeout.

For Agents
In packages/cli/src/daemon/shutdown.ts, make the default-only nature impossible to miss: prefer deleting the compatibility alias and updating remaining tests/comments to DEFAULT_SHUTDOWN_HARD_TIMEOUT_MS; if an external export must remain, mark it deprecated and avoid using it anywhere in the repo. Preserve the default value and env override behavior; tests should prove runtime call sites use resolveShutdownHardTimeoutMs, not the alias.

[otReviewAgent]

🟡 Issue: Do not hide daemon configuration behind a module-level env read

What's wrong
This change turns a previously pure shutdown utility module into a configuration reader with import-time behavior. That is a maintainability regression: readers cannot tell from the lifecycle shutdown path where the timeout comes from, tests/importers can be affected by ambient environment state, and future configuration changes now have two places to look instead of one canonical startup boundary.

Example
Importing shutdown.ts in any CLI/test path now implicitly parses DKG_SHUTDOWN_HARD_TIMEOUT_MS, even when the caller only wants helpers like isForcedShutdownExitCode or encodeForcedShutdownExitCode. The dependency is not visible at the lifecycle callsite that actually passes the timeout into raceShutdownWithTimeout.

Suggested direction
Make resolveShutdownHardTimeoutMs accept an explicit string | undefined and keep shutdown.ts as pure shutdown mechanics plus constants. Resolve process.env.DKG_SHUTDOWN_HARD_TIMEOUT_MS in the canonical daemon configuration/startup layer, next to the other runtime env settings, so the boundary is explicit and import order does not become part of the design.

Confidence note
I only have this diff plus local consumers, but SHUTDOWN_HARD_TIMEOUT_MS is imported by daemon lifecycle and tests as a constant while the actual override source is hidden in the shutdown helper module.

For Agents
Keep the parser pure and move env ownership to the daemon startup/lifecycle layer. Look at packages/cli/src/daemon/lifecycle.ts around the existing process.env-backed settings, resolve DKG_SHUTDOWN_HARD_TIMEOUT_MS there, pass the resolved number into the shutdown race, and preserve the current default/min/max behavior. A focused test should prove the parser accepts/rejects the same values without requiring module re-import side effects.

[otReviewAgent]

🟡 Issue: Avoid burying a second timeout policy inside the stop flow

What's wrong
The new literal cap is a local special case in an otherwise config-driven controller. It makes shutdown orchestration carry policy details and weakens the existing operationTimeoutMs abstraction by adding an undocumented exception at the callsite.

Example
A reader trying to understand stop behavior now has to remember that most adapter waits are governed by operationTimeoutMs, but process-exit-after-shutdown is capped by an unexplained 90s literal embedded in stop(). If another role or phase needs a different shutdown window later, this pattern encourages more one-off Math.min(...) branches in the controller.

Suggested direction
Name this timeout policy at the adapter/config boundary instead of inlining Math.min(90_000, this.cfg.operationTimeoutMs) inside stop(). A small helper like shutdownExitTimeoutMs(cfg) or a parsed config field would make the special cap discoverable and keep timeout policy out of the orchestration path.

Confidence note
The one-line diff does not show the intended RFC-specific timeout model, but the surrounding adapter already has operationTimeoutMs as the named run-wide budget and this line adds a second unnamed shutdown cap directly in the stop flow.

For Agents
Move this policy into a named helper or config-derived field such as shutdownExitTimeoutMs, keeping the current min(90s, operationTimeoutMs) behavior. Look in devnet/rfc64-m1-selective-coverage/testnet-operator-common.ts for the config shape and in testnet-operator-adapter.ts for other timeout callsites; prove the adapter still uses the same effective timeout for shutdown waits.

The new shutdown wait budget is unverified

What's wrong
This changes operational shutdown behavior, but the diff only adds tests for CLI shutdown timeout parsing. There is no regression test showing the adapter now allows longer drains while still respecting the configured operation timeout cap.

Example
With operationTimeoutMs = 120_000, stop() should wait up to 90s for running.exited; with operationTimeoutMs = 45_000, it should keep the shorter 45s bound. A regression that kept the old 30s timeout, or forgot the 90s cap, would not be caught by the added shutdown-timeout tests.

Suggested direction
Add coverage for the new capped timeout calculation in the testnet operator adapter.

Confidence note
I only reviewed the supplied diff and nearby context; I did not find changed tests covering this devnet adapter timeout behavior in the diff.

For Agents
Add a focused unit test around TestnetOperatorController.stop or the adapter timeout path that stubs requestJson success and a never-resolving running.exited, then asserts the timeout duration passed to withTimeout for both above-cap and below-cap operationTimeoutMs values.

[otReviewAgent]

🔴 Bug: Undercut shutdown budgets are detected after shutdown has already started

What's wrong
The new resolver can throw for a misconfigured operator timeout, but it is evaluated only after the adapter has already requested daemon shutdown. Because stop() always deletes the process record in finally, that failure loses lifecycle tracking for a still-running child and can allow duplicate starts or leave cleanup to chance.

Example
With role.environment.DKG_SHUTDOWN_HARD_TIMEOUT_MS='60000' and cfg.operationTimeoutMs=65000, stop('edge') first POSTs /api/shutdown, then the resolver throws because the required budget is 66000ms. The finally block removes edge from this.processes without waiting for running.exited, so the adapter can accept a later start('edge') while the old process is still shutting down against the same data directory/ports.

Suggested direction
Move timeout-budget resolution ahead of the shutdown POST, preferably to config validation/startup, so misconfiguration fails before any daemon process enters shutdown or is removed from tracking.

For Agents
In TestnetOperatorController.stop, resolve and validate the shutdown exit timeout before issuing /api/shutdown, or validate all role shutdown budgets when reading/constructing the operator config. Preserve process tracking until the child exit is observed or an explicit cleanup/kill path has run. Add a focused test where operationTimeoutMs undercuts an extended DKG_SHUTDOWN_HARD_TIMEOUT_MS and assert no shutdown request is sent and the running role remains tracked.

[otReviewAgent]

🟡 Issue: Shutdown budget validation can orphan a running role after issuing shutdown

What's wrong
The new timeout resolver can throw after the adapter has already sent the shutdown request. Because the finally block always removes the role from the process map, the adapter loses lifecycle ownership of a process that may still be alive, which can lead to duplicate starts or shared data-directory races after a failed stop.

Example
With role.environment.DKG_SHUTDOWN_HARD_TIMEOUT_MS='60000' and operationTimeoutMs=65000, stop('edge') successfully asks the node to shut down, then resolveTestnetOperatorShutdownExitTimeoutMs throws because the required budget is 66000ms. The finally block removes the edge process from this.processes while the OS process may still be exiting, so a later start edge command can spawn another process against the same durable data directory.

Suggested direction
Validate the role's derived shutdown timeout before mutating process state or sending the shutdown request, preferably at config load/startup so unsupported configs fail before any worker is launched.

Confidence note
This requires an operator config where a role raises DKG_SHUTDOWN_HARD_TIMEOUT_MS but operationTimeoutMs is below the derived exit budget; generated configs use a large timeout, but the decoder accepts such configs until shutdown time.

For Agents
Move the shutdown-budget validation before sending /api/shutdown, or validate all role shutdown budgets when reading/loading the operator config before any role can start. Preserve the behavior that too-small operationTimeoutMs is rejected, and add a test proving a rejected budget does not call /api/shutdown and does not drop the process from the controller map.

Resolve shutdown timing once at the boundary, not inside stop paths

What's wrong
The shutdown-budget concept now lives in several layers: the daemon parses the hard timeout, the supervisor derives watcher grace, and the testnet adapter derives a separate process-exit timeout during shutdown. That leaves a supposedly validated config carrying unresolved timing invariants and makes the same budget recipe easy to drift over time.

Example
A config with DKG_SHUTDOWN_HARD_TIMEOUT_MS: '60000' and operationTimeoutMs: 65000 can still be decoded as TestnetOperatorConfigV1; it only fails later when stop() asks for resolveTestnetOperatorShutdownExitTimeoutMs(...).

Suggested direction
Keep stop() boring: it should use an already-resolved timeout from the parsed config. The parser or a dedicated shutdown timing module should own env parsing, bounds, and cross-field validation, with one source of truth for the derived observer/process-exit budget.

For Agents
Move shutdown budget resolution to a single config/timing boundary. For the testnet operator, either reject an undercut operationTimeoutMs in readTestnetOperatorConfig or return resolved per-role shutdown budgets on the parsed config. Consider centralizing the shared hard-timeout + forced-cleanup + slack calculation so supervisor and testnet adapter do not maintain parallel formulas.

[otReviewAgent]

🟡 Issue: Centralize the shutdown timing envelope instead of duplicating the formula

What's wrong
The PR adds configurable shutdown timing, but the derived wall-clock budget is now hand-assembled in multiple layers with slightly different names for the same cushion. That spreads one operational invariant across the daemon, supervisor, and devnet adapter, making future changes require synchronized edits rather than a single obvious update.

Example
If LIVENESS_PROBE_TIMEOUT_MS is later changed to 10s, the supervisor grace helper would return 71_000ms for a 60s hard timeout, while the testnet adapter helper would still return 66_000ms. A maintainer would have to discover by inspection that these are two copies of the same shutdown-envelope invariant.

Suggested direction
Move the shared “worker may take hard timeout plus forced cleanup” calculation to the shutdown module that owns those constants, then let the supervisor and testnet adapter add only their truly local cushion. That would make the model explicit and remove the parallel arithmetic.

For Agents
Introduce a canonical helper in packages/cli/src/daemon/shutdown.ts for the shared forced-exit envelope, e.g. hard timeout plus forced-cleanup budget, or a small resolveShutdownTiming(...) model. Have both shutdownGraceMsForHardTimeout and resolveTestnetOperatorShutdownExitTimeoutMs build from that helper, preserving the current returned values. Add a focused test that both consumers stay aligned for the configured-hard-timeout case.

[otReviewAgent]

🔴 Bug: Plane-specific observation test still accepts VM data as SWM evidence

What's wrong
The PR introduces separate VM and SWM assets, but this test gives false confidence because it feeds VM payload to both views and expects both planes to match the VM snapshot. That does not verify the new plane-specific behavior.

Example
A regression where the SWM observer accepts VM asset rows, or where observePlane does not filter expected assets by asset.plane, would still pass because the SWM query fixture also returns the VM subject and the expected value is built from plan.assets[0].

Suggested direction
Make the test fixture and assertions distinguish VM and SWM payloads, ideally with different subjects and digests per plane, so plane-mixing regressions fail.

For Agents
Update devnet/rfc64-m1-selective-coverage/testnet-operator-common.test.ts so the mock server returns VM rows for view: verifiable-memory and SWM rows for view: shared-working-memory, then assert separate expected snapshots from the matching plane assets. Add a negative case proving cross-plane payload is rejected or cannot satisfy the wrong plane.

[otReviewAgent]

🟡 Issue: Model wave/plane assets as the real shape instead of a flat list plus filters

What's wrong
The PR adds plane as another discriminator on a flat asset array, then spreads the resulting regrouping logic across the decoder, corpus preparer, and adapter. That is workable for four assets, but it makes the core invariant implicit and makes future readers audit every graph.assets loop to see whether it is plane-aware. The cleaner move is to make the parsed model match the domain shape directly.

Example
The new invariant is really selected.vm, selected.swm, final.vm, and final.swm, but the type exposes it as readonly assets: TestnetOperatorAssetV1[]. Every caller must remember to reconstruct that shape with filters before doing plane-specific work.

Suggested direction
Introduce a canonical wave/plane asset structure at the config boundary, then make publish/observe/corpus code consume named slots. That would delete the repeated filter chains and make the one-asset-per-wave-and-plane invariant visible in the type rather than re-derived ad hoc.

For Agents
Look at TestnetOperatorGraphV1, readTestnetOperatorConfig, prepare-testnet-corpus.ts, and publishWave. Preserve the same serialized config behavior if needed, but parse it into a typed assetsByWaveAndPlane/WaveAssets model or helper that returns exact slots. Prove publishing and observation still use the same VM/SWM assets for selected and final waves.

[otReviewAgent]

🟡 Issue: No test verifies the new VM-publish/SWM-share adapter contract

What's wrong
The main behavior change for the devnet operator is the publish-wave command's split handling of VM and SWM plane assets. The added tests validate the config requires plane assets, but they do not prove the adapter actually uses the new contract when issuing publisher API calls.

Example
A change that accidentally calls /vm/publish for selected-swm-1, skips /swm/share for it, or publishes SWM before the VM asset could still leave the current unit tests green because no test records the adapter's publish-wave HTTP requests.

Suggested direction
Cover the changed publish-wave behavior at the adapter boundary with a mock HTTP publisher that records requested paths and bodies.

Confidence note
I found tests for config decoding and timeout helpers, but no test driving testnet-operator-adapter.ts's publish-wave handler or asserting the publisher HTTP calls it makes.

For Agents
Add an adapter-level test around publish-wave with a fake publisher API and a minimal operator config containing one VM and one SWM asset for a wave. Assert the ordered calls: VM /swm/share, VM /vm/publish, SWM /swm/share, no SWM /vm/publish, followed by observations reaching the expected counts.