A stamp-shaped directory name is not ownership proof: pruneSkillBackups
counted and RemoveAll'd any 20260819-120000-shaped entry under
~/.dws/skill-backups, so a user or tool that created such a directory
lost its contents once DWS held five backups, and the Go backup path
(MkdirAll) adopted a same-named foreign root outright. The PowerShell
installers already implemented the correct contract; every other
surface now matches it.
Go stamps a freshly created root with the exact marker bytes the
install scripts write (.dws-skill-backup = "dws skill backup v1")
before any payload moves in, claims the root with mkdir so an existing
unproven root bumps to a collision suffix instead of being adopted,
and prunes only roots whose marker verifies — unmarked or wrongly
worded stamp-shaped directories are foreign data, preserved and never
counted against the keep limit. The shell installers (install.sh,
install-skills.sh, install-event.sh, install-devapp.sh) and the npm
installer apply the same rule in their backup collision loops, with
roots recorded as created by the running process exempt from marker
re-verification so a mid-run marker permission failure still reuses
this run's own root and keeps the sibling payload intact.
Regression tests cover every surface: pruning an unmarked/wrongly
marked stamp-shaped directory alongside marked ones, refusing to adopt
a foreign root (payload moves to a suffixed root, foreign data and its
nonexistent marker untouched), same-stamp reuse of a proven root, and
marker-write failure cleaning the empty fresh root.
copy_tree published staged children with mv, which replaces a
concurrently created same-name directory (POSIX rename succeeds over an
empty target) and whose rollback moved every dest child back — including
a concurrent writer's different-named entries — before deleting the
staging tree. Children now publish through kernel-level no-clobber
primitives (mkdir claim + recursion for directories with the recorded
mode restored, ln for regular files, ln -s for symlinks), a manifest
records exactly what this transaction published, the rollback retracts
only those entries in reverse order, and each level re-counts the
destination so a foreign different-named entry aborts the publish with
the destination retained. Read-only staged directories (0555 skill
trees) are made owner-writable for the move; the backup restore uses the
same discipline so a concurrent writer is refused without partially
draining the backup.
Regression tests cover both scripts: a concurrently created same-name
empty child directory and a different-named foreign entry mid-publish
are retained with the original backup kept; both fail against the
previous mv-based implementation.
Linux overlayfs recycles device+inode, so SameFile and a lone inode
token treated a replacement as owned and retracted it. Stamp staged
inodes with an xattr mark, prove Linux/Darwin identity with birth
time, and make shell copied-set rollback check dest first with inode
plus child names.
Record dest on occupy and retract it when confirmation, verify, or
staging cleanup fails. Restore unmatched quarantine with a no-replace
publish. Event/devapp copy uses mkdir-claim; shell rollback claims dest
before delete. Release copy now says npm/PowerShell create junctions and
Go uses os.Symlink, with copy fallback when linking is unavailable.
The shell mono/multi set publishers staged each Skill directory and
published it with a plain mv after the backup; anything another process
created at the destination between the backup and the move was silently
replaced, and restore_multi_skill_set then blind-deleted manifest paths,
so a concurrently replaced object could also be destroyed during
rollback. Publish through an atomic mkdir claim instead — EEXIST refuses
any occupant, staged children move into the claim one by one, and a
failed child move relocates them and removes only the claim. The
published manifest now records <dest>:<inode>, and rollback deletes a
destination only when its inode still matches the publication, skipping
concurrently replaced paths with a warning. Also fixes a latent
unbound-variable expansion where a shell variable was followed directly
by a full-width parenthesis in a message. Regression tests publish a
first Skill, replace it with a foreign directory, fail the second
publication, and assert rollback retains the foreign object untouched
while restoring the rest from backups.
The POSIX shell installers staged shared Skill links and published them
with mv after an existence check; a file or symlink another process
created at the destination between the check and the move was silently
replaced, and the inode confirmation could not detect the loss. Publish
by creating each link directly at its destination instead — symlink(2)
refuses an occupied path with EEXIST, so the creation itself is the
atomic no-replace check. A directory that appears at the destination
turns ln -s into a container; the nested link is removed after an
identity check and the transaction rolls back, leaving the foreign
directory untouched. Applied to install.sh, install-skills.sh,
install-event.sh, and install-devapp.sh. Also covers the remaining
retraction branches of the Go shell-removal fallback so changed-code
coverage is complete. Regression tests inject a concurrent occupant at
the publish instant for regular-file and directory cases and assert the
foreign object and its contents stay completely unchanged.
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.
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).
- 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)
The five internal/app partitions ran end to end inside one job, so the app
shard's wall clock was the sum of all five: 780s in CI, of which the schema
partition owned 357s. Each partition is now its own matrix shard, so they run
concurrently and the shard's wall clock is set by its slowest partition rather
than by their total. Every partition shard still selects the same single
internal/app package, so the impacted-package query maps the shard name back to
app and the partition only chooses which tests run.
The helper gains a partition argument and a list-partitions mode. APP_PARTITIONS
is the single source of truth for the set, and the discovery pass still runs in
every job, so each one independently verifies that the partition patterns cover
every top-level test exactly once before running the one it was asked for.
Two fail-closed checks guard the split, because the helper's own coverage check
can no longer prove the whole package ran once the partitions are separate jobs:
- The helper cross-checks APP_PARTITIONS against the coverage counters in both
directions, so a counted partition that nothing dispatches and a dispatchable
partition with no counter both fail instead of silently skipping tests.
- TestCIAppRacePartitionMatrixMatchesHelper pins the workflow's app-<partition>
shards to list-partitions output in both directions, so a partition cannot
lose its job while every job stays green.
The discovery loop variable is renamed from partition to spec: it would
otherwise shadow the partition requested on the command line, which run mode
reads after the discovery pass completes.
The schema partition's 52 tests assert structural Schema-to-Cobra contracts over
a single goroutine: none of them call t.Parallel or start a goroutine, so the
race detector has no concurrent access to observe there. The process-global lazy
metadata that does need race coverage (schema_source_root's atomic.Value, the
parameter-binding lazy loaders) is exercised by internal/cli's concurrent tests,
which stay instrumented.
The instrumentation was not free here. The partition shares a single sync.Once
Catalog build whose work is allocation-heavy, and -race made it roughly 11x
slower: 26s -> 291s locally, and 357s of the app shard's 780s in CI. Within that
partition TestFinalSchemaToolsHaveExecutableBaseCommands alone accounted for
262s, not because the test is expensive but because it is the first caller to pay
for the shared snapshot; its 1121 subtests together measure 0.00s.
run_partition now takes the instrumentation mode explicitly and fails closed on
an unrecognized value, so a typo cannot silently drop -race from a partition that
is supposed to carry it.
The focused path tested every impacted package in a single job with a
plain `go test -race`, so internal/app ran inside one long-lived process
alongside all of its reverse dependencies. That is exactly the shape
scripts/ci/run-app-race-tests.sh exists to avoid: a single app test
process retains every constructed command tree in framework registries,
so the run grows to 900s and the job stays alive long enough to be
reclaimed by the runner. Recent focused runs failed with SIGTERM after
9-10 minutes without a single test failure, and one earlier run failed
at `internal/app 902.651s`, 2.65s past the package timeout.
Fan the same package plan across the shard matrix test-race already
uses, and run each shard the way test-race runs it: internal/app through
the process-isolating helper, cli/smoke with their wider package budget,
release-scripts without race and with archive tooling.
changed-test-packages.sh gains `list-shard`, which intersects the
impacted set with scripts/ci/test-packages.sh shard membership so shard
definitions stay single-sourced — and so an unknown shard name aborts
there rather than reporting an empty selection, which would let a
mistyped shard skip every test while reporting success.
release-scripts is in the matrix on purpose: its dedicated job only runs
at full-suite or release-sensitive scope, so omitting it here would stop
testing test/scripts changes altogether. A test pins that the shard
selections partition the impacted set exactly, so shard-plan drift
cannot silently shrink focused coverage.
chichuan