fix: correctly handle zero exit code to avoid false positives on "failed" in hasCommandFailed - #1340
awhite0030 wants to merge 5 commits into
Conversation
…ed in update command Ensure that hasCommandFailed correctly ignores benign strings like 'failed' when the command successfully exits with 0. However, it still catches true explicit failures if the output contains messages like 'error:' or 'fatal' despite the 0 exit code.
nc-review: needs work — 1 blocking, 2 important, 1 nit@awhite0030 — there is a blocking item below. Fixes a real bug in hasCommandFailed (false-positive failure when exit code is 0 but output contains 'failed'/'cannot' substrings), and the test added covers the regression. However, the PR mixes in five auto-generated badge SVG rewrites — including stars.svg pointing to the contributor's own fork — that have nothing to do with the bug and one of which would be visually wrong if merged. The same bug is being fixed by open PR #1331, so this is a duplicate of work in flight.
🔴 blocking · Open PR #1331 ( 🟠 important · The badge rewrites are unrelated to the bug fix and are produced by the 🟠 important · The changeset filename and contents are not visible on disk (PR-only file), so this is a check against the diff context. The filename pattern matches other accepted changesets and the package name must be ⚪ nit · The new test ( 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Description
Root cause:
hasCommandFailedincorrectly evaluates to a failure whenexitCode === 0if the command output contains substring regexes from Strategy 2 likefailedorcannot(e.g., "0 failed" in an npm install log).Fix: Added explicit checks inside the exit code 0 block. It will now only trigger a failure if it finds critical explicit strings like
error:orfatal, but explicitly ignores patterns prone to false positives such asfailedorcannot. Otherwise, it returnsfalse.Validation:
Fixes #1304
Fixes #1304
Type of Change
Changeset
pnpm changeset) describing this change for the changelogDocs-only or internal chores need no changeset (or run
pnpm changeset --emptyto note that intentionally).Testing
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist