Skip to content

fix(run): make non-recursive --no-bail exit non-zero when a script fails#12263

Merged
zkochan merged 1 commit into
pnpm:mainfrom
antitoxic:fix/report-failure-flag
Jun 16, 2026
Merged

fix(run): make non-recursive --no-bail exit non-zero when a script fails#12263
zkochan merged 1 commit into
pnpm:mainfrom
antitoxic:fix/report-failure-flag

Conversation

@antitoxic

@antitoxic antitoxic commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

When running a non-recursive pnpm run --no-bail that matches multiple scripts (e.g. via a /regex/ selector), pnpm always exited with code 0 regardless of whether any script failed. This is inconsistent with recursive runs, which aggregate failures and exit non-zero at the end (via throwOnCommandFail).

This PR fixes --no-bail directly so its exit-code behavior is consistent across recursive and non-recursive runs, as requested in #8013:

  • --no-bail still runs every matched script to completion (it no longer short-circuits on the first failure — execution switched from Promise.all to Promise.allSettled).
  • After all scripts settle, the command exits with a non-zero exit code (ERR_PNPM_RUN_FAILED) if any of them failed.

This is a behavior change: previously a non-recursive pnpm run --no-bail with a failing script exited 0. No new flag is introduced — per the issue discussion, a separate flag "would just add confusion without benefit".

Closes #8013


Written by an agent (Claude Code, claude-opus-4-8).

Summary by CodeRabbit

  • Bug Fixes
    • pnpm run --no-bail now returns a non-zero exit code if any matched scripts fail, while still running all matched scripts to completion.
    • Exit-code behavior is now consistent between recursive and non-recursive executions.
  • Documentation
    • Updated the --no-bail help text to clarify that failures don’t stop execution, but the command can still fail overall.

@antitoxic antitoxic requested a review from zkochan as a code owner June 8, 2026 11:41
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Add --report-failure for pnpm run --no-bail

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add --report-failure for pnpm run
• Exit non-zero when --no-bail scripts fail
• Preserve existing --no-bail behavior by default
• Add coverage for success and failure cases
Diagram
flowchart LR
  A["CLI options"] --> B["`--report-failure` added"]
  B --> C["`run` command execution"]
  C --> D["`Promise.allSettled` in `--no-bail` mode"]
  D --> E["Aggregate failed scripts"]
  E --> F["Throw `PnpmError` when requested"]
  C --> G["Keep zero exit by default"]

Loading

Grey Divider

File Changes

1. exec/commands/src/run.ts ✨ Enhancement +34/-11

Add failure reporting to run command

• Registers --report-failure in CLI option types.
• Documents the new flag alongside --no-bail.
• Switches --no-bail execution to Promise.allSettled.
• Throws PnpmError with failure details when enabled.

exec/commands/src/run.ts


2. exec/commands/test/index.ts 🧪 Tests +69/-0

Add run command failure-reporting tests

• Verifies non-zero exit when one matched script fails.
• Verifies zero exit when all matched scripts succeed.
• Confirms backward-compatible zero exit without --report-failure.

exec/commands/test/index.ts


3. .changeset/report-failure-flag.md 📝 Documentation +6/-0

Document package version bumps

• Adds a changeset for @pnpm/exec.commands and pnpm.
• Summarizes the new --report-failure behavior.

.changeset/report-failure-flag.md


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 54bb7325-98c4-4edb-b5c5-044a58aa2296

📥 Commits

Reviewing files that changed from the base of the PR and between f9c6b94 and 9055482.

📒 Files selected for processing (3)
  • .changeset/no-bail-exit-code.md
  • exec/commands/src/run.ts
  • exec/commands/test/index.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/no-bail-exit-code.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • exec/commands/src/run.ts
  • exec/commands/test/index.ts

📝 Walkthrough

Walkthrough

pnpm run --no-bail now exits non-zero when any scripts fail. The handler() bail=false path switches from Promise.all with a suppressing try/catch to Promise.allSettled, then filters rejected results and throws a PnpmError(RUN_FAILED) aggregating failure messages. Tests and changeset are updated to match.

