Fix shell abort classification for Bash and PowerShell by chioarub · Pull Request #1688 · Twigpine/openclaude

chioarub · 2026-06-17T08:44:03Z

Summary

normalize shell abort reasons and attach safe abort metadata to shell command results
classify QueryGuard, user, interrupt, background, and parent-ended abort kills as cancellations instead of ordinary shell failures
keep command nonzero exits and shell tool timeouts as failures, preserving existing background command behavior
surface specific model-facing cancellation messages for Bash and PowerShell without logging raw commands or command output

Root Cause

Shell tools only suppressed ShellError for the narrow interrupt path. QueryGuard and user-cancel flows can arrive with other abort reasons, so abort-driven process kills could be reported as generic shell failures rather than cancellations.

Validation

bun test src/utils/abortReasons.test.ts src/utils/ShellCommand.test.ts src/tools/BashTool/BashTool.errorOutput.test.ts src/utils/toolErrors.test.ts
bun run typecheck
git diff --check

Summary by CodeRabbit

New Features
- Shell command error messages now include specific abort reasons when commands are cancelled or timed out
- Enhanced handling to distinguish between user-initiated cancellations and automatic timeouts
Bug Fixes
- Improved command cancellation tracking and error reporting to properly surface cancellation context

coderabbitai · 2026-06-17T09:16:39Z

📝 Walkthrough

Walkthrough

Introduces a new AbortReason string-union type with normalization, classification, and message helpers. Threads abort metadata (isAbort, abortReason, abortMessage, signalAborted, signal, durationMs) through ExecResult, ShellCommandImpl, Shell.ts, ShellError, and toolErrors. Both BashTool and PowerShellTool output schemas and analytics are updated to surface these fields.

Changes

Abort Reason Infrastructure and Tool Integration

Layer / File(s)	Summary
AbortReason type, normalization, and message helpers `src/utils/abortReasons.ts`, `src/utils/abortReasons.test.ts`	Defines the `AbortReason` union, alias map (`user-cancel` → `user-abort`), `normalizeAbortReason`, `isQueryLevelAbort`, and `getShellAbortMessage`; test suite validates all mappings and message completeness.
ExecResult and ShellCommandImpl abort tracking `src/utils/ShellCommand.ts`, `src/utils/ShellCommand.test.ts`	Extends `ExecResult` with `signal`, `durationMs`, `signalAborted`, `isAbort`, `abortReason`, `abortMessage`, `stdoutLength`, `stderrLength`; `ShellCommandImpl` tracks `#killedByAbort`, `#timedOut`, `#exitSignal`, `#startTime`; `AbortedShellCommand`/`createAbortedCommand` accept `abortReason`; `createFailedCommand` gains optional `code` param; tests cover abort vs. timeout distinctions.
Shell.ts abort normalization at spawn boundaries `src/utils/Shell.ts`	Applies `normalizeAbortReason` to pre-spawn aborts and the exec catch-block abort branch, passing normalized reason into `createAbortedCommand`.
ShellError abort metadata and toolErrors rendering `src/utils/errors.ts`, `src/utils/toolErrors.ts`, `src/utils/toolErrors.test.ts`	`ShellError` gains `abortReason`, `abortMessage`, `isAbort` fields with derivation logic; `formatError` uses `isAbortError`; `getErrorParts` uses `abortMessage` fallback; tests cover override and derivation rules.
BashTool abort output and analytics `src/tools/BashTool/BashTool.tsx`, `src/tools/BashTool/BashTool.errorOutput.test.ts`	Output schema and `Out` type gain abort fields; `call()` switches to `result.isAbort`; `ShellError` throw passes abort metadata; interrupted error rendering uses `abortMessage`; analytics extended; regression tests cover `query-timeout` and `user-cancel` paths.
PowerShellTool abort output and analytics `src/tools/PowerShellTool/PowerShellTool.tsx`	Mirrors BashTool: output schema, `call()` abort detection, `ShellError` throw, error rendering, backgrounded-path `isAbort: false`, analytics, and `Out` payload all updated to match abort contract.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

kevincodex1
jatmn

🚥 Pre-merge checks | ✅ 6 | ❌ 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 (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main focus of the PR: fixing shell abort classification for both Bash and PowerShell tools.
Description check	✅ Passed	The description covers the required sections with clear explanations of what changed, why, and includes specific validation steps, though it deviates from the template structure.
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.
Risk Surface Disclosed	✅ Passed	PR touches background execution through abort classification changes only (no mechanism changes). The single review comment addresses TypeScript exhaustiveness (non-blocking). No risk surface call-...
No Hidden Policy Change	✅ Passed	PR explicitly documents abort classification and telemetry changes: aborts classified as cancellations with safe enum abort_reason values, no hidden trust-model, routing, permission, or network pol...

_{✏️ 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

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

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

coderabbitai

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@src/utils/abortReasons.ts`:
- Around line 48-64: To improve exhaustiveness checking for future
maintainability, refactor the switch statement in getShellAbortMessage to
explicitly list each case separately instead of using fall-through cases. Expand
the grouped cases for user-abort, interrupt, parent-ended, and unknown-abort to
each have their own case statement with the same return message, which will
enable TypeScript to warn if a new AbortReason is added to the type but not
handled in the switch statement.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cae57a5b-66dd-41a9-9f97-1bbf1f4b0696

📥 Commits

Reviewing files that changed from the base of the PR and between 2351811 and ed1dedd.

📒 Files selected for processing (11)

src/tools/BashTool/BashTool.errorOutput.test.ts
src/tools/BashTool/BashTool.tsx
src/tools/PowerShellTool/PowerShellTool.tsx
src/utils/Shell.ts
src/utils/ShellCommand.test.ts
src/utils/ShellCommand.ts
src/utils/abortReasons.test.ts
src/utils/abortReasons.ts
src/utils/errors.ts
src/utils/toolErrors.test.ts
src/utils/toolErrors.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (11)

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript with strict mode and ESM imports

Files:

src/utils/ShellCommand.test.ts
src/utils/toolErrors.test.ts
src/utils/abortReasons.ts
src/utils/abortReasons.test.ts
src/utils/toolErrors.ts
src/tools/BashTool/BashTool.tsx
src/tools/BashTool/BashTool.errorOutput.test.ts
src/utils/Shell.ts
src/tools/PowerShellTool/PowerShellTool.tsx
src/utils/ShellCommand.ts
src/utils/errors.ts

{src/services/**/*.ts,src/utils/**/*.ts}

📄 CodeRabbit inference engine (AGENTS.md)

Use execa for child processes

Files:

src/utils/ShellCommand.test.ts
src/utils/toolErrors.test.ts
src/utils/abortReasons.ts
src/utils/abortReasons.test.ts
src/utils/toolErrors.ts
src/utils/Shell.ts
src/utils/ShellCommand.ts
src/utils/errors.ts

**/*.{ts,tsx,js,jsx,py,json,md}