feat(client): writeSkills materialization and the abuse matrix

[XieX]

Stacked PR 5 of 7 — merge bottom-up into xie/agent-skills-feature-ac9ac7.

1. feat(client): Agent Skills value types #29 — value types
2. feat(client): symlink-refusing filesystem primitives #30 — symlink-refusing filesystem primitives
3. feat(client): Agent Skills store seam, verification, and accessors #31 — store seam, verification, accessors
4. test(client): integrity verification and the accessor telemetry sweep #32 — integrity + telemetry tests
5. feat(client): writeSkills materialization and the abuse matrix #33 — writeSkills + abuse matrix ← you are here
6. test(client): the rest of the writeSkills suite #34 — remaining writeSkills tests
7. docs(client): document Agent Skills #35 — docs

Each PR targets the one below it, so GitHub already shows only this PR's own diff.

---

Fifth of seven, and the heaviest — deliberately. This is the highest-blast-radius layer in the feature: it writes LaunchDarkly-delivered content to a customer's disk.

It lands with the tests that prove its defenses rather than after them, because several of those defenses are the kind a passing functional suite would not notice going missing. That's also why this PR is the largest in the stack; the remaining functional tests are split into the next one.

The API

writeSkills(skills, root, options?) reconciles <root>/<key>/SKILL.md against a .launchdarkly-skills.json manifest and reports every outcome in a ReconcileReport — nothing is silent. It accepts Skill values, SkillReference values, and bare key strings.

⚠️ timeout is in seconds, defaulting to 10, matching the Python signature rather than the usual TypeScript timeoutMs instinct.

Manifest format

Byte-level identical across languages: same filename, same sorted-key ASCII-escaped serialization. A polyglot fleet reconciling one directory has to produce the same bytes from either language, so the filename, the format, and the exported constants are a cross-language breaking change to alter. The manifest is written through the same atomic path as a skill file.

It fails closed, and these checks are not negotiable

• keyRejectionReason and unsafePathReason are shared by the write and the prune paths precisely so the two cannot disagree about which paths this SDK may destroy.
• Keys are re-validated locally against the same anchored pattern, with a tighter path-component bound than the data model's, since no mainstream filesystem accepts a 256-byte component.
• Content is hash-verified again immediately before the write, because a Skill can be constructed by a caller and not just by an accessor. A Skill carries bytes, so this pass hands them straight to the shared verifiedBytes and they are hashed as-is — no encoding happens on the write path. The two-pass design and the integrity signal's property keys are unchanged (and identical to Python's).
• Symlinked roots, directories, and targets are refused.
• A destructive operation is permitted only on a path the manifest lists under a matching key, and a corrupt manifest suppresses every destructive action.
• A target that is not a regular file — a FIFO, a device node — is refused rather than read. See below; this one is a prerequisite rather than a nicety.

Partial reconciles heal themselves (review finding AV-3)

The manifest is written atomically, once, last. A process killed after a skill file lands but before that write leaves the file at a managed path with no manifest entry — exactly the condition clobber protection treats as a customer-placed file. So every later reconcile takes the refusal branch and that skill is permanently wedged until a human intervenes: the rule that protects customer files is what prevents self-healing. Boot-time execution under a 10-second budget makes the crash window realistic.

The fix is to adopt a colliding file when its on-disk sha256 equals the resolved content hash. The read-and-hash already existed in writeOne, gated behind managed; the restructure just runs the hash comparison first. Three details:

• This cannot weaken the clobber guarantee. The only bytes ever adopted are bytes LaunchDarkly resolved, so nothing the customer authored is claimed and nothing is rewritten. Differing unmanaged bytes still fall through to the same refusal, with the same message. The existing clobber tests pass unchanged — which was the check on the design: editing one to make this pass would have been the signal that the guarantee had broken.
• A failed read is never an overwrite. If the read fails, the reconcile refuses. Not knowing what is on disk is the last state in which to overwrite it, and the reason is worded apart from the collision refusal, because "we could not look" and "we looked and the bytes are foreign" call for different operator responses.
• skipped_current is reused rather than a new adopted kind added. Its documented meaning — the bytes on disk already are the resolved content — is exactly this case. Adding a member to the ReconcileActionKind string-literal union would be a breaking change for any consumer with an exhaustive switch, and would drag feat(client): Agent Skills value types #29, six PRs down the stack, into this change.

One caveat, stated plainly. Adoption also creates a manifest entry, so the adopted file becomes prunable on a later reconcile. That is correct and not a weakening: adoption only fires when the bytes are byte-identical to LaunchDarkly-resolved content, so a later prune removes content LaunchDarkly delivered anyway — exactly what would have happened had the crash not occurred. The guarantee that is preserved is the one that matters: differing unmanaged bytes are never overwritten.

The review also floats a write-intent journal as an alternative. It was assessed as over-engineered for the failure it prevents, and is deliberately not built.

Prerequisite: the regular-file read guard

writeOne read the on-disk file with a plain readFile(target). unsafePathReason checks for a symlink, but a FIFO or a device node is neither a symlink nor a directory — and readFile on a FIFO with no writer never returns, hanging the reconcile and the event loop with it. That hazard already existed on the managed path; adoption widens the read to unmanaged, genuinely foreign files, which is what makes fixing it a prerequisite rather than a cleanup.

readRegularFile opens with O_RDONLY | O_NOFOLLOW | O_NONBLOCK, fstats the handle rather than the path, refuses anything that is not a regular file, always closes, and surfaces a failure as a refusal rather than a throw. It mirrors Python's _read_regular_file, including its placement in skills_fs.py rather than in the safe_fs twin. One deliberate difference: no O_BINARY, because Node performs no CRLF translation on a descriptor.

Secondary: orphaned temp files are swept

atomicWrite removes its own temp file on any error it sees — but not after a SIGKILL, and prune walks manifest entries only, so an orphan is invisible to the reconcile forever and blocks the rmdir that would clean up an emptied skill directory.

The sweep runs during the reconcile, ahead of the prune, and is bounded on every axis: only inside <root>/<key>/ for a key that passes keyRejectionReason; only names matching the temp pattern anchored at both ends, derived from tempNamePattern in safe-fs.ts rather than a second copy of the naming rule; only regular files, never following a symlink, and through the same pinned-handle unlinkNoFollow the prune path uses. A failure is a reported error action, never a throw, and never aborts the run. A corrupt manifest suppresses it like every other destructive action.

