[kyle-sexton]

No related issue: Phase 3 of the owner-approved machine-health PC-checkup plan (ADR 0081, #8906; builds on #8917).

Summary

This PR adds four things:

• the only writer for approvals.json and mutes.json: an atomic replace under a lock;
• an exclusive run lock;
• a guard that keeps plugin state out of cloud-synced folders;
• setup apply paths for unmute= and revoke=.

The decision-page picks, the elevated-pass import and the monthly run all write through this path in later phases.

Fix

• lib/Write-StateFile.ps1 writes approvals.json and mutes.json:
  • It writes a temp file in the same folder, calls Flush(true), closes it, then calls File.Replace(tmp, dst, null).
  • The first write uses File.Move without overwrite, because File.Replace throws when the destination is missing. The .NET reference (https://learn.microsoft.com/dotnet/api/system.io.file.replace) lists FileNotFoundException: "The file described by sourceFileName or destinationFileName parameter could not be found."
  • If the destination appears meanwhile, the writer re-reads the file, re-applies the change once and replaces.
  • On a sharing violation it retries 5 times, at 100 ms doubling, then fails, naming both the file and the temp file.
• lib/Set-ApprovalState.ps1 applies mute, unmute, approve and revoke:
  • It holds state/approvals.lock, a lock separate from run.lock, across the read, change and replace.
  • It validates against the schema before replacing, and refuses a synced state folder.
  • Remediation ids come from lib/Get-RemediationId.ps1, and a sync test checks them against scripts/windows/remediations/.
• lib/Enter-RunLock.ps1 (with lib/Get-InnerHResult.ps1):
  • The orchestrator takes run.lock exclusively, waiting up to 2 s.
  • A second run returns one run-lock UNKNOWN result inside a full RunSnapshot (lib/New-RunSnapshot.ps1) and writes nothing.
  • The lock is released in a single try/finally around the run. A test shows it is released after a throw.
• lib/Assert-StateBaseLocation.ps1: a state folder under a OneDrive variable prefix runs with every state write suppressed, and only the markdown report is written. In that mode there is no lock, folder, log, latest.json, history, approvals read, migration or remediation. Log lines go to stderr through Write-MachineHealthLog -ToStderr.
  • The attribute half of the rule (ReparsePoint plus a cloud bit) only warns. On the test host the real OneDrive folders carried no cloud attribute bits, so the plan's fallback applies.
• Approvals migration: the TODO.md migration in Get-ApprovalState now writes through Write-StateFile.
• Docs and changelog:
  • approvals.md states the writer rule once, and the other docs cite it.
  • remediation-policy.md points to setup apply approve= / revoke= instead of telling users to hand-edit.
  • setup SKILL.md documents unmute=<key> and revoke=<remediation>.
  • Changelog fragment (minor).

Consumers found by grep: setup apply was the only writer named outside the scripts, and it now goes through Set-ApprovalState. Other writes in the touched scripts:

File	Writes
Invoke-MachineHealthCheck.ps1	remediation log, latest.json, history.jsonl, the report
Write-StateFile.ps1	the temp file and the replace

None of the scripts writes approvals.json or mutes.json other than through Write-StateFile.

Verification

• run-tests.ps1 exits 0: 677 passed, 0 failed, 1 skipped.
• Each filtered suite exits 0: Write-StateFile, Set-ApprovalState, Get-ApprovalState, Enter-RunLock, Assert-StateBaseLocation, Invoke-MachineHealthCheck and Write-MachineHealthLog.
• The sharing cases hold on this host:
  • replace succeeds against a ReadWrite|Delete reader;
  • replace fails with a sharing violation against a Read or ReadWrite reader.
• Mutation checks:
  • removing the lock dispose from the finally fails the throw test;
  • retries 5 → 4 fails the writer suite.
• Synced-state runtime check: the run had $env:OneDrive set to a folder holding a -StateBase seeded with TODO.md. The folder listing was identical before and after, the run exited 0, and the report lists one state-location UNKNOWN.
• Host smoke (-DryRun) exits 0.
• approvals.schema.json is byte-identical to main.
• Reviews at the phase boundary:
  • Concurrency: 1 CRITICAL (run.lock not released on a throw), fixed.
  • Code-design: 4 CRITICAL and 5 IMPORTANT, all fixed. They covered the hard-coded remediation id list, a duplicate key pattern, a partial RunSnapshot on the lock exit, a shadowed log writer, the writer rule stated four ways, and stale hand-edit docs.
• A fresh-context phase verifier passed 11 of 11 criteria at e49498f.

Related

Follow-up: the description in approvals.schema.json still invites hand edits. This PR keeps that schema byte-identical, so the line will be corrected in the planned follow-up PR that edits that schema (service_exclusions wiring, noise_allowlist removal).

🤖 Generated with Claude Code

[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-10-21T20:10:57.589335Z	e49498f	PR opened

ℹ️ 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.

[github-actions]

claude-security-review reviewed 2ad5a03792ac98f18c07c47a9a5377e680ffb1ae: 0 finding(s), 0 as inline comments.

No security findings in the incremental diff (e49498f → 2ad5a03).

I read .claude-lane/incremental.diff, which covers the three files changed since the last review.

• Invoke-MachineHealthCheck.ps1: The script now saves the two Write-MachineHealthLog:* entries in $PSDefaultParameterValues before the run. A finally block restores or removes them afterward. This only affects session-local logging defaults. It adds no new input, path, command or privilege surface, and it makes the run's state cleaner. The comment edit has no effect on behavior.
• Invoke-MachineHealthCheck.Tests.ps1: A new Pester test checks that no log default is left behind. It writes only to a temp root and restores the caller's defaults in a finally.
• setup/SKILL.md: The argument-hint and the "Arguments" line were shortened. The full set of arguments (deprecate=, demote=) is still listed in the body, so no instruction was removed. No guardrail is deleted, narrowed or softened, so the instruction-surface lens finds nothing.

[github-actions]

claude-review reviewed 2ad5a03792ac98f18c07c47a9a5377e680ffb1ae: 0 finding(s), 0 as inline comments.

Incremental review of 2ad5a03 (since e49498f)

No findings. I reviewed the three changed files by reading .claude-lane/incremental.diff and the surrounding orchestrator code. I did not run the tests or the script. The PR's verification claims, such as the 677 passed tests, are author-claimed and I have not confirmed them.

Invoke-MachineHealthCheck.ps1

• The save of Write-MachineHealthLog:LogPath and :ToStderr sits before the try and after the early run-lock exit. Nothing sets the defaults before that point, so the snapshot is taken at the right time.
• The restore is in the existing finally, so it runs on normal completion, on a throw and on the exit 2 inside the try. It writes back the caller's value when there was one and removes the key otherwise.
• $PSDefaultParameterValues[$k] = … mutates the caller's shared hashtable, so restoring the keys cleans up the caller's session as the comment says.
• The reworded comment about the one-time checkbox migration matches what Get-ApprovalState does according to the PR description.

Invoke-MachineHealthCheck.Tests.ps1

• The new test runs a synced-state run and then a writable run in one session. It asserts that ToStderr is gone after the first run and that the second run writes exactly one run log containing state_writable=True.
• It would fail without the restore, because the leftover ToStderr default would divert the second run's log lines to stderr.
• The test snapshots and restores $PSDefaultParameterValues in its own finally, so it does not leak state into other tests.

setup/SKILL.md

• The shortened argument-hint and the added full-set line in Arguments agree with the frontmatter description. Nothing there contradicts the skill's behaviour.

PR context

• The PR body states No related issue:, so there is no closing-issue criteria to check.
• The PR context file contains no embedded instructions directed at this lane.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e49498f9fb

ℹ️ 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".