publishCanonicalLinkNoReplace checked the destination with lstat and
then published, leaving a window the comment claimed did not exist: on
Windows renameSync replaces a concurrent object outright (libuv passes
MOVEFILE_REPLACE_EXISTING), and on POSIX ln -P source target links INTO
a directory that appeared at the target, leaving a stray link inside
foreign data that the rollback list never recorded. Create the symlink
or junction directly at the destination instead — link creation fails
with EEXIST when anything occupies the path and never treats the target
as a container, so the publication itself is the atomic no-replace
check. Identity confirmation re-reads the live link before the
publication enters the rollback list. Covered by injected concurrent
creators at the publish instant on POSIX and simulated Windows,
asserting the foreign object and its contents stay completely
unchanged.
The mono and multi set copy publishers checked destination existence
with lstat and then called Node's rename, which replaces the target on
every platform (libuv passes MOVEFILE_REPLACE_EXISTING on Windows). A
file, symlink, or empty directory created between the check and the
rename was silently overwritten, and the identity confirmation could not
recover it because the publication record only proved the staged object
arrived. Claim the destination with mkdir — which fails with EEXIST if
anything occupies the path, so the claim itself is the existence check —
and move the staged children into the claim, restoring the source mode
on it. A failed child move relocates the children back and removes only
the claim. Covered for mono, multi, and simulated Windows, including an
injected concurrent creator at the claim instant.
The no-replace file fallback links the destination and then removes the
source. If the removal fails, the caller treats the publish as failed,
but no publication record exists to roll the new destination back, and
a backup restore would refuse the occupied path. Remove the destination
behind an identity check — only the proven linked object may be deleted
— and report when the retraction itself fails or the destination was
concurrently replaced.
The previous wrapper returned the first observed ID for both paths, so
expected == actual still held on Windows and the proof accepted the
swap. Return a distinct ID for the second probe.
The constant file-ID stub made both IDs equal, so the Windows proof
(expected == actual) accepted the publication and the subtest failed
there; Unix stayed green because its proof ignores the ID strings and
the swapped os.SameFile seam already forced the failure. Return the
first observed ID for both paths so staged and published identities
differ on every platform while real IDs still flow through the wrapper.
The physical same-content swap relied on the recreated destination
getting a fresh inode, but CI runners' ext4/overlayfs recycle inodes
eagerly, so the swap was undetectable on Linux and the subtest failed
there (while passing on macOS). Swap the same-file identity seam instead
so the confirmation's fast-path rejection contract is pinned on every
platform.
The four standalone installers pruned the oldest excess stamp
directories regardless of origin, so a migration retiring more than
five batches destroyed its own rollback material mid-run — the same
data loss already fixed for Go via the run-root registry and present
in install.js/install.ps1 as currentRunBackupRoots. Every installer now
records the stamp directories it creates and pruning only removes
earlier-run batches, which is what the changelog already promises.
The degraded directory publication claimed the destination with mkdir, then
— on platforms whose rename refuses to replace a directory (macOS refuses
even an empty target, verified empirically) — removed the claim and retried
a plain rename. Between the unlink and the retry a foreign directory could
appear at the destination and be silently overwritten, breaking the
no-replace contract the fallback exists to provide.
Hold the claim for the whole transaction instead: rename over the claim
where the platform permits it (Linux), otherwise move the source children
into the claim one by one. The destination is never unlinked, so a
concurrent creator can only ever lose the mkdir race; every child rename
targets a nonexistent path inside the empty claim, and a failed move
restores the children and removes only the claim.
The child move legitimately changes the publication's identity, which the
confirmation now handles: a rename that consumed the staged path is still
proven by identity, while a child move is proven by the pre-rename content
fingerprint. The emptied source shell doubles as the signal distinguishing
the two shapes; moveSkillPathRecoverably removes it to keep move semantics.
The backup stamp has second precision and pruning kept only the newest 5
stamps, so a canonical migration that retires copies across many Agent
roots deleted its own earlier backups mid-run. That silently voided the
reversibility guarantee the transaction depends on for rollback: a probe
retiring 8 paths lost 3 of them permanently.
Record every stamp directory this process creates, keyed by normalized
absolute path, and prune only the oldest foreign stamps.
Creating the staged symlink usually succeeds, so the link strategy really
fails at publish time: renameSkillPathNoReplace has no atomic no-clobber
primitive for a symlink source and refuses it whenever the kernel flag is
unavailable (NFS, FUSE, overlayfs). Gating the copy fallback on staging
alone therefore left every non-universal Agent unconfigured on exactly the
filesystems the fallback exists to support.
Retry the whole target transaction as a direct copy after a failure in any
phase, but only when the failed attempt fully restored the originals. The
converter also re-adds the replacement backups the link plan deliberately
skips for destinations already pointing at canonical, which a copy must
replace and no-replace publication would otherwise reject with EEXIST.
skillPathSameFileIdentityImpl on Windows always returns false and is
never reached through skillPathIdentityProven (which uses file IDs
exclusively). Add a direct seam call with synthetic os.FileInfo to
exercise the Windows return-false path and the Unix os.SameFile path
with nil Sys().
Restructure skillPathFileIdentityImpl to use nested if-err-nil with a
named return and skillPathIdentityProven to use a single expression.
Error conditions now fall through to the bare return instead of
occupying separate coverage blocks, eliminating 5 uncovered statements
that the Windows coverage gate flagged at 99.4193%.
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).
A universal Agent whose obsolete private copy cannot be retired installs
nothing there, yet every entry point counted that retirement failure as an
install failure — aborting `npm install`, `dws skill setup`, and the shell
installers even when the canonical store and all links published correctly,
and skipping the skills-state write. Route retirement failures to a separate
warning path across all surfaces (Go upgrade + skill setup, npm, PowerShell,
install.sh, install-skills.sh, install-event.sh, install-devapp.sh).
Also:
- Add a checked-rename fallback for filesystems that reject the atomic
no-replace flag (NFS, FUSE, overlayfs); the no-clobber contract is kept and
the previously unsupported platforms build and work.
- PowerShell multi-mode links only bundle skills, never the shared canonical
store, so third-party/user skills are no longer fanned into every Agent root.
- Prune ~/.dws/skill-backups to the newest 5 on every surface; encode
HOME-relative backup names on PowerShell to preserve origin.
- Add simulated-win32 junction coverage and rewrite the tautological
no-replace test; remove dead code whose tests gave false coverage.
- Soften the overstated Windows ownership-proof comment (NTFS tunneling).
- extend the silent-rollback contract to install-event.sh
- static contract: Restore-MultiSkillSet removes published paths lexically
(section-scoped so identity-anchor refactors keep the guarantee) and link
staging dirs are cleaned via Remove-LinkStageRoot / Remove-DevLinkStageRoot
- install-event.sh integration test: an uninstallable agent target is
skipped loudly while later agents still receive links
- pwsh probe: Test-SamePhysicalSkillRoot must dereference junctions and
symlinks (junction idempotency asserted where junctions are creatable)
- install.ps1: remove published junctions lexically in Restore-MultiSkillSet
(Windows PowerShell 5.1 follows reparse points during Remove-Item -Recurse
and could delete canonical store contents); clean link staging dirs
lexically in Publish-CanonicalSkillLinks and Move-SkillPathRecoverably
- install.ps1: Test-SamePhysicalSkillRoot now dereferences junctions via
Get-PhysicalSkillPath (mirrors EvalSymlinks/realpathSync/cd -P), so reruns
recognize already-published junctions instead of backup churn
- install-event.sh: replace silent 'mv ... 2>/dev/null || true' rollback with
the loud backup-retained failure contract already enforced for devapp
- event/devapp sh+ps1: link→copy fallback and per-agent failures now degrade
per agent like install.sh (skip loudly, continue, report at the end)
instead of aborting mid-loop or swallowing errors
- tests: junction-lexical removal contract, event per-agent degrade
integration test, pwsh junction physical-root recognition + rerun
idempotency (no backup churn)
A failed canonical publish only failed the upgrade when
hasDependentSkillRoot reported a non-universal link target; that helper
explicitly skipped universal agents, which are exactly the direct consumers
of ~/.agents/skills. On a universal-only machine (e.g. only Codex
installed), UpgradeSkillLocations* returned a nil error with nothing
installed, contradicting the documented "canonical publication is
mandatory and fails the upgrade loudly" contract.
Canonical publish failures now return an error unconditionally in both the
mono and multi branches, and hasDependentSkillRoot is removed. The test
that pinned the old standalone-does-not-fail-fast behavior now asserts
error propagation in both modes.
玉澜