Changes

--no-bail Exit Code Fix

Layer / File(s) Summary
Execution logic and help text update
exec/commands/src/run.ts
Updates --no-bail help description to state non-zero exit on failure, and restructures handler() bail=false path to use Promise.allSettled, detect rejections, and throw PnpmError(RUN_FAILED) with failure count and aggregated rejection messages.
Test coverage for updated --no-bail behavior
exec/commands/test/index.ts
Adds util import for error assertions; replaces old --no-bail test with one asserting ERR_PNPM_RUN_FAILED on failure; adds two RegExp-selector tests with bail: false—one for partial failure asserting error and verifying all matched scripts ran, one for all-pass asserting no error.
Changeset entry
.changeset/no-bail-exit-code.md
Documents updated --no-bail exit-code semantics and references the prior inconsistency for non-recursive runs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 Hoppin' down the script path lane,
No bail meant failures hid in vain.
Now allSettled counts the cost,
Non-zero exits when scripts are lost.
Every failure gets its say — 🎉
The rabbit fixed the exit today!

🚥 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and clearly summarizes the main change: fixing exit code behavior of pnpm run --no-bail to return non-zero on script failures, which is directly supported by the changeset and code modifications.
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.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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.

❤️ Share

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

@antitoxic antitoxic mentioned this pull request Jun 8, 2026
4 tasks
@antitoxic

Copy link
Copy Markdown
Contributor Author

Is there any steps that I need to do that I haven't done in this PR?

Is the docstring the only stopper?

@zkochan zkochan force-pushed the fix/report-failure-flag branch from 5a3b48c to e6e8b82 Compare June 16, 2026 08:04
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (3)

Context used

Grey Divider


Action required

1. --no-bail semantics diverge 📎 Requirement gap ≡ Correctness
Description
Recursive --no-bail exits non-zero when any package/script fails, but non-recursive `pnpm run
--no-bail can still exit successfully unless --report-failure` is set. This creates inconsistent
and non-compliant exit-code semantics between recursive and non-recursive runs.
Code

exec/commands/src/run.ts[R312-333]

