feat: pkg 6.21 build hooks — default bump, trust-boundary docs, and the fixes review surfaced - #22
Merged
Merged
Conversation
pkg 6.21.0 added preBuild / postBuild / transform hooks. They are
config-only (no CLI flags), so the action's --config pass-through already
carries them — but the default pkg-version pin of ~6.19.0 resolved to
6.19.x, which does not have them.
- default pkg-version ~6.19.0 -> ~6.21.0
- docs/inputs.md gains a Build hooks section covering the three keys,
the PKG_OUTPUT contract, the config-inline JSON limitation, and a
warning against renaming the binary from postBuild (pkg-output-map
locates outputs by predicted name + basename-prefix scan)
- fix the `config` input description: pkg auto-detects .pkgrc,
.pkgrc.json and pkg.config.{js,cjs,mjs} — never .ts, never
pkg.config.json
There was a problem hiding this comment.
Pull request overview
Updates the action’s default @yao-pkg/pkg version pin to surface pkg 6.21.0 build hooks by default, and regenerates action/docs artifacts to reflect the new defaults and corrected config auto-detection documentation.
Changes:
- Bump default
pkg-versionfrom~6.19.0to~6.21.0across core parsing, generated action manifests, dist bundles, and unit tests. - Document pkg 6.21.0 build hooks in generated
docs/inputs.mdvia a new static section inscripts/gen-action-yml.ts. - Correct
config/config-inlineinput descriptions to match pkg’s actual config filename auto-detection and hook limitations.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| STATUS.yaml | Updates minimum/default pkg-version notes and adds a build-hooks status note. |
| scripts/gen-action-yml.ts | Adds a generated “Build hooks” documentation block into docs/inputs.md. |
| packages/windows-metadata/dist/index.mjs | Regenerated dist bundle reflecting updated input descriptions and defaults. |
| packages/core/test/unit/pkg-runner.test.ts | Updates test baseline default pkgVersion to ~6.21.0. |
| packages/core/test/unit/inputs.test.ts | Updates default-value assertion for pkgVersion. |
| packages/core/src/inputs.ts | Updates input descriptions and default/fallback for pkg-version. |
| packages/build/dist/index.mjs | Regenerated dist bundle reflecting updated input descriptions and defaults. |
| packages/build/action.yml | Regenerated action manifest with updated descriptions and default pkg-version. |
| docs/inputs.md | Regenerated inputs docs: updated config descriptions and new Build hooks section. |
| action.yml | Regenerated composite action manifest with updated descriptions and default pkg-version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg refuses `--config <file>` together with a package.json input
("Specify either 'package.json' or config. Not both"), and the action's
default positional `.` resolves to exactly that. So every run using
`config-inline`, or `config` pointing at a standalone pkg config, failed
outright unless the caller also set `entry`.
Reproduced against 6.21.0 and 6.19.0 alike — this predates the version
bump. Resolve package.json `bin` and pass it as the entry, mirroring
pkg's own precedence (string bin; object keyed by unscoped package name,
else first value). Actionable error when there is no bin.
tokensForTarget narrows to Pick<ProjectInfo, 'name'|'version'> — it never
needed the new field.
`npm i -g @yao-pkg/pkg@${{ inputs.pkg-version }}` pasted the input
straight into the run block, so a crafted specifier executed shell in a
job that can hold signing certs and notarization credentials. Pass it
through `env:`, reference it quoted, and reject anything outside npm
specifier characters before installing.
Also collapses the pkg config filename list to one exported constant.
It had drifted into four independent copies with two different contents:
the cache-key `hashFiles` glob still matched `pkg.config.{js,ts,json}`,
so a `pkg.config.mjs` — the file the hooks docs tell you to write — never
invalidated the pkg-fetch cache, while `.ts` was hashed despite pkg never
auto-detecting it.
Drops the `?? '<literal>'` fallbacks in parseInputs: readInput already
resolves the INPUT_SPECS default, so those were unreachable copies of a
value that has to stay in sync by hand.
`~6.21.0` floats patch releases at CI runtime, so the specifier can't identify the build that ran — exactly what you need when a patch regresses. Query `pkg --version` and surface it in the log and the step summary alongside the specifier. Failing to label a summary never fails a build. Two error messages gain the context the new hooks make necessary: a non-zero pkg exit now names the config file, because a failing preBuild/postBuild/transform hook is indistinguishable from a pkg-internal failure in the log; and the output-map miss suggests a postBuild rename, which is the newly-likely cause.
The default bump to 6.21 silently turned pkg configs from data into
executable code: pkg picks a config up from the checked-out tree, so a
workflow building an untrusted ref now grants that ref command execution
in a job that may hold signing credentials. No such capability existed at
6.19. Documented in docs/inputs.md, README and STATUS.yaml, with the
release-note obligation and the pin-vs-floor rationale recorded.
Corrections to the hooks reference:
- postBuild runs after pkg's own macOS ad-hoc signing, not the action's
signing stage, and not at all on Windows
- the output-map fallback is exact -> case-insensitive -> <os>-<arch>
substring, not a basename-prefix scan
- config-inline needs an entry script; say so
- the transform example scopes to project sources (a bare .endsWith('.js')
also rewrites node_modules), uses a replacer function so `$&` is not
interpreted, and no longer bakes "undefined" outside Actions
- stop telling readers to raise pkg-version to a value that is the default
- setSecret does not mask what a hook prints
Adds an e2e job asserting all three hooks fire through the --config
pass-through, and generator tests covering the cache glob, the install
step's injection guard, and the trust warning.
Fixes an unrelated pre-existing YAML syntax error in STATUS.yaml that
kept the whole file from parsing.
robertsLando
changed the title
Actions expands ${{ }} anywhere inside a run block, shell comments
included, so the comment explaining why pkg-version is passed via env
was itself parsed as an empty expression: "Line 152: An expression was
expected". action.yml failed to load, so every job that invokes the
action failed — including ones this branch never touched.
Rewords the comment and adds a generator test that scans every run
block for workflow expressions. Verified it fails on the exact bug.
…output
`[[ ! "$X" =~ ^[...><...]+$ ]]` is a bash syntax error — [[ ]] parses a
bare > as an operator before the regex engine ever sees it, so the
install step died with "unexpected token `>`" on every job.
The pattern now lives in `specifier_re` and is referenced unquoted, the
standard idiom. Verified against real specifiers (~6.21.0, ^6.21.0,
latest, ">=6.21.0 <7.0.0", 6.x, *, "6.20.0 || 6.21.0") and injection
attempts ("6.21.0; curl …|sh", "$(id)", backticks, "&&whoami").
Root cause of both CI failures is the same gap: shell embedded in a YAML
string is invisible to eslint and to the unit tests, so it could only
fail in CI. Added a test that extracts every `run: |` block from the
generated action.yml and runs `bash -n` over it. Confirmed it fails on
the exact broken conditional.
The entry fix read package.json from GITHUB_WORKSPACE, but a standalone config usually sits beside the project it configures — not at the repo root. The hooks e2e caught it: bin was looked up in pkg-action's own root package.json, which has none. An explicit `config` now names the project — its parent directory is the root whether it is a package.json or a standalone config next to one. A config kept away from its project (configs/pkg.config.mjs) has no package.json beside it, so that still falls back to the workspace, as does an unset config. No back-compat risk: standalone configs never worked at all before 250ebb9, so nothing was relying on the old resolution. Verified the full flow against real pkg 6.21 locally — all three hooks fire, the transform rewrites only packed bytes, and the on-disk source keeps its placeholder.
pkg derives the output base name as --output > package.json name >
config `name` > entry basename. Supplying an entry for a standalone
config moved it off package.json name, so the prediction ("hooks-app")
missed what pkg wrote ("index"), and a single target has no os/arch
suffix for the triple fallback to match on.
The action cannot know the config's `name` without executing the user's
pkg.config.mjs, so prediction here is inherently best-effort — hence the
existing fallback chain. Extends it with the one case that is
unambiguous: a sole target and a sole file in an output directory the
action created empty for this run.
Still refuses to guess when several files are present, or for
multi-target builds where the os/arch fallback already applies. Verified
against the real pkg output dir from the failing CI scenario.
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.
What
pkg 6.21.0 shipped build hooks (yao-pkg/pkg#273). They need no new action surface — config keys, no CLI flags, so the existing
--configpass-through already carries them. The default pin~6.19.0was what made them unreachable.A deep review of the first draft turned up two blockers and a set of real defects, all fixed here. Four commits, separable by concern.
250ebb9— standalone--configwas brokenpkg refuses
--config <file>together with a package.json input (Specify either 'package.json' or config. Not both), and the action's default positional.resolves to exactly that. Every run usingconfig-inline, orconfigpointing at a standalone pkg config, failed outright unless the caller also setentry.Reproduced against 6.21.0 and 6.19.0 alike — this predates the bump; the first draft of this PR merely documented it as working. Fixed by resolving package.json
binand passing it as the entry, mirroring pkg's own precedence.0b132e5— shell injection + a drifted filename listnpm i -g @yao-pkg/pkg@${{ inputs.pkg-version }}pasted the input into the run block; a crafted specifier executed shell in a job that can hold signing certs. Now passed viaenv:, quoted, and validated.The pkg config filename list had drifted into four copies with two different contents. The cache-key
hashFilesglob still matchedpkg.config.{js,ts,json}— so apkg.config.mjs, the file the new hooks docs tell you to write, never invalidated the pkg-fetch cache. Collapsed to one exported constant.Also drops two
?? '<literal>'fallbacks that were unreachable duplicates ofINPUT_SPECSdefaults.5e76562— diagnosability~6.21.0floats patches at CI runtime, so the specifier can't identify the build that ran. Now reports the resolvedpkg --version. A non-zero pkg exit names the config file, because a failing hook is otherwise indistinguishable from a pkg-internal failure; the output-map miss suggests apostBuildrename.013d576— the trust boundary, and the docsThe default bump silently turned pkg configs from data into executable code. pkg picks a config up from the checked-out tree, so a workflow building an untrusted ref now grants that ref command execution in a job that may hold signing credentials. No such capability existed at 6.19. Documented in
docs/inputs.md, README and STATUS.yaml, with the release-note obligation and the pin-vs-floor rationale recorded.Hook-reference corrections:
postBuildruns after pkg's own macOS ad-hoc signing (not the action's, and not at all on Windows); the output-map fallback is exact → case-insensitive →<os>-<arch>substring; thetransformexample scopes to project sources, uses a replacer function so$&isn't interpreted, and no longer bakes"undefined"outside Actions.New e2e job asserts all three hooks fire through the
--configpass-through. Generator tests cover the cache glob, the injection guard, and the trust warning.Verification
yarn typecheck,yarn lintcleanyarn test— 245/245 pass (was 231; +14 covering bin resolution, default consistency, and the generated surfaces)yarn genverified idempotent, so the codegen drift gate passes--configfailure and its fix reproduced directly against both pkg 6.19.0 and 6.21.0Note
Also fixes a pre-existing YAML syntax error in STATUS.yaml that kept the whole file from parsing — unrelated, but the file could not be validated while it was there.
🤖 Generated with Claude Code