feat: publish this repo as a GitHub Action - #49
Open
rossrdme wants to merge 12 commits into
Open
Conversation
lint.js's package.json read and utils/lint.js's validator discovery both relied on paths computed relative to import.meta.url at runtime -- fine for normal CLI use, but broken once this code is relocated into a single bundled file (e.g. by ncc for the upcoming GitHub Action entrypoint). Switched to a static package.json import attribute and static imports of every validator module, preserving identical runtime behavior for normal CLI usage.
Adds a real, publishable GitHub Action (mirroring readmeio/rdme's approach) so customer workflows can reference this repo directly -- e.g. `uses: readmeio/cli@v1` with `readme: lint` -- instead of `npx -y @readme/cli <command>`, which re-resolves against the npm registry on every run and has no way to pin a tested version. - src/gha.js: dedicated entrypoint that imports the lint, oas:validate, and oas:sync commands directly rather than going through src/cli.js's runtime directory scan (which a bundler can't follow statically). Reads the `readme` input (INPUT_README) as a plain command string. - action.yml: node24 JS action, mirroring rdme's action.yml shape. - dist-gha/: single-file bundle produced by `npm run build:gha` (ncc build src/gha.js -o dist-gha), committed so the action works straight off a git ref with no install step -- same pattern rdme uses for its own dist-gha/run.cjs. - src/gha-pre.js: the action's `pre` step. @readme/markdown (pulled in by the components validator for a single helper, mdxishTags) is a ~25MB package with 50+ dependencies including tailwindcss/postcss, and bundling it whole breaks on its embedded CSS/asset references. Rather than vendoring that tree into this repo, this step installs just that one package into the action's own directory (GITHUB_ACTION_PATH) immediately before dist-gha/index.js runs, so plain Node module resolution finds it. Verified end-to-end (pre then main) from a directory with no ambient node_modules, simulating a real Actions runner checkout. - package.json: add @vercel/ncc as a devDependency and a build:gha script.
Adds a `readme` output (mirroring rdme's own action.yml) so a workflow step can inspect what a command printed -- e.g. `contains(steps.sync.outputs.readme, 'Skipped')` -- the same way the existing customer workflows currently do it themselves by piping `npx ... | tee sync-output.txt` and grepping the file. Commands call process.exit() directly on failure, which would skip right past any output-writing code placed after `await entry.run(...)` -- so capturing has to happen via a temporary console.log/error + process.exit patch in gha.js itself, not a simple return value. Verified both the normal-completion path and the process.exit(1) path actually flush to GITHUB_OUTPUT, using a real hard OAS validation error to force the latter. The delimiter written to GITHUB_OUTPUT is randomized per write rather than a fixed string like "EOF" -- the captured value is arbitrary CLI output that can echo back content from a customer's own docs/OAS files, and a guessable delimiter there would let that content prematurely close the heredoc and inject extra key=value pairs into the outputs file.
Supersedes the previous commit's approach. That one worked by monkey-patching console.log/error and process.exit inside gha.js to capture a command's printed text and hand it back as one big string output -- functional, but it threw away structure the CLI already had and made a workflow re-derive facts (like "did oas:sync skip a page?") by grepping rendered prose for a specific word. lint() and oas-validate.js's validateOas()/validateOasFiles() were already pure, side-effect-free functions returning fully structured data -- no console output, no process.exit -- built for exactly this "call me programmatically" case. oas-sync.js was the outlier: syncOas() is pure, but its printing was only ever inlined in run(), not factored out the way validateOasFiles() already is. Extracted that into printSyncResults(), mirroring the existing pattern, so the CLI and the Action can both use it without duplicating it. In an Action, process.exit(1) on failure isn't a problem to route around -- it's exactly what makes the step fail. Each command's run() now just writes its own already-known results (has-errors, skipped-count, etc.) to GITHUB_OUTPUT, via a new shared helper (utils/gha-output.js) that detects GITHUB_ACTIONS and no-ops everywhere else, right before doing whatever it was already about to do. No interception needed anywhere, and gha.js goes back to being as simple as it was originally -- it just calls run(). This also means a customer who runs `npx @readme/cli oas:sync` directly in their own workflow, without adopting the published Action at all, gets the same real outputs. Verified end-to-end from a directory with no ambient node_modules (same isolated-environment test as before): lint's has-errors/results, oas:validate's has-errors/total-errors/total-warnings/total-valid/ file-count, and oas:sync's added-count/deleted-count/skipped-count/ skipped all come back correctly, with proper GITHUB_OUTPUT delimiter framing, for both the normal-exit and process.exit(1) paths.
oas:sync was the odd one out among the three commands: lint and oas:validate both exit non-zero the moment they find a real problem, but oas:sync always exited 0 regardless of whether it skipped writing a page, leaving CI wrappers to notice and fail on that themselves (the existing customer workflows do this with a grep, and the pattern only protects whoever remembers to copy it). A skip already represents something worth a human looking at -- either a spec-crafted path trying to escape reference/, or sync's own bookkeeping not matching what's actually on disk -- so it should fail the same way a real lint or OAS validation error does, by default, without needing to be opted into. oas:sync is still marked alpha, so this is a fine time to fix the inconsistency rather than carry it forward. Also adds a has-errors output (mirroring lint and oas:validate) so this is now symmetric across all three commands.
dist-gha/index.js is committed (see action.yml) so the Action works straight off a git ref with no install step. Nothing previously caught someone editing src/gha.js (or anything it imports) without re-running npm run build:gha -- the Action would just silently keep running whatever was last committed. New build-gha CI job rebuilds it and fails if that produces a diff from what's checked in. Also adds a GitHub Action section to the README, since nothing documented how to actually use this repo as one.
…name Caught by an actual live GitHub Actions run, not local testing: on a real runner, GITHUB_ACTION_PATH came back undefined for this pre step, crashing it immediately (TypeError: path.join received undefined) -- which meant @readme/markdown never got installed, which in turn made every subsequent dist-gha/index.js invocation fail with ERR_MODULE_NOT_FOUND regardless of which command it ran, since the bundle's static import graph needs to resolve @readme/markdown just to load the file at all, before any command dispatch happens. Local testing didn't catch this because it worked by manually setting GITHUB_ACTION_PATH before invoking the script -- which papered over the exact gap that broke on real infrastructure. import.meta.dirname doesn't depend on the runner setting anything: it's just "where is this file," resolved from the module itself. This is also the same approach readmeio/rdme's own pre-step (bin/write-gha-pjson.js) already uses, so this now actually mirrors it rather than just resembling it.
|
| Filename | Overview |
|---|---|
| src/gha.js | Dispatches the supported Action commands, strictly validates trailing options, and publishes has-errors for dispatch and thrown failures; the previously reported parser and fallback-output defects are fixed. |
| src/commands/oas-validate.js | Publishes aggregate validation outputs across normal, empty, and missing-directory paths, including the previously omitted failure state. |
| src/commands/oas-sync.js | Aggregates synchronization outputs, publishes them on controlled exit paths, and now fails when generated pages are skipped. |
| src/utils/gha-output.js | Synchronously appends delimiter-framed Action outputs when running in GitHub Actions. |
| src/utils/lint.js | Replaces runtime validator discovery with an explicit static validator registry suitable for the bundled Action. |
| src/gha-pre.js | Installs the externally bundled Markdown dependency into the Action directory before execution. |
| action.yml | Defines the publishable Node Action, supported input, runtime entrypoints, and structured output contract. |
| .github/workflows/ci.yml | Rebuilds the committed Action distribution and fails CI when generated output has drifted. |
| dist-gha/index.js | Contains the committed bundled Action implementation, including the incremental parser and failure-output fixes. |
Reviews (6): Last reviewed commit: "chore: trigger re-review" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ing them Flagged by Greptile on PR #49. parseInput() collected every --flag off the readme input string, but nothing ever checked those against the command's actual supported flags -- so a typo like "oas:validate --derefrence" silently ran with dereference off, with no error, no warning, nothing. A customer would have no way to know their workflow wasn't doing what they typed, and oas:validate would happily report broken external $refs as valid. Now any flag not in the command's supported list fails loudly with a clear ::error:: annotation, the same way an unknown command already did.
… too Flagged on PR #49: oas-validate.js and oas-sync.js both call process.exit(1) when reference/ doesn't exist, but that happened before either command ever reached its writeGithubActionsOutputs call -- so a downstream workflow step checking has-errors (even one guarded with if: always(), specifically to still run after a failure) would see an empty string, not 'true', on exactly the failure path where distinguishing "this command failed" matters most. Both now write has-errors: 'true' (with the rest of their fields zeroed, since nothing was actually checked) before exiting. Also notes this "outputs are set on every exit path" invariant directly in action.yml, next to has-errors, so it stays visible next to the contract it documents rather than living only in a commit message.
…flag Follow-up to the earlier unknown-option fix, flagged again on PR #49: that fix only validated tokens that already started with "--" -- anything shaped differently, like a short option ("-d" instead of "--dereference") or any other malformed token, was filtered out before ever reaching that check and silently discarded. "oas:validate -d" or "lint -f" would run with default options and no indication anything was wrong, the exact same class of bug as before, just via a path the first fix didn't cover. Rewritten so every trailing token is checked against the command's supported flags directly, instead of pre-filtering by shape first -- anything that isn't exactly "--<a supported flag>" is now rejected. Verified oas:validate -d, lint -f, and the earlier oas:validate --derefrence case all fail loudly, and that valid input (lint, oas:validate --dereference) still works exactly as before.
Comment on lines
+89
to
+92
| run().catch((err) => { | ||
| console.error(err); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
When validation or synchronization throws before its command-specific output call, this rejection handler exits without publishing fallback outputs, causing downstream if: always() steps to receive empty has-errors and command-specific values despite the Action's every-failure-path output contract.
Knowledge Base Used:
Flagged on PR #49: the earlier fixes made oas-validate.js and oas-sync.js publish outputs on every controlled exit path they know about (a real validation failure, a missing reference/ directory), but gha.js's own top-level catch -- for a genuinely unexpected thrown error, as opposed to a command's own deliberate process.exit -- just logged and exited without publishing anything. That's exactly the "every-failure-path" contract action.yml documents for has-errors, just via a path none of the per-command fixes could reach, since by definition the command never got anywhere near its own output call. Also covers gha.js's own two early-exit paths (unknown command, unknown option) the same way, for the same reason -- any exit this file produces should leave has-errors set, not just the ones a command itself is responsible for. Verified with a real forced throw (reference/ existing as a file instead of a directory, so it passes the existence check but throws ENOTDIR partway through) -- has-errors: true now reaches GITHUB_OUTPUT before the process exits, where before this fix nothing would have been written at all.
Contributor
Author
|
@erunion here's the work to make it so it can be a published Github Action |
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
1 participant
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
uses: readmeio/cli@v1withreadme: lint— instead ofnpx -y @readme/cli <command>, which re-resolves against npm on every single run with no way to pin a tested version. (A stale npm-resolved version is exactly what caused a real incident during manual testing of this: it deleted 4 live pages on a customer's synced branch.)lint,oas:validate, andoas:syncnow each publish structured GitHub Actions step outputs (has-errors,skipped-count,total-errors, etc. — seeaction.ymlfor the full list) directly from their ownrun(), gated on theGITHUB_ACTIONSenv var GitHub sets on every runner. This also benefits anyone still calling the CLI directly vianpxin their own workflow, not just Action users.oas:syncnow fails (non-zero exit) when it skips writing a page, matchinglintandoas:validate, which already failed on a real problem on their own. Previouslyoas:syncalways exited 0 regardless, leaving it to whoever wrapped it in CI to notice and fail on that themselves. This command is still alpha, so this felt like the right time to fix the inconsistency rather than carry it forward. This is a deliberate breaking behavior change.lint.js's package.json read,utils/lint.js's validator discovery) — both were doing runtime, path-relative-to-import.meta.urllookups that break once bundled into a single file; switched to static imports with identical runtime behavior.build-gha) that rebuilds the committeddist-gha/index.jsbundle and fails if it's out of sync with the source — otherwise nothing would catch someone editingsrc/gha.js(or anything it imports) without re-runningnpm run build:gha.Test plan
npm test— 181/181).npm run build:ghaproduces a working, self-contained bundle; verifieddist-gha/has no diff after a fresh rebuild (what the new CI job checks).node_modules(simulating a real Actions checkout): success paths, warning paths, and real hard-failure paths (a real invalid OAS spec, a real lint error) — confirmedGITHUB_OUTPUTgets the expected structured fields with correct delimiter framing in every case.gha-pre.jsrelied onGITHUB_ACTION_PATH, which came backundefinedon an actual runner — fixed by switching toimport.meta.dirname, which also now genuinely matches howreadmeio/rdme's own pre-step locates itself. Re-ran after the fix:oas:validate,oas:sync, andlintall completed correctly, includinglintcorrectly catching and failing on a real duplicate-slug issue in the test content.Follow-ups (not blocking this PR)
v1) once merged, soreadmeio/cli@v1actually resolves — none of the docs here work until that happens.