fix(release): include bundled skill in prebuilt archives#302
Conversation
Greptile SummaryThis PR fixes a gap where the bundled
Confidence Score: 4/5The change is well-scoped and the new validation guards in the CI workflow make the release pipeline more robust than before. The binary pre-flight check in stagePrebuiltArtifact is thorough, but the same pattern is not applied to the newly added skills/ copy — a missing skills directory produces a raw ENOENT rather than a clear diagnostic message. Everything else (workflow guards, Homebrew formula update, and test coverage) looks correct. scripts/build-prebuilt-artifact.ts — the cpSync call for skills has no pre-check comparable to the binary validation. Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
scripts/build-prebuilt-artifact.ts:75
Missing pre-flight guard for `skills/` directory — unlike the binary, there is no explicit existence check before calling `cpSync`. If `skills/` is absent (e.g., partial checkout, shallow clone, or a future repo restructure), the function throws a raw `ENOENT` from the OS rather than the actionable message developers see for a missing binary.
```suggestion
const skillsSrc = path.join(repoRoot, "skills");
if (!existsSync(skillsSrc)) {
throw new Error(`Missing skills directory at ${skillsSrc}.`);
}
cpSync(skillsSrc, path.join(outputDir, "skills"), { recursive: true });
```
Reviews (1): Last reviewed commit: "fix(release): include bundled skill in p..." | Re-trigger Greptile |
| chmodSync(stagedBinary, 0o755); | ||
| } | ||
|
|
||
| cpSync(path.join(repoRoot, "skills"), path.join(outputDir, "skills"), { recursive: true }); |
There was a problem hiding this comment.
Missing pre-flight guard for
skills/ directory — unlike the binary, there is no explicit existence check before calling cpSync. If skills/ is absent (e.g., partial checkout, shallow clone, or a future repo restructure), the function throws a raw ENOENT from the OS rather than the actionable message developers see for a missing binary.
| cpSync(path.join(repoRoot, "skills"), path.join(outputDir, "skills"), { recursive: true }); | |
| const skillsSrc = path.join(repoRoot, "skills"); | |
| if (!existsSync(skillsSrc)) { | |
| throw new Error(`Missing skills directory at ${skillsSrc}.`); | |
| } | |
| cpSync(skillsSrc, path.join(outputDir, "skills"), { recursive: true }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build-prebuilt-artifact.ts
Line: 75
Comment:
Missing pre-flight guard for `skills/` directory — unlike the binary, there is no explicit existence check before calling `cpSync`. If `skills/` is absent (e.g., partial checkout, shallow clone, or a future repo restructure), the function throws a raw `ENOENT` from the OS rather than the actionable message developers see for a missing binary.
```suggestion
const skillsSrc = path.join(repoRoot, "skills");
if (!existsSync(skillsSrc)) {
throw new Error(`Missing skills directory at ${skillsSrc}.`);
}
cpSync(skillsSrc, path.join(outputDir, "skills"), { recursive: true });
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Added the explicit skills/ directory guard before copying, and split the tests so the missing directory and missing bundled skill cases both produce actionable errors.
This comment was generated by Pi using OpenAI GPT-5
a170e4f to
eb0b1a2
Compare
eb0b1a2 to
c95e858
Compare
Summary
skills/directory in standalone prebuilt release artifacts.skills/through the generated Homebrew formula sohunk skill pathworks from Brew installs.Closes #299
Tests
bun test ./scriptsbun run typecheckbun run format:checkbun run lintThis PR description was generated by Pi using OpenAI GPT-5