Skip to content

feat(comments): re-run cancelled @claude replies; remove broken new-session#73

Merged
sam-powers merged 1 commit into
mainfrom
fix/claude-cancel-rerun-and-session-cleanup
Jul 10, 2026
Merged

feat(comments): re-run cancelled @claude replies; remove broken new-session#73
sam-powers merged 1 commit into
mainfrom
fix/claude-cancel-rerun-and-session-cleanup

Conversation

@sam-powers

@sam-powers sam-powers commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Stacked on #72 (retry failed @claude replies). Targeted at main so CI runs, so the diff currently includes #72's commitmerge #72 first, then this, and the diff collapses to just this group's changes.

What & why

Three fixes to the @claude reply flow plus a comment-panel layout follow-up, from docs/to-improve/.

Cancel → Re-run (edge case: "cancelling breaks the reply, need a re-run button")

Cancelling an @claude reply used to leave a dead partial. Now cancel resolves to a neutral, retryable state:

  • A cancelled stream event no longer finalizes the reply (finalizing would apply half-streamed tracked edits and masquerade as a finished answer). It marks the reply cancelled and keeps the stashed inputs, and the card shows a Re-run button that re-issues the same request in place against the same reply entry.
  • cancel(replyId) now supersedes the reply synchronously — it bumps the per-reply generation and marks the reply cancelled even when no cancel token is tracked yet. This closes the spawn-await window: a Cancel click landing between startAIReply and spawn_claude_resume resolving used to be silently swallowed (no token → early return), so the reply streamed to completion despite the stop. The parked runSpawn now sees a stale generation on resume and orphan-cancels its own child.
  • cancelled is a transient, UI-only reply flag — stripped from the sidecar and the draft snapshot, same as pending/error.

Remove "Start new session" (edge case: "create new session doesn't really work")

The Quill-minted-session path (createdByQuill / allow_create / --session-id) never reliably produced a resumable session. Every binding now resumes via --resume, and the button is gone (frontend + Rust). Legacy sidecars stamped createdByQuill: true load as plain bindings — the field is dropped by the whitelist sanitizer.

Comment panel: lowest card scrolls fully into view

A dynamic bottom spacer (computeBottomSpacer, derived from document-space card bottoms to avoid a scroll→reflow→scroll loop) extends the scroll range so the lowest comment card can be scrolled into view.

Tests

  • useClaudeReplyRetry.test.ts: cancel → neutral (not finished/errored, no edits applied); re-run in place; superseded-cancel drop; new spawn-await-window regression (Cancel mid-spawn marks cancelled synchronously and orphan-cancels the late child).
  • useComments.test.ts: cancelAIReply marks neutral / cleared by retry / no-op for unknown id.
  • useDocumentReviewCancel.test.ts, commentLayerHelpers.test.ts (computeBottomSpacer), annotationValidation.test.ts (legacy createdByQuill dropped), sidecar/draft strip fixtures.
  • Full bar green: typecheck, eslint, prettier, vitest (292), cargo fmt/clippy -D warnings/test (35).

🤖 Generated with Claude Code

@sam-powers
sam-powers changed the base branch from fix/claude-reply-retry-ux to main July 10, 2026 03:01
@sam-powers sam-powers closed this Jul 10, 2026
@sam-powers sam-powers reopened this Jul 10, 2026
…sion

Two @claude reply fixes plus a comment-panel layout follow-up.

Cancel → Re-run: cancelling an @claude reply now resolves to a neutral,
retryable state instead of a dead partial. A `cancelled` stream event no
longer finalizes the reply (which would apply half-streamed tracked edits)
— it marks the reply `cancelled` and keeps the stashed inputs so a "Re-run"
button re-issues the same request in place against the same reply entry.
`cancel(replyId)` now supersedes the reply synchronously: it bumps the
per-reply generation and marks it cancelled even when no cancel token is
tracked yet, closing the spawn-await window where a Cancel click could land
between startAIReply and the spawn resolving and be silently swallowed —
the reply would then stream to completion despite the stop. The pending
runSpawn sees a stale generation on resume and orphan-cancels its own child.

Remove "Start new session": the Quill-minted-session path (createdByQuill /
allow_create / --session-id) never reliably created a resumable session, so
every binding now resumes via --resume and the button is gone. Legacy
sidecars stamped createdByQuill load as plain bindings (the field is dropped
by the whitelist sanitizer).

Comment panel: a dynamic bottom spacer (computeBottomSpacer, derived from
document-space card bottoms to avoid a scroll→reflow→scroll loop) lets the
lowest card scroll fully into view.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ESi6dnK3Wc1jYRpZN5qfZA
@sam-powers
sam-powers force-pushed the fix/claude-cancel-rerun-and-session-cleanup branch from 9fe1874 to 06081bd Compare July 10, 2026 12:44
@sam-powers
sam-powers merged commit cf06204 into main Jul 10, 2026
3 checks passed
@sam-powers
sam-powers deleted the fix/claude-cancel-rerun-and-session-cleanup branch July 10, 2026 12:56
@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