+  } else {
+    const results = await Promise.allSettled(
+      specifiedScripts.map(script => limitRun(() => _runScript(script)))
+    )
+
+    if (opts.reportFailure) {
+      const failures = results.filter(
+        (r): r is PromiseRejectedResult => r.status === 'rejected'
+      )
+      if (failures.length > 0) {
+        throw new PnpmError(
+          'RUN_NO_BAIL_FAILURES',
+          `Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
+          {
+            hint: failures
+              .map((f) => f.reason?.message ?? String(f.reason))
+              .join('\n'),
+          }
+        )
+      }
  }
}
Evidence
In recursive execution, the code path always calls throwOnCommandFail(...) at the end when any
package failed, which forces a non-zero exit status. In contrast, the updated non-recursive
implementation only throws when opts.reportFailure is true, meaning --no-bail alone can return
exit code 0 even if some matched scripts failed; the newly added test explicitly codifies this
backward-compatible behavior without --report-failure, but compliance requires `pnpm run
--no-bail` to continue running remaining scripts while still exiting non-zero if any invoked script
fails.

Ensure --no-bail exit code semantics are consistent between recursive and non-recursive runs
exec/commands/src/run.ts[312-333]
exec/commands/src/runRecursive.ts[199-206]
cli/utils/src/recursiveSummary.ts[28-32]
exec/commands/test/index.ts[131-149]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Align `--no-bail` exit-code semantics so that both recursive and non-recursive `pnpm run` executions exit non-zero when any matched/invoked script fails, without requiring `--report-failure`.
## Issue Context
Currently, recursive runs reliably throw at the end (via `throwOnCommandFail(...)`) when any package fails, producing a non-zero exit code. Non-recursive runs, however, only throw in `--no-bail` mode when `opts.reportFailure` is enabled, allowing an exit code of 0 even if some scripts fail; compliance requires `--no-bail` to keep running remaining scripts but still return a failure exit code if any sub-run fails, and to behave consistently across recursive and non-recursive execution.
## Fix Focus Areas
- exec/commands/src/run.ts[312-333]
- exec/commands/src/runRecursive.ts[199-206]
- cli/utils/src/recursiveSummary.ts[28-32]
- exec/commands/test/index.ts[59-149]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Docs claim --no-bail always 0 📎 Requirement gap ⚙ Maintainability
Description
The added release note text documents that --no-bail preserves exiting with code 0 unless
--report-failure is used, which contradicts the required documented semantics for --no-bail exit
codes. This fails the documentation alignment requirement for run vs recursive behavior.
Code

.changeset/report-failure-flag.md[6]

+Added a new `--report-failure` flag for `pnpm run`. When used with `--no-bail`, the process exits with a non-zero exit code if any of the matched scripts fail. Without this flag, `--no-bail` preserves its existing behavior of always exiting with code 0.
Evidence
The compliance checklist requires documentation to describe --no-bail as exiting 0 only when all
sub-runs succeed, and non-zero otherwise, consistently for both run and recursive. The added
changeset explicitly documents --no-bail as always exiting 0 without --report-failure, and the
new CLI help entry reinforces this conditional behavior, contradicting the required documentation
semantics.

Documentation alignment: --no-bail behavior and availability for run vs recursive
.changeset/report-failure-flag.md[1-6]
exec/commands/src/run.ts[140-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation added in this PR states `--no-bail` continues to exit 0 unless `--report-failure` is used, but compliance requires `--no-bail` to exit non-zero when any sub-run fails (and to document this consistently for run vs recursive).
## Issue Context
This mismatch risks user confusion and contradicts the required CLI semantics described in the compliance checklist.
## Fix Focus Areas
- .changeset/report-failure-flag.md[1-6]
- exec/commands/src/run.ts[140-147]
- pnpm/src/cmd/recursive.ts[83-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Missing recursive --no-bail tests 📎 Requirement gap ☼ Reliability ⭐ New
Description
The PR adds test coverage for non-recursive pnpm run --no-bail, but it does not add/assert
equivalent exit-code behavior for pnpm run --recursive --no-bail (non-zero when any sub-run fails,
zero when all succeed). This leaves a gap that can allow regressions where recursive and
non-recursive --no-bail semantics diverge again.
Code

exec/commands/test/index.ts[R60-63]

+test('pnpm run --no-bail runs the script to completion but still exits non-zero on failure', async () => {
  prepare({
    scripts: {
      exit1: 'node recordArgs && exit 1',
Evidence
PR Compliance ID 1 requires tests for both recursive and non-recursive --no-bail exit-code
behavior, including failing and all-success scenarios. The PR adds only non-recursive --no-bail
assertions in exec/commands/test/index.ts, while existing recursive run tests do not exercise
--no-bail semantics and the existing recursive --no-bail test checks output but does not assert
the exit status or an all-success case.

Ensure --no-bail exit-code semantics are consistent across pnpm run and pnpm run --recursive
exec/commands/test/index.ts[60-141]
exec/commands/test/runRecursive.ts[18-83]
pnpm/test/errorHandler.test.ts[65-96]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires `--no-bail` exit-code semantics to be consistent between non-recursive and recursive `pnpm run`, with automated tests covering both success and failure cases.

## Issue Context
This PR adds non-recursive tests that assert `ERR_PNPM_RUN_FAILED` on failures and exit success when all matched scripts pass, but there is no equivalent assertion for `pnpm run --recursive --no-bail` covering both (a) at least one failing sub-run and (b) all-success.

## Fix Focus Areas
- exec/commands/test/index.ts[60-141]
- exec/commands/test/runRecursive.ts[18-83]
- pnpm/test/errorHandler.test.ts[65-96]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Unattributed script failures 🐞 Bug ◔ Observability
Description
In the --no-bail path, run.ts throws a single ERR_PNPM_RUN_FAILED whose hint is just a
newline-joined list of error messages, without mapping each message back to its script name. Because
the default reporter prints unknown ERR_PNPM_* errors as title + hint, users may not be able to tell
which matched script(s) failed in multi-script runs.
Code

exec/commands/src/run.ts[R307-322]

+    const results = await Promise.allSettled(
+      specifiedScripts.map(script => limitRun(() => _runScript(script)))
+    )
+    const failures = results.filter(
+      (r): r is PromiseRejectedResult => r.status === 'rejected'
+    )
+    if (failures.length > 0) {
+      throw new PnpmError(
+        'RUN_FAILED',
+        `Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
+        {
+          hint: failures
+            .map((f) => f.reason?.message ?? String(f.reason))
+            .join('\n'),
+        }
+      )
Evidence
The --no-bail implementation throws RUN_FAILED with a hint composed only of failure reason messages,
so script attribution is lost. The default reporter’s fallback for unknown ERR_PNPM_* errors prints
only the error title and hint body, meaning whatever is in the hint is the primary diagnostic detail
shown to the user.

exec/commands/src/run.ts[295-324]
cli/default-reporter/src/reportError.ts[98-107]
cli/utils/src/recursiveSummary.ts[16-33]
cli/default-reporter/src/reportError.ts[335-343]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
In `pnpm run` with `--no-bail`, failures are aggregated via `Promise.allSettled(...)` and then thrown as `new PnpmError('RUN_FAILED', ...)`. The thrown error's `hint` currently concatenates only `reason.message` values, which drops the association between a failure and the script that produced it.
### Issue Context
The default reporter formats unknown `ERR_PNPM_*` errors by printing `err.message` and the `hint` body. Without script attribution, debugging regex/multi-script runs becomes unnecessarily difficult.
### Fix Focus Areas
- exec/commands/src/run.ts[307-322]
### Suggested change
When building the aggregated hint, keep the index/script name so each failure line is prefixed with the script, e.g.:
- Map `results` to `{ script: specifiedScripts[i], result }`
- Filter rejected
- Build `hint` lines like `"<script>: <message>"`
Optionally, attach a structured `failures: Array<{script: string, message: string}>` field to the error (similar to recursive summary) and add a dedicated formatter later, but the minimal fix is to include script names in the hint.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 9055482

Results up to commit f9c6b94


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (2)

Context used

Action required
1. --no-bail semantics diverge 📎 Requirement gap ≡ Correctness
Description
Recursive --no-bail exits non-zero when any package/script fails, but non-recursive `pnpm run
--no-bail can still exit successfully unless --report-failure` is set. This creates inconsistent
and non-compliant exit-code semantics between recursive and non-recursive runs.
Code

exec/commands/src/run.ts[R312-333]

+  } else {
+    const results = await Promise.allSettled(
+      specifiedScripts.map(script => limitRun(() => _runScript(script)))
+    )
+
+    if (opts.reportFailure) {
+      const failures = results.filter(
+        (r): r is PromiseRejectedResult => r.status === 'rejected'
+      )
+      if (failures.length > 0) {
+        throw new PnpmError(
+          'RUN_NO_BAIL_FAILURES',
+          `Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
+          {
+            hint: failures
+              .map((f) => f.reason?.message ?? String(f.reason))
+              .join('\n'),
+          }
+        )
+      }
   }
 }
Evidence
In recursive execution, the code path always calls throwOnCommandFail(...) at the end when any
package failed, which forces a non-zero exit status. In contrast, the updated non-recursive
implementation only throws when opts.reportFailure is true, meaning --no-bail alone can return
exit code 0 even if some matched scripts failed; the newly added test explicitly codifies this
backward-compatible behavior without --report-failure, but compliance requires `pnpm run
--no-bail` to continue running remaining scripts while still exiting non-zero if any invoked script
fails.

Ensure --no-bail exit code semantics are consistent between recursive and non-recursive runs
exec/commands/src/run.ts[312-333]
exec/commands/src/runRecursive.ts[199-206]
cli/utils/src/recursiveSummary.ts[28-32]
exec/commands/test/index.ts[131-149]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Align `--no-bail` exit-code semantics so that both recursive and non-recursive `pnpm run` executions exit non-zero when any matched/invoked script fails, without requiring `--report-failure`.
## Issue Context
Currently, recursive runs reliably throw at the end (via `throwOnCommandFail(...)`) when any package fails, producing a non-zero exit code. Non-recursive runs, however, only throw in `--no-bail` mode when `opts.reportFailure` is enabled, allowing an exit code of 0 even if some scripts fail; compliance requires `--no-bail` to keep running remaining scripts but still return a failure exit code if any sub-run fails, and to behave consistently across recursive and non-recursive execution.
## Fix Focus Areas
- exec/commands/src/run.ts[312-333]
- exec/commands/src/runRecursive.ts[199-206]
- cli/utils/src/recursiveSummary.ts[28-32]
- exec/commands/test/index.ts[59-149]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Docs claim --no-bail always 0 📎 Requirement gap ⚙ Maintainability
Description
The added release note text documents that --no-bail preserves exiting with code 0 unless
--report-failure is used, which contradicts the required documented semantics for --no-bail exit
codes. This fails the documentation alignment requirement for run vs recursive behavior.
Code

.changeset/report-failure-flag.md[6]

+Added a new `--report-failure` flag for `pnpm run`. When used with `--no-bail`, the process exits with a non-zero exit code if any of the matched scripts fail. Without this flag, `--no-bail` preserves its existing behavior of always exiting with code 0.
Evidence
The compliance checklist requires documentation to describe --no-bail as exiting 0 only when all
sub-runs succeed, and non-zero otherwise, consistently for both run and recursive. The added
changeset explicitly documents --no-bail as always exiting 0 without --report-failure, and the
new CLI help entry reinforces this conditional behavior, contradicting the required documentation
semantics.

Documentation alignment: --no-bail behavior and availability for run vs recursive
.changeset/report-failure-flag.md[1-6]
exec/commands/src/run.ts[140-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation added in this PR states `--no-bail` continues to exit 0 unless `--report-failure` is used, but compliance requires `--no-bail` to exit non-zero when any sub-run fails (and to document this consistently for run vs recursive).
## Issue Context
This mismatch risks user confusion and contradicts the required CLI semantics described in the compliance checklist.
## Fix Focus Areas
- .changeset/report-failure-flag.md[1-6]
- exec/commands/src/run.ts[140-147]
- pnpm/src/cmd/recursive.ts[83-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
3. Unattributed script failures 🐞 Bug ◔ Observability ⭐ New
Description
In the --no-bail path, run.ts throws a single ERR_PNPM_RUN_FAILED whose hint is just a
newline-joined list of error messages, without mapping each message back to its script name. Because
the default reporter prints unknown ERR_PNPM_* errors as title + hint, users may not be able to tell
which matched script(s) failed in multi-script runs.
Code

exec/commands/src/run.ts[R307-322]

+    const results = await Promise.allSettled(
+      specifiedScripts.map(script => limitRun(() => _runScript(script)))
+    )
+    const failures = results.filter(
+      (r): r is PromiseRejectedResult => r.status === 'rejected'
+    )
+    if (failures.length > 0) {
+      throw new PnpmError(
+        'RUN_FAILED',
+        `Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
+        {
+          hint: failures
+            .map((f) => f.reason?.message ?? String(f.reason))
+            .join('\n'),
+        }
+      )
Evidence
The --no-bail implementation throws RUN_FAILED with a hint composed only of failure reason messages,
so script attribution is lost. The default reporter’s fallback for unknown ERR_PNPM_* errors prints
only the error title and hint body, meaning whatever is in the hint is the primary diagnostic detail
shown to the user.

exec/commands/src/run.ts[295-324]
cli/default-reporter/src/reportError.ts[98-107]
cli/utils/src/recursiveSummary.ts[16-33]
cli/default-reporter/src/reportError.ts[335-343]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In `pnpm run` with `--no-bail`, failures are aggregated via `Promise.allSettled(...)` and then thrown as `new PnpmError('RUN_FAILED', ...)`. The thrown error's `hint` currently concatenates only `reason.message` values, which drops the association between a failure and the script that produced it.

### Issue Context
The default reporter formats unknown `ERR_PNPM_*` errors by printing `err.message` and the `hint` body. Without script attribution, debugging regex/multi-script runs becomes unnecessarily difficult.

### Fix Focus Areas
- exec/commands/src/run.ts[307-322]

### Suggested change
When building the aggregated hint, keep the index/script name so each failure line is prefixed with the script, e.g.:

- Map `results` to `{ script: specifiedScripts[i], result }`
- Filter rejected
- Build `hint` lines like `"<script>: <message>"`

Optionally, attach a structured `failures: Array<{script: string, message: string}>` field to the error (similar to recursive summary) and add a dedicated formatter later, but the minimal fix is to include script names in the hint.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit e6e8b82


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (2)

Context used

Action required
1. --no-bail semantics diverge 📎 Requirement gap ≡ Correctness
Description
Recursive --no-bail exits non-zero when any package/script fails, but non-recursive `pnpm run
--no-bail can still exit successfully unless --report-failure` is set. This creates inconsistent
and non-compliant exit-code semantics between recursive and non-recursive runs.
Code

exec/commands/src/run.ts[R312-333]

+  } else {
+    const results = await Promise.allSettled(
+      specifiedScripts.map(script => limitRun(() => _runScript(script)))
+    )
+
+    if (opts.reportFailure) {
+      const failures = results.filter(
+        (r): r is PromiseRejectedResult => r.status === 'rejected'
+      )
+      if (failures.length > 0) {
+        throw new PnpmError(
+          'RUN_NO_BAIL_FAILURES',
+          `Some scripts failed: ${failures.length} of ${specifiedScripts.length}`,
+          {
+            hint: failures
+              .map((f) => f.reason?.message ?? String(f.reason))
+              .join('\n'),
+          }
+        )
+      }
    }
  }
Evidence
In recursive execution, the code path always calls throwOnCommandFail(...) at the end when any
package failed, which forces a non-zero exit status. In contrast, the updated non-recursive
implementation only throws when opts.reportFailure is true, meaning --no-bail alone can return
exit code 0 even if some matched scripts failed; the newly added test explicitly codifies this
backward-compatible behavior without --report-failure, but compliance requires `pnpm run
--no-bail` to continue running remaining scripts while still exiting non-zero if any invoked script
fails.

Ensure --no-bail exit code semantics are consistent between recursive and non-recursive runs
exec/commands/src/run.ts[312-333]
exec/commands/src/runRecursive.ts[199-206]
cli/utils/src/recursiveSummary.ts[28-32]
exec/commands/test/index.ts[131-149]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Align `--no-bail` exit-code semantics so that both recursive and non-recursive `pnpm run` executions exit non-zero when any matched/invoked script fails, without requiring `--report-failure`.

## Issue Context
Currently, recursive runs reliably throw at the end (via `throwOnCommandFail(...)`) when any package fails, producing a non-zero exit code. Non-recursive runs, however, only throw in `--no-bail` mode when `opts.reportFailure` is enabled, allowing an exit code of 0 even if some scripts fail; compliance requires `--no-bail` to keep running remaining scripts but still return a failure exit code if any sub-run fails, and to behave consistently across recursive and non-recursive execution.

## Fix Focus Areas
- exec/commands/src/run.ts[312-333]
- exec/commands/src/runRecursive.ts[199-206]
- cli/utils/src/recursiveSummary.ts[28-32]
- exec/commands/test/index.ts[59-149]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Docs claim --no-bail always 0 📎 Requirement gap ⚙ Maintainability
Description
The added release note text documents that --no-bail preserves exiting with code 0 unless
--report-failure is used, which contradicts the required documented semantics for --no-bail exit
codes. This fails the documentation alignment requirement for run vs recursive behavior.
Code

.changeset/report-failure-flag.md[6]

+Added a new `--report-failure` flag for `pnpm run`. When used with `--no-bail`, the process exits with a non-zero exit code if any of the matched scripts fail. Without this flag, `--no-bail` preserves its existing behavior of always exiting with code 0.
Evidence
The compliance checklist requires documentation to describe --no-bail as exiting 0 only when all
sub-runs succeed, and non-zero otherwise, consistently for both run and recursive. The added
changeset explicitly documents --no-bail as always exiting 0 without --report-failure, and the
new CLI help entry reinforces this conditional behavior, contradicting the required documentation
semantics.

Documentation alignment: --no-bail behavior and availability for run vs recursive
.changeset/report-failure-flag.md[1-6]
exec/commands/src/run.ts[140-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Documentation added in this PR states `--no-bail` continues to exit 0 unless `--report-failure` is used, but compliance requires `--no-bail` to exit non-zero when any sub-run fails (and to document this consistently for run vs recursive).

## Issue Context
This mismatch risks user confusion and contradicts the required CLI semantics described in the compliance checklist.

## Fix Focus Areas
- .changeset/report-failure-flag.md[1-6]
- exec/commands/src/run.ts[140-147]
- pnpm/src/cmd/recursive.ts[83-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread exec/commands/src/run.ts
Comment thread .changeset/report-failure-flag.md Outdated
@zkochan zkochan force-pushed the fix/report-failure-flag branch from e6e8b82 to f9c6b94 Compare June 16, 2026 08:35
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f9c6b94

@zkochan zkochan changed the title feat(run): add --report-failure flag for use with --no-bail fix(run): make non-recursive --no-bail exit non-zero when a script fails Jun 16, 2026
Comment thread exec/commands/src/run.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
exec/commands/test/index.ts (2)

68-84: ⚡ Quick win

Guard caught errors before asserting code.

These new assertions read err.code from any catches. Use util.types.isNativeError(err) before checking the pnpm error code to follow the repo’s cross-realm Jest guidance. As per coding guidelines, “Jest/TS error checks: avoid instanceof Error; use util.types.isNativeError(err) when asserting error types/code across Jest realms.”

♻️ Proposed test assertion shape
 import fs from 'node:fs'
 import path from 'node:path'
+import { types as utilTypes } from 'node:util'
-  let err!: PnpmError
+  let err: unknown
   try {
     await run.handler({
       ...DEFAULT_OPTS,
@@
-  } catch (_err: any) { // eslint-disable-line
+  } catch (_err: unknown) {
     err = _err
   }
 
-  expect(err).toBeDefined()
-  expect(err.code).toBe('ERR_PNPM_RUN_FAILED')
+  expect(utilTypes.isNativeError(err)).toBe(true)
+  expect((err as PnpmError).code).toBe('ERR_PNPM_RUN_FAILED')

Apply the same pattern to the regex failure test.

Also applies to: 99-115

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@exec/commands/test/index.ts` around lines 68 - 84, The error caught in the
try-catch block is typed as any, and the code directly accesses err.code without
validation, which violates cross-realm Jest error handling guidelines. Add a
guard check using util.types.isNativeError(err) before asserting on err.code to
ensure the error is a native error before accessing its properties. Apply the
same guard pattern to the regex failure test mentioned in the "Also applies to"
section (lines 99-115) to maintain consistency across all error assertions in
this test file.

Source: Coding guidelines


90-116: ⚡ Quick win

Make the regex failure test prove every matched script ran.

The test name says every matched script runs, but the scripts only exit; there is no side effect proving lint:a and lint:c completed when lint:b fails. Add output assertions so a regression back to fail-fast behavior is caught.

✅ Proposed coverage improvement
   prepare({
     scripts: {
-      'lint:a': 'node -e "process.exit(0)"',
-      'lint:b': 'node -e "process.exit(1)"',
-      'lint:c': 'node -e "process.exit(0)"',
+      'lint:a': 'node -e "require(\'fs\').writeFileSync(\'./output-lint-a.txt\', \'a\', \'utf8\')"',
+      'lint:b': 'node -e "require(\'fs\').writeFileSync(\'./output-lint-b.txt\', \'b\', \'utf8\'); process.exit(1)"',
+      'lint:c': 'node -e "require(\'fs\').writeFileSync(\'./output-lint-c.txt\', \'c\', \'utf8\')"',
     },
   })
@@
   expect(err).toBeDefined()
   expect(err.code).toBe('ERR_PNPM_RUN_FAILED')
+  expect(fs.readFileSync('output-lint-a.txt', { encoding: 'utf-8' })).toBe('a')
+  expect(fs.readFileSync('output-lint-b.txt', { encoding: 'utf-8' })).toBe('b')
+  expect(fs.readFileSync('output-lint-c.txt', { encoding: 'utf-8' })).toBe('c')
 })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@exec/commands/test/index.ts` around lines 90 - 116, The test `pnpm run with
regex and --no-bail runs every matched script but exits non-zero when one fails`
does not actually prove that all three matched scripts (lint:a, lint:b, lint:c)
executed. Currently the test only verifies that an error was thrown with the
correct code, but there is no observable output proving each script ran when one
failed. Modify each script in the prepare block to produce distinct output (for
example, echoing a unique identifier), then add assertions in the test that
verify each expected output is present in the command's output. This will
demonstrate that execution continued through all scripts despite the --no-bail
flag and lint:b failing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@exec/commands/test/index.ts`:
- Around line 68-84: The error caught in the try-catch block is typed as any,
and the code directly accesses err.code without validation, which violates
cross-realm Jest error handling guidelines. Add a guard check using
util.types.isNativeError(err) before asserting on err.code to ensure the error
is a native error before accessing its properties. Apply the same guard pattern
to the regex failure test mentioned in the "Also applies to" section (lines
99-115) to maintain consistency across all error assertions in this test file.
- Around line 90-116: The test `pnpm run with regex and --no-bail runs every
matched script but exits non-zero when one fails` does not actually prove that
all three matched scripts (lint:a, lint:b, lint:c) executed. Currently the test
only verifies that an error was thrown with the correct code, but there is no
observable output proving each script ran when one failed. Modify each script in
the prepare block to produce distinct output (for example, echoing a unique
identifier), then add assertions in the test that verify each expected output is
present in the command's output. This will demonstrate that execution continued
through all scripts despite the --no-bail flag and lint:b failing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc2c1be8-9f41-4e3e-b6a1-ab89c9f6d453

📥 Commits

Reviewing files that changed from the base of the PR and between e6e8b82 and f9c6b94.

📒 Files selected for processing (3)
  • .changeset/no-bail-exit-code.md
  • exec/commands/src/run.ts
  • exec/commands/test/index.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/no-bail-exit-code.md

Previously `pnpm run --no-bail` (without -r) always exited with code 0 even
when a matched script failed, while recursive runs failed at the end via
throwOnCommandFail. --no-bail now runs every matched script to completion and
exits non-zero if any failed, making the behavior consistent across recursive
and non-recursive runs. Closes pnpm#8013.
@zkochan zkochan force-pushed the fix/report-failure-flag branch from f9c6b94 to 9055482 Compare June 16, 2026 08:58
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9055482

@zkochan

zkochan commented Jun 16, 2026

Copy link
Copy Markdown
Member

Addressed the latest review feedback in 9055482:

  • Qodo (failure→script attribution): the RUN_FAILED hint now prefixes each failed line with its script name.
  • CodeRabbit (cross-realm error checks): the new tests now guard caught errors with util.types.isNativeError(err) before asserting err.code.
  • CodeRabbit (prove every matched script ran): the regex --no-bail test now has each script write a distinct marker file and asserts all three exist, so a regression back to fail-fast (Promise.all) would be caught.

All exec/commands tests pass and lint/typecheck are clean.


Written by an agent (Claude Code, claude-opus-4-8).

Comment thread exec/commands/test/index.ts
@zkochan zkochan merged commit 179ebc4 into pnpm:main Jun 16, 2026
18 checks passed
@antitoxic

Copy link
Copy Markdown
Contributor Author

@zkochan 🙇

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.

--no-bail behavior inconsistent

2 participants