Skip to content

fix(review): preserve the originating profile through idle dispatch - #108538

Open
Liuzikaii wants to merge 1 commit into
NousResearch:mainfrom
Liuzikaii:fix/deferred-review-profile
Open

Liuzikaii wants to merge 1 commit into
NousResearch:mainfrom
Liuzikaii:fix/deferred-review-profile

Conversation

@Liuzikaii

Copy link
Copy Markdown

What does this PR do?

A review deferred on the managed local runtime is dispatched by a shared thread without the ContextVars of the session that queued it. With multiple profiles in one process, the dispatch-time enabled check reads the ambient profile, and the review worker inherits that same ambient context. This can drop an enabled profile's review, run a disabled profile's review, or resolve profile-aware memory paths to the ambient home.

Capture contextvars.copy_context() per pending item and enter it around both the dispatch-time gate and worker spawn. The existing worker propagation can then carry the correct context onward. Coalescing replaces the context together with the snapshot; the original age-out time is unchanged.

Related Issue

Fixes #108537

Type of Change

  • Bug fix

Changes Made

  • agent/review_idle_queue.py
  • tests/agent/test_review_idle_profile_scope.py

How to Test

Base: 7469c0f2a52baf70c574b110af0772b81bf84227.

bash scripts/run_tests.sh tests/agent/test_review_idle_profile_scope.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 tests/agent/test_review_idle_profile_scope.py
git diff --check

New regression: 2 failed on unmodified main, 2 passed with the fix. Final related suite: 43 passed. Earlier expanded review/session/memory/toolset regression: 44 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

Validated with real temporary profile files and a real dispatch thread; only the model-spawn boundary is substituted with real MemoryStore I/O. No live model/GPU run was performed. No profile inheritance, global environment mutation, configuration keys, or prompt/tool schemas are added.

Duplicate check

Searched open/closed Issues and PRs for ReviewIdleQueue, deferred review/profile, and background review/ContextVars. #108234 / #108260 cover retained queue memory, not context propagation. #91509 changes provider alias/credential resolution after entering a scope. #24392 and #54937 concern earlier direct-worker/global-home paths; the extra idle-dispatch hop still loses scope on current main. Their fixes do not cover this queue boundary.

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/profiles Multi-profile isolation, HERMES_HOME scoping area/memory Memory subsystem: store, providers, sync, background reviews labels Sep 11, 2026
teknium1 pushed a commit that referenced this pull request Sep 28, 2026
…le's context (#108537, salvage #108538)

ReviewIdleQueue stored (agent, session_key, kwargs, enqueued_at) with no
context, and the shared dispatcher thread called _still_enabled(item) and
item.agent._spawn_background_review_now(**item.kwargs) under its own
AMBIENT context. On a multiplexed gateway that meant the wrong profile's
background_review.enabled gate decided whether a queued review ran, and the
spawned worker inherited the ambient home / no secret scope (the
propagate_context_to_thread in _spawn_background_review_now copies a
context that no longer carries the originating profile).

Capture contextvars.copy_context() at enqueue() on _PendingReview and run
both the enabled re-check and the spawn via item.context.run(...).

Test: two profile homes (enabled / disabled) plus a disabled ambient home
under set_multiplex_active(True); only the enabled profile's item spawns,
and it observes its own home override + secret scope.

Salvaged from #108538 by @Liuzikaii (mechanism kept, test trimmed into the
existing test module).

Fixes #108537

(cherry picked from commit 4f8a64fd680e40de270a18b52c8f44a414334c56)
teknium1 pushed a commit that referenced this pull request Sep 28, 2026
…le's context (#108537, salvage #108538)

ReviewIdleQueue stored (agent, session_key, kwargs, enqueued_at) with no
context, and the shared dispatcher thread called _still_enabled(item) and
item.agent._spawn_background_review_now(**item.kwargs) under its own
AMBIENT context. On a multiplexed gateway that meant the wrong profile's
background_review.enabled gate decided whether a queued review ran, and the
spawned worker inherited the ambient home / no secret scope (the
propagate_context_to_thread in _spawn_background_review_now copies a
context that no longer carries the originating profile).

Capture contextvars.copy_context() at enqueue() on _PendingReview and run
both the enabled re-check and the spawn via item.context.run(...).

Test: two profile homes (enabled / disabled) plus a disabled ambient home
under set_multiplex_active(True); only the enabled profile's item spawns,
and it observes its own home override + secret scope.

Salvaged from #108538 by @Liuzikaii (mechanism kept, test trimmed into the
existing test module).

Fixes #108537

(cherry picked from commit 4f8a64fd680e40de270a18b52c8f44a414334c56)
teknium1 pushed a commit that referenced this pull request Sep 28, 2026
…le's context (#108537, salvage #108538)

ReviewIdleQueue stored (agent, session_key, kwargs, enqueued_at) with no
context, and the shared dispatcher thread called _still_enabled(item) and
item.agent._spawn_background_review_now(**item.kwargs) under its own
AMBIENT context. On a multiplexed gateway that meant the wrong profile's
background_review.enabled gate decided whether a queued review ran, and the
spawned worker inherited the ambient home / no secret scope (the
propagate_context_to_thread in _spawn_background_review_now copies a
context that no longer carries the originating profile).

Capture contextvars.copy_context() at enqueue() on _PendingReview and run
both the enabled re-check and the spawn via item.context.run(...).

Test: two profile homes (enabled / disabled) plus a disabled ambient home
under set_multiplex_active(True); only the enabled profile's item spawns,
and it observes its own home override + secret scope.

Salvaged from #108538 by @Liuzikaii (mechanism kept, test trimmed into the
existing test module).

Fixes #108537

(cherry picked from commit 4f8a64fd680e40de270a18b52c8f44a414334c56)
teknium1 pushed a commit that referenced this pull request Sep 28, 2026
…le's context (#108537, salvage #108538)

ReviewIdleQueue stored (agent, session_key, kwargs, enqueued_at) with no
context, and the shared dispatcher thread called _still_enabled(item) and
item.agent._spawn_background_review_now(**item.kwargs) under its own
AMBIENT context. On a multiplexed gateway that meant the wrong profile's
background_review.enabled gate decided whether a queued review ran, and the
spawned worker inherited the ambient home / no secret scope (the
propagate_context_to_thread in _spawn_background_review_now copies a
context that no longer carries the originating profile).

Capture contextvars.copy_context() at enqueue() on _PendingReview and run
both the enabled re-check and the spawn via item.context.run(...).

Test: two profile homes (enabled / disabled) plus a disabled ambient home
under set_multiplex_active(True); only the enabled profile's item spawns,
and it observes its own home override + secret scope.

Salvaged from #108538 by @Liuzikaii (mechanism kept, test trimmed into the
existing test module).

Fixes #108537

(cherry picked from commit 4f8a64fd680e40de270a18b52c8f44a414334c56)

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 area/profiles Multi-profile isolation, HERMES_HOME scoping 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.

Deferred background reviews lose the originating profile before the enabled check and worker spawn

2 participants