This is the one change here that touches safe-fs.ts (a file introduced in #30): tempNamePattern is exported next to the generator so the two spellings of the naming rule cannot drift. Deriving it was worth one small addition to an earlier file rather than hardcoding a copy in skills-fs.ts.

Windows reserved device names (review finding DV-2)

The key grammar ^[a-z0-9][a-z0-9-]*$ admits con, prn, aux, nul, com1–com9 and lpt1–lpt9 — 22 names Windows resolves as devices rather than as paths, so none can be a directory name there. They are now rejected in keyRejectionReason, ordered after isValidSkillKey to match the existing discipline in that function.

Deliberately in the filesystem layer, not the shared key grammar. isValidSkillKey and SKILL_KEY_PATTERN in types.ts are untouched, and that is not an oversight:

• parseAiConfig fails closed on a bad skills entry, by design. A grammar-level rejection would therefore invalidate the entire AI Config — model, provider, instructions, tools — for a Linux or macOS customer, over a constraint that only exists on Windows.
• skillRefs would silently shrink, and a dropped reference lets writeSkills prune the skill's on-disk copy. That converts "this skill fails to write on Windows" into "this skill gets deleted on Linux."
• Precedent: the 255-byte path-component bound already lives in this layer for exactly this reason, and types.ts documents it in those words — "no mainstream filesystem allows a 256-byte path component, so writeSkills applies a tighter bound of its own."
• keyRejectionReason is already shared by the write and prune paths, so one edit covers both destructive paths.

Unconditional, not platform-gated. A managed root written by a Linux container and read from a Windows host is an ordinary deployment, so the on-disk result must not depend on the writer's OS. And neither repo has a Windows CI runner — every job in both matrices is ubuntu-latest — so a process.platform === 'win32' branch would be untestable in CI, which is the exact condition that produced this gap.

The honest trade: a customer who legitimately names a skill aux now gets a reported error action where it would previously have worked off Windows.

The set is exactly the 22 reserved names, as a Set. No com0/lpt0 — those are not reserved. No suffix stripping and no case folding, and the code says why: the grammar admits no . and no $, so the con.txt and CONIN$/CONOUT$ forms are unreachable, and keys are lowercase-only already.

Residual, documented rather than implemented: the 255-byte bound is per component and does not bound the total path, so root + key + /SKILL.md can still exceed Windows' 260-character MAX_PATH with a legal 200-character key. The SDK cannot validate that — the root is the customer's — so it is one README sentence, not a check.

Not in scope

Neither change emits the Gap 1 integrity-failure log record that landed in #31: a key rejection and a clobber refusal are ReconcileAction errors, not integrity failures. Gap 2 (a typed outcome distinguishing integrity failure from absence) is a separate public-API decision and is not here.

The abuse matrix that holds those in place

Path traversal, over seventeen hostile keys — parent traversal, absolute paths, backslashes, drive letters, an NTFS alternate data stream, an embedded null byte, uppercase, leading and trailing whitespace, a 257-byte key. Each asserts that no filesystem operation was attempted, not merely that none succeeded: the OS would reject several of these on its own, so a failed write is not evidence of a defense.

Symlink attacks on the root, on a skill directory, and on the target file, plus the swap-race pair. Those two are skipped off SUPPORTS_DIR_FD and written out in full so they become live if Node ever grows the *at() family. The capability probe is itself tested, so a probe that silently reported "unsupported" could not also silently skip the cases that would have caught it.

Clobber protection: a file at a managed path with no manifest entry is never overwritten — unless its bytes already are the resolved content, in which case it is adopted. Adoption, refusal on a one-byte difference, a mixed run that adopts one skill and refuses another, and the follow-on reconcile being an ordinary no-op are all covered.

Crash-mid-reconcile recovery, which the review asks for by name in the abuse matrix: a skill file on disk with no manifest entry and byte-identical content is adopted, reported skipped_current, recorded in the manifest, and the next reconcile is a plain no-op.

Targets that must not be read as ordinary files: a FIFO — with an explicit test timeout, so a regression fails rather than stalls CI — a directory standing where SKILL.md belongs, and an unreadable file, which must refuse rather than overwrite and whose reason must be distinguishable from the collision refusal.

All 22 reserved device names, through both writeSkills and the prune path, plus the assertion that makes the layer choice load-bearing: isValidSkillKey('con') is still true and parseAiConfig still accepts a config referencing it. com0 and lpt0 are not reserved and still write.

Corrupt manifest, over eight variants. Six are unparseable, which makes "performed no destructive action" arithmetic rather than a defense — with no entries to act on there's nothing to prune and clobber protection covers the rest. The two *_live_entries variants are the ones that actually test the rule: corrupt only in manifestVersion, with a valid entries map listing the managed path under a matching key. The implementation has everything it needs to overwrite and to prune, and must refuse anyway.

Also here

The last of the package-root exports and the assertions pinning their exact values — a caller needs MANIFEST_FILENAME to gitignore the manifest and MAX_SKILL_CONTENT_BYTES to pre-check content.

One line from the accessor layer

resolveReference builds Resolution values of its own for the two conditions only this path treats as data — an exhausted deadline and an absent store — so it now states the reason field that #31 made required. Both are store_unavailable: neither is the store answering "no", and reporting either as an absence is what would let a prune delete working files over a non-answer. No behaviour change.

Verification

Client package 452 → 555 tests, +2 skipped (the swap-race pair). typecheck, biome, and sherif clean. The FIFO case skips on Windows and the unreadable-file case skips as root; both run on the CI matrix.

Known coverage gap at this commit: writeSkills's defenses are proven here, but its ordinary behaviour — reconcile semantics, atomicity, resilience — is covered in the next PR. The suite is green either way; this is a deliberate ordering so the non-relaxable checks ship alongside the tests that prove them.

🤖 Generated with Claude Code

---

Note

Overview
Introduces writeSkills to reconcile LaunchDarkly skill content under a managed root (<key>/SKILL.md) using a .launchdarkly-skills.json manifest, returning a ReconcileReport instead of failing silently. The package root now exports writeSkills, manifest/filename constants, and WriteSkillsOptions (prune, timeout in seconds, onUnavailable).

The reconcile is manifest-driven and fail-closed: shared keyRejectionReason / unsafePathReason guard writes and prunes; only manifest-owned paths may be overwritten or removed; corrupt or unreadable manifests block destructive work while still allowing new writes where safe. Post-crash adoption records orphaned files as skipped_current when on-disk bytes match the resolved hash (without weakening clobber protection). readRegularFile avoids blocking reads on FIFOs/devices; Windows reserved device names are rejected at the filesystem layer; safe-fs gains tempNamePattern for bounded orphan temp sweeps.

Adds a large skills-fs.test.ts abuse matrix (traversal, symlinks, clobber/adoption, corrupt manifest, etc.) plus package export assertions in skills.test.ts.

^{Reviewed by Cursor Bugbot for commit afbd8f0. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit afbd8f0. Configure here.}

[andrewklatzke]

Worth taking a look at the bugbot on this one, might not be real but we should confirm at least