Skip to content

fix(release): include bundled skill in prebuilt archives#302

Open
benvinegar wants to merge 1 commit into
mainfrom
fix/prebuilt-skill-artifacts
Open

fix(release): include bundled skill in prebuilt archives#302
benvinegar wants to merge 1 commit into
mainfrom
fix/prebuilt-skill-artifacts

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • Include the bundled skills/ directory in standalone prebuilt release artifacts.
  • Verify GitHub release archives contain the Hunk review skill and restore binary execute bits before tarring.
  • Install skills/ through the generated Homebrew formula so hunk skill path works from Brew installs.

Closes #299

Tests

  • bun test ./scripts
  • bun run typecheck
  • bun run format:check
  • bun run lint

This PR description was generated by Pi using OpenAI GPT-5

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 12, 2026

Greptile Summary

This PR fixes a gap where the bundled skills/ directory was absent from standalone prebuilt release archives, causing hunk skill path to fail after extracting a tarball or installing via Homebrew.

  • build-prebuilt-artifact.ts is refactored into an exported stagePrebuiltArtifact function that now copies skills/ into the output directory and restores the binary's execute bit (chmod 0755) on non-Windows platforms.
  • The GitHub Actions create-github-release job gains explicit guards that fail fast if the binary or skills/hunk-review/SKILL.md is missing from a downloaded artifact before tarring.
  • The generated Homebrew formula gains libexec.install "skills" so both the binary and skills land in libexec side-by-side; a new unit test covers the staging logic end-to-end.

Confidence Score: 4/5

The 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

Filename Overview
scripts/build-prebuilt-artifact.ts Refactored to export a testable stagePrebuiltArtifact function; now copies skills/ and chmods the binary. Missing pre-flight check for the skills directory before cpSync.
scripts/build-prebuilt-artifact.test.ts New test covering skill inclusion and binary execute-bit restoration after staging; well-structured with proper temp-dir cleanup.
.github/workflows/release-prebuilt-npm.yml Adds binary existence/skills presence guards and a defensive chmod 0755 before tarring in the create-github-release job; handles both hunk and hunk.exe correctly.
scripts/update-homebrew-formula.ts Adds libexec.install 'skills' to the generated Homebrew formula so the skills directory lands alongside the binary in libexec.
scripts/update-homebrew-formula.test.ts Adds assertion that the generated formula includes libexec.install 'skills'; minimal, focused change.
CHANGELOG.md Adds a changelog entry for the fix under the upcoming release section.
Prompt To Fix All With AI
Fix 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

Comment thread scripts/build-prebuilt-artifact.ts Outdated
chmodSync(stagedBinary, 0o755);
}

cpSync(path.join(repoRoot, "skills"), path.join(outputDir, "skills"), { recursive: true });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Suggested change
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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@benvinegar benvinegar force-pushed the fix/prebuilt-skill-artifacts branch from eb0b1a2 to c95e858 Compare May 12, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hunk skill path fails: SKILL.md missing from all v0.11.1 release tarballs

1 participant