feat(platform_util,recovery_flow): Windows handle-anchored owned-spec restoration (supersedes the DW-309/DW-310 fail-closed pause) by pbean · Pull Request #799 · bmad-code-org/bmad-loop

pbean · 2026-09-17T00:50:26Z

What

Thirteenth slice on top of Wave 5's stack, and the only hand-written one: it replaces the Windows fail-closed pause that S12 (#797) shipped for DW-309/DW-310 with a real handle-anchored restoration path. New module src/bmad_loop/win32_at.py; platform_util.HANDLE_ANCHORED_WRITES joins the POSIX dir_fd arm to it; recovery_flow restores and normalizes attempt-owned specs on Windows instead of pausing for manual adoption.

Why

S12 closed DW-308 … DW-310 by refusing attempt-owned spec restoration on any host without descriptor-relative writes — correct, and on Windows it meant every rollback that has to put a bound spec back stops with "ACTION REQUIRED — attempt-owned spec needs manual recovery" (seven test_engine.py rows were skipif'd there rather than fixed). The user decided that Windows should get the same guarantee rather than a pause. CPython exposes the *at() family only where the C library has it, but the NT kernel has the same primitive: every open takes an OBJECT_ATTRIBUTES.RootDirectory handle the name is resolved against, and a rename or delete is a NtSetInformationFile on the file's own handle with the destination again spelled relative to a handle. The reasons the POSIX arm is race-free apply unchanged once those calls are bound.

How

win32_at.py — ctypes bindings: open_directory(path, follow=True) (a CreateFileW directory handle, follow=False refuses a symlink/junction AT the path with ELOOP); open_at(dir_fd, name, flags) (NtCreateFile relative to the handle, speaking O_RDONLY/O_WRONLY/O_RDWR/O_CREAT/O_EXCL plus AT_NOFOLLOW/AT_DIRECTORY/AT_NONBLOCK; O_TRUNC/O_APPEND refused loudly); stat_at (lstat semantics — a reparse link reports S_IFLNK so S_ISREG/S_ISDIR answer False); replace_at / unlink_at (FileRenameInformationEx / FileDispositionInformationEx with POSIX semantics, the classic classes as the fallback where a volume refuses them). "Do not follow" is FILE_OPEN_REPARSE_POINT plus an attribute check on the opened handle — only the symlink and mount-point tags count, as platform_util._LINK_REPARSE_TAGS already draws the line. Every handle is wrapped into a CRT descriptor (msvcrt.open_osfhandle) so os.fstat/read/fsync/fdopen/samestat/close work unchanged. NTSTATUS → RtlNtStatusToDosError → OSError(winerror), so the subclass is the one Win32 raises; IsADirectoryError/NotADirectoryError are pinned by hand because the Win32 mapping loses them. Importable everywhere; ENOSYS off Windows.
platform_util — HANDLE_ANCHORED_WRITES = DIR_FD_ANCHORED_WRITES or win32_at.AVAILABLE; wrappers open_at/stat_at/replace_at/unlink_at whose POSIX arm is the bare dir_fd syscall (byte-identical behavior) and whose Windows arm is win32_at (replace_at inside _retry_on_sharing_violation, as atomic_replace is). open_dir_confined, _open_exclusive_at, _atomic_write_at, _refuse_unwritable_target_at, _atomic_write_confined and create_exclusive_confined route through them, so the confined spec writers (set_frontmatter_status among them — needed for DW-310's lifecycle normalization) anchor at a handle on Windows too. The confined text writer keeps atomic_write_text's translating newline default on the anchored arm, so a ledger or decisions file still lands CRLF on Windows — pinned by a new row whose ablation reddens there. Callers that need the rest of the POSIX family (scandir(fd), symlink with dir_fd, directory fsync: artifact publication, journal, sweep, TUI launch, verify snapshots) stay on DIR_FD_ANCHORED_WRITES, untouched.
recovery_flow — both refusals gate on HANDLE_ANCHORED_WRITES ("cannot be verified without handle-anchored writes"; the journaled problem reads "lacks handle-anchored writes"); target_stat_at/read_target_at/verify_published_inode use the wrappers. Only a host with neither arm still pauses.
Docs — the docs/FEATURES.md attempt-owned-spec and confined-rewrite paragraphs describe both arms. deferred-work.md is not hand-edited: DW-309/DW-310's recorded decisions ("fail closed without descriptors") are superseded by this slice for Windows, and the ledger is the orchestrator's to write.

Tests

tests/test_win32_at.py (new, skipped off Windows): every primitive; junction refusal with ELOOP and following without the flag; O_EXCL; read-open of a directory allowed / write-open EISDIR; the read-only-attribute PermissionError the writable probe relies on; stat_at S_IFLNK for a junction; replace_at keeping the writer's own handle as the published inode, across directories, over a target another share-delete handle holds open (POSIX rename semantics), and the WinError 5/32 sharing violation the retry exists for; unlink_at refusing a directory and removing a name while the writer's handle is open (POSIX delete); the OS refusing to rename a directory with a handle open beneath it; the anchor following a renamed directory while a junction planted at the old name is never consulted. One ENOSYS row runs on POSIX.
test_recovery_flow.py / test_engine.py — requires_descriptor_restoration and the ~30 "dir-fd anchoring is POSIX-only" gates flip to HANDLE_ANCHORED_WRITES, so the seven engine rows and the recovery rows run on the Windows legs; test_owned_spec_restore_native_windows_refuses_before_path_writer becomes …_anchors_at_a_handle (asserts the anchored write landed and the generic confined PATH writer was never reached — what the refusal existed to guarantee). Rows that rename a directory out from under the open writer are POSIX-only by the OS's own rule (Windows refuses that rename with ERROR_ACCESS_DENIED; posix_parent_swap_under_writer, pinned from the other side in test_win32_at.py). Directory redirects are planted as junctions on Windows (_plant_directory_redirect), fifo/socket/file-symlink entry types skip there and a junction parameter joins the final-entry substitution row.
test_platform_util.py — an ANCHORED marker runs the platform-neutral open_dir_confined / *_at / confined-callback rows on both arms (_dir_fd opens through win32_at on Windows); EBADF asserted by errno ("Invalid handle" on Windows); the confined text writer's platform line ending pinned. Forced-fallback rows everywhere patch HANDLE_ANCHORED_WRITES beside DIR_FD_ANCHORED_WRITES; _refuse_link joins the portability guard's refusal-helper inventory.
The Windows arm was driven under Wine before CI (relative opens, renames, link refusal, error subclasses, stat_at); Wine diverges from Windows on delete-on-close, directory renames with open handles and a handle-relative rename after the anchored directory moved, so those rows are settled by the Windows CI legs.

Testing

At this tip: uv run pytest -q -n 4 (10,245 passed, 86 skipped), uv run pyright (0 errors, 0 warnings), trunk check --all --no-fix (263 files, no issues), uv run --no-project python scripts/release.py check and uv build all clean. Every one of the 13 tips of the rebased stack was re-verified the same way (round 6). Negative assertions were ablated before being trusted.

Changelog

One Added entry under ## [Unreleased] (trailing docs(changelog): wave5 S13 entries commit). The S12 entries for DW-308 … DW-310 stand: the no-descriptor refusal still exists for a host with neither arm.

Notes

Merge with S12. S12 alone ships the fail-closed Windows pause; this slice is what the user asked for instead. gh stack merge is bottom-up, so S12 lands first and this immediately after — do not stop between them.
Two commits, both hand-written: the feature and its changelog entry. Net: 10 files, +1,293/−141, of which win32_at.py +541 and test_win32_at.py +425.

Stack (merge bottom-up): S01 → … → S12 → S13 (this). Base is S12 (pbean/wave5-12-migration-ledger-byte-binding), not main. Stacked on #797.

Summary by CodeRabbit

New Features
- Added Windows support for safely restoring and normalizing attempt-owned specifications.
- Extended safe, handle-anchored file operations to policy and related artifact updates.
- Prevented writes through symlinks and junctions on supported platforms.
Bug Fixes
- Recovery now proceeds on supported Windows and POSIX systems.
- Unsupported platforms pause before staging or normalization and report that handle-anchored writes are unavailable.
- Preserved platform-appropriate line endings during confined text updates.

coderabbitai · 2026-09-17T00:50:34Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7e7c2be1-ae5e-4bd0-94bb-36734c8509cd

📥 Commits

Reviewing files that changed from the base of the PR and between b592123 and 1e139fd.

📒 Files selected for processing (1)

tests/test_portability_guard.py

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Walkthrough

Changes

Anchored write support

Layer / File(s)	Summary
Windows NT filesystem operations `src/bmad_loop/win32_at.py`, `tests/test_win32_at.py`, `tests/test_portability_guard.py`	Adds Windows handle-relative open, stat, replace, and unlink operations. The implementation refuses symlinks and junctions when requested and provides non-Windows `ENOSYS` behavior.
Cross-platform anchored write dispatch `src/bmad_loop/platform_util.py`, `tests/conftest.py`, `tests/test_platform_util.py`, `tests/test_artifact_publication.py`	Adds `HANDLE_ANCHORED_WRITES` and dispatches confined walks, atomic writes, replacement, cleanup, and exclusive creation through POSIX or Windows primitives.
Attempt-owned spec recovery `src/bmad_loop/recovery_flow.py`, `tests/test_recovery_flow.py`, `docs/FEATURES.md`, `CHANGELOG.md`	Recovery uses handle-anchored capability checks and platform shims for inspection, verification, and restoration. Hosts without either anchored-write arm pause before the write.
Cross-platform validation seams `tests/test_engine.py`, `tests/test_operatoractions.py`, `tests/test_runs.py`	Tests use shared publication-rename hooks, updated capability markers, forced fallback paths, and concurrent publication coverage.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant RecoveryFlow
  participant PlatformUtil
  participant Win32At
  participant FileSystem
  RecoveryFlow->>PlatformUtil: request anchored spec restoration
  PlatformUtil->>Win32At: dispatch Windows handle-relative operations
  Win32At->>FileSystem: inspect, stage, and publish within directory handle
  FileSystem-->>RecoveryFlow: return verified restoration result

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.03% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 156 functions across 11 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: Windows handle-anchored restoration for attempt-owned specifications, replacing the previous fail-closed pause.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.}

_{Comment @coderabbitai help to get the list of available commands.}

…open Windows CI (#799, both legs): 283 rows failed with PermissionError [WinError 5] out of _refuse_unwritable_target_at → open_at(O_WRONLY | AT_NOFOLLOW) → _refuse_link. FILE_GENERIC_WRITE carries WRITE_ATTRIBUTES only, so the write-only open succeeded and GetFileInformationByHandleEx (FileAttributeTagInfo) was then denied on that handle. CreateFileW adds FILE_READ_ATTRIBUTES to every open implicitly; do the same here. Wine granted the read regardless, which is why the harness never showed it.

Three rows inject a fault at `os.replace` (the floor every POSIX publish reaches, path-based or dir_fd-relative): the two no-tmp-residue rows in test_operatoractions and the interleaved-writer row in test_runs. With the Windows confined writer now handle-anchored, its publish is `win32_at.replace_at` (NtSetInformationFile), so that patch fired on nothing there — DID NOT RAISE / `[] == ['b']` on both Windows legs of #799. `conftest.patch_publish_rename` patches `os.replace` and, on the arm where it is live, `win32_at.replace_at` with the argument order adapted; `real_publish_rename` is the matching fall-through, bound to the originals at import. Linux behaviour is unchanged (same `os.replace` patch). Ablations: dropping either arm's patch reddens the rows on that arm (Linux; Wine).