feat(disk-hygiene): add read-only managed-state owner registry

[kyle-sexton]

Refs: #4006

Summary

Adds the read-only slice of #4006: a managed-state owner registry as data (owner-registry.json with a schema), a report design for registry matches, and tests. No destructive command, credentials, elevation or deletion was added, and the engine is unchanged.

The engine has two deletion lanes. apply needs --execute, --confirm-tier and a fresh token, then runs apply_plan. handoff-apply needs --execute and one exact path, re-verifies it in process, then runs handoff_apply. Both remove through anchored_remove. The registry adds no route into either: the engine blocks a plan that claims a registry owner as native-managed-report-only and issues no token for it, and handoff_apply takes no plan, so it carries no owner claim.

The report neither shows nor runs a product-native destructive command; the registry keeps each as data to inspect. Whether the report may show one, or offer one behind the engine's tier and exact-list approval, is the owner's call, and building a route needs an engine change. #4006 stays open.

Managed state with no registry match still gets the native cleanup handoff under the existing SKILL.md text: the documented native command and its current dry-run result. A registry match follows managed-state-report.md alone (SKILL.md §4 says so), and its step 4 shows no destructive command. The owner's decision on whether a report may show a destructive command therefore covers both cases.

Fix

• skills/clean/reference/owner-registry.json and owner-registry.schema.json: six owners, each with platform-scoped path patterns and a note that a hint is not authorization. Each entry carries a required verification record (claim, basis, as-of date, recheck trigger).
• skills/clean/reference/managed-state-report.md: how a registry match is reported, with a table checking it against the design constraints. It says the report neither shows nor runs the destructive command and that no route for either is built. Its presence check and read-only command are tool calls or operator actions, never shipped code: the skill's Bash guard denies them, so the agent runs them in the PowerShell lane (where the guard gives no decision) or the operator runs them and the report records the output. The test fence names read-only commands as it names destructive ones, and the design-check rows and the test docstring say so.
• skills/clean/scripts/test_owner_registry.py (wrapper owner_registry.test.sh), 24 tests:
  • Registry grants no approval (RegistryGrantsNoApprovalTest): a registry path gets the same verdict as a neutral path, the token depends only on snapshot and plan, apply keeps its gates, and an owner-claimed plan stays report-only. These show the registry adds no approval. They do not test a destructive route, because none exists.
  • Only the engine's lanes delete (OnlyTheEngineLanesDestroyTest, EngineSourceTest):
    • A scan of every shipped non-test file under plugins/disk-hygiene (including hooks/hooks.json) fails any file outside the engine that deletes, names the registry, one of its commands (matched up to a placeholder) or one of its tools as a string constant, references apply_plan, handoff_apply, anchored_remove or purge_directory_contents, or imports a process runner (only the engine and the telemetry emitter may). A non-Python file also fails on a deletion verb or child_process beyond the launchers' existing count.
    • Inside the engine, only apply_plan, handoff_apply, anchored_remove, purge_directory_contents and write_text_atomic and run_inventory may delete. write_text_atomic and run_inventory remove only their own temporary file, which a test pins. A registry command or tool may appear only in apply_plan, so a route built on the registry would sit behind the tier and token gates.
    • apply_plan and handoff_apply each have main as their only caller. anchored_remove has exactly those two callers. purge_directory_contents is called only from anchored_remove, and only handoff_apply asks for it. handoff_apply takes (snapshot, relative, vcs_evidence), so no plan and no owner claim.
    • Planted parallel paths, in Python and shell, and a registry command planted in handoff_apply, anchored_remove or purge_directory_contents, are asserted to fail; the two lanes, their removers and the telemetry emitter are asserted to pass.
    • These tests pin which code deletes and who calls it. handoff_apply's own gates (exact path, in-process clear verdict) are covered by test_hygiene.py.
  • Engine unchanged: the engine reads no registry file and runs no registry tool during scan and preview, and the baseline policy loads identically without the registry. The engine file is unchanged versus origin/main.
• Limit of the scan: it is static, so getattr, eval, importlib and a command assembled at run time are not seen. A move or overwrite (os.replace, shutil.move) is not counted as a deletion, a non-Python file is not checked for every process it starts, and test files are not scanned.
• skills/clean/SKILL.md §4 (the file stays at 499 lines under the 500-line cap) states the precedence: a registry match follows managed-state-report.md alone, and only managed state with no registry match gets the native-command handoff. The §1 managed-state bullet and README.md point a registry match at §4 and the report. reference/safety-model.md points to the report doc.
• SKILL.md §2 question 3 tells the agent to match a path against the registry first. The report gates only commands on tool presence, keeps manual_step when the tool is absent, and runs probes by the resolved application executable.
• plugin.json 0.40.0 with a matching CHANGELOG entry above main's 0.39.0.
• test_owner_registry.py is mode 100755, as its shebang and its sibling test_hygiene.py require.

