fix(webui): let a failed run's error bubble capture a trace - #7370
Merged
Merged
Conversation
A terminal run failure renders as a `role: "error"` chat message, which never carried a `turnRunId` and was excluded from the existing "download run artifact" action (gated on `role === assistant`). The action-button row itself was also gated on `showActions || showRetryAction`, neither true for an error bubble, so the button stayed hidden even once its role check was relaxed. Net effect: a failed run had no way to capture a trace for it (#7369). Thread `turnRunId` onto the run-failure message at creation, in-place update, and through the unknown-to-known-run-id promotion path, and let `showArtifactAction` fire for error messages that carry one. The existing download handler needed no changes — it was already keyed only on `threadId`/`turnRunId`, not role. Fixes #7369. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
🚅 Deployed to the ironclaw-pr-7370 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7370
August 7, 2026 19:16
Destroyed
📝 WalkthroughSummary by CodeRabbit
WalkthroughFailed run messages now retain ChangesFailed-run artifact downloads
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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. Comment |
github-actions
Bot
added
risk: low
Contributor
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟥 Final result · Could not complete
Automatic trigger · attempt 1 of 3 · failed after 5m 49s The generated review did not pass diff verification, so IronLoop did not publish it. Inspect the current pull request and start a new Review Run if appropriate. Failure details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
`@crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsx`:
- Around line 320-324: Restrict showArtifactAction in message-bubble.tsx to
errors with a normalized terminal failureStatus such as failed or
recovery_required, while preserving the existing assistant final-reply
conditions. In message-bubble.test.ts, set failureStatus to failed for the
eligible error case and add coverage confirming a nonterminal error does not
render the artifact action.
🪄 Autofix
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: af6e9eae-679f-4b6e-a82c-9fd4727df894
📒 Files selected for processing (5)
crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.tscrates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsxcrates/product/ironclaw_webui/frontend/src/pages/chat/lib/message-types.tscrates/product/ironclaw_webui/frontend/src/pages/chat/lib/useChatEvents.test.tscrates/product/ironclaw_webui/frontend/src/pages/chat/lib/useChatEvents.ts
Comment on lines
320
to
+324
| const showArtifactAction = Boolean( | ||
| role === CHAT_MESSAGE_ROLES.ASSISTANT && | ||
| message.isFinalReply === true && | ||
| !isOptimistic && | ||
| (isError || | ||
| (role === CHAT_MESSAGE_ROLES.ASSISTANT && | ||
| message.isFinalReply === true && | ||
| !isOptimistic)) && |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict run-artifact downloads to terminal run failures.
isError accepts every error message. A generic or nonterminal error with turnRunId can display the run-artifact action. Require a normalized terminal failureStatus, such as failed or recovery_required, before rendering the action.
crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsx#L320-L324: add a terminal failure-status check toshowArtifactAction.crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts#L394-L439: setfailureStatus: "failed"for the eligible error case, and add a nonterminal error case that does not render the action.
📍 Affects 2 files
crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsx#L320-L324(this comment)crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts#L394-L439
🤖 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
`@crates/product/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsx`
around lines 320 - 324, Restrict showArtifactAction in message-bubble.tsx to
errors with a normalized terminal failureStatus such as failed or
recovery_required, while preserving the existing assistant final-reply
conditions. In message-bubble.test.ts, set failureStatus to failed for the
eligible error case and add coverage confirming a nonterminal error does not
render the artifact action.
Collaborator
|
@ironloopai review |
Contributor
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Manual command by serrrfirat · attempt 1 of 3 · completed in 6m 9s IronLoop completed the review and posted it to GitHub. 🔗 Result |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 IronLoop review
Completed review of the exact merge-base-to-head diff. Found one medium-severity gap in the failed-run artifact flow.
Findings: 🟠 Medium 1
🟠 Medium · Backfill IDs on connection-loss failure bubbles
Inline on crates/product/ironclaw_webui/frontend/src/pages/chat/lib/useChatEvents.ts:868. See the inline comment for details.
Validation
- ✅ Diff hygiene — `git diff --check refs/ironloop/merge-base refs/ironloop/head` passed.
- ✅ Frontend lint — `pnpm lint` passed using Node 22.22.0 and pnpm 11.7.0.
- ✅ Focused frontend tests — Changed message-bubble and useChatEvents suites passed: 81 tests.
- ✅ Full frontend tests — `pnpm test` passed: 132 files, 1,140 tests.
- ✅ Frontend build — `pnpm build` passed.
Review details
- Run:
0e2f84dc-927a-4680-af36-5f0d2dfc6a54 - Workflow: Review
- Attempts: 1
| failureStatus: status, | ||
| failureCategory, | ||
| failureSummary, | ||
| turnRunId: runId, |
Contributor
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🟠 Medium · Backfill IDs on connection-loss failure bubbles
This assignment is unreachable when a known-run connection-loss bubble already exists and the later terminal `failed` projection has neither `failure_category` nor `failure_summary` (both are optional). `upsertConnectionLostRunFailure` creates `err-<runId>` without `turnRunId`; then `hasUsefulUpdate` is false and the early return preserves that bubble before this assignment runs. The newly enabled artifact action requires `turnRunId`, so a run that disconnects and then fails still has no trace-download button. Backfill the ID even when the visible failure content does not change (and preserve it on the connection-loss-created bubble), with a regression test for that sequence.
serrrfirat
approved these changes
Aug 7, 2026
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
) A terminal run failure renders as a `role: "error"` chat message, which never carried a `turnRunId` and was excluded from the existing "download run artifact" action (gated on `role === assistant`). The action-button row itself was also gated on `showActions || showRetryAction`, neither true for an error bubble, so the button stayed hidden even once its role check was relaxed. Net effect: a failed run had no way to capture a trace for it (nearai#7369). Thread `turnRunId` onto the run-failure message at creation, in-place update, and through the unknown-to-known-run-id promotion path, and let `showArtifactAction` fire for error messages that carry one. The existing download handler needed no changes — it was already keyed only on `threadId`/`turnRunId`, not role. Fixes nearai#7369. Co-authored-by: Sergey <sergey@Mac.attlocal.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
) A terminal run failure renders as a `role: "error"` chat message, which never carried a `turnRunId` and was excluded from the existing "download run artifact" action (gated on `role === assistant`). The action-button row itself was also gated on `showActions || showRetryAction`, neither true for an error bubble, so the button stayed hidden even once its role check was relaxed. Net effect: a failed run had no way to capture a trace for it (nearai#7369). Thread `turnRunId` onto the run-failure message at creation, in-place update, and through the unknown-to-known-run-id promotion path, and let `showArtifactAction` fire for error messages that carry one. The existing download handler needed no changes — it was already keyed only on `threadId`/`turnRunId`, not role. Fixes nearai#7369. Co-authored-by: Sergey <sergey@Mac.attlocal.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
) A terminal run failure renders as a `role: "error"` chat message, which never carried a `turnRunId` and was excluded from the existing "download run artifact" action (gated on `role === assistant`). The action-button row itself was also gated on `showActions || showRetryAction`, neither true for an error bubble, so the button stayed hidden even once its role check was relaxed. Net effect: a failed run had no way to capture a trace for it (nearai#7369). Thread `turnRunId` onto the run-failure message at creation, in-place update, and through the unknown-to-known-run-id promotion path, and let `showArtifactAction` fire for error messages that carry one. The existing download handler needed no changes — it was already keyed only on `threadId`/`turnRunId`, not role. Fixes nearai#7369. Co-authored-by: Sergey <sergey@Mac.attlocal.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
role: "error"chat message. That message never carried aturnRunId, and the existing "download run artifact" action was gated onrole === assistant && isFinalReply, so it never appeared for a failed run — matching the report: no button to capture a trace when the agent errors.showActions || showRetryAction, both false for an error bubble, so relaxing the role check alone would still have been a no-op.turnRunIdonto the run-failure message at creation, at in-place update, and through the unknown→known-run-id promotion path (err-unknown→err-<runId>), and letshowArtifactAction/the button row fire for error messages that carry aturnRunId. The existingdownloadArtifacthandler needed no changes — it was already keyed only onthreadId/turnRunId, never role.Fixes #7369.
Test plan
message-bubble.test.ts(button renders for a failed-run error bubble with a known run id, respects the deployment gate, and stays hidden with no run id yet) anduseChatEvents.test.ts(three mutation paths: fresh creation, in-place update, and unknown→known-run-id promotion all setturnRunId).npx vitest run— full frontend suite: 1139 passed, 1 pre-existing unrelated failure (theme.test.tsx, alocalStoragemocking issue, confirmed to fail identically onmainbefore this change).pnpm lint(conventions +tsc --noEmit) — clean.pnpm build— production build succeeds, bundle budgets pass.🤖 Generated with Claude Code