fix: make schema-sync directory renames EXDEV-safe for docker builds#2357
fix: make schema-sync directory renames EXDEV-safe for docker builds#2357mikemeier wants to merge 1 commit into
Conversation
During docker build /app is overlayfs, and renameSync on a directory that came from a lower image layer (COPY . .) fails with EXDEV even though src and dest are on the same mount. This broke sync-schemas (and thus schemas:ensure / build:lib) in any image build without a pre-populated schema cache, misreported as 'AdCP not reachable' by the fallback path. Fall back to copy+delete on EXDEV and include the original error in the GitHub-dist fallback warning so extraction failures are no longer misreported as network failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
|
Reviewed — the fix itself is solid, approving on the code. One thing before merge: please add a patch changeset. You'll need a changes though - Even though scripts/ isn’t in package.json files, sync-schemas runs inside build:lib and produces published artifacts (skills/, compliance/cache/), and a broken container build is release-relevant — so this needs a changeset (and the Changeset Check job will flag it anyway). npm run changeset, pick patch. On the code, confirmed the diagnosis holds up: Existing mitigation at sync-schemas.ts:370 (temp workdir inside the repo) covers the cross-mount EXDEV case but not the overlayfs lower-layer case — so this fix is complementary, not redundant. |
|
Thanks @andybevan-scope3 — agreed on the rationale. I wasn't able to push directly to the fork branch (session scope is limited to the upstream repo), but I've staged the changeset at Or add the file manually:
---
"@adcp/sdk": patch
---
Fixed `sync-schemas` crashing with EXDEV inside docker builds. Directory renames now fall back to copy+delete when `renameSync` fails on overlayfs lower-layer directories. Also corrects the misleading "was not reachable" fallback warning to include the original error.Once that's pushed the Changeset Check should go green and the PR will be ready to merge. Generated by Claude Code |
Problem
npm run sync-schemas(and thereforeschemas:ensure/build:lib) fails inside anydocker buildwhere the schema cache is not pre-populated.During an image build, the working tree is overlayfs.
renameSyncon a directory that came from a lower image layer (e.g. viaCOPY . .) fails withEXDEV: cross-device link not permittedeven though src and dest are on the same mount — overlayfs cannot rename lower-layer directories. This hitscopySkillTreewhen it snapshots an existing skill dir (skills/adcp-brand->skills/adcp-brand.previous) and can equally hitreplaceTree.The failure is then misreported: the catch in
sync()swallows the error and printsAdCP <version> was not reachable from adcontextprotocol.org, and the GitHub-dist fallback 404s (nodist/onmain), so the build dies with a misleading network error. Bind mounts and normal checkouts are unaffected, which makes this look flaky/environment-specific.Fix
moveTreeSync: tryrenameSync, and onEXDEVfall back tocpSync(recursive)+rmSync— overlayfs handles copy+delete fine. Used at all three directory-rename sites (replaceTreex2,copySkillTree).Notes
scripts/-only change (dev tooling, not published infiles), so no changeset per CONTRIBUTING policy — happy to add one if you want it anyway.docker builda context containing the repo, runnpm run sync-schemaswith an emptyschemas/cache/. Verified fixed in node:22-alpine builds.🤖 Generated with Claude Code