Verification

Run on 29643714b, which contains origin/main at 52c4885a2 (git merge-tree HEAD origin/main is clean).

• owner_registry.test.sh: 24 tests OK. The planted Python and shell parallel paths, and the registry command planted in each engine function, are asserted inside the suite, so they run every time; this head was not re-checked by planting files in the tree.
• hygiene.test.sh (648 tests, 1 skipped), engine_context.test.sh (8), guard_launch_monitor.test.sh (46) and the setup skill's kill_switch_probe.test.sh (20) and python3_alias_probe.test.sh (10): OK.
• scripts/affected-tests.sh --run: every shell suite it runs passes; it lists 5 Python suites (including test_owner_registry.py) as NOT RUN because they belong to their own lane, and they were run through their wrappers as above.
• scripts/run-ruff.sh check and format on test_owner_registry.py pass.
• Guard check behind the report's probe lanes: destructive_guard.py fed PreToolUse payloads returned deny for Bash docker system df, chezmoi doctor, command -v pulumi and pulumi about, and no decision for PowerShell docker system df, Get-Command docker and chezmoi doctor.
• scripts/check-changelog-parity.sh --check and --check-bump origin/main pass (0.40.0 against main's 0.39.0).
• scripts/check-changed-skills.sh origin/main passes (skills/clean/SKILL.md is 499 lines, cap 500).
• No tracked file in plugins/disk-hygiene with a shebang has index mode 100644.

Related

• Issue disk-hygiene: add a gated managed-state lane with a bundled owner registry #4006 (kept open).
• Supersedes the earlier closed attempts docs(disk-hygiene): park gated managed-state lane until funded (#4006) #5002 and fix(disk-hygiene): keep owner commands off the unlink blockers (#4006) #5095.

🤖 Generated with Claude Code

[github-actions]

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-30T19:35:34.694875Z	331e267	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 2m 8s —— View job

---

Security review (/review:security-review)

• Gather context (PR body, diff via gh pr diff 5562, changed files)
• Invoke /review:security-review per custom instructions
• Post findings

Scope: all 10 changed files (git diff HEAD 331e26741 vs origin/main): owner-registry.json/.schema.json (data), managed-state-report.md (new doc), SKILL.md/README.md/safety-model.md (doc edits), test_owner_registry.py/owner_registry.test.sh (new tests), plugin.json/CHANGELOG.md (version bump).

No security issues found.

Reasoning:

• The engine (skills/clean/scripts/hygiene.py) is not touched by this diff — confirmed both by the changed-files list and by the PR's own test_engine_reads_no_registry_file_and_runs_no_registry_tool test, which spies on Path.read_text/subprocess.run/subprocess.Popen during scan_tree/preview and asserts the registry file is never read and no registry tool is ever spawned.
• managed-state-report.md#L104-L108 (step 4) keeps the destructive command as inert data — never shown, never run — and the engine still blocks any plan that claims a native-managed-report-only owner (issues no approval token for it), so a registry match has no route into either deletion lane (apply_plan needs --execute/--confirm-tier/a fresh token; handoff_apply takes (snapshot, relative, vcs_evidence) — no plan, no owner claim). OnlyTheEngineLanesDestroyTest/EngineSourceTest in test_owner_registry.py back this with an AST-based static scan across every shipped file in the plugin.
• Checked the diff under the instruction-surface-deletion lens (docs/conventions/instruction-exception-register/README.md is not present in this checkout, so recognition was by consequence): the SKILL.md, README.md, and safety-model.md edits only add pointers to the new report doc — none narrow or remove the existing "managed state is never eligible for this engine" / "report-only handoff" guardrail language.
• managed-state-report.md#L112-L121 ("Who runs the probes") routes the registry's read_only_command (e.g. docker system df, pulumi about; pulumi plugin ls) through the PowerShell lane instead of the fail-closed Bash guard. That lane is pre-existing (SKILL.md#L480-L494, unchanged by this PR) and was already documented as "open for read-only support work, hard-denying engine invocations" — this PR reuses it rather than opening a new gap, and the PR's own verification section confirms destructive_guard.py still returns deny for these same commands over Bash and no decision (not allow) over PowerShell.
• No path traversal, injection, or credential-handling surface: path_patterns are static literal relative segments (not derived from runtime input), and owner-registry.schema.json is a plain validator with no external refs or code execution.
• GitHub Actions: no workflow files changed; out of scope for this lane regardless (zizmor's).

[claude]

Claude code review in progress

• Gather context (PR body, changed files list)
• Fetch diff against origin/main
• Read changed files
• Review for correctness / maintainability issues
• Adversarially verify candidate findings
• Post inline comments and summary

View job run

[claude]

Claude finished @kyle-sexton's task in 5m 5s —— View job

---

Claude finished the code review

• Gather context (PR body, diff, changed files)
• Check skip gate — open, not draft, non-trivial: proceeded
• Fetch diff against origin/main and read changed files
• Review for correctness / maintainability against CLAUDE.md/AGENTS.md conventions
• Adversarially verify candidate findings
• Post inline comments and summary

Ran /review:code-review (scoped to correctness/maintainability; security findings left to the separate /review:security-review lane already running on this PR) against 36d3c17 vs origin/main.

Findings (posted inline)

1. SKILL.md:370 — The registry-match branch has no trigger. §1 and §4 both describe what happens once a "registry match" exists, but nothing in SKILL.md tells the agent to actually compare a candidate path against reference/owner-registry.json. §2's triage step is unchanged and never mentions the registry. As written, an agent hitting e.g. ~/Library/Containers/com.docker.docker will never discover a match and will fall through to the old ad hoc research path — managed-state-report.md is exercised only by the test suite, never by the documented workflow.
2. managed-state-report.md:14 — Step 2's tool-presence gate suppresses manual_step too, but three seeded entries (nvidia-shader-cache, cursor, openai-codex-cli) have manual_step as their only content (read_only_command/destructive_native_command both null). Gating that manual step behind an unrelated CLI's presence on PATH (e.g. the optional cursor shim, which most Cursor users never install) means the report shows nothing for exactly the entries that exist because the product has no scriptable command.

What I verified directly

• The diff matches the listed changed files; HEAD at 36d3c17 merges cleanly with origin/main at 7106c28.
• owner-registry.json validates against owner-registry.schema.json by inspection (required keys, types, platforms/path_patterns consistency across all six entries).
• Traced hygiene.py's preview/apply_plan (lines ~3932, ~4749): candidate["owner"] != "unmanaged" unconditionally appends native-managed-report-only and blocks — confirms the engine is generically owner-aware already and untouched by this PR, matching the "engine unchanged" claim.
• Confirmed the EXECUTORS/LAUNCHER_ACTIONS pinned deletion-verb counts in test_owner_registry.py (run-python-hook.sh: 2, exec-bash.mjs: 1) match the current file contents.
• Confirmed test_owner_registry.py and owner_registry.test.sh are mode 100755 via git ls-files -s.
• Confirmed ## Gotchas and the PowerShell-lane bullet that managed-state-report.md links to actually exist in SKILL.md and support its claims about the guard giving read-only PowerShell commands no decision.

Not independently verified

This sandbox's Bash tool required interactive approval for every python3/git fetch/piped command beyond plain git status/log/diff/ls-files, so I could not execute owner_registry.test.sh or the other test suites myself. The PR body's "22 tests OK" and the other listed verification commands are author-claimed, not independently confirmed by me — I instead verified the relevant logic by reading hygiene.py and the test source directly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 331e26741b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 14s —— View job

---

Claude security review complete

• Gather context (PR body, changed files list)
• Fetch diff against origin/main
• Read changed files
• Review for security issues (logic flaws, authz gaps, injection, secrets handling, instruction-surface deletions)
• Adversarially verify candidate findings
• Post inline comments and summary

No security issues found.

Reviewed all 10 changed files (git diff origin/main...HEAD at 36d3c17e5ae2f50ff799d0d4dc5b47858278cf9b). Summary of what was checked:

• Engine untouched. hygiene.py is not in the diff, confirmed by git diff origin/main...HEAD --name-only. The native-managed-report-only blocker and the two gated deletion lanes (apply_plan, handoff_apply) already exist on origin/main unchanged — this PR adds no new code path that can delete or run a registry command.
• owner-registry.json / owner-registry.schema.json (blob): static, repo-committed data, not sourced from untrusted input at runtime. read_only_command/destructive_native_command values are fixed strings (e.g. pulumi about; pulumi plugin ls) with no template/interpolation, so there's no injection surface — they're read by a human or pasted into the PowerShell lane, never assembled from variable input.
• managed-state-report.md: explicitly keeps destructive_native_command as inert data — step 4 states it is "neither shown nor run by the report," and the engine issues no approval token for a plan that claims a registry owner. The PowerShell-lane / Bash-guard split it relies on (open for read-only support work, hard-denying engine invocations) is pre-existing behavior documented in SKILL.md §Gotchas, not something this PR weakens.
• Instruction-surface check (per this skill's deletion lens against docs/conventions/instruction-exception-register/README.md, which is present in this checkout): README.md and SKILL.md narrow the blanket "managed state is always report-only handoff" sentence to carve out a registry-match exception, but the carved-out path (managed-state-report.md step 4) is strictly more conservative than the prior universal rule — it shows no destructive command at all, versus previously reporting a documented native dry-run/prune command. This is a tightening, not a softening, of an irreversible-action-class guardrail, so it isn't a finding.
• test_owner_registry.py: the OnlyTheEngineLanesDestroyTest/EngineSourceTest suite's static scan (AST-based for Python, regex-based for shell/JS) has documented, disclosed blind spots (getattr, eval, importlib, runtime-assembled commands) — this is stated in the test's own docstring, not a hidden gap. Spot-checked against hygiene.py: apply_plan/handoff_apply have main as sole caller, anchored_remove has exactly those two callers, matching what the tests assert.
• File modes match the PR's claims (test_owner_registry.py and owner_registry.test.sh are both 100755).

No CRITICAL, IMPORTANT, or SUGGESTION findings. GitHub Actions hardening is out of scope for this lane (zizmor's advisory), and no workflow files are touched by this PR anyway.

[claude]

Claude finished @kyle-sexton's task in 1m 40s —— View job

---

Claude security review complete

• Gather context (PR body, diff, changed files, custom instructions)
• Invoke /review:security-review per custom instructions
• Post findings

Ran /review:security-review against the full diff origin/main...79dbe34f (10 files, 978/−9), with focus on what changed since the two prior security-review passes on this PR (those covered 331e267/36d3c17): the new commit 79dbe34 ("answer review on the managed-state report and registry").

No security issues found.

What changed since the last review, and why it's clean

1. PowerShell alias/function-shadowing (Codex P1, now fixed). The prior finding on managed-state-report.md warned that Get-Command <tool> alone considers a profile alias or function "present," so a shadow could run instead of the vendor executable during a purportedly read-only probe. managed-state-report.md#L33-L39 now requires Get-Command <tool> -CommandType Application (excludes Alias/Function command types), invocation via & '<Source>' <arguments> using the resolved absolute path, and explicitly treats a name that resolves only to an alias or function as absent-tool. This closes the exploit path the Codex finding described — a shadowed docker/pulumi can no longer satisfy the presence check or run in place of the real binary.
2. owner-registry.json verification records. Each of the six entries now carries a verification object (claim, basis, as_of, recheck), enforced by owner-registry.schema.json#L41-L55 as a required field. This is inert metadata (strings/dates), no new code path, no injection surface — addresses the prior staleness/auditability finding, not a security concern either way.
3. Registry-match trigger wired into the workflow. SKILL.md#L272 now tells the agent to match reference/owner-registry.json path_patterns before falling to the native-command path. This only routes more managed-state candidates into the strictly more conservative registry report (which still shows/runs no destructive command); it doesn't add a new destructive path, so no change to the security posture.
4. manual_step no longer suppressed by an absent tool. managed-state-report.md#L12-L16 now shows manual_step regardless of tool presence. manual_step is prose pointing at the product's own UI, never a command the skill or an agent runs, so this doesn't create an execution or injection surface.
5. test_owner_registry.py: added test_every_entry_carries_a_complete_verification_record and a pattern check in the local JSON-schema-subset validate() helper. Test-only, no runtime/production code path.

Reconfirmed from the unchanged parts of the diff (still holds)

• Containment is unaffected: hygiene.py (the engine) is untouched by this PR; apply_plan still requires --execute/--confirm-tier/a fresh token, handoff_apply still takes (snapshot, relative, vcs_evidence) with no plan/owner claim, and the engine still blocks any plan claiming a registry owner (native-managed-report-only, no token issued). OnlyTheEngineLanesDestroyTest/EngineSourceTest back this with a static AST/regex scan across the plugin.
• The destructive command in each registry entry is kept as inert data — step 4 of the report never shows or runs it, and no route exists from a registry match into either deletion lane.
• path_patterns are static literal relative segments, not runtime-derived; the schema and registry have no external $refs or executable content.
• Instruction-surface check: the SKILL.md/README.md/safety-model.md wording changes only narrow/tighten the existing "managed state is report-only" rule (routing more paths into the strictly-safer registry report), never loosen it. docs/conventions/instruction-exception-register/README.md is present in this checkout and was consulted; no protected-class rule is removed or weakened by this diff.

GitHub Actions hardening is out of scope for this lane (zizmor's advisory); no workflow files are touched by this PR.

[claude]

Claude finished @kyle-sexton's task in 3m 5s —— View job

---

Claude finished the code review

• Gather context (PR body, diff, changed files, prior review comments)
• Invoke /review:code-review per custom instructions
• Post inline comments for findings anchored to changed lines
• Post summary comment

Ran /review:code-review against 79dbe34 (fix(disk-hygiene): answer review on the managed-state report and registry), which directly follows up on the four review findings already posted on 331e267/36d3c17 (Codex's two P1s, Codex's P2, and my own SKILL.md reachability finding, duplicated by a second bot run). This review is scoped to whether that fix commit actually resolves those findings and to any new issue it introduces, per the CI code-review lane's high-signal bar (correctness/maintainability only; a separate security lane already ran on this PR, so security-flavored concerns stay there).

Prior findings: resolution check

1. Codex P1 — PowerShell alias/profile-function shadowing (managed-state-report.md:34-39): fixed for a single-invocation command — now requires Get-Command <tool> -CommandType Application and & '<Source>' <arguments> instead of a bare name. New inline finding below: the template doesn't handle the one registry entry whose read_only_command is compound (pulumi), so the fix is incomplete for that case.
2. Codex P1 — verification records (owner-registry.json, owner-registry.schema.json:42-56): resolved. All six entries now carry a verification object (claim/basis/as_of/recheck) matching the four-part record this repo's .claude/rules/skill-bodies-state-current-rules.md convention requires, the schema makes it required, and test_every_entry_carries_a_complete_verification_record exercises both the missing-field and malformed-field cases. Verified the validator's new pattern check (test_owner_registry.py:65-66) is safe for schemas without a pattern key (defaults to "", which re.search always matches, so it never spuriously fires).
3. Codex P2 / my duplicate — manual-step suppressed by unrelated tool absence: resolved. Step 2 in managed-state-report.md now gates only the commands on tool presence; manual_step is shown regardless, with a caveat that it applies only if the product is installed. Matches the registry's own stated design (owner-registry.json:3).
4. My finding — registry-match branch unreachable from the documented workflow (SKILL.md:271): resolved. §2 step 3 ("Establish evidence and ownership," which runs before §4) now explicitly instructs matching reference/owner-registry.json's path_patterns first, wiring the registry into the live workflow rather than leaving it exercised only by the test suite.

New finding (posted inline)

managed-state-report.md:38 — the resolved-executable invocation template (& '<Source>' <arguments>) is written for a single invocation, but the pulumi entry's read_only_command is "pulumi about; pulumi plugin ls" — two invocations of the tool name joined by ;. Applying the template once leaves the second clause calling the bare, potentially-shadowed pulumi name, reopening the exact alias-shadowing gap this fix closes for every other entry. Detail and a fix link are in the inline comment.

Not independently verified

Doc-only change; no test suite covers PowerShell invocation prose. owner_registry.test.sh's 22 tests and the other suites listed in the PR body's Verification section are author-claimed — I did not execute them myself in this session.

[claude]

Claude finished @kyle-sexton's task in 1m 1s —— View job

---

Claude security review complete

• Gather context (PR body, diff, changed files, custom instructions)
• Invoke /review:security-review per custom instructions
• Post findings

No security issues found.

Skip-gate check (per /review:security-review)

Confirmed condition 3 of the skip gate applies: this head (e0456092) carries no security-relevant change since the last full review at 79dbe34fc, which already passed two independent security-review passes (no CRITICAL/IMPORTANT/SUGGESTION findings).

• git diff 79dbe34..e0456092 -- plugins/disk-hygiene/skills/clean/ is empty. Every file this PR actually touches in its logic/data surface — SKILL.md, managed-state-report.md, owner-registry.json, owner-registry.schema.json, safety-model.md, test_owner_registry.py, owner_registry.test.sh — is byte-identical to the commit two prior reviews already cleared, and the review scope for those runs covered git diff origin/main...HEAD, which still resolves to this same 10-file, 978/−9 set (git diff origin/main...HEAD --stat, confirmed against this checkout's current origin/main).
• The only change between 79dbe34 and this head (e0456092, a "merge remote-tracking branch 'origin/main'" commit) touches plugin.json, CHANGELOG.md, README.md, plus files outside this PR's scope (hooks/run-python-hook.sh, skills/check/SKILL.md, skills/check/evals/evals.json) — all conflict resolution absorbing an unrelated /disk-hygiene:check feature (hooks: disk-hygiene's missing-Python notice names /disk-hygiene:setup check, which the model cannot run #5436/feat(guardrails): declare node requirement and warn at session start when it is missing #5547) that landed on main in the interim. That forced a version renumber (0.38.0 → 0.39.0) and a one-line README wording tweak to avoid colliding with the unrelated feature's own /disk-hygiene:check mention. No logic, data, or guardrail content changed.
• Re-checked changelog/version parity post-merge: plugin.json on this branch is 0.39.0; origin/main's is 0.38.0 — parity still holds.

Nothing new to review beyond what the last two passes already covered (engine untouched, no route from a registry match into either deletion lane, PowerShell alias-shadowing fix intact, manual_step no longer gated on tool presence). GitHub Actions hardening stays out of scope for this lane; no workflow files are touched.
· branch feat/4006-managed-state-registry

[claude]

Claude finished @kyle-sexton's task in 1m 10s —— View job

---

Claude finished the code review

• Gather context (PR body, diff, changed files, prior review threads on this branch)
• Invoke /review:code-review per custom instructions
• Check skip gate
• Post summary comment

Skip gate: stopping early, no new findings posted.

This trigger fired on a synchronize event, but the new commit (e045609) is only a merge of
origin/main into this branch. I diffed 79dbe34 (the commit this lane already reviewed,
see the 19:39–19:56 UTC review threads above) against the current head (e0456092) restricted to
the review-relevant paths (plugins/disk-hygiene/skills/clean/**) and it is empty — every file
under skills/clean/ is byte-identical to what was already reviewed:

git diff 79dbe34..HEAD -- plugins/disk-hygiene/skills/clean/reference/managed-state-report.md plugins/disk-hygiene/skills/clean/reference/owner-registry.json
# (0 lines)

The only files the merge touched are plugin.json (version 0.38.0 → 0.39.0) and
CHANGELOG.md (reordered to slot above main's own new 0.38.0 entry for #5436) — routine
merge-conflict resolution to keep check-changelog-parity.sh passing against origin/main's
advanced tip, not new review surface.

Per this lane's skip gate ("this head already has a successful review from this lane that still
applies"), this is a repeat of already-reviewed content:

• The one finding still outstanding from that review — the PowerShell resolved-executable
  invocation template (managed-state-report.md "Who runs the probes") not handling the
  ;-joined pulumi read_only_command, so its second clause (plugin ls) still calls the bare,
  potentially alias-shadowed pulumi name — is unchanged and already posted as an inline comment
  on this same text (managed-state-report.md:38, 19:55:47 UTC review). Confirmed still present:
  owner-registry.json:122 still reads "pulumi about; pulumi plugin ls", and the invocation
  template in managed-state-report.md is unmodified.
• The three earlier Codex/Claude findings (alias-shadowing, missing verification records,
  manual-step suppression) were fixed in 79dbe34 and reconfirmed in the prior review pass; still
  fixed here since nothing changed.

Posting no duplicate inline comments. No new findings from this synchronize.

[claude]

Claude finished @kyle-sexton's task in 1m 3s —— View job

---

Claude finished the code review

• Gather context (PR body, diff, changed files, prior review threads on this branch)
• Invoke /review:code-review per custom instructions
• Check skip gate
• Post summary comment

No new findings. The new commit fully resolves the last open finding.

This synchronize fired on b8a3254 ("fix(disk-hygiene): resolve the executable for each invocation in a compound probe"), a 2-line, single-file change on top of e045609 (already reviewed by this lane, which found nothing new beyond a routine merge).

That commit is the fix for the one finding still open from the 19:55:47 UTC review: the resolved-executable invocation template in managed-state-report.md ("Who runs the probes") covered only a single invocation, so the ;-joined pulumi read_only_command (pulumi about; pulumi plugin ls) left its second clause calling the bare, potentially alias-shadowed pulumi name.

managed-state-report.md:38 now reads:

& '<Source>' <arguments> with that resolved path, once for each ;-separated invocation in a
compound command (pulumi about; pulumi plugin ls is two), and treat a name that resolves only to an alias

Checked this against the registry data (owner-registry.json): pulumi (line 122) is the only read_only_command with a ;-joined compound form in the registry — docker-desktop's compound command (line 14) is a destructive_native_command, which step 4 of the report never runs, so it isn't affected by this gap either way. The fix's instruction to resolve-and-invoke per ;-separated clause closes the gap for the one case that mattered.

No correctness or maintainability issues in this commit: it's a documentation-only clarification, doesn't affect the SKILL.md 500-line cap (that file is untouched here), and doesn't reopen any of the four findings this branch already resolved in 79dbe34fc (SKILL.md registry-match trigger, owner-registry.json verification records, manual_step suppression, PowerShell alias-shadowing).
· branch feat/4006-managed-state-registry

[claude]

Claude finished @kyle-sexton's task in 38s —— View job

---

Security review (/review:security-review)

• Gather context (PR body, diff via gh pr diff 5562, changed files)
• Invoke /review:security-review per custom instructions
• Post findings

No security issues found.

Skip-gate check

This lane already cleared this PR's full 10-file diff (origin/main...HEAD, 979/−9) across four
prior passes on 331e267, 36d3c17, 79dbe34, and e045609, each with no security findings.
The only change since the last pass (e045609 → b8a3254, git diff e045609..HEAD) is a
2-line prose edit to one doc:

managed-state-report.md:38-39
— "Who runs the probes" now says to resolve-and-invoke once per ;-separated invocation in a
compound read_only_command (pulumi about; pulumi plugin ls is two), not once for the whole
string.

This closes the exact gap flagged in the code-review lane's 79dbe34 pass: the prior wording
wrapped & '<Source>' <arguments> around the full compound string, so only the first clause
(pulumi about) used the resolved, alias-proof executable path while the second clause
(plugin ls) still resolved the bare pulumi name through PowerShell's normal command lookup —
reopening the alias/profile-function-shadowing route the surrounding fix (Get-Command <tool> -CommandType Application, invoke via & '<Source>') exists to close. The new wording applies the
resolve-and-invoke step per clause, so both pulumi invocations now go through the resolved
executable. docker-desktop's destructive_native_command has the same ;-joined shape but is
still never shown or run by the report (step 4), so it isn't reachable by this probe path either
way.

This is prose only — no code, schema, or test file changed — and no new security-relevant surface
is introduced. Everything already confirmed in prior passes still holds unchanged: the engine
(hygiene.py) is untouched, no route exists from a registry match into either deletion lane,
read_only_command/destructive_native_command are static inert data with no injection surface,
and no workflow files are touched (GitHub Actions hardening stays out of scope for this lane).
· branch feat/4006-managed-state-registry

[claude]

Claude finished @kyle-sexton's task in 2m 0s —— View job

---

Claude finished the code review

• Gather context (PR body, diff, changed files, prior review threads on this branch)
• Invoke /review:code-review per custom instructions
• Check skip gate
• Post summary comment

No new findings.

This synchronize fired on 2964371, a merge of origin/main into this branch (pulling in the unrelated, already-merged #5585 "attended deep inventory" feature). The PR's actual diff vs origin/main (git diff origin/main...HEAD --stat) is still the same 10-file, +1000/−9 set this lane already reviewed across 331e267 → b8a3254, and SKILL.md/safety-model.md's content in that diff is byte-identical to what was already cleared (verified via git diff origin/main...HEAD on those two files — only the two hunks already reviewed in 79dbe34fc).

The one new thing since the last pass (b8a3254) is a 41-line adaptation in test_owner_registry.py, required because the merged-in deep-inventory feature added hygiene.py's run_inventory and scripts/deep_inventory.py — neither of which this PR introduces, but both of which the PR's static-scan test (OnlyTheEngineLanesDestroyTest/EngineSourceTest) now has to account for:

• DELETERS gains run_inventory (test_owner_registry.py:366). Traced run_inventory in hygiene.py (~line 4923): it deletes only its own partial temp file via partial.unlink(missing_ok=True) in a finally block, matching the new parametrized test_the_temporary_file_writers_remove_only_their_own_temporary_file case ("run_inventory", "partial.unlink").
• LABEL_ONLY = {SCRIPTS / "deep_inventory.py"} (test_owner_registry.py:349) exempts only the registry-tool-name string-constant check for that one file, because deep_inventory.py's TMP_PRODUCERS table uses "codex" as a /tmp-entry producer label (unrelated to the registry's openai-codex-cli entry, which happens to share the same tool name). Confirmed the exemption is narrowly scoped: the new test_a_label_only_file_is_still_checked_for_deletion_and_runners still asserts deep_inventory.py is flagged for a deletion call or a subprocess.run call, so only the tool-name-constant heuristic is relaxed, not the deletion/process-runner checks. REGISTRY_FRAGMENTS/REGISTRY_COMMANDS text matching (full command strings, e.g. "docker system df") is unconditional and untouched by the exemption.
• run_inventory is added to DELETERS (which only gates the AST exclusion for path == ENGINE_PATH) but not to FENCED (the caller-count-pinned lane/remover set), which is correct — it's a separate read-only CLI subcommand, not part of the apply_plan/handoff_apply deletion-lane chain, so it shouldn't be asserted to have main as its sole caller alongside those.

No correctness or maintainability issue found in this adaptation. The CHANGELOG.md/plugin.json renumbering to 0.40.0 (above main's own new 0.39.0 deep-inventory entry) is also consistent and correctly ordered.

Not independently verified: this sandbox's Bash tool requires interactive approval for python3, so I could not execute owner_registry.test.sh myself; I verified the adaptation's correctness by reading hygiene.py and deep_inventory.py source directly rather than running the suite.
· branch feat/4006-managed-state-registry

[claude]

Claude finished @kyle-sexton's task in 2m 4s —— View job

---

Security review (/review:security-review)

• Gather context (PR body, diff via gh/git diff origin/main...HEAD, changed files)
• Invoke /review:security-review per custom instructions
• Post findings

No security issues found.

Skip-gate check, then what's actually new

This lane already cleared this PR's full diff across five prior passes (331e267, 36d3c17,
79dbe34, e045609, b8a3254). This synchronize fired on
2964371, another "merge origin/main" commit, so I checked what it actually changes rather than skip
unconditionally.

• git diff origin/main...HEAD --stat (the PR's real content) still resolves to the same 10 files,
  1000/−9, confirming no new file entered the PR's own diff.

• The merge pulled in an unrelated, already-merged feature from main
  (feat(disk-hygiene): attended deep inventory..., feat(disk-hygiene): attended deep inventory with justified KEEP reasons #5585, hygiene.py's new inventory
  subcommand and skills/clean/scripts/deep_inventory.py). Those files are not part of this
  PR's diff against origin/main — they were already on main before this merge landed them here.

• The one file this PR owns that changed content since the last review is
  test_owner_registry.py:
  a merge-driven update adding LABEL_ONLY = {SCRIPTS / "deep_inventory.py"} and run_inventory to
  DELETERS, needed because the newly-merged deep_inventory.py uses "codex" and "cursor-agent"
  as /tmp-producer classification labels (deep_inventory.py:71-72), which would otherwise
  false-positive against REGISTRY_TOOLS ("codex" is the openai-codex-cli entry's tool value)
  in the static "only the engine deletes" scan.

  Checked whether this exemption weakens OnlyTheEngineLanesDestroyTest/EngineSourceTest's
  security guarantee: it doesn't. LABEL_ONLY is consulted in exactly one place
  (test_owner_registry.py:453-459),
  gating only the registry-tool-name-as-string-constant check. Every other check in violations() —
  deletion verbs (unlink/rmdir/rmtree/removedirs/os.remove), process-runner imports
  (subprocess, os.system, etc.), and the registry-fragment/command substring check — still runs
  against deep_inventory.py unconditionally. A new test,
  test_a_label_only_file_is_still_checked_for_deletion_and_runners
  (test_owner_registry.py:506-513),
  plants both a deletion call and a subprocess.run call into a fixture deep_inventory.py source
  string and asserts both still get flagged. Confirmed deep_inventory.py itself only uses these
  names as inert prefix→producer labels (no subprocess/os.system call sites reference them).

• run_inventory's addition to DELETERS is covered by the renamed
  test_the_temporary_file_writers_remove_only_their_own_temporary_file, which pins it (like
  write_text_atomic) to removing only its own partial temp file via AST inspection of the
  function body — not a broadened deletion surface.

• Confirmed no changes since b8a3254 to owner-registry.json, owner-registry.schema.json,
  managed-state-report.md, or owner_registry.test.sh (git diff b8a3254..HEAD on those paths is
  empty).

• Re-diffed SKILL.md/README.md/safety-model.md against origin/main directly (not against
  b8a3254, since the merge advanced the baseline): the PR's own content in those three files is
  byte-identical in substance to what the four prior passes already cleared — the larger b8a3254..HEAD
  diff in those files is entirely the unrelated feat(disk-hygiene): attended deep inventory with justified KEEP reasons #5585 doc additions already on main, not new PR
  content.

Nothing new to flag: the engine (hygiene.py) remains untouched by this PR's own diff, no route
exists from a registry match into either deletion lane, and the merge-driven test-file update
narrows a false-positive without loosening the static scan's actual guarantees. GitHub Actions
hardening stays out of scope for this lane; no workflow files are touched.
· branch feat/4006-managed-state-registry