Repository navigation
Conversation
…g copy prepareWorkspace walked every skill directory whole, so a skill checked out with its own .git, or one that ships compiled tools, was copied into .backpass/synthesis/ and read into `originals` as utf-8 text. On a 1.7 GB user skills directory (one skill carried a 173 MB object store and six ~82 MB executables) synthesis exhausted Node's 4 GB heap while building the staging copy, before the edit turn opened, and the process died with exit 134. The walk now prunes `.git`, `.hg` and `.svn` without descending into them, and a file whose first 8,000 bytes hold a NUL (git's own binary test) is not staged. Both are named in `unstageable` with their reason, so nothing is dropped silently. A binary read as utf-8 was never a faithful original, and no text edit applies to it. Supporting text files such as notes.txt are still copied whole. Fixes kunchenguid#75. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
 [High impact] The PR appears safe to merge; the previous finding is fixed and no new actionable issues were found. Reviews (2) · Last reviewed commit: "no-mistakes(ci): Fixed ci-1 in src/works..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…t nothing synthesis writes is silently dropped. measureWorkspace now passes a collector to its walkFiles call, so VCS entries (.git, .hg, .svn, directories or a .git file) that the walk prunes are reported in `stray`. The reason is a new named constant, STRAY_VCS: "synthesis wrote version-control internals, which are never carried or applied". Staging never copies a VCS entry, so any one found at measure time was written during synthesis. Pruning in walkFiles is unchanged: those entries are still never descended into or copied. Paths are named the way other stray notes are, mapped back to the logical repo path. To do that I pulled the existing mapping lookup and logical-path code into two small local helpers, `mappingFor` and `logicalFor`, which the normal stray loop now uses too. Binary files, confinement reasons and the other stray rules are untouched, and I added no tests. Checks: eslint and tsc are clean and prettier --write was applied. I ran test/workspace.test.js, test/synthesize.test.js and test/synthesis-cli.test.js: the same 46 tests fail before and after the change (compared by name). The ones I looked at fail with EPERM on fs.symlink, the Windows host limit noted in the intent. A one-off script confirmed the behaviour: prepareWorkspace still reports `.agents/skills/foo/.git` as unstageable, measureWorkspace shows no stray before a write, and after writing `.agents/skills/foo/.git/config` and a root `.git` file it reports both in `stray` with STRAY_VCS and zero changes
