Skip to content

fix(setup-owner): keep keeper alive after CLI exit - #33

Merged
BramVR merged 1 commit into
mainfrom
codex/setup-owner-breakaway
Sep 3, 2026
Merged

BramVR merged 1 commit into
mainfrom
codex/setup-owner-breakaway

Conversation

@BramVR

@BramVR BramVR commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Problem

The setup-owner CLI launches its detached keeper inside the Windows OpenSSH outer Job. When the CLI exits, that outer Job tears down the keeper before it can finish, leaving a published receipt that later reconciles as owner_lost / cleanup_unverified.

Fix

Request CREATE_BREAKAWAY_FROM_JOB together with the existing process-group and detached flags. The keeper can then escape an outer Job that permits breakaway while preserving the exact unnamed kill-on-close Job assigned to the PowerShell setup tree. Missing Windows flag support fails closed with SetupOwnerError.

Verification

  • ./.venv/bin/python -m pytest -q — 203 passed, 9 skipped
  • ./.venv/bin/ruff check src tests — passed
  • git diff --check — passed
  • Added a Windows-fake regression asserting all three keeper creation flags and a fail-closed compatibility test.

The live Hermes repro is authorized and should be rerun by the integration owner after this dependency lands.

Model: GPT-5.6
Harness: Codex desktop

Copilot AI lite review requested due to automatic review settings September 3, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A newly added test assertion is vulnerable to Python operator precedence (== vs |), so it may not actually verify the combined creation flags value.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the Windows setup-owner keeper spawn so it can survive the CLI exiting under an outer Windows OpenSSH Job, by requesting CREATE_BREAKAWAY_FROM_JOB (and failing closed when that capability isn’t available).

Changes:

  • Switch keeper spawning to use a dedicated _windows_keeper_creation_flags() helper that includes CREATE_BREAKAWAY_FROM_JOB.
  • Add a Windows-faked regression test asserting the keeper is spawned with the expected creation flags.
  • Add a compatibility test asserting an unsupported breakaway flag fails closed with SetupOwnerError.
File summaries
File Description
tests/test_setup_owner.py Adds regression/compat tests around keeper creation flags and fail-closed behavior.
src/blendersessiond/setup_owner.py Adds Windows keeper creation flags helper and uses it when spawning the keeper.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_setup_owner.py

setup_owner._spawn_keeper(tmp_path)

assert calls[0]["creationflags"] == 0x200 | 0x400 | 0x800
@BramVR

BramVR commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Verified exact head 3a3f21a68d6584a64d60a73a7681171dffa82693.

  • Local gate: Ruff clean; 203 tests passed, 9 platform skips; build and diff checks clean.
  • CI: all eight exact-head checks passed, including both Windows jobs and required Ubuntu Blender smoke.
  • Independent Terra-high review: PASS_WITH_NOTES, no actionable code blocker. The Python assertion parses as equality against the combined bitmask; AST verification confirms it exercises all three flags.
  • Owned-host proof: a wheel built from this exact head launched a minimal setup owner through Windows OpenSSH. The launch returned owned; after the SSH parent exited immediately, exact fenced status converged to process_succeeded / exited / tree_gone, exit code 0, with untruncated output. This directly reproduces and fixes the parent-lifetime failure.

No Blender Session was launched or stopped. Operator host details remain private.

@BramVR
BramVR merged commit 6d40e40 into main Sep 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants