Add FILE_FLAG_OPEN_REPARSE_POINT to the Windows CreateFile call in
skillPathFileIdentityImpl so symlinks are opened as reparse points
rather than followed to their target. Staged symlinks carry relative
targets computed for the final destination, which may not resolve from
the staging directory; following them caused CreateFile to fail,
yielding an empty file ID that rejected publication and broke canonical
skill layout migration on Windows.
Make skillPathSameFileIdentity a seam variable so the tunneled
replacement test can deterministically simulate the identity change on
Unix. On tmpfs (used by Linux CI runners), os.SameFile can return true
for a recreated file due to inode reuse, making the test flaky. On
Windows the swap is a no-op because skillPathIdentityProven compares
file IDs from GetFileInformationByHandle and ignores
skillPathSameFileIdentity.
NTFS file tunneling can restore the original creation time for a
recreated same-named object, which defeated the creation-time
incarnation check and allowed rollback to delete a concurrent
replacement. Replace the platform-specific identity pair with a single
skillPathIdentityProven function:
- Unix: delegates to os.SameFile (inode/dev), ignoring file ID strings
- Windows: compares VolumeSerialNumber:FileIndexHigh:FileIndexLow from
GetFileInformationByHandle, which uniquely identifies the file on the
volume for its lifetime and is unaffected by tunneling
When the file ID cannot be obtained at publish time, identity is not
proven and the auto-delete is refused. Add a regression test that
simulates tunneled creation time and verifies rollback still refuses
the concurrent replacement.
On Windows isNoReplaceRenameUnsupported always returns false, so the
fallback is never entered from the invalid-path test. Force the fallback
and swap skillPathLink to a non-EEXIST error to cover line 94 on all
platforms.
Add tests for mkdir non-EEXIST error, remove failure after rename
failure, first-rename-succeeds path (Linux behavior), retry-rename
path, and non-regular source safe-fail. All 24 changed executable
statements now covered on both macOS and Windows.
The fallback path for filesystems without RENAME_NOREPLACE/EXCL (NFS,
FUSE, overlayfs) used Lstat-then-Rename, which could overwrite a
concurrently created destination between the check and the rename.
Replace the TOCTOU-prone check with truly atomic no-clobber primitives:
- Directories: os.Mkdir atomically claims the destination (fails with
EEXIST if occupied). On Linux rename(2) replaces the empty dir
directly; on Darwin/Windows rename refuses existing dirs so the empty
dir is removed and the rename retried — any concurrent creation
between remove and rename is detected by the second rename failing.
- Files: os.Link atomically fails if the destination exists, then
os.Remove completes the move.
Add concurrent-creation test covering the mkdir→rename race window.
The !os.IsNotExist(statErr) branch in renameSkillPathNoReplace was
uncovered on Windows. Inject errNoReplaceRenameUnsupported for the
atomic rename and os.ErrPermission for skillPathLstat so the stat-error
path is exercised on every platform.
The smoke test creates temp home directories with .config/kimchi markers
for agent detection. On Linux CI runners XDG_CONFIG_HOME may point to the
runner's real config path, causing resolvedAgentTargets to look outside
the temp home. Unset it so detection resolves against the test's temp dir.
Restrict backup pruning to directories whose names match the DWS stamp
format (YYYYmmdd-HHMMSS with optional -N suffix) across all 8 installer
surfaces (Go, npm, 4 shell, 2 PowerShell). Unknown directories in
~/.dws/skill-backups are now preserved. Also fixes Windows coverage test
portability and covers the remaining macOS changed-code gap (retire
warning loop in runUpgrade).
Dennis