Split out of #11, which tracks this together with the Windows timing drift. The drift half is a test-budget problem; this one is a product bug and stands on its own.
What happens
Discovery.pull refreshes a skill whose version changed by downloading into a staging directory and then swapping: root → backup, staging → root, then delete backup. On Windows the first rename is refused while anything still holds a handle inside the cached directory, and the whole refresh is abandoned: the downloaded version is deleted with the staging directory and the stale one stays in place. The only trace is a log line, and pull returns the stale directory, so the caller — and the person — go on using the old skill believing it current.
Observed on unit (windows) at 968ba5e490 (job):
ERROR failed to refresh skill { skill: "mutable",
reason: { module: "FileSystem", method: "rename",
cause: EPERM: operation not permitted, rename
'…\cache\opencode\skills\mutable' -> '…\cache\opencode\skills\mutable.old-4ab5a758…' } }
(fail) Discovery.pull > refreshes a remote skill when its version changes
Expected: "# New"
Received: "# Old"
That is the failing assertion #11 has been quoting since it was filed; what is new is the cause, visible because #13 made a failing test replay the console output it had been swallowing.
Why it is the swap and not the download
The download succeeded — this is not one of the two paths that abandon a refresh on a missing file. rename was refused, and on Windows that refusal is transient: whatever was reading inside the directory, or the scanner that followed it in, lets go in a moment. Nothing retried it.
Reproduced locally against the real syscall
Not by injecting an error: on Linux, making the cached directory immutable (chattr +i) makes the kernel refuse to rename it with the same EPERM, while still allowing the sibling staging directory to be created, so only the swap is affected. Clearing the flag after 400ms and pulling a new version:
|
result |
| as it is today |
# Old — the refresh is discarded, exactly the CI symptom |
| with the swap retried |
# New |
A real EPERM from fs.rename arrives as reason._tag: "Unknown" carrying reason.cause.code: "EPERM" — @effect/platform-node-shared gives EACCES and EBUSY reasons of their own and leaves everything else Unknown, so the cause is what tells a held directory apart from a genuinely unknown failure.
Fix
Retry the two renames on the codes that mean the directory is held, bounded in time so a refusal that will not pass still gives up and leaves the cached copy — which is the right fallback: a stale skill is worth more than none.
Note what this does not fix, and is worth a separate decision: a refresh that fails for good is still silent to the caller. pull returns the stale directory and only a log says otherwise.
Split out of #11, which tracks this together with the Windows timing drift. The drift half is a test-budget problem; this one is a product bug and stands on its own.
What happens
Discovery.pullrefreshes a skill whose version changed by downloading into a staging directory and then swapping:root→backup,staging→root, then deletebackup. On Windows the first rename is refused while anything still holds a handle inside the cached directory, and the whole refresh is abandoned: the downloaded version is deleted with the staging directory and the stale one stays in place. The only trace is a log line, andpullreturns the stale directory, so the caller — and the person — go on using the old skill believing it current.Observed on
unit (windows)at968ba5e490(job):That is the failing assertion #11 has been quoting since it was filed; what is new is the cause, visible because #13 made a failing test replay the console output it had been swallowing.
Why it is the swap and not the download
The download succeeded — this is not one of the two paths that abandon a refresh on a missing file.
renamewas refused, and on Windows that refusal is transient: whatever was reading inside the directory, or the scanner that followed it in, lets go in a moment. Nothing retried it.Reproduced locally against the real syscall
Not by injecting an error: on Linux, making the cached directory immutable (
chattr +i) makes the kernel refuse to rename it with the sameEPERM, while still allowing the sibling staging directory to be created, so only the swap is affected. Clearing the flag after 400ms and pulling a new version:# Old— the refresh is discarded, exactly the CI symptom# NewA real
EPERMfromfs.renamearrives asreason._tag: "Unknown"carryingreason.cause.code: "EPERM"—@effect/platform-node-sharedgivesEACCESandEBUSYreasons of their own and leaves everything elseUnknown, so the cause is what tells a held directory apart from a genuinely unknown failure.Fix
Retry the two renames on the codes that mean the directory is held, bounded in time so a refusal that will not pass still gives up and leaves the cached copy — which is the right fallback: a stale skill is worth more than none.
Note what this does not fix, and is worth a separate decision: a refresh that fails for good is still silent to the caller.
pullreturns the stale directory and only a log says otherwise.