Skip to content

fix(review): keep preempted retries from replacing newer queued snapshots - #108540

Open
Liuzikaii wants to merge 1 commit into
NousResearch:mainfrom
Liuzikaii:fix/review-retry-snapshot-order
Open

Liuzikaii wants to merge 1 commit into
NousResearch:mainfrom
Liuzikaii:fix/review-retry-snapshot-order

Conversation

@Liuzikaii

Copy link
Copy Markdown

What does this PR do?

When a foreground turn preempts a managed-local background review, the old worker may unwind after the foreground turn has queued its updated conversation. The old worker's retry then overwrites that newer pending review. The next reviewer receives the old conversation and misses the latest correction, contrary to the queue's documented newest-snapshot-wins contract.

Carry an internal monotonic snapshot creation timestamp through dispatch and requeue. Reject an incoming older snapshot when a newer item is already pending, while preserving the existing age-out timestamp and normal coalescing behavior.

Related Issue

Fixes #108539

Type of Change

  • Bug fix

Changes Made

  • agent/review_idle_queue.py
  • run_agent.py
  • tests/agent/test_review_requeue_order.py

How to Test

Base: 7469c0f2a52baf70c574b110af0772b81bf84227.

bash scripts/run_tests.sh tests/agent/test_review_requeue_order.py tests/agent/test_review_idle_queue.py tests/run_agent/test_background_review.py tests/run_agent/test_background_review_cache_parity.py --file-retries 0 -j 3 -q
python -m ruff check .
python scripts/check_compat_pointers.py
python scripts/check-windows-footguns.py agent/review_idle_queue.py run_agent.py tests/agent/test_review_requeue_order.py
git diff --check

New regression on unmodified main: 2 failed, 1 passed (the already-correct opposite arrival order). With the fix: all 3 passed. Final related suite: 44 passed; additional session/memory/toolset regression: 29 passed.

Ruff, compatibility-pointer checks, changed-file Windows checks and whitespace checks passed. No effective test was removed or skipped to obtain a passing result.

Compatibility and limitations

The test drives the real queue and AIAgent requeue method in both arrival orders, plus an actual worker thread for timestamp propagation; the managed-endpoint classifier and model execution are substituted. This protects a newer pending item and does not claim to serialize every reviewer or cross-session memory write.

Duplicate check

Searched all states for _maybe_requeue_preempted_review, background review/requeue, and review/snapshot/stale. #108260 addresses queue memory bounds. #101074 fixes shallow snapshot aliasing. #9055 / #39806 guard memory/skill writes against live-state changes; they do not prevent the queue from discarding the newer pending review. This change is confined to pending-item selection, not a global stale-write guard.

Checklist

  • Read the contributing guide; Conventional Commit; one focused fix.
  • Searched existing open/closed/merged work and current main.
  • Added regression tests and ran the related suite via scripts/run_tests.sh.
  • Tested on Windows with Python 3.13; considered platform impact.
  • Full repository suite: not run.
  • Configuration/schema/architecture documentation changes: N/A; local comments/docstrings updated where needed.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/memory Memory subsystem: store, providers, sync, background reviews labels Sep 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A late retry of a preempted background review replaces a newer pending conversation snapshot

2 participants