Skip to content

fix: Report failed GitHub PR and issue actions - #521

Merged
Ziinc merged 3 commits into
mainfrom
claude/github-action-errors
Sep 27, 2026
Merged

fix: Report failed GitHub PR and issue actions#521
Ziinc merged 3 commits into
mainfrom
claude/github-action-errors

Conversation

@Ziinc

@Ziinc Ziinc commented Sep 26, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Collaborator

Problem

  • In the GitHub panel's PR and issue detail panes, the comment, close, reopen and draft/ready actions had no onError handler. When gh failed (auth, permissions, network), the button reset and the user saw no message.
  • useMutation.mutate() called void mutateAsync(...), and mutateAsync re-throws. Every failed fire-and-forget mutation in the app therefore became an unhandled promise rejection.

Fix

  • github-panel/shared.tsx: add a useGhErrorToast() helper that returns an onError which shows an error toast containing the gh message.
  • PrDetail.tsx and IssueDetail.tsx: every action now has a titled error toast, for example "Failed to close pull request" or "Failed to post comment". When a comment fails to post, the typed comment stays in the box.
  • useMutation.ts: mutate() now catches the rejection. The error still reaches error, isError and onError, and mutateAsync still throws for callers that await it.

Tests

  • Integration (test/integration/github-panel-detail.test.tsx): failing Close PR, failing PR comment (the draft is kept) and failing Close Issue. All three failed, with unhandled rejections, before the fix and pass after it.
  • Screenshot spec github-action-error-toast.spec.tsx: capture checked against its expectations.
  • npm run test:unit, lint, check and format are clean.
    Generated by Claude Code

Ziinc
Ziinc commented Sep 26, 2026
View reviewed changes

@Ziinc Ziinc left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for reviewers explaining why the error handling takes this approach.

Generated by Claude Code

Comment thread src/hooks/useMutation.ts

@Ziinc Ziinc Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutate is the fire-and-forget form, so it now catches the error instead of letting it escape as an unhandled rejection.

  • The error still reaches error, isError and onError, so no error UI changes.
  • mutateAsync still throws, so callers that await it keep their try/catch behavior.

Generated by Claude Code

Comment thread src/components/github-panel/shared.tsx

@Ziinc Ziinc Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGhErrorToast lives here because both detail panes need it.

  • It uses a toast rather than inline text, which matches the existing openOrCreateWorkspace error path in PrDetail.
  • String(error) covers Tauri invoke rejections, which arrive as plain strings rather than Error objects.

Generated by Claude Code

@Ziinc Ziinc mentioned this pull request Sep 26, 2026
Merged
@Ziinc Claude

Ziinc commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator Author

test-js / test:integration failed on two runs here, on two different tests. Neither failure comes from this PR.

  • Run 1: remote-workspace-ui.test.tsx > refreshes when the workspace change marker advances. This test polls the filesystem with an 8s wait. It passed 5 of 5 times locally on this branch.
  • Run 2: workspace/reviews.test.tsx > is able to mark a file as viewed and back to unviewed timed out at the 5s default. The Viewed toggle does not go through useMutation or the GitHub panel. The same test fails on main too: 2 of 5 local runs, and 1 of 4 when every core is busy.

The reviews test runs too close to the 5s default when the machine is loaded. I opened #536 to give it the same 15s budget as the other heavy Dashboard tests, and ported that commit here (3c35ac6). The ported change becomes a no-op once #536 merges. With every core busy, the file passed 6 of 6 runs with the fix.

Generated by Claude Code

@claude
fix: Report failed GitHub PR and issue actions
eb4d4f6 
Comment, close, reopen and draft/ready actions in the GitHub panel's PR
and issue detail panes had no error handling, so a gh failure just reset
the button with no message. They now raise an error toast carrying the gh
message.

useMutation.mutate() also re-threw into a void promise, so every failed
fire-and-forget mutation surfaced as an unhandled rejection. The error
already reaches onError and the error state, so mutate() now swallows it.
@Ziinc
Ziinc force-pushed the claude/github-action-errors branch from 3c35ac6 to eb4d4f6 Compare September 27, 2026 03:01
@Ziinc Ziinc mentioned this pull request Sep 27, 2026
Merged
@claude
fix: Keep a file marked Viewed when its diff finishes loading
8670207 
handleMarkFileViewed hashed whatever hunks were in allFileHunks at click
time. While a diff is loading that entry is a placeholder with empty
hunks, so marking a file Viewed early stored the hash of the placeholder.
Once the real hunks arrived, the stale-content effect saw a mismatch and
silently un-marked the file. This is also the root cause of the flaky
reviews "mark a file as viewed" integration test.

Only hash loaded hunks; an empty hash is never treated as stale.

(cherry picked from commit 6ef2cf8)
@claude
test: Persist the agentMessageQueue preview flag in its flag-off test
8968e61 
"hides the message queue when the agentMessageQueue preview is off"
only set the in-memory feature preview store. Rendering the Dashboard
hydrates settings, and hydrateFlags rebuilds every flag from defaults
(all on in test mode) plus persisted settings, so whenever hydration
resolved after the setState the flag flipped back on and the queue
button rendered. Persist the setting through setSetting, as
feature-preview.test.tsx does, and restore it when the test finishes.

(cherry picked from commit 88ab81c)
@Ziinc
Ziinc force-pushed the claude/github-action-errors branch from 37df85c to 8968e61 Compare September 27, 2026 03:29
@Ziinc Claude

Ziinc commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator Author

Update after rebasing onto main: #536's longer timeout covered only one way the "mark file as viewed" test fails. The other is a real race in useReview: a file marked Viewed while its diff is still loading un-marks itself once the diff arrives. It is fixed in #540.

main also has a new flaky test from #530, terminal-pane.test.tsx "agentMessageQueue preview is off". It is fixed in #539.

I copied both fixes into this branch (8670207, 8968e61). They become no-ops once those PRs merge.

Generated by Claude Code

@Ziinc Claude

Ziinc commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator Author

test-js / test:integration failed on 8968e61 in three tests unrelated to this PR:

  • settings-repo-yaml-config.test.tsx: "syncs a newly written .treq/config.yaml when Settings is reopened"
  • browser/review.test.tsx: "rejects a non-localhost URL…" and "adds multiple element comments…", which hit the 5s timeout

None of these components go through useMutation or the GitHub panel, which are the only areas this diff touches. Both files pass 3 of 3 locally on this branch and on main. Three unrelated timing-sensitive tests failing together in one run suggests an overloaded runner. No fix exists yet. I'm re-running the failed job once. If it fails again I'll treat it as real and root-cause it.

Generated by Claude Code

@Ziinc
Ziinc merged commit 5420f03 into main Sep 27, 2026
34 of 35 checks passed
@Ziinc
Ziinc deleted the claude/github-action-errors branch September 27, 2026 08:45

This branch was successfully deployed

1 active deployment
preview — 8968e619 Deployed Sep 27, 2026 by Ziinc via build #1295
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Ziinc @claude