Repository navigation
ci: run on Blacksmith, add npm packaging gate and a scoped @planmonster publish path - #2
Conversation
Establishes a CI and distribution pathway for this fork. Runners: all Linux jobs move to blacksmith-4vcpu-ubuntu-2404 with useblacksmith/checkout + useblacksmith/setup-go. Measured on this module, a cold cross-compile costs ~8 min per target and a warm one ~3 s, so the restored Go build cache dominates release time. Two jobs deliberately stay on GitHub-hosted runners: macOS (Blacksmith has none, and the darwin build needs CGO for the Keychain) and any npm publish job (npm Trusted Publishing accepts cloud-hosted runners only — a Blacksmith runner reports itself as self-hosted in the OIDC claims). ci.yml also gains workflow_dispatch, so a maintainer can force a run without pushing an empty commit, and a `package` job that runs the new scripts/test-npm-package.sh: it builds a host binary, stamps and packs the wrapper packages, installs the tarballs, runs `olk version` through the JS launcher, and checks exit-code propagation — in both the unscoped upstream layout and the scoped fork layout. npm packaging breakage is now a PR failure instead of a tag-time failure. npm: publish-npm.yml publishes @planmonster/olkcli plus six @planmonster/olk-<os>-<arch> packages to npmjs.org via Trusted Publishing. The unscoped names belong to upstream, so the fork takes a scope. The trigger tag is npm-v* rather than v*, so it does not also fire release.yml (which cannot run here: no TAP_GITHUB_TOKEN, and io.github.rlrghb/outlook is not our MCP namespace). Versions must carry a fork suffix such as 0.9.5-pm.1; a bare X.Y.Z is rejected because upstream owns those numbers. scripts/build-npm.mjs gains --scope, --registry, --repository, --access and --skip-binary-check (one-time bootstrap only); re-scoping is idempotent. npm/olk/bin/olk.js now resolves its platform package from its own optionalDependencies instead of a hardcoded name, so one launcher serves both layouts. Also corrects the golangci-lint version in AGENTS.md: the workflow pins v2.11.4, and v2.5.0 cannot lint a module targeting go 1.26.4.
📝 WalkthroughWalkthroughThe PR adds scoped npm packaging for the fork, platform-specific binary publishing, launcher resolution for scoped dependencies, end-to-end package tests, Blacksmith-backed CI, and documentation for fork release and consumption workflows. ChangesForked npm package tooling
Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant GitHubActions
participant BuildJobs
participant NPM
Maintainer->>GitHubActions: Dispatch workflow or push npm-v tag
GitHubActions->>BuildJobs: Build platform binaries
BuildJobs-->>GitHubActions: Upload binary artifacts
GitHubActions->>NPM: Publish scoped packages
GitHubActions->>NPM: Verify package installation
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.23.0)AGENTS.mdmarkdownlint-cli2 wrapper config setup failed docs/npm-publishing.mdmarkdownlint-cli2 wrapper config setup failed Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/publish-npm.yml (1)
176-177: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the npm CLI used by the publish job.
npm@latestmakes release builds non-reproducible and pulls an unreviewed registry version. Usenpm@11.5.1instead.Source: Linters/SAST tools
scripts/test-npm-package.sh (1)
42-46: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDefensive-quote the
rm -rftargets.
$pkgdir/bin/$execan't realistically be empty today (guarded by the case-statement bailout andset -u), but Shellcheck's"${var:?}"pattern is a cheap safeguard against future refactors that could leave the variable empty.🛡️ Optional hardening
- rm -rf "$tmp" "$pkgdir/bin/$exe" + rm -rf "${tmp:?}" "${pkgdir:?}/bin/$exe"Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b7d323f-f300-4949-b88e-f4deabe8cf2e
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/publish-npm.yml.github/workflows/release.ymlAGENTS.mddocs/npm-publishing.mdnpm/olk/bin/olk.jsscripts/build-npm.mjsscripts/test-npm-package.sh
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||
| version="${{ inputs.version }}" | ||
| dry_run="${{ inputs.dry_run }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove dispatcher input from shell source.
inputs.version is rendered into the script before Bash runs. An authorized workflow dispatcher can break the quoted assignment and execute commands in the version job. Pass it through env and read $GITHUB_EVENT_NAME instead; also reject CR/LF before writing $GITHUB_OUTPUT.
Proposed fix
- name: Resolve version and mode
id: v
+ env:
+ DISPATCH_VERSION: ${{ inputs.version }}
+ DISPATCH_DRY_RUN: ${{ inputs.dry_run }}
run: |
set -euo pipefail
- if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
- version="${{ inputs.version }}"
- dry_run="${{ inputs.dry_run }}"
+ if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
+ version="$DISPATCH_VERSION"
+ dry_run="$DISPATCH_DRY_RUN"
else
version="${GITHUB_REF_NAME#npm-v}"
dry_run="false"
fi
version="${version#v}"
+ if [[ "$version" == *$'\n'* || "$version" == *$'\r'* ]]; then
+ echo "version must be a single line" >&2
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | |
| version="${{ inputs.version }}" | |
| dry_run="${{ inputs.dry_run }}" | |
| - name: Resolve version and mode | |
| id: v | |
| env: | |
| DISPATCH_VERSION: ${{ inputs.version }} | |
| DISPATCH_DRY_RUN: ${{ inputs.dry_run }} | |
| run: | | |
| set -euo pipefail | |
| if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then | |
| version="$DISPATCH_VERSION" | |
| dry_run="$DISPATCH_DRY_RUN" | |
| else | |
| version="${GITHUB_REF_NAME#npm-v}" | |
| dry_run="false" | |
| fi | |
| version="${version#v}" | |
| if [[ "$version" == *$'\n'* || "$version" == *$'\r'* ]]; then | |
| echo "version must be a single line" >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 51-51: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
Source: Linters/SAST tools
1 participant
Scope
Three things, all fork-specific infrastructure:
blacksmith-4vcpu-ubuntu-2404withuseblacksmith/checkout+useblacksmith/setup-go.workflow_dispatchand a newpackagejob that tests npm packaging on every PR.@planmonster/olkcli+ six@planmonster/olk-<os>-<arch>packages, published from CI with Trusted Publishing (no secrets).Why two jobs stay on GitHub-hosted runners
build-darwin,releasemacos-latestubuntu-latestMeasurements that drove the design
Cross-compile cost for this module, measured locally:
Hence
useblacksmith/setup-gofor the build cache. Artifact sizes: 119 MB unpacked per binary package, 23 MB compressed (matches upstreamolk-linux-x64@1.10.0at 118.6 MB).npm details
olkcli,olk-linux-x64, …) belong to upstream.npm-v*, notv*, so it does not also firerelease.yml.0.9.5-pm.1); a bareX.Y.Zis rejected.npm/olk/bin/olk.jsresolves its platform package from its ownoptionalDependencies, so one launcher serves the unscoped and scoped layouts.One-time bootstrap is required before the first real publish (npm Trusted Publishing can only be configured on a package that already exists). Documented in
docs/npm-publishing.md.Testing
Local gates, all green (Go 1.26.4, golangci-lint 2.11.4):
go mod tidy→ no drift;go vet ./...,go build ./...→ cleango test -race -count=1 ./...→ all packages passgolangci-lint run→ 0 issuesbash scripts/test-npm-package.sh→ PASS in both unscoped and scoped modes: packs, installs, runsolk version --jsonthrough the launcher (returns the stamped version), and confirms exit-code propagation (80)This PR is also the first real test of whether Actions now dispatches on this fork.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.