Show automation run error detail#1413
Conversation
|
@DevinVinson is attempting to deploy a commit to the openhands Team on Vercel. A member of the Team first needs to authorize it. |
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable — the implementation is small and the core UI path is sound, but the PR still lacks the required runtime evidence for a visible UI change.
Reviewer verification:
npm run test -- --run src/components/features/automations/detail/run-logs-modal.test.tsx✅npm run typecheck✅npm run build✅
[TESTING GAPS]
- [PR description] No end-to-end evidence: The
How to TestandVideo/Screenshotssections are empty. This changes visible automation-run UI behavior, so please add concrete evidence from the real product path — for example, a screenshot/video of a failed automation run witherror_detailshown in the logs modal, plus the exact steps or commands used to produce it. The unit test is useful, but by itself it does not prove the backend payload and automation detail page work end-to-end.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
Localized frontend-only change with no dependency or backend API changes, and the code-level data flow is straightforward. Risk is medium because this surfaces backend-provided automation error detail in the UI and changes the debugging path users rely on; the missing runtime evidence should be addressed before approval.
VERDICT:
❌ Needs more evidence before approval: I did not find a code correctness blocker, but the PR description needs concrete UI/runtime evidence before this should be approved.
KEY INSIGHT:
AutomationRun.error_detail is treated as supplemental debug information, preserving fetched stdout/stderr instead of replacing them — that is the right data-flow shape for this feature.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.- Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it's merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
HUMAN:
I stumbled into this error detail being available from the automations server and wanted to help get it surfaced.
AGENT:
Why
I want to surface the errror_detail content if available without getting rid of the other bits. I didn't dig into what is already showing and why.
Summary
Issue Number
#1037 related although it doesn't "fix" this.
How to Test
Video/Screenshots
Type
Notes