Fix silent branch-delete, GitHub refresh-button spin, and Insights first-load - #17
Merged
Merged
Conversation
…is gone When a remote branch is deleted elsewhere (e.g. a merged PR whose branch was auto-deleted), the local refs/remotes/<remote>/<branch> tracking ref lingers until a fetch --prune. "Delete remote branch" then ran `git push --delete`, which git rejects with "remote ref does not exist" — so the phantom never cleared and every retry re-failed, with no useful feedback. delete_remote_branch now treats that specific case as success: it prunes the stale tracking ref locally (show-ref --verify, then update-ref -d) and returns Ok. Every other failure (auth, network, protected branch) still surfaces. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Failed nav/branch ops only set a muted status-bar line that was easy to miss and got overwritten by the next refresh, and the graph was not repainted on failure — so a partial success (local branch deleted, remote delete failed) looked like nothing happened. The run() wrapper now repaints the graph even on failure and surfaces the full error in a new message-only "alert" dialog, so a destructive op that fails is impossible to miss. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The spin animation was applied to the 30x30 bordered .refresh button, so transform: rotate() spun the border and background too. Wrap the glyph in an inline-block span and rotate that instead; the button box stays put. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GitHub computes /stats/commit_activity lazily (HTTP 202 on a cold cache), so the first Insights load can throw a transient error. The old retry loop only handled the `computing` return value, so a thrown error hard-errored the panel until the user switched tabs and back (which forced a re-fetch — the reported symptom). Add fetchActivityWithRetry: retries through both `computing:true` and thrown transient errors with backoff, aborts early on permanent errors (NotFound/auth), and resolves to the soft "computing" marker if stats aren't ready in time. The retry policy is a pure, injected helper with unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b54706de95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… edits Code review caught that reloadGraph() in the run() failure path resets graphCommits via setGraphCommits(), which clears newDates (queued commit-time edits), selected, and currentSha. That silently discarded the user's queued edits and selection on ANY failed op — including non-mutating ones (rejected checkout, branch-already-exists, fetch error) that cannot have partially applied. Swap it for the non-destructive refreshRefs(): the sidebar still updates for a genuine partial success (local branch deleted, remote delete failed), but graphCommits and the queued edits are left intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fada2ad27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…h locales Code review caught that delete_remote_branch matched git's English "remote ref does not exist" stderr to detect an already-deleted remote branch. Under a non-English git locale that message is translated, so the self-heal was skipped and the delete returned an error without pruning the stale tracking ref. Pin LC_ALL=C on the push --delete command so the diagnostic is always git's stable English text (Command::env overrides the child's locale regardless of the user's LANG/LC_*). Behavior is otherwise unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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.
Three UX bug fixes, each root-caused and committed independently.
1. Silent branch-delete failures (
fix(git)+fix(ui))Symptom: deleting a branch spun briefly then did nothing, with no feedback — reproducible on stale remote-tracking refs (a branch already deleted on the remote, e.g. by a merged PR).
Root cause: "Delete remote branch" ran
git push --delete, which git rejects withremote ref does not existonce the remote ref is already gone, so the phantom tracking ref was never pruned. On top of that, failures only set a muted status-bar line (no dialog, no error styling) and the view wasn't repainted on failure — so a partial success (local branch deleted, remote delete failed) looked like nothing happened.Fix:
delete_remote_branchself-heals: when the remote ref is already gone it prunes the stale local tracking ref (show-ref --verify→update-ref -d) and returns success. Every other failure (auth, network, protected branch) still surfaces. New unit test.run()now repaints the graph even on failure and surfaces the full error in a new message-only alert dialog, so a destructive op that fails is impossible to miss.2. GitHub refresh button spins the whole square (
fix(github))transform: rotate()was applied to the 30×30 bordered.refreshbutton, so the border and background rotated too. Wrapped the↻in an inline-block glyph and moved the rotation there; the button box now stays put (reduced-motion still dims instead of spinning).3. GitHub Insights "commit activity" errors on first load (
fix(github))Symptom: commit activity often errored on the first Insights visit, then loaded fine after switching to another tab and back.
Root cause: GitHub computes
/stats/commit_activitylazily (HTTP 202 on a cold cache), so the first request can throw a transient error. The retry loop only handled thecomputing: truereturn value — a thrown error bypassed it entirely and hard-errored the panel. The only recovery was the manual tab-switch, which re-fetches becausemakePaneldoesn't dedup theerrorstatus (andreloadNonceisn't bumped on tab-switch).Fix:
fetchActivityWithRetryretries through bothcomputing: trueand thrown transient errors with backoff, aborts early on genuinely-permanent errors (NotFound/NotAuthed/…), and resolves to the soft "still computing" marker if stats aren't ready in time. The retry policy is a pure, injected, unit-tested helper.Testing
npm run check— 0 errorsnpm test— 296 passed (28 files; +8 new activity-retry tests)cargo test— all crates green (+1 new self-heal test)The branch-delete self-heal was reproduced end-to-end against a throwaway remote, and the exact
remote ref does not existfailure was confirmed against a real stale tracking ref via a dry-run. A live 202 for the Insights fix couldn't be captured (GitHub's stat caches stayed warm across ~150 probed repos), so that fix is deliberately agnostic to the exact transient shape — it retries through any transient failure.🤖 Generated with Claude Code