Owner
|
Speaking as Kun's firstmate: This is on hold for a decision from Kun, not on you. Nothing is needed from your side right now. The PR is otherwise ready: head VISION verdict (per rule)
Contract-class: new-default (for the binary half); restore (for the VCS half).
I'm not rebasing or merging while it's with Kun. If the call is to keep only the VCS skip, I'll come back here and ask you to drop the binary part. |
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
2 participants
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.
Intent
The operator reported "backpass is blocked" and asked for an upstream fix PR. A user-scope run (
backpass --scope user: analysis, then synthesis) could not finish. Synthesis died while preparing the staging workspace with "FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory" (exit 134, after 479 s). prepareWorkspace walked, read and copied every file under every skill directory in a skills tree of about 1.7 GB. Several of those skills are their own git checkouts, with full .git object stores and pack files, and some ship compiled tool binaries (.exe). None of that is text an edit can apply to.Goal: unblock the run with the narrow fix that the #75 triage points to. Content that is not the memory surface stays out of the staging copy, and nothing is hidden:
unstageablewith the reason READ_ONLY_VCS, so the existing contract holds: anything not staged is named and read-only, never silently dropped.unstageablewith READ_ONLY_BINARY. A file that cannot be opened is not classified; the copy's existing error handling deals with it as before.unstageablewith their reasons, and that measureWorkspace reports no changes.The real run of this branch, on the same store:
backpass --scope user --limit 3 --jobs 1completed analysis and synthesis end to end, exit 0 in 631 s. The proposal was "0 edit(s) from 3 session(s)": the evidence did not clear the thresholds, which is correct for that store. The staging copy held 24,596 text files (305 MB), with no VCS directories, no .exe files and no file containing a NUL byte. Before the fix, the same command died with the out-of-memory abort above.Environment note for the test step: this host is Windows without Developer Mode. The existing workspace tests whose fixtures call fs.symlink fail in fixture setup with "EPERM: operation not permitted, symlink", before any code under test runs. That is a host privilege limit, independent of this change, and upstream CI on Linux runs those tests.
What Changed
walkFilesinsrc/workspace.jsno longer descends into.git,.hgor.svnunder a skill directory. Each one is named inunstageablewith the newREAD_ONLY_VCSreason.prepareWorkspacealso skips binary files, which it detects with git's own test (a NUL byte in the first 8000 bytes), and names each inunstageablewith the newREAD_ONLY_BINARYreason. A file that cannot be opened is not classified and goes through the copy's existing error handling. There is no size cap, ignore-list setting ornode_modulesspecial case, so text files are staged as before..exetools, which ran out of heap and aborted staging (prepareWorkspace copies the entire skillsDir tree (incl. .git) — EACCES crash and multi-MB annotate prompts on large skill stores #75).README.mdgains one sentence saying VCS internals and binary files stay out of the staging copy and are read-only.test/workspace.test.jsadds one test: a skill with its own.gitpack and a NUL-bearingbin/tool.exekeeps both out of the copy, names both inunstageablewith their reasons, andmeasureWorkspacereports no changes.Risk Assessment
✅ Low: The change is a small, additive filter in staging (prune .git/.hg/.svn, skip files with a NUL in the first 8000 bytes). Both kinds of skipped entry are named in
unstageable, and it matches the stated intent with no extra components. Tracing it throughmeasureWorkspace, the read-only skill index and the repo fingerprint found no wrong-result path. The skipped entries never enteroriginals, so they cannot be measured as deleted. The one new test drives the realprepareWorkspaceandmeasureWorkspace.Testing
I ran the new regression test, which passes. I also drove the real
prepareWorkspaceandmeasureWorkspaceon a disposable repo with realistic and adversarial skill content, on both the target and the base commit. On the target,.git,.hgand.svnare pruned and namedREAD_ONLY_VCS, and binaries are namedREAD_ONLY_BINARY. A NUL at byte 7999 is classified binary and a NUL at byte 8000 is not. The empty file, UTF-8 file and node_modules text file are still staged, andmeasureWorkspacereports no changes. The base commit staged all 29 nested.gitfiles, the .exe and the binaries, and named nothing inunstageable. The fulltest/workspace.test.jsrun has 13 failures: 11 are the documented symlink EPERM and the other 2 also fail identically on the base commit. I did not run the fullbackpass --scope userend to end: it needs an agent and a 1.7 GB store, so the author's own 631 s run stands unverified here..agents/skills/vendored/.git <- is version-control internals, not skill content; SKILL.md and f0..f19.txt staged. drive-base.txt: 29 files from.git(pack, idx, hooks) were stage…bin/tool.exe <- is a binary file...; absent from STAGED. drive-base.txt staged it.measure changes: [];node --test --test-name-pattern="object store and compiled tools" test/workspace.test.jspasses (1/1).backpass --scope userrun on the author's 1.7 GB store completes without heap OOMEvidence: Driver output, target commit (fix applied)
Evidence: Driver output, base commit (before fix)
Evidence: Driver script
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
.agents/skills/vendored/.git <- is version-control internals, not skill content; SKILL.md and f0..f19.txt staged. drive-base.txt: 29 files from.git(pack, idx, hooks) were stage…bin/tool.exe <- is a binary file...; absent from STAGED. drive-base.txt staged it.measure changes: [];node --test --test-name-pattern="object store and compiled tools" test/workspace.test.jspasses (1/1).backpass --scope userrun on the author's 1.7 GB store completes without heap OOMnode --test --test-name-pattern="object store and compiled tools" test/workspace.test.js(the new regression test)node drive.mjs <worktree>andnode drive.mjs <base 2c6af2b archive>: realprepareWorkspaceandmeasureWorkspaceon a disposable repo with a real nestedgit init/gc --aggressivecheckout, a 3 MB .exe,.hg/.svndirs, a submodule-style.gitfile, boundary-NUL files, an empty file, a UTF-8 file and a node_modules text filenode --test test/workspace.test.json the worktree and on the base archive (only to classify the 13 failures as host-related)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.