fix: prevent shell failure render crash by Astro-Han · Pull Request #174 · Astro-Han/pawwork

Astro-Han · 2026-04-22T22:08:25Z

Summary

Fixes the renderer crash risk when a user-entered shell command fails through composer shell mode.

Why

Issue #171 showed that shell failures such as definitely-not-a-command-171 --help produce valid command-not-found output, but the renderer could crash while displaying the completed bash tool part. This is separate from #170, which covers PATH and GitHub CLI environment assembly.

Related Issue

Fixes #171.

How To Verify

cd packages/ui
bun test src/components/basic-tool-trigger.test.ts src/components/message-part-stale.test.ts
bun run typecheck
cd ../app
bun run test:e2e e2e/prompt/prompt-shell.spec.ts
bun run typecheck
bun run test:unit

Screenshots or Recordings

Not included. This is a renderer crash fix for an existing shell output view, not a visual design or copy change. The e2e regression verifies failed shell output renders without the Solid Switch page error.

Checklist

I linked the related issue, or stated why there is no issue
This PR has type, scope, and priority labels, or I requested maintainer labeling
I listed the relevant verification steps, including tests when behavior changed
I manually checked visible UI or copy changes when needed, with screenshots or recordings
I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, or generated/local file changes when relevant
I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

Tests
- Added an end-to-end test to verify shell command failure is shown and backend output is validated
- Added a unit test to ensure trigger rendering no longer uses legacy branching patterns
Refactor
- Updated trigger rendering logic in the basic tool to a consolidated implementation (no behavior changes)

coderabbitai · 2026-04-22T22:08:33Z

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 29 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 29 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9fc931a0-68e3-43f2-b422-498ab059c1b9

📥 Commits

Reviewing files that changed from the base of the PR and between 00530f5 and 846bf56.

📒 Files selected for processing (3)

packages/app/e2e/prompt/prompt-shell.spec.ts
packages/ui/src/components/basic-tool-trigger.test.ts
packages/ui/src/components/basic-tool.tsx

📝 Walkthrough

Walkthrough

Refactors trigger rendering in BasicTool to remove Solid Switch/Match, adds a Bun unit test to assert the trigger implementation avoids those patterns, and introduces an e2e Playwright test plus a page-error capture helper that runs an invalid shell command and asserts no renderer crash occurs.

Changes

Cohort / File(s)	Summary
E2E Test & Helper `packages/app/e2e/prompt/prompt-shell.spec.ts`	Adds a Page-scoped `pageerror` listener helper with `dispose()` and a test that executes an invalid shell command, asserts expected failure output via renderer and backend session messages, and verifies no captured `pageerror` contains `switchFunc(...) is not a function`.
Component refactor `packages/ui/src/components/basic-tool.tsx`	Replaces SolidJS `Switch`/`Match` rendering with a `triggerInfo()` helper that returns either structured trigger markup or the raw `props.trigger`, and removes `Switch`/`Match` imports.
Unit test `packages/ui/src/components/basic-tool-trigger.test.ts`	Adds a Bun test that reads the `basic-tool.tsx` source, extracts the trigger implementation, and asserts it does not contain `<Switch>` or a Solid `<Match when={isTriggerTitle(...)}>` pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I sniffed the code, a cautious hop,
Replaced the Switch so renders won't drop,
Tests chase errors, catch the fall,
Backend, renderer, and tests stand tall,
Hooray — no more crash, just carrots and code 🍃

🚥 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
Title check	✅ Passed	The title 'fix: prevent shell failure render crash' directly summarizes the main change - fixing a renderer crash when shell commands fail.
Description check	✅ Passed	The PR description covers all required template sections: summary, why, related issue, verification steps, and checklist items completed.
Linked Issues check	✅ Passed	Code changes address issue `#171` objectives: refactoring Switch/Match logic to prevent the Solid Switch TypeError, adding e2e test to confirm failed shell output renders without page errors, and ensuring renderer resilience to command failures.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the shell command failure render crash: refactoring trigger rendering logic, adding regression tests, and improving component resilience.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/fix-i171-shell-render-crash

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

gemini-code-assist

Code Review

This pull request addresses a renderer crash by refactoring the BasicTool component to use a helper function instead of SolidJS Switch and Match components. It includes a new E2E test for shell command failures and a regression test that checks the source code for the removed components. Feedback indicates that the source-code-based regression test is fragile and should be made more robust.

Astro-Han · 2026-04-23T00:20:10Z

@coderabbitai review

coderabbitai · 2026-04-23T00:20:18Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app/e2e/prompt/prompt-shell.spec.ts`:
- Around line 120-121: Remove the wall-clock wait: delete the call to
page.waitForTimeout(250) in the test and replace it with a deterministic wait
that asserts the desired state (for example use page.waitForFunction to poll
until errors.pageErrors does not contain "switchFunc(...) is not a function", or
waitForSelector / waitForResponse that indicates the operation completed).
Specifically, update the assertion block around errors.pageErrors.join("\n") so
the test waits on a condition tied to application state instead of fixed time,
keeping the final expect(...) check the same.
- Around line 79-117: The test asserts the exact shell error text "command not
found", which is platform-specific; update the assertions in the shell flow
(references: project.shell, withSession,
page.locator('[data-component="bash-output"]'), the poll that reads
project.sdk.session.messages and uses isBash) to accept platform variations
(e.g., test for either "command not found" OR Windows "is not recognized" or use
a regex/substring check that matches both), and replace both the immediate UI
expect and the polled message assertion to use this resilient check so CI won't
fail on Windows.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8e7de3ff-2dcd-4a3a-a5e7-851513ff257b

📥 Commits

Reviewing files that changed from the base of the PR and between 0e39c3e and 8f87b81.

📒 Files selected for processing (3)

packages/app/e2e/prompt/prompt-shell.spec.ts
packages/ui/src/components/basic-tool-trigger.test.ts
packages/ui/src/components/basic-tool.tsx

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app/e2e/prompt/prompt-shell.spec.ts`:
- Around line 79-80: Rename the test-level constant identifiers to
SCREAMING_SNAKE_CASE: change command to COMMAND (or COMMAND_TEST) and
commandFailurePattern to COMMAND_FAILURE_PATTERN, and apply the same renaming
for the other test constants referenced (lines grouping around 88-93, 107-113,
118) so all test constants use SCREAMING_SNAKE_CASE; update every usage of these
identifiers in prompt-shell.spec.ts (e.g., references inside tests and helper
calls) to the new names to avoid breakage.

In `@packages/ui/src/components/basic-tool-trigger.test.ts`:
- Around line 6-8: The test currently slices source using indices from
source.indexOf("const triggerInfo = () =>") and source.indexOf("const trigger =
() =>", triggerInfoStart) without verifying those calls found the markers; add
guard assertions to ensure triggerInfoStart !== -1, triggerStart !== -1 and
triggerStart > triggerInfoStart before creating triggerSource (i.e., assert the
marker strings exist and are in order), so the test fails deterministically
instead of producing false-positive passes when markers are missing or
reordered.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6849ce00-2179-4c4e-a19b-0956f6ad4aff

📥 Commits

Reviewing files that changed from the base of the PR and between 8f87b81 and 00530f5.

📒 Files selected for processing (3)

packages/app/e2e/prompt/prompt-shell.spec.ts
packages/ui/src/components/basic-tool-trigger.test.ts
packages/ui/src/components/basic-tool.tsx

Astro-Han added bug Something isn't working P1 High priority app Application behavior and product flows labels Apr 22, 2026

gemini-code-assist Bot reviewed Apr 22, 2026

View reviewed changes

Comment thread packages/ui/src/components/basic-tool-trigger.test.ts Outdated

coderabbitai Bot reviewed Apr 23, 2026

View reviewed changes

Comment thread packages/app/e2e/prompt/prompt-shell.spec.ts Outdated

Comment thread packages/app/e2e/prompt/prompt-shell.spec.ts Outdated

Astro-Han force-pushed the codex/fix-i171-shell-render-crash branch from 8f87b81 to 00530f5 Compare April 23, 2026 00:32

coderabbitai Bot reviewed Apr 23, 2026

View reviewed changes

Comment thread packages/app/e2e/prompt/prompt-shell.spec.ts Outdated

Comment thread packages/ui/src/components/basic-tool-trigger.test.ts

fix: prevent shell failure render crash

846bf56

Astro-Han force-pushed the codex/fix-i171-shell-render-crash branch from 00530f5 to 846bf56 Compare April 23, 2026 01:06

Astro-Han merged commit 5c8fddf into dev Apr 23, 2026
23 checks passed

Astro-Han deleted the codex/fix-i171-shell-render-crash branch April 23, 2026 01:27

This was referenced Apr 23, 2026

feat(app): home redesign per #151 #180

Merged

[Bug] Shell-mode any command triggers renderer Switch crash (beyond #174) #182

Closed

fix: prevent home shell session crash #183

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent shell failure render crash#174

fix: prevent shell failure render crash#174
Astro-Han merged 1 commit into
devfrom
codex/fix-i171-shell-render-crash

Astro-Han commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Astro-Han commented Apr 23, 2026

Uh oh!

coderabbitai Bot commented Apr 23, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented Apr 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

How To Verify

Screenshots or Recordings

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Astro-Han commented Apr 23, 2026

Uh oh!

coderabbitai Bot commented Apr 23, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading