fix(env): destroy works from wrappers that mention the target path

[hannesrudolph]

Closes #104

Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

ocm env destroy can classify its own caller wrapper as a process owned by the target environment when the wrapper command line mentions the environment root. Descendant expansion then pulls the active OCM invocation and transient inspection processes into the candidate set, so the two guarded process scans can disagree and fail with:

environment process state changed during inspection; retry the command

This blocks preview and guarded destroy even when the target service is stopped and no target-owned process is running.

Why This Change Was Made

Exclude the active OCM invocation from environment ownership discovery: the current OCM process, its caller ancestry, and descendants spawned by that OCM process are ignored as candidate seeds and during descendant expansion.

The exclusion deliberately does not remove siblings or all descendants of the caller wrapper, and it does not weaken path-based ownership matching. Independent processes whose command line or working directory belongs to the target environment remain detectable and terminable. This keeps the repair scoped to the invocation tree that cannot safely be treated as destroyable target work.

User Impact

Operators can preview and perform guarded environment destruction from shell wrappers, maintenance scripts, and diagnostic flows whose command lines mention the target path. The destroy guard remains stable instead of counting its own invocation.

Genuinely target-owned processes are still discovered, while independent sibling-environment processes remain untouched.

Evidence

• The new wrapper regression failed on unchanged main with the exact reported inspection-state error.
• After the fix, the same wrapper preview reports processCount: 0, guarded destroy succeeds, and an independent sibling process remains running.
• cargo test --locked invocation_processes_include_ancestors_and_current_descendants_only -- --nocapture: passed.
• cargo test --locked --test env_destroy_tests: 14 passed, 0 failed.
• cargo fmt --all -- --check: passed.
• git diff --check origin/main..HEAD: passed.
• Hosted CI passed on macOS, Ubuntu, Windows, formatting, and the Rust 1.88 minimum lane.
• No OCM binary was installed locally, and no resident daemon or managed gateway was restarted.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[hannesrudolph]

@clawsweeper re-review

This PR implements the focused repair proposed on #104.

Change selected

Process discovery now excludes:

• the active OCM process;
• its caller ancestry; and
• descendants spawned by that OCM process, including transient inspection helpers.

The exclusion applies both when selecting environment-owned seed processes and when expanding their descendants.

Deliberate boundary

I did not exclude the caller wrapper's entire descendant tree, and I did not weaken command-line or working-directory path ownership. A blanket wrapper-tree exclusion could hide a genuinely target-owned process launched independently by the same maintenance shell. The helper therefore excludes current-process descendants, not siblings, and the unit test proves that distinction.

Evidence

• The wrapper regression failed on unchanged main with environment process state changed during inspection; retry the command.
• With the repair, the same wrapper reports processCount: 0 and guarded destroy succeeds.
• The regression keeps an independent sibling process alive through destroy.
• Invocation-tree unit coverage passes.
• Full env_destroy_tests: 14 passed, 0 failed.
• Formatting and git diff --check passed.

No local OCM deployment or gateway restart was performed.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 21, 2026, 4:09 PM ET / 20:09 UTC.

ClawSweeper review

What this changes

The PR excludes the running OCM command, its ancestors, and its descendants from Unix environment-process discovery, and adds a wrapper-path destroy regression test.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open for normal maintainer merge review: this member-authored PR has a focused, source-consistent fix with no actionable correctness finding. Likely related people: shakkernerd (environment-destroy history) and Vincent Koc (guarded-destroy history).

Priority: P2
Reviewed head: ad9e1d3c9bdf6a611a493eb981a0bc6751dae340

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused implementation, real workflow evidence, regression coverage, and green platform checks support normal maintainer merge review.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR supplies a real wrapper-based before/after destroy result, including a successful guarded destroy and retained sibling process, with completed platform checks.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR supplies a real wrapper-based before/after destroy result, including a successful guarded destroy and retained sibling process, with completed platform checks.
Evidence reviewed	6 items	Invocation-tree exclusion: The new helper collects the current process, its ancestry, and descendants into an exclusion set before ownership discovery. Both discovery phases honor the boundary: Candidate seeds and descendant expansion both skip invocation-tree processes, while existing command-path and working-directory ownership matching remains intact. Focused end-to-end regression: The added Unix test invokes destroy through a wrapper that carries the target path, requires a zero process count and successful guarded destroy, and keeps a sibling process alive.
Findings	None	None.
Security	None	None.

Live Verification

Command: cargo test --locked --test env_destroy_tests env_destroy_ignores_its_invocation_wrapper_when_the_command_mentions_the_target_root -- --nocapture

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

• FAIL expect_output: test result: ok

How this fits together

ocm env destroy reads the host process table to find environment-owned work before previewing or removing an environment. Its candidate list feeds the guarded state token and process termination steps, so it must not classify the command performing the inspection as target work.

flowchart LR
  A[Shell wrapper] --> B[OCM destroy command]
  B --> C[Process-table snapshot]
  C --> D[Exclude active invocation tree]
  D --> E[Find environment processes]
  E --> F[Preview or guarded teardown]

Loading

Before merge

• Resolve merge risk (P1) - On Unix, an environment-owned ancestor of the OCM command is intentionally no longer eligible to block or be terminated by that same destroy invocation; maintainers should accept this invocation-tree safety boundary.
• Complete next step (P2) - No repair action is identified; this member-authored PR has a coherent focused patch and needs ordinary maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed surface	2 files; production +64/-3, tests +84/-0	The patch narrowly alters Unix process selection and adds an end-to-end guarded-destroy regression.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #104
Summary: This PR is the focused candidate fix for the wrapper self-classification behavior reported in the canonical issue.

