ci: add pkg.pr.new PR preview workflow#152
Merged
liangshuo-1 merged 12 commits intolarksuite:mainfrom Mar 31, 2026
Merged
Conversation
📝 Walkthrough🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR introduces a
Confidence Score: 4/5Safe to merge; the only remaining item is a stale comment that does not affect runtime behaviour. All three previously raised concerns (unpinned action SHA, missing pagination, missing platform binaries) have been resolved. The sole finding in this pass is a misleading TODO comment that says a subset of binaries is shipped when in fact all six are built — this is a documentation inconsistency, not a functional bug. scripts/build-pkg-pr-new.sh lines 52–53 (stale TODO comment) Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Script as build-pkg-pr-new.sh
participant PkgNew as pkg.pr.new
participant PR as Pull Request
Dev->>GH: Push / open PR (non-draft, targeting main)
GH->>Script: ./scripts/build-pkg-pr-new.sh
Script->>Script: go build x6 targets (darwin, linux, windows)
Script->>Script: Generate run.js + package.json
GH->>PkgNew: npx pkg-pr-new publish --json output.json --comment=off
PkgNew-->>GH: output.json (package URL)
GH->>PR: Find existing bot comment (github.paginate)
alt Comment exists
GH->>PR: updateComment (install URL)
else No comment yet
GH->>PR: createComment (install URL)
end
Reviews (2): Last reviewed commit: "chore: enable pkg PR build targets" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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/pkg-pr-new.yml (2)
54-65: Consider handling pagination for PRs with many comments.Using
per_page: 100without pagination could miss the existing bot comment if the PR has more than 100 comments, resulting in duplicate comments. While uncommon, this is a potential edge case.♻️ Proposed fix using paginate
- const comments = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issueNumber, - per_page: 100, - }); - - const existing = comments.data.find((comment) => + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + per_page: 100, + }); + + const existing = comments.find((comment) => comment.user?.login === "github-actions[bot]" && typeof comment.body === "string" && comment.body.startsWith("Install this PR change globally:") );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pkg-pr-new.yml around lines 54 - 65, The current call to github.rest.issues.listComments with per_page: 100 can miss older comments and cause duplicate bot posts; replace the single-page fetch with Octokit's pagination by using github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, issue_number: issueNumber, per_page: 100 }) to retrieve all comment pages into the comments array before running the existing detection logic that finds the bot comment (the existing variable and the comment.user?.login / comment.body.startsWith checks).
35-35: Pinactions/github-scriptwith SHA for consistency.Other actions in this workflow use SHA pinning (e.g.,
actions/checkout@34e114876b...), butactions/github-script@v7uses only a version tag. For supply-chain security consistency, pin this action as well.🔒 Proposed fix
- name: Comment install command - uses: actions/github-script@v7 + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: script: |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pkg-pr-new.yml at line 35, The workflow currently references the action using a floating tag "uses: actions/github-script@v7"; replace that with the corresponding pinned commit SHA (same style used for actions/checkout@<sha>) to ensure supply-chain consistency: update the line referencing actions/github-script@v7 to actions/github-script@<commit-sha> (use the official action repo's latest stable commit SHA) and verify the SHA is immutable and documented in the workflow commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build-pkg-pr-new.sh`:
- Around line 40-74: The platform/arch mappings in the generated run.js
(platformMap, archMap) advertise platforms that aren't actually built and can
produce misleading "Unsupported platform" messages; update run.js to only
include the concrete build targets you produce (e.g., restrict
platformMap/archMap to the exact platform/arch combos you build) or keep current
maps but add an existence check for the resolved binary path (the `binary`
variable) before calling execFileSync and throw a clear, actionable error via
console.error that the binary for `${platform}-${arch}` is missing and suggest
supported targets; locate symbols platformMap, archMap, binary and execFileSync
in the run.js template and implement the chosen fix there.
---
Nitpick comments:
In @.github/workflows/pkg-pr-new.yml:
- Around line 54-65: The current call to github.rest.issues.listComments with
per_page: 100 can miss older comments and cause duplicate bot posts; replace the
single-page fetch with Octokit's pagination by using
github.paginate(github.rest.issues.listComments, { owner: context.repo.owner,
repo: context.repo.repo, issue_number: issueNumber, per_page: 100 }) to retrieve
all comment pages into the comments array before running the existing detection
logic that finds the bot comment (the existing variable and the
comment.user?.login / comment.body.startsWith checks).
- Line 35: The workflow currently references the action using a floating tag
"uses: actions/github-script@v7"; replace that with the corresponding pinned
commit SHA (same style used for actions/checkout@<sha>) to ensure supply-chain
consistency: update the line referencing actions/github-script@v7 to
actions/github-script@<commit-sha> (use the official action repo's latest stable
commit SHA) and verify the SHA is immutable and documented in the workflow
commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db5da584-6b14-4f8c-ac5b-60a429d613bd
📒 Files selected for processing (2)
.github/workflows/pkg-pr-new.ymlscripts/build-pkg-pr-new.sh
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/build-pkg-pr-new.sh (1)
47-75:⚠️ Potential issue | 🟡 MinorAdd explicit missing-binary handling in generated
run.js.This still maps platforms/arches that are not built, then fails with an opaque exit when the file is missing. Please guard
binaryexistence and print supported targets before exiting.💡 Suggested patch
const path = require("path"); +const fs = require("fs"); const { execFileSync } = require("child_process"); @@ const ext = isWindows ? ".exe" : ""; const binary = path.join(__dirname, "..", "bin", `lark-cli-${platform}-${arch}${ext}`); +const supportedTargets = ["darwin-arm64", "windows-amd64"]; + +if (!fs.existsSync(binary)) { + console.error(`No prebuilt binary found for ${platform}-${arch}.`); + console.error(`Supported PR preview targets: ${supportedTargets.join(", ")}.`); + process.exit(1); +} try { execFileSync(binary, process.argv.slice(2), { stdio: "inherit" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-pkg-pr-new.sh` around lines 47 - 75, The script currently constructs binary (variable `binary`) and calls `execFileSync` without checking the file exists, causing an opaque exit; update the runtime in this file to explicitly check existence of `binary` (e.g., using fs.existsSync or fs.statSync) before calling `execFileSync`, and if missing print a clear error that includes the attempted `platform` and `arch`, the full `binary` path, and a list of supported targets derived from `platformMap` and `archMap` (iterate over their keys to show valid combos), then exit with a non-zero status; ensure references to `isWindows`, `platformMap`, `archMap`, `platform`, `arch`, and `execFileSync` are used so the fix is applied in the correct spot.
🧹 Nitpick comments (1)
.github/workflows/pkg-pr-new.yml (1)
1-16: Consider adding workflow concurrency to avoid duplicate publish/comment races.Rapid successive pushes can run multiple publish jobs in parallel for the same PR. A per-PR concurrency group will reduce churn and comment flapping.
💡 Suggested patch
name: PR Preview Package @@ permissions: contents: read pull-requests: write + +concurrency: + group: pkg-pr-new-${{ github.event.pull_request.number }} + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pkg-pr-new.yml around lines 1 - 16, Add a top-level GitHub Actions concurrency stanza to the workflow "PR Preview Package" to prevent duplicate publish/comment races for the "publish" job; specifically add a concurrency block (e.g. group using github.workflow and github.event.pull_request.number and cancel-in-progress: true) so that only one publish run per PR is active at a time and in-flight runs are cancelled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/build-pkg-pr-new.sh`:
- Around line 47-75: The script currently constructs binary (variable `binary`)
and calls `execFileSync` without checking the file exists, causing an opaque
exit; update the runtime in this file to explicitly check existence of `binary`
(e.g., using fs.existsSync or fs.statSync) before calling `execFileSync`, and if
missing print a clear error that includes the attempted `platform` and `arch`,
the full `binary` path, and a list of supported targets derived from
`platformMap` and `archMap` (iterate over their keys to show valid combos), then
exit with a non-zero status; ensure references to `isWindows`, `platformMap`,
`archMap`, `platform`, `arch`, and `execFileSync` are used so the fix is applied
in the correct spot.
---
Nitpick comments:
In @.github/workflows/pkg-pr-new.yml:
- Around line 1-16: Add a top-level GitHub Actions concurrency stanza to the
workflow "PR Preview Package" to prevent duplicate publish/comment races for the
"publish" job; specifically add a concurrency block (e.g. group using
github.workflow and github.event.pull_request.number and cancel-in-progress:
true) so that only one publish run per PR is active at a time and in-flight runs
are cancelled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e3b8275-9044-410f-8689-ecdc9afa4e98
📒 Files selected for processing (2)
.github/workflows/pkg-pr-new.ymlscripts/build-pkg-pr-new.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/build-pkg-pr-new.sh (2)
52-53: Consider updating the TODO comment.The comment states "we only ship a subset of binaries," but lines 32-37 now build all 6 platform/arch combinations. Either update the comment to reflect the current state, or clarify that it's tracking a potential future constraint from the upstream 20MB limit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-pkg-pr-new.sh` around lines 52 - 53, The TODO comment near the top of scripts/build-pkg-pr-new.sh incorrectly claims "we only ship a subset of binaries" although the script currently builds all six platform/arch combos (see the platform/arch build loop); update that TODO to reflect the current behavior — either remove the outdated claim and note that all 6 targets are built, or explicitly state it is documenting a potential future constraint from pkg.pr.new's 20MB limit and link the upstream issue for context (keep the reference to pkg.pr.new 20MB and the PR URL).
71-75: Error handling is correct but could be more informative.If the binary is missing (ENOENT), users would see a raw Node.js stack trace. Consider checking for
err.code === 'ENOENT'to provide a clearer message, though this is an edge case that shouldn't occur if the build succeeded.💡 Optional: More informative error for missing binary
try { execFileSync(binary, process.argv.slice(2), { stdio: "inherit" }); } catch (err) { + if (err.code === "ENOENT") { + console.error(`Binary not found: ${binary}`); + console.error(`This PR preview may not support ${process.platform}-${process.arch}.`); + } process.exit(err.status || 1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-pkg-pr-new.sh` around lines 71 - 75, Catch the execFileSync failure and handle the ENOENT case explicitly: when execFileSync(binary, ...) throws, check err.code === 'ENOENT' and log a clear message that the binary was not found (including the binary variable value) before exiting; otherwise preserve the existing behavior of exiting with err.status || 1 and optionally log the error for other failures. Use the existing identifiers execFileSync, binary, err, and process.exit to locate and update the catch block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build-pkg-pr-new.sh`:
- Around line 14-17: The script sets VERSION via node -p
"require('./package.json').version" which can return the literal "undefined";
add a guard right after VERSION is assigned to verify it's non-empty and not
"undefined" (e.g., test -z "$VERSION" || test "$VERSION" = "undefined") and if
it fails print an error and exit 1 so the build stops early; ensure this check
sits before LDFLAGS is constructed so LDFLAGS and subsequent steps never embed
an invalid VERSION.
---
Nitpick comments:
In `@scripts/build-pkg-pr-new.sh`:
- Around line 52-53: The TODO comment near the top of
scripts/build-pkg-pr-new.sh incorrectly claims "we only ship a subset of
binaries" although the script currently builds all six platform/arch combos (see
the platform/arch build loop); update that TODO to reflect the current behavior
— either remove the outdated claim and note that all 6 targets are built, or
explicitly state it is documenting a potential future constraint from
pkg.pr.new's 20MB limit and link the upstream issue for context (keep the
reference to pkg.pr.new 20MB and the PR URL).
- Around line 71-75: Catch the execFileSync failure and handle the ENOENT case
explicitly: when execFileSync(binary, ...) throws, check err.code === 'ENOENT'
and log a clear message that the binary was not found (including the binary
variable value) before exiting; otherwise preserve the existing behavior of
exiting with err.status || 1 and optionally log the error for other failures.
Use the existing identifiers execFileSync, binary, err, and process.exit to
locate and update the catch block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 080c3b13-2884-4bb7-aa54-f02f666f2939
📒 Files selected for processing (1)
scripts/build-pkg-pr-new.sh
liangshuo-1
approved these changes
Mar 31, 2026
liangshuo-1
approved these changes
Mar 31, 2026
This was referenced Apr 1, 2026
tuxedomm
pushed a commit
that referenced
this pull request
Apr 3, 2026
* ci: publish PR preview builds to pkg.pr.new * chore: limit pkg-pr-new build targets to 3 platforms * ci: use node lts in pkg-pr-new workflow * chore: trim pkg-pr-new to single target to fit size limit * ci: publish pkg-pr-new package path without --bin * ci: disable compact pkg-pr-new urls for fork previews * ci: post minimal npm -g pkg-pr-new install comment * chore: enable windows amd64 build for pkg-pr-new * ci: format pkg-pr-new install comment as markdown code block * ci: tweak pkg-pr-new comment wording * ci: pin github-script and paginate PR comments * chore: enable pkg PR build targets --------- Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
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.
This PR adds
pkg.pr.newpreview publishing for CLI PRs.Summary by CodeRabbit