Skip to content

fix(autorun): resolve paused batch session for resume/skip/abort#726

Merged
pedramamini merged 3 commits intomainfrom
fix/aut-bann-sty
Apr 4, 2026
Merged

fix(autorun): resolve paused batch session for resume/skip/abort#726
pedramamini merged 3 commits intomainfrom
fix/aut-bann-sty

Conversation

@pedramamini
Copy link
Copy Markdown
Collaborator

@pedramamini pedramamini commented Apr 4, 2026

Summary

Changes

  • src/renderer/hooks/batch/useBatchHandlers.ts — add resolveBatchSessionIdForPausedError() that reads errorPaused from batch store, replacing the old activeBatchSessionIds[0] || activeSessionId fallback
  • src/__tests__/renderer/hooks/useBatchHandlers.test.ts — update tests to set errorPaused in batch store state and verify the tightened resolution logic

Test plan

  • All 68 hook tests pass
  • Full test suite passes (22,445 tests)
  • TypeScript and ESLint clean

Fixes #707

Summary by CodeRabbit

  • Bug Fixes

    • Improved batch processing to correctly resolve which session should be acted upon when errors pause processing, prioritizing the active session if error-paused.
  • Tests

    • Updated batch handler tests to validate error handling behavior across concurrent sessions.

nehaaprasad and others added 3 commits April 3, 2026 16:50
Resolve merge conflict with main (consumeGroupChatAutoRun import).

Also tighten resolveBatchSessionIdForPausedError to only return sessions
that are actually error-paused, removing fallbacks to activeBatchSessionIds[0]
or activeSessionId which could target sessions not paused on error.
Remove non-error-paused fallbacks from resolveBatchSessionIdForPausedError
so skip/resume/abort only target sessions that are actually error-paused.
Update tests to set errorPaused state in batchStore and verify the new
behavior.
@pedramamini pedramamini merged commit 97fdc15 into main Apr 4, 2026
1 check passed
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 25caf14e-8237-4f9b-bbb2-ec49c6542f57

📥 Commits

Reviewing files that changed from the base of the PR and between 48651b6 and 5697833.

📒 Files selected for processing (2)
  • src/__tests__/renderer/hooks/useBatchHandlers.test.ts
  • src/renderer/hooks/batch/useBatchHandlers.ts

📝 Walkthrough

Walkthrough

The PR refactors session ID resolution for batch error-paused handlers by introducing a new resolveBatchSessionIdForPausedError function that determines which session should be acted upon when an error pause occurs, replacing direct reliance on activeBatchSessionIds. Tests are updated to validate this new selection logic.

Changes

Cohort / File(s) Summary
Implementation
src/renderer/hooks/batch/useBatchHandlers.ts
Added resolveBatchSessionIdForPausedError helper that prioritizes the active session if error-paused, falling back to the first error-paused session in batchRunStates. Updated handleSkipCurrentDocument, handleResumeAfterError, and handleAbortBatchOnError to use this resolver instead of activeBatchSessionIds array.
Tests
src/__tests__/renderer/hooks/useBatchHandlers.test.ts
Updated test cases for the three batch handlers to drive session selection via batchRunStates instead of mockActiveBatchSessionIds. Tests now verify handlers correctly identify and operate on error-paused sessions, and assert no action is taken when no error-paused session exists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A session once lost in the error-paused way,
Now finds itself through our resolver's new day!
Which batch shall we wake from its slumber of strife?
The logic rings clear—let the right one have life! 🌟

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/aut-bann-sty

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 4, 2026

Greptile Summary

This PR fixes a longstanding Auto Run bug where Resume/Skip/Abort could operate on the wrong session by replacing the activeBatchSessionIds[0] || activeSessionId heuristic with resolveBatchSessionIdForPausedError(), which strictly reads errorPaused from the batch store and prefers the active session when it is paused. Tests are updated to set errorPaused explicitly and verify the resolver's preference and bail-out logic.

Confidence Score: 5/5

Safe to merge — targeted fix with correct logic and good test coverage; only finding is a P2 test coverage gap for one handler.

All three error-action handlers now use a strict errorPaused check. The logic is correct, the tests validate the key branches, and no existing behavior outside the error-pause path is changed.

No files require special attention.

Important Files Changed

Filename Overview
src/renderer/hooks/batch/useBatchHandlers.ts Introduces resolveBatchSessionIdForPausedError() that strictly reads errorPaused from the batch store; all three error-action handlers now call it instead of the old `activeBatchSessionIds[0]
src/tests/renderer/hooks/useBatchHandlers.test.ts Tests updated to set errorPaused: true in the batch store before triggering skip/resume/abort handlers; coverage for the resolver's "prefer active session" and "bail when nothing is error-paused" branches is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["User triggers Resume / Skip / Abort"] --> B["Read batchRunStates from\nuseBatchStore.getState() at call time"]
    B --> C{"activeSessionId is set AND\nbatchRunStates[activeSessionId]\n.errorPaused is true?"}
    C -- Yes --> D["return activeSessionId"]
    C -- No --> E["Object.keys(batchRunStates)\n.find(id => errorPaused)"]
    E -- "found" --> F["return first errorPaused sessionId"]
    E -- "not found" --> G["return undefined"]
    D --> H["Call action handler\nresumeAfterError / skip / abort"]
    F --> H
    G --> I["bail — no-op"]
    H --> J["handleClearAgentError clears error banner"]
Loading

Comments Outside Diff (1)

  1. src/__tests__/renderer/hooks/useBatchHandlers.test.ts, line 557-603 (link)

    P2 Missing "prefers active session" test case for handleAbortBatchOnError

    The handleSkipCurrentDocument and handleResumeAfterError suites each include a test verifying that when the active session is error-paused it is preferred over a different session found by the fallback scan. handleAbortBatchOnError only tests the non-active-session path and the "nothing is paused" guard, leaving the active-session preference branch uncovered for this handler.

    Consider adding a parity test:

    it('prefers active session when it is error-paused', () => {
        const session = createMockSession({ id: 'session-1' });
        useSessionStore.setState({ sessions: [session], activeSessionId: 'session-1' });
        useBatchStore.setState({
            batchRunStates: {
                'session-1': createDefaultBatchState({ isRunning: true, errorPaused: true }),
            },
        });
        mockActiveBatchSessionIds = [];
    
        const { result } = renderHook(() => useBatchHandlers(createDeps()));
    
        act(() => { result.current.handleAbortBatchOnError(); });
    
        expect(mockAbortBatchOnError).toHaveBeenCalledWith('session-1');
        expect(mockHandleClearAgentError).toHaveBeenCalledWith('session-1');
    });

Reviews (1): Last reviewed commit: "fix: tighten error-paused session resolu..." | Re-trigger Greptile

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.

bug: Auto Run error banner stays visible after clicking Resume

2 participants