Skip to content

[Frontend][Review] Stop paper review batch retries after abort - #2754

Merged
Chris0Jeky merged 2 commits into
mainfrom
issue-2581/abort-no-retry
Sep 6, 2026
Merged

[Frontend][Review] Stop paper review batch retries after abort#2754
Chris0Jeky merged 2 commits into
mainfrom
issue-2581/abort-no-retry

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Refs #2581

Summary

  • Preserve caller ownership when deciding whether a failed paper-review evidence batch may receive the one same-action retry.
  • Skip that retry when the caller signal has already been aborted, while retaining genuine live-failure recovery.
  • Add regression coverage for both scope disposal and caller abort after the underlying read later rejects.

Verification

  • npx vitest --run --maxWorkers=2 src/tests/composables/usePaperReviewSelectors.spec.ts (53 passed)
  • npx vitest --run --maxWorkers=2 src/tests/composables/usePaperReviewSelectors.spec.ts src/tests/views/paper/review/PaperReviewView.spec.ts (263 passed)
  • npm run typecheck
  • npm run build
  • npx eslint src/composables/usePaperReviewSelectors.ts src/tests/composables/usePaperReviewSelectors.spec.ts --max-warnings=20
  • git diff --check

Scope

No merge or issue closure performed. Hosted checks and independent review remain to be observed on the open PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Fresh-context review record (coordinator, 2026-09-06; read-only Opus 5 reviewer over the diff against the merge base; route: the standing D-12 rule for Codex-session PRs).

VERDICT: SHIP, nothing fixed in-PR. The one source hunk is sound: callerSignal.aborted can only be true after the waiter's own abort listener ran, so the guard never denies a live waiter its retry, only the orphan batch that used to launch for an action nobody was waiting on; termination unchanged; the failed batch stays retained so the next deliberate wait gets a real retry; no half-applied state (batchRecord = failed, loading clears); AbortError versus real failure is discriminated by caller ownership, the stronger discriminator, and the pre-existing "live failure still retries" test holds.

Recorded for #2581, not fixed: MEDIUM — the "does not retry after its owning scope is disposed" test cannot fail without the source change (scope disposal already supersedes and nulls activeCoreBatch on main), so the body's "regression coverage for both scope disposal and caller abort" overstates it; keep it as characterization. LOWs: nothing asserts the retry is deferred rather than consumed (a future edit to the activeCoreBatch === batch retention would strand the rail on failed silently); the doc comment still says only "one explicit wait retries a failed batch once" and the new callerSignal parameter is undocumented; the user-visible consequence (after a timeout or cancel the rail stays failed instead of being repaired by the background retry) is intended but unrecorded.

@Chris0Jeky
Chris0Jeky merged commit d8c83d8 into main Sep 6, 2026
35 checks passed
@Chris0Jeky
Chris0Jeky deleted the issue-2581/abort-no-retry branch September 6, 2026 12:31
@github-project-automation github-project-automation Bot moved this from Pending to Done in Taskdeck Execution Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant