Skip to content

fix: complete setup and recover interrupted session state - #51

Merged
paulcam206 merged 14 commits into
feat/session-diagnosticsfrom
fix/session-lifecycle-recovery
Sep 17, 2026
Merged

paulcam206 merged 14 commits into
feat/session-diagnosticsfrom
fix/session-lifecycle-recovery

Conversation

@paulcam206

@paulcam206 paulcam206 commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Interrupted setup and lifecycle transitions need to recover accurately without binding gateway state to the wrong session or hiding the reason a gateway exited.

User Impact

Setup can reconcile its durable state, retry a launch whose outcome was not confirmed, and use a guest-visible working directory. Status and stop commands expose the gateway detail when it is available.

Why This Change Was Made

The lifecycle requires a durable completion marker and reconciles stale provisions rather than accepting a superficially completed setup. Gateway records are scoped to the current session, and teardown removes gateway recovery registration.

Review fixes addressed

  • Production direct execution restores host runtime preparation through NodeRuntimeInstaller.EnsureInstalled.
  • The helper allowlist retains --install-tools, and the default working directory is the guest-visible session workspace unless explicitly configured.
  • Gateway inspection carries its supervisor detail into status; status and stop render that detail and startup guidance points to clawctl gateway-service status.
  • Collection-result reads are validated through handle-bound containment before any host-authority read.
  • Pending launch reconciliation uses a valid guest protocol. Explicit forced teardown can reach backend removal when guest inspection is unavailable, while the ordinary refusal remains covered.

Current validation

Latest hosted follow-up: The lifecycle-recovery test fixture no longer references the later-layer AliasCommand option, restoring this PR's independent layer compile. Its static-analysis quality gate and 22 focused ProgramTests pass, and the replayed final tip passes the full 628-test suite.

Handle-bound cleanup follow-up: diagnostics collection now retains a generation-aware workspace operation across guest collection, final bundle reads, and success/failure cleanup. Staged files are opened through that operation, and cleanup deletes relative to its validated workspace handle only while the recorded session generation remains current; the previous path-based recursive Directory.Delete flow is removed.

Current layer head: 6b08ee1a23890cb39260a917c588fe4deccc46f2. This layer is included in the final integrated stack tip cce2b02f3acd5791654b7a6b1a5a9c27db5ff63b rebased onto 685ee93b7ebbec1e784205a3544c460bea740e11.

Integrated local gates: exact .NET SDK 10.0.100; Test-DotNetQuality.ps1 with 0 warnings/errors; full solution tests 642/642; NativeAOT x64 and ARM64 publishes for both launcher and session host; NativeAOT CLI, deployment, MXC, signing, runtime-input, release-identity, bundle, isolation-plugin, and packaging-relevance policy suites.

Live x64 MXC evidence: final-tip Developer Mode deployment registered OpenClaw.Gateway_0.1.2451.40134_x64__kaa03rpbbqef6 from workflow payload run 35191206689; openclaw --version returned OpenClaw 2026.9.4 (3a9d69d); clawctl status confirmed the isolated session was running. Earlier final-tip validation also exercised setup, Node.js 24.20.0 reuse, package-qualified activation, detached gateway launch, and redacted diagnostics collection.

Signed package evidence: local NativeAOT x64 and ARM64 packages and a multi-architecture bundle were composed and test-signed. Elevated upgrade validation passed all four proof-release transitions (v0.0.0.0 and v0.0.0.1, standalone and bundle), retained package-family LocalState in every transition, and accepted fresh standalone and bundle installs. The temporary certificate and test package were removed, then the Developer Mode registration was restored.

Layer 9 of 12. Parent: #50 - feat/session-diagnostics. Child: #52 - feat/session-fresh-reset

@clawsweeper

clawsweeper Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

Comment thread src/OpenClaw.Launcher/Gateway/GatewayPersistenceManager.cs Fixed
Comment thread src/OpenClaw.Launcher/Gateway/GatewayPersistenceManager.cs Fixed
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 15, 2026
@clawsweeper

clawsweeper Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 17, 2026, 4:01 PM ET / 20:01 UTC (Revision 27).

ClawSweeper review

What this changes

The Windows launcher adds diagnostic bundles and session-free setup, reconciles interrupted session state, and coordinates gateway recovery with teardown.

Merge readiness

⛔ Blocked before merge - 4 items remain

This remains useful, unmerged work. Earlier cleanup and fixture defects are addressed in the current source; the remaining blocker is proof of rejection at the guest-to-host diagnostics boundary.

Priority: P2
Reviewed head: 6b08ee1a23890cb39260a917c588fe4deccc46f2

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation and supplied Windows validation are useful, with readiness limited by the remaining diagnostics authority proof.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Authority-chain proof required: captured integrated Windows x64 collection and package-transition reports support allowed behavior, but do not show redirected staging or invalidated session generations rejected before GatewayRuntime's final host reads and cleanup. Only this scoped requirement overrides the collaborator exemption; the existing runtime evidence remains credited. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Authority-chain proof required: captured integrated Windows x64 collection and package-transition reports support allowed behavior, but do not show redirected staging or invalidated session generations rejected before GatewayRuntime's final host reads and cleanup. Only this scoped requirement overrides the collaborator exemption; the existing runtime evidence remains credited. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Pinned review boundary: The locally inspected introduction delta is a6fe3ed..6b08ee1: 35 files, with production +1175/-210 and tests +1015/-69. The supplied test merge is stale and was not used to infer removal of current-main behavior.
Still necessary on the released default branch: The fetched default branch contains the direct launcher but not the Session, SessionHost, or managed Gateway subsystem introduced by this stack. Its README describes isolated sessions as future work. The release tag v2026.9.4-msix.0 points at that default-branch revision.
Cleanup repair is present: Collection retains a SessionWorkspaceOperation, uses it for final staged-file reads, and calls operation.Delete for success and failure cleanup. The previously reported recursive path-based staging deletion is absent.
Findings None None.
Security Needs attention Verify rejection before host reads and cleanup: Guest-writable staging becomes input to host-authority file reads and deletion. Current containment and generation guards are meaningful, but supplied evidence does not demonstrate rejection of redirected staging or invalidated generations through the complete production path; this is an unresolved verification concern, not a demonstrated exploit.

How this fits together

The Windows launcher prepares and manages OpenClaw inside an isolated agent session. Its diagnostics path collects guest files through a shared workspace, then reads and removes those files using the signed-in user's authority.

flowchart TD
  A[Operator commands] --> B[Windows launcher]
  B --> C[Recorded session and setup state]
  C --> D[Isolated agent session]
  D --> E[Shared diagnostics staging]
  E --> F[Generation and file containment checks]
  F --> G[Redacted ZIP and staging cleanup]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: captured integrated Windows x64 collection and package-transition reports support allowed behavior, but do not show redirected staging or invalidated session generations rejected before GatewayRuntime's final host reads and cleanup. Only this scoped requirement overrides the collaborator exemption; the existing runtime evidence remains credited. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve security concern: Verify rejection before host reads and cleanup - Guest-writable staging becomes input to host-authority file reads and deletion. Current containment and generation guards are meaningful, but supplied evidence does not demonstrate rejection of redirected staging or invalidated generations through the complete production path; this is an unresolved verification concern, not a demonstrated exploit.
  • Resolve merge risk (P1) - The available Windows evidence does not establish that guest staging redirection or session-generation invalidation is rejected before final host reads and recursive cleanup; a boundary failure could expose or remove files outside the authorized collection.
  • Complete next step (P2) - Provide the remaining diagnostics authority proof before merge. Redacted terminal output, logs, or recordings with diagnostics are suitable; remove credentials, private endpoints, and personal details. Update the PR body to trigger review, or ask a maintainer to comment @clawsweeper re-review if needed.

Findings

  • [medium] Verify rejection before host reads and cleanup — src/OpenClaw.Launcher/Gateway/DiagnosticsBundle.cs:392
Agent review details

Security

Needs attention: The prior cleanup defect is repaired, but diagnostics boundary rejection still needs final-effect evidence.

Review metrics

Metric Value Why it matters
Production and test growth Production +1175/-210, net +965; tests +1015/-69, net +946 The stated growth supports diagnostics bundling and lifecycle recovery, with a comparable amount of regression coverage.

Merge-risk options

Maintainer options:

  1. Complete diagnostics boundary proof (recommended)
    Supply Windows evidence that redirected staging and an invalidated session generation cannot cause outside-file reads or deletion through the production collection path.

Technical review

Best possible solution:

Retain generation-bound, handle-validated diagnostics access and establish its rejection behavior through the production collection boundary before landing.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a current-main bug reproduction: this extends an unmerged isolation stack. Source and regression tests establish the intended recovery cases, but no runtime reproduction was executed during this review.

Is this the best way to solve the issue?

Yes, the current handle-based cleanup and persisted reconciliation are a maintainable direction; final-effect boundary proof remains necessary to establish merge readiness.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 685ee93b7ebb.

Labels

Label justifications:

  • P2: This is a bounded Windows setup and recovery improvement in an unmerged isolation stack.
  • merge-risk: 🚨 security-boundary: New host reads and cleanup consume guest-writable staging, and adversarial final-effect behavior remains unproven.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: captured integrated Windows x64 collection and package-transition reports support allowed behavior, but do not show redirected staging or invalidated session generations rejected before GatewayRuntime's final host reads and cleanup. Only this scoped requirement overrides the collaborator exemption; the existing runtime evidence remains credited. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [medium] Verify rejection before host reads and cleanup — src/OpenClaw.Launcher/Gateway/DiagnosticsBundle.cs:392
    Guest-writable staging becomes input to host-authority file reads and deletion. Current containment and generation guards are meaningful, but supplied evidence does not demonstrate rejection of redirected staging or invalidated generations through the complete production path; this is an unresolved verification concern, not a demonstrated exploit.
    Confidence: 0.9

What I checked:

  • Pinned review boundary: The locally inspected introduction delta is a6fe3ed..6b08ee1: 35 files, with production +1175/-210 and tests +1015/-69. The supplied test merge is stale and was not used to infer removal of current-main behavior. (6b08ee1a2389)
  • Still necessary on the released default branch: The fetched default branch contains the direct launcher but not the Session, SessionHost, or managed Gateway subsystem introduced by this stack. Its README describes isolated sessions as future work. The release tag v2026.9.4-msix.0 points at that default-branch revision. (README.md, 685ee93b7ebb)
  • Cleanup repair is present: Collection retains a SessionWorkspaceOperation, uses it for final staged-file reads, and calls operation.Delete for success and failure cleanup. The previously reported recursive path-based staging deletion is absent. (src/OpenClaw.Launcher/Gateway/DiagnosticsBundle.cs:208, 6b08ee1a2389)
  • Authority checks and remaining proof scope: SessionWorkspaceOperation checks the saved sandbox generation before reads and deletion. TrustedPath verifies opened-file containment and performs deletion relative to protected directory handles. These are substantive safeguards; the unresolved verification concerns guest staging redirection and generation invalidation across the complete collection-to-cleanup path. (src/OpenClaw.Launcher/Session/SessionWorkspaceOperation.cs:94, 6b08ee1a2389)
  • Captured proof and review continuity: The complete supplied body, captured under sourceRevision e1f477f2f7b6fd5c7098d0b2cace9e48832cfc94f9fd8c84eebb5ba0d38ebf35, reports integrated Windows x64 setup, diagnostics collection, NativeAOT validation, and fresh/upgrade package checks. It does not report redirected staging or invalidated-generation rejection before final host reads/deletes. The previous completed review requested those same observations. The earlier local review object could not be loaded, so no unchanged-code or late-finding attribution is made. (6b08ee1a2389)
  • Focused regression coverage: Current tests cover stale-session replacement, preservation of superseded identities, refusal to replace an ambiguous pending gateway, host runtime preparation, and rejection of a reparse-point collection result. The collection regression substitutes the MXC transport, so it supplements rather than supplies the requested final-effect proof. (tests/OpenClaw.Launcher.Tests/Session/SessionExecutorTests.cs:170, 6b08ee1a2389)

Likely related people:

  • paulcam206: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Linus Huang: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add redacted Windows production-path evidence showing staging redirection and session-generation invalidation rejected before host reads or cleanup, with outside files preserved.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (26 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-17T16:24:24.333Z sha 166d399 :: needs real behavior proof before merge. :: [P1] [P1] Remove the obsolete AliasCommand test argument
  • reviewed 2026-09-17T16:38:43.164Z sha 22d31b2 :: needs real behavior proof before merge. :: [P1] Remove the obsolete AliasCommand test argument
  • reviewed 2026-09-17T16:57:37.163Z sha 3ba6c64 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-17T17:05:08.811Z sha 3ba6c64 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-17T18:19:37.026Z sha 024730b :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-17T19:02:23.029Z sha e456f4f :: needs real behavior proof before merge. :: [P1] [P1] Restore the session-state fixture path | [P2] [P2] Inject setup readiness in the shared session fixture | [P2] [P2] Supply valid application paths to ownership tests
  • reviewed 2026-09-17T19:23:27.591Z sha 41ed7ca :: needs real behavior proof before merge. :: [P1] [P1] Bind staging cleanup to validated directory handles
  • reviewed 2026-09-17T19:40:37.788Z sha 7b0c030 :: needs real behavior proof before merge. :: none

@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from b862f23 to 364ceec Compare September 15, 2026 20:32
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 364ceec to cb1939e Compare September 15, 2026 20:41
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from cb1939e to 7ed8a25 Compare September 15, 2026 20:57
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 7ed8a25 to f03c305 Compare September 15, 2026 21:28
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from f03c305 to 0fecd32 Compare September 15, 2026 23:23
@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Sep 17, 2026
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 261464c to 166d399 Compare September 17, 2026 16:18
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 17, 2026
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 166d399 to 4389d95 Compare September 17, 2026 16:27
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 4389d95 to 22d31b2 Compare September 17, 2026 16:33
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch from 22d31b2 to 3ba6c64 Compare September 17, 2026 16:51
@paulcam206
paulcam206 force-pushed the fix/session-lifecycle-recovery branch 2 times, most recently from 024730b to e456f4f Compare September 17, 2026 18:55
@clawsweeper clawsweeper Bot added the rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. label Sep 17, 2026
paulcam206 and others added 14 commits September 17, 2026 12:54
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 97c31f3f-0c7b-43e8-b979-6418dcb7fedc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants