Skip to content

fix(cli): preserve build cancellation through npx - #3231

Merged
WcaleNieWolny merged 1 commit into
mainfrom
wolny/fix-build-cancel-duplicate-sigint
Aug 30, 2026
Merged

fix(cli): preserve build cancellation through npx#3231
WcaleNieWolny merged 1 commit into
mainfrom
wolny/fix-build-cancel-duplicate-sigint

Conversation

@WcaleNieWolny

@WcaleNieWolny WcaleNieWolny commented Aug 30, 2026

Copy link
Copy Markdown
Member

Summary

  • ignore the immediate duplicate SIGINT that npx can emit for one physical Ctrl+C
  • preserve the existing later Ctrl+C escape hatch for force-quitting a stuck cancellation
  • wait for and validate the build-cancellation response instead of silently treating HTTP errors as success
  • tell the user whether the remote builder accepted cancellation

Root cause

When invoked through npx, one Ctrl+C can reach the CLI twice: once through the foreground process group and once through the npx wrapper. The first signal started the asynchronous cancellation request; the immediate second signal saw cancellation as already requested and called process.exit(1), terminating the process before the POST reached Capgo.

Test plan

  • bun run test:build-cancellation
  • bun run cli:check

The full CLI gate was run with an isolated test home and canonical temporary directory so local credentials and the macOS /var symlink could not leak into existing tests.

Screenshots

Not applicable; this changes terminal cancellation behavior and adds explicit success/failure messages.

Checklist

  • Relevant CLI lint, typecheck, build, and full test suite pass via bun run cli:check.
  • The regression is covered for duplicate signals, later force-quit, HTTP rejection, and successful POST payloads.
  • No documentation change is required.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added more reliable build cancellation when interrupting a running build.
    • The first interrupt requests cancellation and stops log streaming.
    • A repeated interrupt shortly afterward is safely ignored, while later interrupts force the process to exit.
    • Cancellation success or failure is now reported clearly.
  • Tests

    • Added coverage for successful, failed, repeated-interrupt, and stalled cancellation scenarios.

@codspeed-hq

codspeed-hq Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing wolny/fix-build-cancel-duplicate-sigint (f78def6) with main (3346033)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c4408eb2-0389-4e0c-aae1-f93296529002

📥 Commits

Reviewing files that changed from the base of the PR and between 3346033 and f78def6.

📒 Files selected for processing (4)
  • cli/package.json
  • cli/src/build/cancellation.ts
  • cli/src/build/request.ts
  • cli/test/test-build-cancellation.mjs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Cap-go/capacitor-updater (manual)

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

The CLI now uses shared utilities to request build cancellation, handle repeated SIGINT signals, abort log streaming, and report cancellation outcomes. Native tests cover success, failure, duplicate-signal, and forced-exit paths.

Changes

Build cancellation

Layer / File(s) Summary
Cancellation utilities
cli/src/build/cancellation.ts
Added configurable HTTP cancellation requests with structured results. Added SIGINT handling for pending cancellation, duplicate signals, log-stream abortion, and forced exit.
Build request integration
cli/src/build/request.ts
Replaced local cancellation logic with the shared handler and request helper. The flow sends the authenticated request, logs the result, aborts log streaming, and force-exits after later interrupts.
Cancellation validation and test wiring
cli/test/test-build-cancellation.mjs, cli/package.json
Added native tests for cancellation requests and signal behavior. Added the test runner to the package test commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f78de

This change preserves build cancellation through npx and reports whether cancellation was accepted without introducing an evidenced merge-blocking risk; it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as Build request CLI
  participant Handler as Cancellation signal handler
  participant API as Cancellation request
  participant Logs as Log streaming
  CLI->>Handler: Receive SIGINT
  Handler->>API: Send cancellation request
  API->>API: POST application ID
  Handler->>Logs: Abort after request completes
Loading

Suggested reviewers: riderx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preserving build cancellation when the CLI runs through npx.
Description check ✅ Passed The description includes a summary, root cause, test plan, screenshots note, and completed checklist items. It accurately explains the cancellation behavior and regression coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

@sonarqubecloud

Copy link
Copy Markdown

@WcaleNieWolny
WcaleNieWolny merged commit a94674a into main Aug 30, 2026
46 of 50 checks passed
@WcaleNieWolny
WcaleNieWolny deleted the wolny/fix-build-cancel-duplicate-sigint branch August 30, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant