Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Dec 21, 2025

Extract refresh logic into a dedicated useReviewRefreshController hook that centralizes all refresh decision-making and eliminates race conditions.

Changes

  • New useReviewRefreshController hook (~300 LoC) with:

    • Debounced tool completion handling (3s window)
    • Hidden tab detection (queue + flush on visibilitychange)
    • User interaction pausing (queue + flush on blur)
    • In-flight coalescing (single follow-up after fetch completes)
    • Scroll position preservation
  • Simplified ReviewPanel:

    • Removed ~125 lines of inline refresh logic
    • Single integration point via the hook

Race condition fixes

Issue Before After
Stale closure capture Timer captured filters.diffBase at schedule time All state read from refs at execution time
Hidden tab ❌ Refresh dropped silently ✅ Queued, flush on visibilitychange
Refresh during origin fetch ❌ Dropped silently ✅ Single follow-up queued
Timer cleanup Closure variable could go stale Refs allow cleanup from any path

Regression risk: MEDIUM-LOW

Preserved behaviors:

  • 3s debounce timing (same constant)
  • Origin fetch logic (same shell injection protection)
  • User interaction pausing (same isPanelFocused tracking)
  • Scroll position preservation

New behaviors (improvements):

  • Hidden-tab refreshes now queued instead of dropped
  • In-flight requests coalesce to single follow-up instead of dropped

Manual smoke test checklist:

  • Open ReviewPanel with origin/main base
  • Run bash tool that modifies files → refresh ~3s after
  • Trigger tool during refresh → one additional refresh after first completes
  • Hide tab during tool, show tab → refresh fires on visible
  • Focus panel, trigger tool, blur → refresh fires after blur

Generated with mux • Model: anthropic:claude-opus-4-5 • Thinking: high

Extract refresh logic into a dedicated `useReviewRefreshController` hook that
centralizes all refresh decision-making and eliminates race conditions.

## Changes

- New `useReviewRefreshController` hook (~300 LoC) with:
  - Debounced tool completion handling (3s window)
  - Hidden tab detection (queue + flush on visibilitychange)
  - User interaction pausing (queue + flush on blur)
  - In-flight coalescing (single follow-up after fetch completes)
  - Scroll position preservation

- Simplified ReviewPanel:
  - Removed 125 lines of inline refresh logic
  - Single integration point via the hook
  - isPanelFocused controls setInteracting()

## Race condition fixes

1. **Stale closure capture**: All state read from refs at execution time
2. **Hidden tab drops**: Now queues and flushes on visibilitychange
3. **In-flight race**: Requests during fetch coalesce to single follow-up
4. **Timer cleanup**: Refs allow cleanup from any code path

## Tests

- Unit tests for getOriginBranchForFetch (shell injection protection)
- Behavioral contract documentation as test cases

---
_Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
@ammar-agent ammar-agent force-pushed the fix-git-refresh-debounce-race branch from 4089f3c to c9750a0 Compare December 21, 2025 02:08
@ammario ammario merged commit d246709 into main Dec 21, 2025
20 checks passed
@ammario ammario deleted the fix-git-refresh-debounce-race branch December 21, 2025 02:25
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.

2 participants