Members:

• canonical: env destroy can count its own wrapper and inspection processes #104 - The PR body explicitly identifies that issue as the behavior addressed, and the regression implements its requested wrapper scenario.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Accept the invocation-tree boundary (recommended)
   Merge with the documented rule that caller ancestry and current-command descendants are not destroy targets, as covered by the wrapper regression.

Technical review

Best possible solution:

Merge the narrow invocation-tree exclusion while retaining the wrapper regression as the contract for guarded destroy process discovery.

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

Yes, from source and the supplied focused regression: prior command-path matching can select a wrapper that mentions the target root, while the branch test exercises preview and guarded destroy through that wrapper. This review did not independently run the current-main failure because the base tree is unavailable locally.

Is this the best way to solve the issue?

Yes: excluding only the active invocation tree preserves the existing command-path and working-directory ownership rules for independent processes, making it narrower than weakening path matching globally.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 768714ae3491.

Labels

Label justifications:

• P2: The bug can prevent guarded environment destruction for wrapper-based operator workflows without indicating broad runtime failure.
• merge-risk: 🚨 compatibility: The PR deliberately changes which already-running Unix processes can be considered destroy targets.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR supplies a real wrapper-based before/after destroy result, including a successful guarded destroy and retained sibling process, with completed platform checks.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies a real wrapper-based before/after destroy result, including a successful guarded destroy and retained sibling process, with completed platform checks.

Evidence

What I checked:

• Invocation-tree exclusion: The new helper collects the current process, its ancestry, and descendants into an exclusion set before ownership discovery. (src/cli/env.rs:1711, ad9e1d3c9bdf)
• Both discovery phases honor the boundary: Candidate seeds and descendant expansion both skip invocation-tree processes, while existing command-path and working-directory ownership matching remains intact. (src/cli/env.rs:1764, ad9e1d3c9bdf)
• Focused end-to-end regression: The added Unix test invokes destroy through a wrapper that carries the target path, requires a zero process count and successful guarded destroy, and keeps a sibling process alive. (tests/env_destroy_tests.rs:760, ad9e1d3c9bdf)
• Feature-history routing: Environment-destroy history is dominated by Shakker; commits 3260b95 and 796ddbe specifically stabilized detection and path boundaries, while e455c4f added guarded destruction. (src/cli/env.rs:1696, 3260b95aa322)
• Release check: No checked-out release tag contains the PR head, so this change is not shipped; the latest supplied release is v0.2.33. (ad9e1d3c9bdf)
• Base availability limitation: The review worktree is grafted at the PR head without a merge base; current-main source could not be fetched because GitHub name resolution failed, so no claim that main already contains this fix is made. (768714ae3491)

Likely related people:

• shakkernerd: Authored the original environment-destroy command and the later process-detection and path-boundary fixes; 58 commits touch these central files. (role: feature owner and recent area contributor; confidence: high; commits: a4a118503127, 3260b95aa322, 796ddbeeb596; files: src/cli/env.rs, tests/env_destroy_tests.rs)
• Vincent Koc: Added conditional environment destruction and related process-boundary hardening in the same code path. (role: guarded-destroy contributor; confidence: medium; commits: e455c4f993c6, 3f852323c1fe; files: src/cli/env.rs, tests/env_destroy_tests.rs)

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 (1 earlier review cycle)

• reviewed 2026-08-21T19:57:02.752Z sha ad9e1d3 :: needs maintainer review before merge. :: none

[hannesrudolph]

@clawsweeper

The platform checks requested by the latest review are now complete:

• macOS: passed
• Ubuntu: passed
• Windows compile: passed
• Rust 1.88 minimum: passed
• Format: passed

I made no additional code change because the review identified no actionable correctness finding. The remaining merge-risk note is already covered by the checked-in wrapper regression: it proves the invocation tree is excluded while an independent sibling process remains detectable and survives destroy.

The review's live-verification command did not reach the Rust test because its setup attempted a Corepack download. That setup failure does not contradict the focused local before/after reproduction or the now-green hosted platform runs. The PR body has been updated with the completed CI evidence.

[clawsweeper]

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: The platform checks requested by the latest review are now complete: - macOS: passed - Ubuntu: passed - Windows compile: passed - Rust 1.88 minimum: passed - Format: passed I made no additional code change because the review identified no actionable correctness finding. The remaining merge-risk note is already covered by the checked-in wrapper regression: it proves the invocation tree is excluded while an independent sibling process remains detectable and survives destroy. The review's live-veri...

[clawsweeper]

ClawSweeper assist: The completed platform checks strengthen the existing evidence, but the PR body was updated after the latest ClawSweeper review, so a fresh correctness review is needed for the current PR state.

Evidence:

• Prior review covered head ad9e1d3c… and reported no findings: fix(env): destroy works from wrappers that mention the target path #105 (comment)
• The updated CI evidence was posted afterward at 20:06 UTC: fix(env): destroy works from wrappers that mention the target path #105 (comment)
• The supplied context reports macOS, Ubuntu, Windows compile, Rust 1.88, and format passing.
• The wrapper regression documents both invocation-tree exclusion and sibling-process preservation.

Suggested next action: Request a current-state review with @clawsweeper review.

---

Source: #105 (comment)
Assist reasoning: high.