Refactor release workflow into separate build and publish pipelines#984
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the monolithic release workflow with modular CI: adds build-platforms, publish, and build_and_verify workflows; introduces packages/build-envio tools to build, verify, and produce platform-specific npm artifacts; removes old package.json templates; refactors verify workflow to consume uploaded artifacts and sequence tests after builds. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Build as Build Jobs
participant Art as Artifact Storage
participant Pack as build-envio (Package Builder)
participant Test as Test Jobs
participant Pub as Publish Job
GH->>Build: run platform build matrix (linux/darwin variants)
Build->>Art: upload built binaries & platform npm packages
GH->>Pack: build-envio job downloads binary artifacts
Pack->>Art: download binary artifacts
Pack->>Art: upload envio npm package artifact
GH->>Test: template/scenario/e2e jobs depend on envio package
Test->>Art: download envio binary & npm artifact
Test->>Test: execute tests using downloaded artifacts
GH->>Pub: on release, locate Build & Verify run and download artifacts
Pub->>Art: download 4 platform packages + envio package
Pub->>Pub: patch package.json versions and determine npm tag
Pub->>Registry: publish platform packages and envio package
sequenceDiagram
participant Release as GitHub Release
participant Publish as Publish Workflow
participant CI as Build & Verify
participant NPM as NPM Registry
Release->>Publish: trigger on release creation
Publish->>CI: locate successful Build & Verify run for commit
CI->>Publish: provide artifact URLs
Publish->>Publish: patch package.json versions & determine npm tag
Publish->>NPM: publish platform packages and envio package
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/verify.yml (1)
4-75:⚠️ Potential issue | 🔴 CriticalAvoid running untrusted artifacts with secrets via
workflow_run.
workflow_runruns with full secrets, but it downloads and executes artifacts produced by the Build workflow. For PRs from forks, this allows untrusted binaries to run withENVIO_API_TOKEN/DISCORD_WEBHOOKaccess. Gate this workflow to trusted branches/events or run PR verification without secrets/artifact execution.🔒 Example hardening (limit to trusted runs)
on: workflow_run: workflows: ["Build"] types: - completed + branches: + - main jobs: check-build: runs-on: ubuntu-latest - if: github.event.workflow_run.conclusion == 'success' + if: github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/verify.yml around lines 4 - 75, The workflow currently triggers on workflow_run and exposes secrets (ENVIO_API_TOKEN) and runs downloaded artifacts (see workflow_run trigger, env ENVIO_API_TOKEN, and the template-tests job's Download envio binary / Add envio to PATH steps), which lets untrusted forked workflows execute binaries with secrets; fix by gating execution to trusted runs: restrict the top-level trigger or add an if-check on sensitive jobs/steps (e.g., template-tests and any job that consumes artifacts or secrets) to ensure github.event.workflow_run.head_repository.owner.login == github.repository_owner (or another trusted condition), and reduce permissions by removing secrets/env access for untrusted triggers or scoping permissions to read-only for non-sensitive jobs.
🧹 Nitpick comments (1)
packages/build-envio/src/build-artifact.ts (1)
176-181: Prefer the canonical ESM main-module guard overendsWith.The
endsWithcheck is brittle — it can produce a false positive if any parent path segment happens to end inbuild-artifact.ts. The idiomatic ESM approach compares URLs:♻️ Suggested fix
+import { pathToFileURL } from "node:url"; -if ( - process.argv[1] && - (process.argv[1].endsWith("build-artifact.ts") || - process.argv[1].endsWith("build-artifact.js")) -) { +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/build-envio/src/build-artifact.ts` around lines 176 - 181, Replace the brittle endsWith guard with the canonical ESM main-module guard: compute the current file path using fileURLToPath(import.meta.url) (import fileURLToPath from 'url' if not already imported) and compare it to process.argv[1]; call main() only when they are equal (e.g. if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { main(); }). This ensures the check around main() in build-artifact.ts is exact and avoids false positives from parent path segments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-platforms.yml:
- Around line 25-56: The workflow uses deprecated actions: replace
actions-rs/toolchain@v1 and actions-rs/cargo@v1 with maintained Node16+
compatible alternatives (e.g., use the official rust setup action such as
rust-lang/setup-rust or another actively maintained setup/cargo action) and
update Swatinem/rust-cache@v1 to Swatinem/rust-cache@v2; update the use blocks
for the Install toolchain step (actions-rs/toolchain), Build binaries step
(actions-rs/cargo), and the rust cache step (Swatinem/rust-cache) so the new
actions preserve the same inputs (toolchain/target/override, command/args/env,
cache-on-failure) and environment variables.
In @.github/workflows/build.yml:
- Around line 42-48: The workflow step using the archived action
actions-rs/cargo@v1 should be replaced with a plain run step that invokes cargo
directly: remove the uses: actions-rs/cargo@v1 key and add a run: cargo build
--release --bins --target x86_64-unknown-linux-musl (preserving the
SVM_TARGET_PLATFORM env and any args semantics shown), and ensure the Rust
toolchain is installed earlier in the job (e.g., add a setup rust toolchain step
such as actions-rs/toolchain or rust-lang/setup-rust) so the direct cargo
invocation in the "Build envio CLI" step succeeds.
In @.github/workflows/publish.yml:
- Around line 93-109: The "Patch version and publish" step currently uses npm;
change it to use pnpm per repo policy: before calling publish, install/enable
pnpm (e.g. run "corepack enable && corepack prepare pnpm@latest --activate") in
the same job, then replace the "npm publish --access public" invocation with
"pnpm publish --access public" and keep the existing environment vars
(NODE_AUTH_TOKEN, VERSION) and version-patching node script intact so the job
behavior is unchanged except using pnpm; apply the same replacement for the
other publish job referenced in the comment.
In `@packages/build-envio/src/build-artifact.test.ts`:
- Around line 134-165: The tests "generates linux-x64 package.json" and
"generates darwin-arm64 package.json" spread assertions across many expect()
calls; consolidate each test to a single assertion that compares the entire
parsed package object (pkg) to the expected object. Update the first test (using
variables out, execSync, pkg) to assert expect(pkg).toEqual({ name:
"envio-linux-x64", version: "v1.2.3", os: ["linux"], cpu: ["x64"], ...any other
expected fields }); and update the second test similarly to assert the full
object (name, os, cpu, license, repository, etc.) in one
expect(pkg).toEqual(...). Ensure all existing expected fields are included in
each single expected object so the tests remain equivalent.
- Around line 220-253: The test "produces a publish-ready package.json from the
real dev package" performs many individual expects on the parsed package
(variable result); replace these sequential asserts with a single assertion
using toMatchObject to validate expected fields and preserved values (version,
name, type, main, types, bin, optionalDependencies, keywords, dependencies) and
a separate minimal assertion for files if needed (e.g., contains README.md and
svm.schema.json and not local-bin.mjs). Update the it block that calls
build(...) and reads result to use expect(result).toMatchObject({...})
referencing result and devPkg so the overall structure and preserved
dependencies are asserted in one call rather than multiple expect lines.
- Around line 20-115: The tests in the buildPackageJson describe block are split
into many single-field assertions; collapse them into a few assertions that
validate the whole returned object at once (use toMatchObject / toEqual against
an expected object shape) while keeping a dedicated test for immutability;
update tests that currently call buildPackageJson(devPkg, ...) repeatedly to
build a single result variable per it and replace per-field expects (version,
private, scripts, bin, keywords, optionalDependencies, files entries,
dependencies, name/type/main/types/repository/engines) with one or two
comprehensive assertions (e.g., expect(result).toMatchObject({...}) and
expect(result.files).toEqual([...]) or expect(result).toContain(...)) and
preserve the existing "does not mutate the input" check using a deep-cloned
original of devPkg.
---
Outside diff comments:
In @.github/workflows/verify.yml:
- Around line 4-75: The workflow currently triggers on workflow_run and exposes
secrets (ENVIO_API_TOKEN) and runs downloaded artifacts (see workflow_run
trigger, env ENVIO_API_TOKEN, and the template-tests job's Download envio binary
/ Add envio to PATH steps), which lets untrusted forked workflows execute
binaries with secrets; fix by gating execution to trusted runs: restrict the
top-level trigger or add an if-check on sensitive jobs/steps (e.g.,
template-tests and any job that consumes artifacts or secrets) to ensure
github.event.workflow_run.head_repository.owner.login == github.repository_owner
(or another trusted condition), and reduce permissions by removing secrets/env
access for untrusted triggers or scoping permissions to read-only for
non-sensitive jobs.
---
Nitpick comments:
In `@packages/build-envio/src/build-artifact.ts`:
- Around line 176-181: Replace the brittle endsWith guard with the canonical ESM
main-module guard: compute the current file path using
fileURLToPath(import.meta.url) (import fileURLToPath from 'url' if not already
imported) and compare it to process.argv[1]; call main() only when they are
equal (e.g. if (process.argv[1] && fileURLToPath(import.meta.url) ===
process.argv[1]) { main(); }). This ensures the check around main() in
build-artifact.ts is exact and avoids false positives from parent path segments.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/build-platforms.yml.github/workflows/build.yml.github/workflows/publish.yml.github/workflows/release.yml.github/workflows/verify.ymlpackages/build-envio/build-platform-package.tspackages/build-envio/package.jsonpackages/build-envio/src/build-artifact.test.tspackages/build-envio/src/build-artifact.tspackages/build-envio/tsconfig.jsonpackages/build-envio/vitest.config.tspackages/cli/npm/package.json.tmplpackages/envio/package.json.tmpl
💤 Files with no reviewable changes (3)
- packages/cli/npm/package.json.tmpl
- .github/workflows/release.yml
- packages/envio/package.json.tmpl
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/build.yml (1)
42-48: Replace archived actions-rs/cargo@v1 (already flagged).This was already noted in the prior review; please switch to a maintained Rust setup action and invoke
cargodirectly as previously suggested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 42 - 48, Replace the archived uses: actions-rs/cargo@v1 step named "Build envio CLI" with a maintained Rust setup action (e.g., rust-lang/setup-rust or actions/setup-rust) and invoke cargo directly; keep the env SVM_TARGET_PLATFORM, configure the setup action to install the required toolchain, and replace the with: command/args block by a run: cargo build --release --bins --target x86_64-unknown-linux-musl so the workflow uses a supported setup action and runs cargo directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 65-83: The workflow currently invokes the TypeScript script
packages/build-envio/build-platform-package.ts directly with node; update the
job to install pnpm and the package deps and run the script with a TypeScript
runner: add a step that runs corepack enable && corepack prepare pnpm@latest
--activate then run pnpm install (or pnpm install --filter
./packages/build-envio... to install only that package), and replace the node
invocation with pnpm dlx tsx packages/build-envio/build-platform-package.ts
--version "0.0.0-build" --platform "linux" --arch "x64" --out "envio-linux-x64"
so the build-platform-package.ts script executes with tsx and required deps are
installed.
---
Duplicate comments:
In @.github/workflows/build.yml:
- Around line 42-48: Replace the archived uses: actions-rs/cargo@v1 step named
"Build envio CLI" with a maintained Rust setup action (e.g.,
rust-lang/setup-rust or actions/setup-rust) and invoke cargo directly; keep the
env SVM_TARGET_PLATFORM, configure the setup action to install the required
toolchain, and replace the with: command/args block by a run: cargo build
--release --bins --target x86_64-unknown-linux-musl so the workflow uses a
supported setup action and runs cargo directly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
.github/workflows/build.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
.github/workflows/build_and_verify.yml (2)
49-81:⚠️ Potential issue | 🔴 Critical
build-linux-packagewill likely fail:node packages/build-envio/build-platform-package.tsruns TS without a runner.Same issue as in Build Platforms: Line 70 invokes a
.tsfile directly withnode, and this job never installs build tooling deps.Proposed fix (install pnpm + deps; run with tsx)
build-linux-package: @@ steps: - uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 24 + cache: "pnpm" + + - uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: false + + - name: Install build-envio dependencies + run: pnpm install --frozen-lockfile --filter ./packages/build-envio... - name: Download linux-x64 binary uses: actions/download-artifact@v4 with: name: envio-binary-linux-x64 path: ./binary - name: Build platform npm package run: | - node packages/build-envio/build-platform-package.ts \ - --version "0.0.0-build" --platform "linux" --arch "x64" --out "envio-linux-x64" + pnpm --dir packages/build-envio exec tsx build-platform-package.ts \ + --version "0.0.0-build" --platform "linux" --arch "x64" --out "../../envio-linux-x64" mkdir -p envio-linux-x64/bin cp ./binary/envio envio-linux-x64/bin/ chmod +x envio-linux-x64/bin/envioBased on learnings: "Applies to **/{package.json,pnpm-lock.yaml,.npmrc,.pnpmfile.cjs,.yml,.yaml} : Use
pnpmovernpm/npx".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 49 - 81, The CI job build-linux-package currently invokes the TypeScript script packages/build-envio/build-platform-package.ts directly with node (the command in the Build platform npm package step), which will fail because no TS runner or project deps are installed; add a step to install pnpm (or use actions/setup-node + pnpm), run pnpm install to fetch devDependencies, and replace the direct `node packages/build-envio/build-platform-package.ts` invocation with a TypeScript runner (e.g., tsx via pnpm exec tsx or pnpm run script) so packages/build-envio/build-platform-package.ts executes with the proper toolchain and dependencies available.
103-129:⚠️ Potential issue | 🔴 Critical
build-envio-packagealso runs TypeScript vianode(Line 122) — switch to a TS runner.Line 122 calls
node packages/build-envio/src/build-artifact.ts .... Unless Node is configured to execute TS, this breaks your core artifact generation.Proposed fix
- name: Build envio package - run: node packages/build-envio/src/build-artifact.ts --version 0.0.0-build --out envio-dist + run: pnpm --dir packages/build-envio exec tsx src/build-artifact.ts --version 0.0.0-build --out ../../envio-dist🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 103 - 129, The workflow step named "Build envio package" currently invokes the TypeScript file build-artifact.ts directly with node; update that step to execute the script with a TypeScript runner (e.g., use pnpm ts-node, npx ts-node or pnpm dlx ts-node) so the .ts file is executed correctly, or alternatively precompile packages/build-envio/src/build-artifact.ts to JS as part of the job and run the compiled JS with node; ensure the replacement preserves the same CLI flags (--version and --out) and the step name "Build envio package" and target file build-artifact.ts are the references you modify.packages/build-envio/src/build-artifact.test.ts (2)
68-121:⚠️ Potential issue | 🔴 CriticalThese tests spawn
node …/build-platform-package.ts: will fail if Node can’t execute TS in subprocesses.Even if vitest handles TS for the test runner, the subprocess is plain
node(Lines 86-87, 108-109). Use the same TS execution strategy you choose for CI (e.g.,pnpm exec tsx …) or test via an imported API (preferred if available).Example adjustment (pnpm + tsx)
it("generates linux-x64 package.json and copies README", () => { const out = path.join(tmpDir, "envio-linux-x64"); execSync( - `node ${scriptPath} --version v1.2.3 --platform linux --arch x64 --out "${out}"`, + `pnpm --dir ${path.dirname(scriptPath)} exec tsx build-platform-package.ts --version v1.2.3 --platform linux --arch x64 --out "${out}"`, ); @@ it("generates darwin-arm64 package.json", () => { const out = path.join(tmpDir, "envio-darwin-arm64"); execSync( - `node ${scriptPath} --version v1.0.0 --platform darwin --arch arm64 --out "${out}"`, + `pnpm --dir ${path.dirname(scriptPath)} exec tsx build-platform-package.ts --version v1.0.0 --platform darwin --arch arm64 --out "${out}"`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/build-envio/src/build-artifact.test.ts` around lines 68 - 121, Tests invoke the TypeScript script via execSync using plain `node` (see the scriptPath constant and the execSync calls in the two it blocks), which fails when Node can't run TS files; change the tests to either spawn the script with the project's TS runner (e.g., replace `node ${scriptPath}` with `pnpm exec tsx ${scriptPath}` or equivalent CI command) or—preferred—avoid subprocesses by importing the build script's API and calling its exported function(s) directly (update the two execSync usages to call the imported function with the same args or invoke the TS runner command string instead).
21-57:⚠️ Potential issue | 🟠 MajorTests still violate the single-assert guideline; consolidate to whole-value assertions.
Several
itblocks assert many individual fields / file-existence checks (e.g., Lines 42-47, 91-102, 167-179, 234-255, etc.). Please consolidate each test to one “whole value” assertion by building an expected object (or an “exists map”) and comparing once.Example refactor pattern (one expect per test)
it("rewrites dev package.json for publishing", () => { const result = buildPackageJson(devPkg, "v1.2.3", "v1.2.3"); - expect(result).toMatchObject({ - name: "envio", - version: "v1.2.3", - type: "module", - main: "./index.js", - types: "./index.d.ts", - bin: "./bin.js", - repository: devPkg.repository, - engines: { node: ">=22.0.0" }, - dependencies: devPkg.dependencies, - keywords: ["blockchain", "indexer", "ethereum", "evm", "fuel", "data", "dapp"], - optionalDependencies: { - "envio-linux-x64": "v1.2.3", - "envio-linux-arm64": "v1.2.3", - "envio-darwin-x64": "v1.2.3", - "envio-darwin-arm64": "v1.2.3", - }, - }); - expect(result.private).toBeUndefined(); - expect(result.scripts).toBeUndefined(); - expect(result.files).toEqual([ - "bin.js", "evm.schema.json", "fuel.schema.json", "svm.schema.json", - "rescript.json", "index.d.ts", "index.js", "src", "README.md", - ]); + const expected = { + ...devPkg, + version: "v1.2.3", + bin: "./bin.js", + keywords: ["blockchain", "indexer", "ethereum", "evm", "fuel", "data", "dapp"], + optionalDependencies: { + "envio-linux-x64": "v1.2.3", + "envio-linux-arm64": "v1.2.3", + "envio-darwin-x64": "v1.2.3", + "envio-darwin-arm64": "v1.2.3", + }, + files: [ + "bin.js", "evm.schema.json", "fuel.schema.json", "svm.schema.json", + "rescript.json", "index.d.ts", "index.js", "src", "README.md", + ], + } as Record<string, unknown>; + delete (expected as any).private; + delete (expected as any).scripts; + expect(result).toEqual(expected); });As per coding guidelines: "
**/*.{test.res,test.js,test.ts,test.tsx,spec.res,spec.js,spec.ts,spec.tsx}: Always use single assert to check the whole value instead of multiple asserts for every field" and "Prefer public module API for testing".Also applies to: 83-103, 105-121, 163-200, 218-255, 296-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/build-envio/src/build-artifact.test.ts` around lines 21 - 57, The tests in build-artifact.test.ts use many individual assertions against fields of the buildPackageJson result; instead consolidate each it block to a single whole-value assertion: for the "rewrites dev package.json for publishing" and "supports different platformPkgVersion" cases, construct an expected object (or an existence map) that represents the entire output of buildPackageJson(devPkg, version, platformPkgVersion) and replace the multiple expect(...) calls with a single expect(result).toEqual(expected) or expect(result).toMatchObject(expected) assertion. Locate the checks referencing buildPackageJson, result.private, result.scripts, result.files and optionalDependencies and merge them into that single expected object per test, removing the extra individual expects..github/workflows/build-platforms.yml (1)
52-72:⚠️ Potential issue | 🔴 CriticalDon’t run
build-platform-package.tswith plainnode; install deps and use a TS runner (also chmod the binary).Line 68 runs a
.tsfile directly vianode. Unless you’re intentionally relying on Node’s TypeScript execution (and the right flags/loader), this will fail at runtime. Also, the workflow doesn’t installpackages/build-enviodeps, which may be needed by the packager.Proposed update (pnpm + tsx runner, and ensure executable bit)
- name: Install node uses: actions/setup-node@v4 with: node-version: 24 + cache: "pnpm" + + - uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: false + + - name: Install build-envio dependencies + run: pnpm install --frozen-lockfile --filter ./packages/build-envio... - name: Build platform npm package shell: bash env: TARGET: ${{ matrix.job.target }} NAME: ${{ matrix.job.name }} VERSION: ${{ inputs.version }} run: | - cd packages/cli/npm node_os=$(echo "${NAME}" | cut -d '-' -f1) node_arch=$(echo "${NAME}" | cut -d '-' -f2) node_pkg="envio-${node_os}-${node_arch}" - node ../../../packages/build-envio/build-platform-package.ts \ - --version "${VERSION}" --platform "${node_os}" --arch "${node_arch}" --out "${node_pkg}" - mkdir -p "${node_pkg}/bin" - cp "../../../target/${TARGET}/release/envio" "${node_pkg}/bin" + pnpm --dir packages/build-envio exec tsx build-platform-package.ts \ + --version "${VERSION}" --platform "${node_os}" --arch "${node_arch}" --out "../cli/npm/${node_pkg}" + mkdir -p "packages/cli/npm/${node_pkg}/bin" + cp "target/${TARGET}/release/envio" "packages/cli/npm/${node_pkg}/bin/envio" + chmod +x "packages/cli/npm/${node_pkg}/bin/envio"Node.js v24: can `node path/to/script.ts` execute TypeScript without flags/loader? If not, what’s the recommended approach (e.g., `--experimental-strip-types` vs `tsx`)?#!/bin/bash set -euo pipefail # Find any direct node execution of .ts in workflows rg -n '(^|\s)node\s+[^|]*\.(ts)\b' .github/workflows -S || true # Check whether build-envio declares a TS runner cat packages/build-envio/package.json | rg -n '"tsx"|"ts-node"' || trueBased on learnings: "Applies to **/{package.json,pnpm-lock.yaml,.npmrc,.pnpmfile.cjs,.yml,.yaml} : Use
pnpmovernpm/npx".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-platforms.yml around lines 52 - 72, The workflow runs build-platform-package.ts directly with node and doesn't install packages/build-envio deps, which will fail; change the Build platform step to install pnpm and run pnpm install in packages/build-envio (or workspace root) and invoke the TypeScript script with a proper TS runner (e.g., run it via tsx or the package script from packages/build-envio rather than plain node, referencing build-platform-package.ts), and after copying the built binary set the executable bit (chmod +x) on envio to ensure it is runnable; update the step that currently references build-platform-package.ts and the cp of target/${TARGET}/release/envio accordingly.
🧹 Nitpick comments (4)
packages/build-envio/src/build-artifact.ts (1)
38-46:REPO_ROOTresolution is brittle; preferfileURLToPath(import.meta.url)overimport.meta.dirname/URL pathname.
import.meta.dirnameis non-standard, andnew URL(import.meta.url).pathnamecan be percent-encoded (spaces, etc.). UsingfileURLToPathis the robust Node ESM pattern.Proposed change
import * as fs from "node:fs"; import * as path from "node:path"; import { execSync } from "node:child_process"; import { parseArgs } from "node:util"; +import { fileURLToPath } from "node:url"; @@ -const REPO_ROOT = path.resolve( - import.meta.dirname ?? path.dirname(new URL(import.meta.url).pathname), - "../../../" -); +const REPO_ROOT = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../" +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/build-envio/src/build-artifact.ts` around lines 38 - 46, REPO_ROOT resolution is brittle; replace the current import.meta.dirname/new URL(import.meta.url).pathname logic used to compute REPO_ROOT (the const named REPO_ROOT) with the robust Node ESM pattern using fileURLToPath(import.meta.url) and path.dirname: import fileURLToPath from 'url' and compute const __filename = fileURLToPath(import.meta.url) then const __dirname = path.dirname(__filename) and use that to resolve REPO_ROOT so ENVIO_DIR and README_PATH remain correct and avoid percent-encoding/non-standard import.meta.dirname issues..github/workflows/publish.yml (1)
34-47: Harden publish job: set explicit permissions and pin pnpm version (avoidpnpm@latest).
- You’re using cross-workflow artifact download via
run-idandgh api; consider explicitly settingpermissions: actions: readto avoid repo-default permission drift.corepack prepare pnpm@latest --activatecan introduce non-reproducible publishing behavior; you already standardized on pnpm 9 elsewhere.Suggested tightening
name: Publish to NPM +permissions: + contents: read + actions: read @@ - name: Enable pnpm - run: corepack enable && corepack prepare pnpm@latest --activate + uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: false @@ - name: Enable pnpm - run: corepack enable && corepack prepare pnpm@latest --activate + uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: falseBased on learnings: "Applies to **/{package.json,pnpm-lock.yaml,.npmrc,.pnpmfile.cjs,.yml,.yaml} : Use
pnpmovernpm/npx".Also applies to: 69-71, 115-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 34 - 47, Add explicit repository permissions and pin pnpm to v9: update the publish job to include permissions: actions: read (so the cross-workflow artifact download using the find_build step/id find_build and gh api call keeps working even if repo defaults change) and replace any uses of corepack prepare pnpm@latest --activate (and any other pnpm@latest invocations referenced around the steps at ~69-71 and ~115-117) with a pinned version such as corepack prepare pnpm@9 --activate (or install pnpm 9 explicitly) to ensure reproducible publishing behavior..github/workflows/build_and_verify.yml (1)
114-116: Usepnpm install --frozen-lockfilein CI for deterministic builds.All jobs currently run
pnpm installwithout lockfile enforcement. If the lockfile diverges, CI can “work” locally but fail (or drift) in automation.Also applies to: 194-196, 272-274, 378-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 114 - 116, Replace the loose install step used by the "Install dependencies" GitHub Actions job(s) with a lockfile-enforcing command: change the step that currently runs "pnpm install" in each affected job to run "pnpm install --frozen-lockfile" so CI fails fast if the lockfile and package manifest diverge; locate the steps named "Install dependencies" (the ones with run: pnpm install) and update their run commands accordingly..github/workflows/build-platforms.yml (1)
17-39: Verify darwin-x64 builds onmacos-latest(runner arch drift can break cross-target linking).You’re building both
x86_64-apple-darwinandaarch64-apple-darwinonmacos-latest(Lines 18-19). Ifmacos-latestis arm64 (common), the x64 build may require additional linker/SDK configuration or using an Intel runner (e.g.,macos-13) for the x64 leg.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-platforms.yml around lines 17 - 39, The macOS x86_64 build ("darwin-x64", target "x86_64-apple-darwin") may fail on an arm64 GitHub runner; update the workflow to guarantee an Intel runner or proper cross-linking when matrix.job.target == "x86_64-apple-darwin": either set runs-on to an Intel macOS image (e.g., "macos-13") for that matrix entry, or add explicit linker/SDK/arch setup (Rosetta install, SDKROOT/MACOSX_DEPLOYMENT_TARGET and appropriate CARGO_TARGET_* linker flags) in the Apple setup step that checks matrix.job.target; reference the matrix.job.target, the job names "darwin-x64" and "darwin-arm64", and the "Apple M1 setup" step when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build_and_verify.yml:
- Around line 223-233: The CI workflow exposes a test-only secret in the hasura
service env var HASURA_GRAPHQL_DATABASE_URL which triggers Checkov CKV_SECRET_4;
to fix, either suppress the finding or avoid embedding the credential directly
by constructing the URL from separate non-secret env vars (e.g., POSTGRES_USER,
POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB) and reference that assembled var
in the hasura env block, or add a Checkov suppression comment/metadata for this
test-only credential so the scanner ignores it; update the workflow to use one
of these approaches and ensure HASURA_GRAPHQL_DATABASE_URL is no longer a
hardcoded secret.
---
Duplicate comments:
In @.github/workflows/build_and_verify.yml:
- Around line 49-81: The CI job build-linux-package currently invokes the
TypeScript script packages/build-envio/build-platform-package.ts directly with
node (the command in the Build platform npm package step), which will fail
because no TS runner or project deps are installed; add a step to install pnpm
(or use actions/setup-node + pnpm), run pnpm install to fetch devDependencies,
and replace the direct `node packages/build-envio/build-platform-package.ts`
invocation with a TypeScript runner (e.g., tsx via pnpm exec tsx or pnpm run
script) so packages/build-envio/build-platform-package.ts executes with the
proper toolchain and dependencies available.
- Around line 103-129: The workflow step named "Build envio package" currently
invokes the TypeScript file build-artifact.ts directly with node; update that
step to execute the script with a TypeScript runner (e.g., use pnpm ts-node, npx
ts-node or pnpm dlx ts-node) so the .ts file is executed correctly, or
alternatively precompile packages/build-envio/src/build-artifact.ts to JS as
part of the job and run the compiled JS with node; ensure the replacement
preserves the same CLI flags (--version and --out) and the step name "Build
envio package" and target file build-artifact.ts are the references you modify.
In @.github/workflows/build-platforms.yml:
- Around line 52-72: The workflow runs build-platform-package.ts directly with
node and doesn't install packages/build-envio deps, which will fail; change the
Build platform step to install pnpm and run pnpm install in packages/build-envio
(or workspace root) and invoke the TypeScript script with a proper TS runner
(e.g., run it via tsx or the package script from packages/build-envio rather
than plain node, referencing build-platform-package.ts), and after copying the
built binary set the executable bit (chmod +x) on envio to ensure it is
runnable; update the step that currently references build-platform-package.ts
and the cp of target/${TARGET}/release/envio accordingly.
In `@packages/build-envio/src/build-artifact.test.ts`:
- Around line 68-121: Tests invoke the TypeScript script via execSync using
plain `node` (see the scriptPath constant and the execSync calls in the two it
blocks), which fails when Node can't run TS files; change the tests to either
spawn the script with the project's TS runner (e.g., replace `node
${scriptPath}` with `pnpm exec tsx ${scriptPath}` or equivalent CI command)
or—preferred—avoid subprocesses by importing the build script's API and calling
its exported function(s) directly (update the two execSync usages to call the
imported function with the same args or invoke the TS runner command string
instead).
- Around line 21-57: The tests in build-artifact.test.ts use many individual
assertions against fields of the buildPackageJson result; instead consolidate
each it block to a single whole-value assertion: for the "rewrites dev
package.json for publishing" and "supports different platformPkgVersion" cases,
construct an expected object (or an existence map) that represents the entire
output of buildPackageJson(devPkg, version, platformPkgVersion) and replace the
multiple expect(...) calls with a single expect(result).toEqual(expected) or
expect(result).toMatchObject(expected) assertion. Locate the checks referencing
buildPackageJson, result.private, result.scripts, result.files and
optionalDependencies and merge them into that single expected object per test,
removing the extra individual expects.
---
Nitpick comments:
In @.github/workflows/build_and_verify.yml:
- Around line 114-116: Replace the loose install step used by the "Install
dependencies" GitHub Actions job(s) with a lockfile-enforcing command: change
the step that currently runs "pnpm install" in each affected job to run "pnpm
install --frozen-lockfile" so CI fails fast if the lockfile and package manifest
diverge; locate the steps named "Install dependencies" (the ones with run: pnpm
install) and update their run commands accordingly.
In @.github/workflows/build-platforms.yml:
- Around line 17-39: The macOS x86_64 build ("darwin-x64", target
"x86_64-apple-darwin") may fail on an arm64 GitHub runner; update the workflow
to guarantee an Intel runner or proper cross-linking when matrix.job.target ==
"x86_64-apple-darwin": either set runs-on to an Intel macOS image (e.g.,
"macos-13") for that matrix entry, or add explicit linker/SDK/arch setup
(Rosetta install, SDKROOT/MACOSX_DEPLOYMENT_TARGET and appropriate
CARGO_TARGET_* linker flags) in the Apple setup step that checks
matrix.job.target; reference the matrix.job.target, the job names "darwin-x64"
and "darwin-arm64", and the "Apple M1 setup" step when making the change.
In @.github/workflows/publish.yml:
- Around line 34-47: Add explicit repository permissions and pin pnpm to v9:
update the publish job to include permissions: actions: read (so the
cross-workflow artifact download using the find_build step/id find_build and gh
api call keeps working even if repo defaults change) and replace any uses of
corepack prepare pnpm@latest --activate (and any other pnpm@latest invocations
referenced around the steps at ~69-71 and ~115-117) with a pinned version such
as corepack prepare pnpm@9 --activate (or install pnpm 9 explicitly) to ensure
reproducible publishing behavior.
In `@packages/build-envio/src/build-artifact.ts`:
- Around line 38-46: REPO_ROOT resolution is brittle; replace the current
import.meta.dirname/new URL(import.meta.url).pathname logic used to compute
REPO_ROOT (the const named REPO_ROOT) with the robust Node ESM pattern using
fileURLToPath(import.meta.url) and path.dirname: import fileURLToPath from 'url'
and compute const __filename = fileURLToPath(import.meta.url) then const
__dirname = path.dirname(__filename) and use that to resolve REPO_ROOT so
ENVIO_DIR and README_PATH remain correct and avoid percent-encoding/non-standard
import.meta.dirname issues.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.github/workflows/build-platforms.yml.github/workflows/build_and_verify.yml.github/workflows/publish.ymlpackages/build-envio/build-platform-package.tspackages/build-envio/src/build-artifact.test.tspackages/build-envio/src/build-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/build-envio/build-platform-package.ts
Comment on lines
+223
to
+233
| hasura: | ||
| image: hasura/graphql-engine:v2.43.0 | ||
| env: | ||
| HASURA_GRAPHQL_DATABASE_URL: postgres://postgres:testing@postgres:5432/envio-dev | ||
| HASURA_GRAPHQL_ENABLE_CONSOLE: "true" | ||
| HASURA_GRAPHQL_ENABLED_LOG_TYPES: startup, http-log, webhook-log, websocket-log, query-log | ||
| HASURA_GRAPHQL_NO_OF_RETRIES: 10 | ||
| HASURA_GRAPHQL_ADMIN_SECRET: testing | ||
| HASURA_GRAPHQL_STRINGIFY_NUMERIC_TYPES: "true" | ||
| HASURA_GRAPHQL_UNAUTHORIZED_ROLE: public | ||
| PORT: 8080 |
Contributor
There was a problem hiding this comment.
Checkov CKV_SECRET_4: consider suppressing or clearly marking test-only credentials.
Static analysis flagged credentials in HASURA_GRAPHQL_DATABASE_URL (Line 226). If Checkov is gating, you may need a suppression comment or to assemble the URL from non-flagged env vars to avoid noisy failures—especially since this is a test-only container credential.
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 226-227: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build_and_verify.yml around lines 223 - 233, The CI
workflow exposes a test-only secret in the hasura service env var
HASURA_GRAPHQL_DATABASE_URL which triggers Checkov CKV_SECRET_4; to fix, either
suppress the finding or avoid embedding the credential directly by constructing
the URL from separate non-secret env vars (e.g., POSTGRES_USER,
POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB) and reference that assembled var
in the hasura env block, or add a Checkov suppression comment/metadata for this
test-only credential so the scanner ignores it; update the workflow to use one
of these approaches and ensure HASURA_GRAPHQL_DATABASE_URL is no longer a
hardcoded secret.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/build_and_verify.yml (2)
48-50:⚠️ Potential issue | 🟠 MajorExecute TypeScript build scripts with a TS runner and installed deps.
nodecan’t run.tsdirectly. These steps will fail unless the scripts are precompiled or a TS runner is installed. Also, thebuild-linux-packagejob doesn’t install build-envio deps before invoking the script. Please install deps and run viatsx(or point to compiled JS).✅ Suggested fix (pnpm + tsx)
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 24 + - uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: false + + - name: Install build-envio deps + run: pnpm install --filter ./packages/build-envio... + - name: Build platform npm package run: | - node packages/build-envio/build-platform-package.ts \ + pnpm --dir packages/build-envio exec tsx build-platform-package.ts \ --version "0.0.0-build" --platform "linux" --arch "x64" --out "envio-linux-x64"- name: Build envio package - run: node packages/build-envio/src/build-artifact.ts --version 0.0.0-build --out envio-dist + run: pnpm --dir packages/build-envio exec tsx src/build-artifact.ts --version 0.0.0-build --out envio-dist - name: Verify artifact - run: node packages/build-envio/src/verify-artifact.ts envio-dist + run: pnpm --dir packages/build-envio exec tsx src/verify-artifact.ts envio-distVerification script (confirm compiled JS / TS runner availability):
#!/bin/bash # Check for TS runner or compiled JS output in build-envio cat packages/build-envio/package.json rg -n 'tsx|ts-node' packages/build-envio/package.json fd -t f 'build-platform-package\.(ts|js)$' packages/build-envio fd -t f 'build-artifact\.(ts|js)$' packages/build-envio find packages/build-envio -maxdepth 2 -type d -name 'dist' -o -name 'lib'Also applies to: 95-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 48 - 50, The workflow currently invokes the TypeScript script directly with node which fails because .ts files aren’t executable; update the build-linux-package job to install the package dependencies for packages/build-envio (e.g., run pnpm install or pnpm --filter ./packages/build-envio install) and invoke the script with a TS runner (e.g., tsx) or point to the compiled JS output instead of calling node on packages/build-envio/build-platform-package.ts; ensure similar fixes are applied to the other occurrences referenced around lines 95-99 so the job installs deps before running build-platform-package.ts or uses packages/build-envio/dist/build-platform-package.js if you precompile.
200-203:⚠️ Potential issue | 🟡 MinorAvoid inline DB credentials to satisfy Checkov.
Checkov CKV_SECRET_4 flags the inline credential in
HASURA_GRAPHQL_DATABASE_URL. Prefer assembling it from non-secret env vars or add a suppression if this is strictly test-only.🔧 Example suppression
hasura: image: hasura/graphql-engine:v2.43.0 env: + # checkov:skip=CKV_SECRET_4: test-only credential for local CI service HASURA_GRAPHQL_DATABASE_URL: postgres://postgres:testing@postgres:5432/envio-dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 200 - 203, The HASURA_GRAPHQL_DATABASE_URL is flagged by Checkov CKV_SECRET_4 for inline DB credentials; change the workflow to build the URL from non-secret env vars (e.g., HASURA_DB_USER, HASURA_DB_PASSWORD, HASURA_DB_HOST, HASURA_DB_NAME) and set HASURA_GRAPHQL_DATABASE_URL by interpolating those variables, or move the sensitive value into a secret (e.g., github secret) and reference that secret instead; alternatively, if this is strictly test-only and you accept the risk, add a Checkov suppression comment for CKV_SECRET_4 near the HASURA_GRAPHQL_DATABASE_URL declaration to silence the warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/build-envio/src/verify-artifact.ts`:
- Around line 49-57: The package.json read/parse can throw and currently aborts
error aggregation; wrap the read+JSON.parse for pkgPath in a try/catch inside
verify-artifact.ts, catch any errors and push a descriptive message onto the
existing errors array (e.g. "package.json read/parse error: <error.message> for
pkgPath"), and only run the subsequent pkg property checks (pkg.private,
pkg.bin, pkg.optionalDependencies, pkg.dependencies) if parsing succeeded (pkg
is defined); this ensures errors[] is always populated and the script reports a
clean failure summary.
---
Duplicate comments:
In @.github/workflows/build_and_verify.yml:
- Around line 48-50: The workflow currently invokes the TypeScript script
directly with node which fails because .ts files aren’t executable; update the
build-linux-package job to install the package dependencies for
packages/build-envio (e.g., run pnpm install or pnpm --filter
./packages/build-envio install) and invoke the script with a TS runner (e.g.,
tsx) or point to the compiled JS output instead of calling node on
packages/build-envio/build-platform-package.ts; ensure similar fixes are applied
to the other occurrences referenced around lines 95-99 so the job installs deps
before running build-platform-package.ts or uses
packages/build-envio/dist/build-platform-package.js if you precompile.
- Around line 200-203: The HASURA_GRAPHQL_DATABASE_URL is flagged by Checkov
CKV_SECRET_4 for inline DB credentials; change the workflow to build the URL
from non-secret env vars (e.g., HASURA_DB_USER, HASURA_DB_PASSWORD,
HASURA_DB_HOST, HASURA_DB_NAME) and set HASURA_GRAPHQL_DATABASE_URL by
interpolating those variables, or move the sensitive value into a secret (e.g.,
github secret) and reference that secret instead; alternatively, if this is
strictly test-only and you accept the risk, add a Checkov suppression comment
for CKV_SECRET_4 near the HASURA_GRAPHQL_DATABASE_URL declaration to silence the
warning.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build_and_verify.ymlpackages/build-envio/src/build-artifact.test.tspackages/build-envio/src/verify-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/build-envio/src/build-artifact.test.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/build_and_verify.yml (2)
200-210:⚠️ Potential issue | 🟡 MinorAvoid hardcoded DB credentials to satisfy Checkov (CKV_SECRET_4).
Line 203 embeds credentials in
HASURA_GRAPHQL_DATABASE_URL, which is known to trip Checkov. Consider a suppression comment for this test-only URL or construct it from non-secret env vars. Apply the same fix in the e2e-test hasura block (Line 309).Suggested suppression
hasura: image: hasura/graphql-engine:v2.43.0 env: - HASURA_GRAPHQL_DATABASE_URL: postgres://postgres:testing@postgres:5432/envio-dev + # checkov:skip=CKV_SECRET_4: test-only local DB credentials + HASURA_GRAPHQL_DATABASE_URL: postgres://postgres:testing@postgres:5432/envio-dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 200 - 210, The workflow currently hardcodes database credentials in the hasura service env var HASURA_GRAPHQL_DATABASE_URL (and the e2e-test hasura block), which triggers Checkov CKV_SECRET_4; update both places to avoid embedding secrets by either (A) building the URL from non-secret environment variables (e.g., POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB) or (B) pulling the full URL from a GitHub Actions secret (e.g., use a secret like ${{ secrets.HASURA_DATABASE_URL }}), and if this is strictly a test-only URL you must instead add a Checkov suppression comment for CKV_SECRET_4 scoped to the service entry; locate the hasura blocks by the service name "hasura" and the env var key "HASURA_GRAPHQL_DATABASE_URL" and apply the same change in the e2e-test hasura block.
42-50:⚠️ Potential issue | 🟠 MajorPotential CI failure:
.tsscripts executed with plainnode.Line 49 (and likewise Lines 96 and 99) invoke TypeScript files directly; unless Node 24 is explicitly configured to strip/transform types, this will fail at runtime. Please confirm the intended TS execution path or switch to a TS runner after installing build-envio deps.
Proposed fix (pnpm + tsx)
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 24 + - uses: pnpm/action-setup@v4 + with: + version: 9 + run_install: false + + - name: Install build-envio deps + run: pnpm install --filter ./packages/build-envio... + - name: Build platform npm package run: | - node packages/build-envio/build-platform-package.ts \ + pnpm --dir packages/build-envio exec tsx build-platform-package.ts \ --version "0.0.0-build" --platform "linux" --arch "x64" --out "envio-linux-x64"#!/bin/bash # Check whether a TS runner is available or Node TS flags are configured rg -n 'tsx|ts-node' rg -n 'strip-types|transform-types|loader'As per coding guidelines, Use
pnpmovernpm/npx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build_and_verify.yml around lines 42 - 50, The workflow currently invokes TypeScript scripts directly (e.g., build-platform-package.ts) with plain node which will fail; update the CI step to install the package dev deps with pnpm and execute the TS entry via a TypeScript runner (e.g., tsx or ts-node) or run the compiled JS output instead. Specifically, ensure the job installs pnpm, runs pnpm install in the repo or packages/build-envio, and replace the direct "node packages/build-envio/build-platform-package.ts" invocation with a runner invocation (e.g., "pnpm dlx tsx packages/build-envio/build-platform-package.ts" or run the built JS artifact) so build-platform-package.ts (and the other TS invocations) execute correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/build_and_verify.yml:
- Around line 200-210: The workflow currently hardcodes database credentials in
the hasura service env var HASURA_GRAPHQL_DATABASE_URL (and the e2e-test hasura
block), which triggers Checkov CKV_SECRET_4; update both places to avoid
embedding secrets by either (A) building the URL from non-secret environment
variables (e.g., POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB)
or (B) pulling the full URL from a GitHub Actions secret (e.g., use a secret
like ${{ secrets.HASURA_DATABASE_URL }}), and if this is strictly a test-only
URL you must instead add a Checkov suppression comment for CKV_SECRET_4 scoped
to the service entry; locate the hasura blocks by the service name "hasura" and
the env var key "HASURA_GRAPHQL_DATABASE_URL" and apply the same change in the
e2e-test hasura block.
- Around line 42-50: The workflow currently invokes TypeScript scripts directly
(e.g., build-platform-package.ts) with plain node which will fail; update the CI
step to install the package dev deps with pnpm and execute the TS entry via a
TypeScript runner (e.g., tsx or ts-node) or run the compiled JS output instead.
Specifically, ensure the job installs pnpm, runs pnpm install in the repo or
packages/build-envio, and replace the direct "node
packages/build-envio/build-platform-package.ts" invocation with a runner
invocation (e.g., "pnpm dlx tsx packages/build-envio/build-platform-package.ts"
or run the built JS artifact) so build-platform-package.ts (and the other TS
invocations) execute correctly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build_and_verify.ymlpackages/build-envio/src/verify-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/build-envio/src/verify-artifact.ts
- Replace release.yml/verify.yml with build_and_verify.yml, publish.yml, build-platforms.yml - Add packages/build-envio with build-artifact.ts, verify-artifact.ts, build-platform-package.ts - Move keywords (+ solana) and files list into packages/envio/package.json - Remove package.json.tmpl files (build script rewrites package.json directly) - Use 0.0.1-dev for CI build version - Install deps first, then overlay envio dist (no package.json swap) - Remove vitest from build-envio (verify-artifact.ts is sufficient) https://claude.ai/code/session_01VvSR6vCjTtv3gA8hryEkmb
DZakh
force-pushed
the
claude/fix-npm-publish-workflow-Qak6u
branch
from
b7f7bb9 to
ca0deac
Compare
This was referenced Feb 25, 2026
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
2 participants
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
This PR refactors the monolithic
release.ymlworkflow into a modular three-workflow system:build.yml,verify.yml, andpublish.yml. It also introduces a newbuild-enviopackage with TypeScript-based build tooling to replace shell script templating for npm package generation.Key Changes
Workflow Architecture
release.yml- the old monolithic release workflow that handled building, testing, and publishing in one jobbuild.yml- builds the envio CLI binary for linux-x64 and creates npm packages (base + platform-specific)build-platforms.yml- reusable workflow called by publish to build remaining platform binaries (linux-arm64, darwin-x64, darwin-arm64)publish.yml- publishes npm packages to registry, triggered by GitHub releases or manual dispatchverify.yml- now triggered bybuild.ymlcompletion viaworkflow_run, downloads pre-built binaries instead of building locallyBuild Tooling
packages/build-envio/- new TypeScript package for programmatic npm artifact generationsrc/build-artifact.ts- main build script that rewrites package.json for publishing and compiles ReScriptbuild-platform-package.ts- generates platform-specific package.json filessrc/build-artifact.test.ts) with unit and integration testsPackage Configuration
codegenerator/cli/npm/package.json.tmplandcodegenerator/cli/npm/envio/package.json.tmpl- replaced by programmatic generationpackages/build-envio/package.json- build tool configuration with test and build scriptsImplementation Details
build.ymlis reused byverify.ymlandpublish.ymlvia GitHub artifacts, eliminating redundant buildspublish.ymlverifies that both Build and Verify workflows passed before publishingpublish.ymlsupports manual triggering with custom version and npm dist-tag selectionhttps://claude.ai/code/session_01VvSR6vCjTtv3gA8hryEkmb
Summary by CodeRabbit