* fix(specialist): make overwrites atomic
* fix(specialist): harden atomic replacement
* fix(specialist): replace through a Windows-atomic, DACL-preserving primitive
Addresses both P1 findings on #757.
os.Rename — which fsutil.RenameWithRetry wraps — is documented non-atomic
outside Unix, and the retry loop only handles sharing violations. Worse, the
replacement is a freshly created temporary file, so renaming it over the
destination also publishes the directory's inherited DACL in place of the
destination's own: overwriting a specialist that had been explicitly restricted
inside a group-readable directory would expose its system prompt to that group.
temp.Chmod(0o600) cannot prevent that on Windows, where Go maps only the
owner-write bit.
New fsutil.ReplaceWithRetry publishes through a platform primitive:
- Windows uses ReplaceFileW with REPLACEFILE_WRITE_THROUGH, one operation that
carries the replaced file's security descriptor, attributes, and streams over
to the replacement. REPLACEFILE_IGNORE_ACL_ERRORS is deliberately not passed —
silently losing the descriptor is the failure being fixed — so a merge failure
surfaces and leaves the destination untouched. A missing destination falls back
to a rename (nothing to replace or preserve), as do volumes that cannot perform
a replace at all (FAT/exFAT, some network redirectors), which carry no ACL
either. The transient-lock retry is unchanged.
- Unix keeps rename(2), which is already atomic within one filesystem.
Tests: Windows coverage asserts the destination's creation time survives (a
rename would not, proving the call goes through ReplaceFileW) and that a
protected owner-only DACL survives both at the fsutil layer and through the real
`Create --force` path; a plain rename was verified to replace that DACL with the
directory's inherited ACEs, so the assertion has teeth. POSIX covers both
destination shapes.
* fix(fsutil): stop ignoring ACL merge errors, recover partial ReplaceFileW
Addresses jatmn's two #757 review findings:
- The call passed REPLACEFILE_IGNORE_MERGE_ERRORS (0x2) while the
comment claimed ACL failures were fail-closed. Microsoft documents
0x2 and REPLACEFILE_IGNORE_ACL_ERRORS (0x4) identically: with
either set, a call lacking WRITE_DAC "succeeds but the ACLs are not
preserved". Passing 0x2 therefore let a --force overwrite silently
publish the temp file's inherited directory DACL over a restricted
specialist and expose its system prompt. Pass no flags at all.
- With no backup file, ERROR_UNABLE_TO_MOVE_REPLACEMENT (1176) and
ERROR_UNABLE_TO_MOVE_REPLACEMENT_2 (1177) leave the destination
deleted (1176) or renamed to an unreachable name (1177) while the
replacement survives only under its temporary name. Returning those
unrepaired let the caller's deferred temp-file cleanup remove the
sole surviving copy, so one failed 'specialist create --force'
destroyed both the old manifest and the new one. recoverPartialReplace
now moves the replacement into place and still reports the error,
since the destination's descriptor died with the destination.
Also drops the doc claim that REPLACEFILE_WRITE_THROUGH (0x1) flushes
before returning: Microsoft documents that value as "not supported".
The two fsutil regression tests are verified to fail against the
pre-fix code. The specialist-side test guards the caller half of the
recovery contract and does not fail pre-fix; its comment says so.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* fix(fsutil): preserve Windows replacement backups
Use a managed ReplaceFileW backup so partial failures can restore the original file and DACL. Fail closed when atomic replacement is unsupported, retry rollback and backup cleanup, and retain a known backup if rollback cannot complete.\n\nCover unsupported volumes, 1176/1177 semantics, cleanup, rollback retries, and symlink rejection.
Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-be8e-75eb-b274-b7e22af373a0
Co-authored-by: Pierre Bruno <[email protected]>
* fix(specialist): report committed cleanup warnings
Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8
Co-authored-by: Pierre Bruno <[email protected]>
* docs(specialist): describe what a failed overwrite actually leaves behind
The Windows partial-replace recovery errors told an operator the replacement
"remains at" the caller's temporary path, but the only caller removes that file
in a deferred cleanup before the error ever surfaces — it named a file that was
already gone. Recovery from a failed overwrite is always about the original, so
the errors now describe only what the replace itself left on disk: the state of
the destination and of the managed backup. The replacement path is no longer a
parameter, since nothing reports it.
The one terminal state that needs a human — 1177 whose rollback exhausted its
retries — now says so outright. The original then exists only under an opaque
.zero-replace-*.backup name, and the loader reads *.md, so the specialist stays
invisible until somebody moves it back. The error names both paths and the
action; docs/SPECIALISTS.md walks through it; and loadDirectory records why the
non-.md siblings an interrupted overwrite can leave are skipped rather than
guessed at.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
* fix(specialist): serialize managed loads during replacement
Amp-Thread-ID: https://ampcode.com/threads/T-019faf74-99d4-75cf-ac7a-661c308240ef
Co-authored-by: Amp <[email protected]>
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Amp <[email protected]>
