fix(skills): copy skill supporting files byte-faithfully - #2575
Merged
Conversation
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Part of #2563 — this PR takes only the companion-file byte-fidelity defect reported there; the issue stays open for the remaining shared-frontmatter question.
DirFeatureProcessor.writeAiDirssplit a skill's supporting files (everything besideSKILL.md) into a text path and a binary path usingisBinaryBuffer, which detects only content a UTF-8 round-trip would mangle. Everything else went throughaddTrailingNewline(file.fileBuffer.toString("utf-8")), so any supporting file that happens to be valid UTF-8 had its CRLF line endings flattened to LF, its trailing whitespace stripped, and a trailing newline appended.docs/reference/file-formats.mddocuments that Rulesync "preserves supporting files beside directory-layoutSKILL.md", which that rewrite contradicts: a fixture whose exact bytes matter, a CSV that needs RFC 4180 CRLF, or a file whose missing trailing newline is deliberate came out different from the source.Solution
Supporting files are now copied byte for byte, unconditionally — both the comparison and the write stay on the buffer path.
SKILL.mditself is unchanged: rulesync composes its body and frontmatter, so it keeps its trailing-newline normalization and structured-equivalence comparison.Change detection is byte-exact too, via a new
companionFileContentsEquivalent, with one narrow exception. Not every companion is a user asset: Codex CLI'sagents/openai.yamlis composed by rulesync from frontmatter, and comparing it byte-exactly would make every generate report the whole skill directory as changed once a formatter re-indented it. Such files are now flaggedcomposed: trueonAiDirFile, and only they fall back to the structured verdict of the existing comparison when the bytes differ. A carried-through user asset never does — whatever its extension, a drifted copy is repaired rather than tolerated, which is what "byte for byte" has to mean. There is deliberately no text fallback either: trailing-whitespace-insensitive text equality is exactly the normalization companion files no longer get.Consequences:
otherFileContentsstaging array are gone, which also removes the "synchronization issue between otherFiles and otherFileContents arrays" internal-error guard that only existed to keep the two arrays aligned.isBinaryBufferinsrc/utils/file.tshad this as its only caller and is removed with it.docs/reference/file-formats.md.Deliberately out of scope, to keep the PR to byte fidelity: symlink and executable-bit preservation.
Testing
dir-feature-processortests: a CRLF, newline-less text companion is written byte-identical rather than normalized; an identical existing file is skipped; a file differing only in those trailing bytes is rewritten; a change in a later companion is still detected; a re-indented composedagents/openai.yamlcounts as unchanged. The existing JPEG-buffer tests still cover the invalid-UTF-8 case.companionFileContentsEquivalentcovering the binary, CRLF, composed-structured and carried-through-structured cases, and a real-filesystem test inskills-processor.test.tsthat writes a PNG header and a CRLF fixture throughwriteAiDirs, reads the bytes back unchanged, and confirms a second write is a no-op.npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-skills.spec.ts— 130 passed.pnpm cicheck— all green.🤖 Generated with Claude Code