Skip to content

fix(run): add missing binary check for lark-cli execution#362

Merged
sang-neo03 merged 1 commit intomainfrom
fix/run-js-missing-binary-check
Apr 9, 2026
Merged

fix(run): add missing binary check for lark-cli execution#362
sang-neo03 merged 1 commit intomainfrom
fix/run-js-missing-binary-check

Conversation

@sang-neo03
Copy link
Copy Markdown
Collaborator

@sang-neo03 sang-neo03 commented Apr 9, 2026

⏺ Summary

When @larksuite/cli is installed with ignore-scripts=true, the postinstall script that
downloads the Go binary is skipped. Currently scripts/run.js silently exits with code
1, giving users no indication of what went wrong or how to fix it.

Changes

  • Add fs.existsSync pre-check in scripts/run.js before attempting to execute the binary
  • Output a clear error message explaining the cause (postinstall skipped or download
    failed) and recovery command (node scripts/install.js)

Test Plan

  • Unit tests pass
  • Manual local verification confirms the lark xxx command works as expected
  • Verified: removing bin/lark-cli and running node scripts/run.js prints the expected
    error message with recovery instructions
  • Verified: with binary present, lark-cli --help works as before (no behavior change on
    happy path)

Related Issues

Summary by CodeRabbit

  • Chores
    • Enhanced error messaging and validation to provide clearer guidance when required binary dependencies are missing, including instructions to complete manual installation steps if needed.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6def0af-ddba-4235-b8c4-e161138b123c

📥 Commits

Reviewing files that changed from the base of the PR and between 284e5b6 and d535fb6.

📒 Files selected for processing (1)
  • scripts/run.js

📝 Walkthrough

Walkthrough

scripts/run.js now checks whether the platform-specific bin/lark-cli binary exists before execution. If missing, it prints a diagnostic error message with recovery instructions and exits with code 1. Otherwise, it executes the binary as before, retaining existing error handling for runtime failures.

Changes

Cohort / File(s) Summary
Binary existence validation
scripts/run.js
Added fs module import and pre-flight check. If bin/lark-cli (platform-suffixed) does not exist, prints multi-line error message describing likely causes and instructs user to run node scripts/install.js manually, then exits with code 1. Preserves existing execution and error handling logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit checked the path so near,
Before the binary could appear,
"Dear friend, please install with care,
The scripts must run—I'm waiting there!" 🐰✨
Now users see the missing clue,
And know exactly what to do.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a binary check before executing lark-cli in the run script.
Description check ✅ Passed The description covers all required template sections: Summary (problem and context), Changes (specific implementation), Test Plan (verification steps), and Related Issues (Closes #321).
Linked Issues check ✅ Passed The PR meets the key coding requirement from issue #321: scripts/run.js now detects missing bin/lark-cli and prints an actionable error message with recovery instructions.
Out of Scope Changes check ✅ Passed All changes are scoped to the primary objective: adding a pre-flight binary check to scripts/run.js. No out-of-scope modifications (documentation or alternative distribution approaches) are included.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/run-js-missing-binary-check

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the size/M Single-domain feat or fix with limited business impact label Apr 9, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 9, 2026

Greptile Summary

Adds an fs.existsSync guard in scripts/run.js that prints an actionable error message — including cause and recovery command — when the lark-cli binary is absent (e.g. after --ignore-scripts install). The file mode is also corrected from 0644 to 0755 to match the existing shebang line.

Confidence Score: 5/5

Safe to merge — minimal, correct change with no P0 or P1 findings.

The diff is a focused, one-concern fix: add an existence check, emit a helpful error, and exit. The happy path is unchanged, the error path is straightforward, and all remaining observations are P2 style suggestions.

No files require special attention.

Vulnerabilities

No security concerns identified. The change only adds an existence check on a locally resolved binary path and emits a static diagnostic string — no user input is interpolated into shell commands.

Important Files Changed

Filename Overview
scripts/run.js Adds fs.existsSync pre-check before binary execution; emits a clear error message with recovery instructions when binary is missing. File mode also corrected to 0755.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([lark CLI invoked]) --> B[Resolve binary path\nbin/lark-cli or bin/lark-cli.exe]
    B --> C{fs.existsSync bin?}
    C -- No --> D[console.error:\nBinary not found\nCauses + recovery cmd]
    D --> E([process.exit 1])
    C -- Yes --> F[execFileSync bin\nwith stdio: inherit]
    F -- success --> G([exit 0])
    F -- throws --> H{e.status?}
    H -- non-null --> I([exit e.status])
    H -- null signal kill --> J([exit 1])
Loading

Reviews (1): Last reviewed commit: "fix(run): add missing binary check for l..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d535fb60632c5e38ce3487312e7bb9c0c69e967a

🧩 Skill update

npx skills add larksuite/cli#fix/run-js-missing-binary-check -y -g

@luojiyin1987
Copy link
Copy Markdown

Whether to add executable permission check ? @sang-neo03

@sang-neo03
Copy link
Copy Markdown
Collaborator Author

Whether to add executable permission check ? @sang-neo03

No need to add an executable permission check. install.js already sets the correct permissions via fs.chmodSync(dest, 0o755) after download. On Windows, executability is determined by file extension, not permission bits, and fs.accessSync(X_OK) behaves unreliably there. This PR focuses on the missing binary caused by ignore-scripts=true — permission issues are a rare edge case from manual user operations and out of scope for this fix.

@sang-neo03 sang-neo03 merged commit 3db4f42 into main Apr 9, 2026
12 checks passed
@sang-neo03 sang-neo03 deleted the fix/run-js-missing-binary-check branch April 9, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs/install: npm install succeeds with ignore-scripts=true but lark-cli is unusable

3 participants