[Bugfix #693] Restore executable bit on forge scripts after install#696
Merged
Conversation
pnpm pack/publish strips executable bits from every file outside the `bin` field, so `scripts/forge/<provider>/*.sh` ship as 0644 on fresh installs. `afx spawn` then fails with `Permission denied` when `executeForgeCommand` invokes `issue-view` (and every other concept routed through the shell dispatcher). Fix by moving the existing one-liner postinstall into a dedicated `scripts/postinstall.mjs` that also chmods every forge script back to 0755 after install. The node-pty spawn-helper fix is preserved.
Gemini PR review flagged a correctness nit: the original code called
statSync on each subdir entry, which throws on broken symlinks and would
crash postinstall. readdirSync(..., { withFileTypes: true }) gives the
same info in one syscall without the throw path, and is more idiomatic.
Also drops the unused count return value.
3 tasks
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.
Summary
Fixes #693 —
afx spawnfailing withPermission deniedon fresh installs because forge concept scripts (scripts/forge/<provider>/*.sh) ship as0644.Root Cause
pnpm pack/pnpm publishstrips executable bits from every file outside thebinfield, even when the source is tracked in git with mode100755. The scripts atpackages/codev/scripts/forge/**/*.share all100755in the repo (confirmed viagit ls-files -s), but the produced tarball has them as0644:npm packon the same tree preserves the bits, so this is a pnpm-specific packaging bug rather than a git state issue.The issue's suggested fix (
chmod +x+ commit) does not help — the files are already 100755 in git.Fix
Move the existing one-liner
postinstallinto a dedicatedpackages/codev/scripts/postinstall.mjsthat chmods everyscripts/forge/<provider>/*.shback to0755. The pre-existing node-ptyspawn-helperchmod is preserved.scripts/postinstall.mjsis added tofilesso it ships in the tarball.Implementation uses
readdirSync(..., { withFileTypes: true })so a broken symlink in the provider dir wouldn't crash postinstall.This is robust against both pnpm-packed and npm-packed tarballs, and does not require changing the release workflow.
Verification
All 42 forge scripts (github/16, gitlab/14, gitea/12) end up at
0755.Test Plan
packages/codev/src/__tests__/bugfix-693-forge-exec-bit.test.tspackage.jsonwires postinstall + ships the script viafiles.postinstall.mjsagainst a fixture tree of mode-0644scripts and asserts they become0755.scripts/forge/<provider>/<name>.sh(one level deep), so new scripts are not silently missed.pnpm vitest run→Test Files 126 passed (126), Tests 2523 passed | 13 skipped (2536)pnpm pack, installed into a scratch dir, confirmed-rwxr-xr-xon all forge scripts.CMAP Review
statSyncloop could throw on a broken symlink and crash postinstall. Fixed by switching toreaddirSync(..., { withFileTypes: true })(commit 6779092). Gemini's other two "issues" (missingcodev/reviews/doc,status.yamlstill atinvestigate) don't apply: the BUGFIX protocol keeps the review in the PR body, andstatus.yamlis now advanced toverifiedviaporch done— builders aren't allowed to edit it directly.