-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Generate package-lock.json when missing from claude-code tarball #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
|
@copilot if these files change for the local claude we should run the action that updates claude (try to build it only no publishing) |
Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
Added PR trigger to the update-claude-code workflow. When files in |
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the DCO. |
|
@copilot why did the update claude code package not run? |
The "Update claude-code package" step is being skipped on PRs by design (via However, I realize this means we're not testing the
Which approach would be more useful?
Fixed in d12b23d. The step was being skipped on PRs (by design to avoid creating update PRs). Now the workflow:
This ensures changes to |
Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the update-claude-code workflow by adding fallback logic to generate package-lock.json when it's missing from npm tarballs (encountered when upgrading from version 2.1.15 to 2.1.25). It also adds PR validation to test update script changes before they're merged.
Changes:
- Added fallback generation of
package-lock.jsonfrompackage.jsonusing npm when the lock file is absent from the tarball - Added pull_request trigger to the workflow to validate update script changes in CI before merge
- Added appropriate error handling to verify package.json exists and lock file generation succeeds
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nix/claude-code/update.sh | Added fallback logic with error handling to generate package-lock.json when missing from tarball using npm install --package-lock-only --ignore-scripts |
| .github/workflows/update-claude-code.yml | Added pull_request trigger with conditional execution to test update script changes without creating PRs, and guarded production steps to only run on schedule/manual triggers |
| run: | | ||
| # On PRs, run the update script to test it works | ||
| # but don't commit or create PRs with the changes | ||
| ./nix/claude-code/update.sh || echo "::warning::Update script execution test failed" |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test step swallows failures with || echo "::warning::...", which means the job will always pass even if the update script fails. This defeats the purpose of testing the script changes in PRs. Consider either:
- Removing the
|| echofallback to let the job fail when the script has issues - Using
set +eand explicit exit code checking to fail the job while still providing a warning message
The script should fail the PR check if it cannot successfully execute, so that broken changes to update.sh are caught before merge.
| ./nix/claude-code/update.sh || echo "::warning::Update script execution test failed" | |
| set +e | |
| ./nix/claude-code/update.sh | |
| status=$? | |
| if [ "$status" -ne 0 ]; then | |
| echo "::warning::Update script execution test failed (exit code: $status)" | |
| exit "$status" | |
| fi |
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
Applied both review comments in a5051c3:
|
Fix Update Claude Code CI Action Failure
The CI action was failing with "Error: No package-lock.json in tarball" when trying to update claude-code from 2.1.15 to 2.1.25.
Plan:
nix/claude-code/update.shto generatepackage-lock.jsonif not present in tarballSummary of Changes:
Root Cause: Newer versions of
@anthropic-ai/claude-codenpm package don't includepackage-lock.jsonin their published tarball.Solution 1 - Update Script: Modified
nix/claude-code/update.shto:package-lock.jsonexists in the downloaded tarballpackage.jsonusingnpm install --package-lock-only --ignore-scripts.npmrcfile from tarball when generating lock file (ensures npm registry configs are respected)package.jsonexists before attempting generationpackage-lock.jsonwas generatedSolution 2 - Workflow Trigger: Modified
.github/workflows/update-claude-code.ymlto:pull_requesttrigger for changes tonix/claude-code/**filesValidation:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.