Skip to content

test(memory): reset the sticky tool tracker between memory gate tests - #4873

Open
chopratejas wants to merge 1 commit into
mainfrom
fix/memory-injected-gate-after-merge
Open

chopratejas wants to merge 1 commit into
mainfrom
fix/memory-injected-gate-after-merge

Conversation

@chopratejas

@chopratejas chopratejas commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Description

On main, test_memory_call_the_proxy_did_not_inject_reaches_the_client[not_injected-anthropic] and [not_injected-anthropic_stream] fail when tests/test_memory_tool_user_scope.py runs earlier in the same process (CI shard test (3)).

This is a test-isolation problem, not a regression in the proxy. Memory tool definitions are sticky per session (SessionToolTracker, a process-wide tracker in headroom/proxy/helpers.py). The user-scope test sends an identical Anthropic request with memory tools injected. When the later not_injected case sends the same request, the session tracker replays those tool definitions, so the proxy really does offer memory_delete upstream on that request. Running the call is then the intended behaviour. Each PR passed alone because nothing else in its own shard had injected tools for that request first.

The fix resets the tracker before and after each test in tests/test_memory_tools_only_when_injected.py. The same reset is already used in test_memory_tool_stream.py and test_corrupt_golden_bytes_recovery.py.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)

Changes Made

  • tests/test_memory_tools_only_when_injected.py: an autouse fixture calls _reset_session_tool_tracker_for_test() around each test. No production code changes.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom) (tests-only change)
  • New tests added for new functionality
  • Manual testing performed

Test Output

Same order as CI (tests/test_memory_tool_user_scope.py then tests/test_memory_tools_only_when_injected.py), on origin/main fda75de:

FAILED tests/test_memory_tools_only_when_injected.py::test_memory_call_the_proxy_did_not_inject_reaches_the_client[not_injected-anthropic]
FAILED tests/test_memory_tools_only_when_injected.py::test_memory_call_the_proxy_did_not_inject_reaches_the_client[not_injected-anthropic_stream]
2 failed, 16 passed

On this branch:

18 passed

Injected controls first, then the not-injected cases, in one process: 4 failed on main, 11 passed on this branch.
tests/test_memory_tool_user_scope.py tests/test_memory_tools_only_when_injected.py tests/test_memory/ tests/test_memory_handler_native_ops.py: 780 passed, 2 skipped. ruff check and format are clean.

Real Behavior Proof

  • Environment: macOS, Python 3.12, in-process create_app with a MockTransport upstream, LITELLM_LOCAL_MODEL_COST_MAP=True.
  • Exact command / steps: run the two test files above in the CI order on main and on this branch.
  • Observed result: on main the not-injected Anthropic cases fail with assert ['delete'] == [] because the replayed session tools are injected; on this branch all pass.
  • Not tested: n/a (test-only change).

Runtime Rollout Safety

  • Rollout-managed feature(s): none.
  • Minimum rollout channel: n/a.
  • Stable/default behavior changed: no.
  • Kill switch / disable path: n/a.
  • Unsafe override required: no.
  • Qualification impact: none.
  • Rollback path: revert this PR.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Additional Notes

Sticky replay only matters within a single process that serves the same session under different memory settings, which a running proxy (with one fixed config) does not do.

🤖 Generated with Claude Code


Devin Review

Memory tool definitions are sticky per session in a process-wide tracker.
When a test that injected them ran earlier in the same process with an
identical request, a later "not injected" case replayed the tools, and
the proxy correctly ran the calls it had offered, failing the test. Reset
the tracker around each test.

Assay-Kind: tests
Assay-Ask: The memory "not injected" tests failed on main when a test that injected memory tools for an identical request ran earlier in the same process.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chopratejas chopratejas added bug Something isn't working size: small At most 100 added/deleted lines. risk: low Scope estimate: documentation/tests-only change; verification still required. area: memory Touches memory files; triage routing, not a CODEOWNERS assignment. labels Nov 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@github-actions github-actions Bot added the status: ready for review Pull request body is complete and the author marked it ready for human review label Nov 14, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment on lines +26 to +33
@pytest.fixture(autouse=True)
def _fresh_tool_tracker():
# Memory tools are sticky per session in a process-wide tracker: a test
# that injected them would make a later identical request replay them,
# and the proxy then correctly runs the calls it offered.
_reset_session_tool_tracker_for_test()
yield
_reset_session_tool_tracker_for_test()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Prohibited standalone test-isolation fix

Adding _fresh_tool_tracker makes this a test-only PR addressing a known main-branch failure. The contribution policy explicitly prohibits that submission category.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

This branch was successfully deployed

1 active deployment
Preview — ebe6b4aa Deployed Nov 14, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: memory Touches memory files; triage routing, not a CODEOWNERS assignment. bug Something isn't working risk: low Scope estimate: documentation/tests-only change; verification still required. size: small At most 100 added/deleted lines. status: ready for review Pull request body is complete and the author marked it ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants