refactor(state): share Shields deadline recovery decisions

[HOYALIM]

Summary

Shields deadline recovery previously repeated security decisions across synchronous and asynchronous executors. This change routes committed containment, owner recovery, reentrancy, generation resumption, and takeover authority through one pure decision model while preserving separate I/O, waiting, and callback behavior. Fault-injection tests also prove that authority rotation cannot strand an exact deadline or ambiguously published main generation.

Related Issue

Closes #9959.

Impact and Mechanism

Question	Answer
User/operator impact	Deadline recovery remains fail-closed without duplicating security decisions that can drift between sync and async paths.
Root cause	Recovery policy was embedded separately in each executor around shared lock observations.
Durable prevention	A pure phase-based model owns containment, protected-owner, reentrant-owner, and publication-authority decisions; executors perform only their contract-specific effects.
Non-goals	No Shields posture, timer duration, storage primitive, public command, or synchronous waiting contract changes.

Changes

• Add a shared Shields deadline-recovery decision model and decision-table coverage.
• Route asynchronous and synchronous deadline acquisition, protected-path clearing, and main-owner publication through that model.
• Preserve generation-specific cleanup before authority refusal, including lost publication replies and takeover-token rotation.
• Compose the decision model with completed auto-restore authority from current main.
• Add async and sync fault tests for post-link authority drift, ambiguous publication, and completed recovery without a timer marker.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — independent security and concurrency review passed the current-main integration. Fault-injection regressions cover publication cleanup and authority loss. Maintainer review remains pending.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: Not applicable; no DGX host preparation path changes.
• Station profile/scenario: Not applicable.
• Result: Not applicable.
• Supporting evidence: Deterministic filesystem and process-identity tests cover this internal state refactor; issue refactor(state): reuse Shields deadline fence decisions #9959 does not require live E2E unless those boundaries cannot be exercised deterministically.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — The 65-test focused acquisition file and the remaining 332 lifecycle tests passed in separate runs (397 total). The 27 completed-recovery and destroy integration tests also passed. Two platform-specific cases were skipped on macOS as designed. A combined lifecycle run exposed a pre-existing 10 ms publication-hook timing case under suite load; both affected cases pass when run alone.
• Applicable broad gate passed — CLI and plugin builds, CLI type-check, repository checks, source and type-aware lint, formatting, diff checks, normal commit hooks, and npm run validate:pr passed after refreshing current main.
• Large change reviewed — four files, +603/−95 (698 lines of churn). Production is +316/−95; tests are +287.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Ho Lim subhoya@gmail.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

• Bug Fixes
  • Improved lifecycle recovery when timer or takeover authority changes during an operation.
  • Prevented operations from running when recovery authority cannot be verified.
  • Improved handling of active, stale, reentrant, and unavailable ownership states.
  • Ensured interrupted recovery attempts clean up both primary and deadline locks.
  • Added safer retry, waiting, containment, and resume behavior for lifecycle operations.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9144de84-0af6-48c3-a804-21b512db1463

📥 Commits

Reviewing files that changed from the base of the PR and between d64957c and d3b9260.

📒 Files selected for processing (1)

• src/lib/state/mcp-lifecycle-lock/decisions.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/state/mcp-lifecycle-lock/decisions.test.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

The change centralizes Shields deadline-recovery decisions for synchronous and asynchronous lifecycle-lock flows. Acquisition, publication, owner recovery, containment retries, and fence execution now revalidate timer authority. Tests cover authority rotation and ambiguous publication failures.

Changes

Deadline recovery

Layer / File(s)	Summary
Deadline recovery decision model src/lib/state/mcp-lifecycle-lock/decisions.ts, src/lib/state/mcp-lifecycle-lock/decisions.test.ts	Adds discriminated input and output types plus decideMcpLifecycleDeadlineRecovery. Tests cover containment, publication, owner identity, waiting, resuming, and refusal outcomes.
Acquisition and publication integration src/lib/state/mcp-lifecycle-lock-acquisition.ts	Uses centralized decisions across async and synchronous deadline acquisition, protected-path clearing, owner recovery, publication, containment retries, and fence execution.
Authority drift and publication failure tests src/lib/state/mcp-lifecycle-lock-acquisition.test.ts	Adds async and synchronous tests for timer-authority rotation, failed reconciliation, completed auto-restore recovery, skipped operation execution, and cleanup of both generations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to d3b92

This refactor centralizes Shields deadline-recovery decisions while preserving executor behavior, and no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant DeadlineFence
  participant RecoveryDecision
  participant LockGenerations
  participant ProtectedOperation
  DeadlineFence->>RecoveryDecision: validate deadline authority
  RecoveryDecision->>LockGenerations: inspect owner and generation state
  LockGenerations-->>RecoveryDecision: recovery outcome
  RecoveryDecision-->>DeadlineFence: proceed, wait, contain, resume, or refuse
  DeadlineFence->>ProtectedOperation: invoke after accepted authority

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: sharing Shields deadline recovery decisions across state handling paths.
Linked Issues check	✅ Passed	The changes implement the shared deadline-recovery decision model for synchronous and asynchronous acquisition, protected-path clearing, authority validation, publication, recovery, and generation cle…
Out of Scope Changes check	✅ Passed	The changes remain within issue #9959. They refactor deadline recovery decisions and add focused tests without changing Shields posture, policy restoration, timer duration, storage primitives, user-fa…

Full details: Linked Issues check

Explanation

The changes implement the shared deadline-recovery decision model for synchronous and asynchronous acquisition, protected-path clearing, authority validation, publication, recovery, and generation cleanup. The added tests cover the required authority drift, reentrancy, containment, publication failure, and recovery cases for issue #9959.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #9959. They refactor deadline recovery decisions and add focused tests without changing Shields posture, policy restoration, timer duration, storage primitives, user-facing commands, or synchronous waiting behavior.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[wscurran]

✨ Thanks for the refactor. This consolidates deadline recovery decisions into a pure decision model with fault-injection tests — ready for maintainer review.

---

Related open issues:

• #9959 refactor(state): reuse Shields deadline fence decisions

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[github-actions]

PR Review Advisor finished for commit 2b674a0. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs