Skip to content

feat(comments): retry failed @claude replies in place#72

Merged
sam-powers merged 1 commit into
mainfrom
fix/claude-reply-retry-ux
Jul 10, 2026
Merged

feat(comments): retry failed @claude replies in place#72
sam-powers merged 1 commit into
mainfrom
fix/claude-reply-retry-ux

Conversation

@sam-powers

Copy link
Copy Markdown
Owner

What & why

A failed @claude reply used to offer only a single, heavyweight "Re-link session…" button — even when the failure was a transient API hiccup and the right fix was a plain retry. Re-linking opens the session picker and is the wrong tool for a temporary error; there was no way to retry the identical request in place.

This PR classifies the failure and surfaces the affordance that actually fixes it, and stops transient reply state from ever reaching disk.

What changed

  • classifyReplyError(message){ retryable, kind } (co-located with the pure helpers in useClaudeReply.ts, exported + unit-tested). Ordered rules, specific before broad: sessionauthtransientunknown (unknown defaults to retryable). Case-insensitive. The 401 and login auth markers are word-boundary-anchored so a port number like 24010 or a hostname like mylogin.internal can't force a false non-retryable auth verdict.
  • retryAIReply(commentId, replyId) (useComments.ts) resets the same reply entry to a fresh pending state (pending: true, error: undefined, text: '') — it never appends a new reply, so the id is stable and the count is unchanged. Unknown replyId is a no-op.
  • useClaudeReply.retry(replyId) stashes each ask's inputs in a transient in-memory map, then re-issues the identical spawn reusing the same replyId. A per-replyId generation guard drops late/stale stream events and orphan-cancels a superseded spawn, so a retry that supersedes a slow original isn't clobbered by the original's late done/error. Each spawn cancels by its own captured token, not a replyId re-lookup — otherwise a stale original's late terminal event would cancel the live retry (see the race test). A retryingRef set prevents a double-fire retry. This generation-guard shape is factored to be reused by the upcoming cancel-during-review fix.
  • UI by classifier kind (CommentCard.tsx, threaded through CommentLayer.tsx and App.tsx): transient/unknown → Retry primary, no Re-link; sessionRe-link primary, Retry secondary; authRe-link only (not retryable).
  • stripTransientReplyState(comments) drops errored and pending AI replies before serialization, applied at both persistence choke points (the .comments.json sidecar and the draft.json autosave snapshot) so transient UI state never lands on disk. No schema change (SidecarFile stays version: 2).

Testing

  • npm run typecheck, npm run lint, npm run format:check — clean.
  • npm test275 vitest tests pass, including:
    • classifyReplyError — one case per kind, case-insensitivity, ordering (session before transient), and word-boundary negative controls (24010/mylogin don't false-match; a plain word isn't auth; empty/whitespace/undefined → unknown, no throw).
    • retryAIReply — clears error, sets pending, resets text, count unchanged, id stable, unknown replyId no-op.
    • stripTransientReplyState — errored + pending AI replies dropped; user replies and finished AI replies kept.
    • A generation-guard race test (useClaudeReplyRetry.test.ts) that reproduces the cross-generation orphan-cancel hazard: a superseded original's late done is dropped and cancels its own orphan (not the live retry), and a double-fire retry is coalesced to one spawn.
  • Rust bar (cargo fmt --check, cargo clippy -- -D warnings, cargo test) unaffected — this is a frontend-only change and stays green.

The full racing-spawn flow end-to-end needs the packaged runtime (a live claude CLI); it's verified manually and the driving window.__quillMock seam is exercised by the vitest race test above. agent-browser is not installed, so no Playwright run here.

🤖 Generated with Claude Code

A failed @claude reply previously offered only a heavyweight
"Re-link session…" button, even when the right fix was a simple
retry. Classify the failure and surface the right affordance:

- classifyReplyError (in useClaudeReply.ts) maps an error message to
  { retryable, kind } with ordered rules (session → auth → transient →
  unknown, unknown defaulting to retryable). 401/login markers are
  word-boundary-anchored so a port number or hostname can't force a
  false auth verdict.
- retryAIReply (useComments.ts) resets the SAME reply entry to a fresh
  pending state (never appends); useClaudeReply.retry re-issues the
  identical spawn, reusing the replyId.
- A per-replyId generation guard drops late/stale events and
  orphan-cancels a superseded spawn, so a retry that supersedes a slow
  original isn't clobbered by the original's late terminal event. Each
  spawn cancels by its OWN captured token, not a replyId re-lookup —
  otherwise a stale original's late event would cancel the live retry.
- CommentCard renders Retry/Re-link by classifier kind: transient or
  unknown → Retry primary; session → Re-link primary, Retry secondary;
  auth → Re-link only.
- stripTransientReplyState drops errored/pending AI replies before
  serialization, at both the sidecar and draft choke points, so
  transient UI state never reaches disk.

Coverage: vitest (classifyReplyError incl. word-boundary negative
controls, retryAIReply, stripTransientReplyState, and a
generation-guard race test that reproduces the cross-generation
orphan-cancel hazard). The full racing-spawn e2e needs the packaged
runtime and is verified manually — agent-browser is not installed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ESi6dnK3Wc1jYRpZN5qfZA
@sam-powers
sam-powers merged commit c0a7e9b into main Jul 10, 2026
3 checks passed
@sam-powers
sam-powers deleted the fix/claude-reply-retry-ux branch July 10, 2026 12:42
@sam-powers sam-powers mentioned this pull request Jul 10, 2026
sam-powers added a commit that referenced this pull request Jul 10, 2026
Patch release bundling the fixes from PRs #70#74:

- @claude CLI resolution: working PATH + prefer configured binary (#70)
- desktop: menu stale-closure, window-close capability, deep-link
  window surfacing (#71)
- retry failed @claude replies in place (#72)
- re-run cancelled @claude replies; remove broken new-session (#73)
- surface @claude in composer placeholders; widen reading measure (#74)

Also removes the stray docs/to-improve and docs/plans files that landed
via #70 — those are downstream porting notes, not public-repo content.


Claude-Session: https://claude.ai/code/session_01ESi6dnK3Wc1jYRpZN5qfZA

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant