Skip to content

fix(memory): prevent stale-head search misses during concurrent append - #1253

Open
thunguo wants to merge 2 commits into
oceanbase:masterfrom
thunguo:fix/concurrent-mem
Open

fix(memory): prevent stale-head search misses during concurrent append#1253
thunguo wants to merge 2 commits into
oceanbase:masterfrom
thunguo:fix/concurrent-mem

Conversation

@thunguo

@thunguo thunguo commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Closes #1249 .

Rationale for this change

Memory search had a TOCTOU race during concurrent appends. A search could validate revision N, then query the mutable index projection after another request had advanced the head to revision N+1. The results would still be filtered using revision N, potentially producing an empty result even when an unchanged matching entry existed.

This failure was silent: callers received a successful response associated with revision N but no indication that the result was inconsistent.

What changes are included in this PR?

  • Validate requested memory revisions before and after the index query.
  • Detect when the memory head advances during index lookup or reranking.
  • Retry the search with the latest head for a bounded number of attempts.
  • Raise RevisionConflictError instead of returning potentially inconsistent results if concurrent updates continue beyond the retry limit.
  • Add a deterministic concurrency regression test using synchronization barriers.
  • Cover SQLite and OceanBase across FTS, vector, and hybrid search modes.

Are there any user-facing changes?

NONE

How was this change tested?

Added tests/e2e/test_memory_search_concurrency.py, which deterministically pauses a search after initial head validation but before the index query, performs a concurrent append, and verifies that:

  • the search retries against the new head;
  • the unchanged matching entry is returned;
  • the returned memory_ref points to the new revision;
  • the expected FTS, vector, and hybrid channels are used.

Code quality checks:

make check

Result: all formatting, lint, and type checks passed.

Full test suite:

make test

Result:

534 passed, 12 skipped

The OceanBase matrix was tested against a temporary real OceanBase CE 4.3.5.6 instance:

POWERCONTEXT_TEST_OCEANBASE_URL="<dedicated-oceanbase-url>" \
  .venv/bin/python -m pytest \
  tests/e2e/test_memory_search_concurrency.py \
  -k oceanbase -q

Result:

3 passed, 3 deselected in 22.71s

The three passing OceanBase cases cover FTS, vector, and hybrid search.

AI usage statement

Copilot AI lite review requested due to automatic review settings August 17, 2026 09:59

Copilot AI 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.

Pull request overview

Fixes a TOCTOU race in Memory search under concurrent appends by detecting when the memory head changes during search execution and ensuring results are not silently computed against a mixed revision/index projection.

Changes:

  • Add bounded retry logic in the runtime Memory search path, re-checking the head after the backend search (and rerank) completes, and raising RevisionConflictError after repeated contention.
  • Add backend-side head validation around the index query to reject searches when a requested head advances during projection lookup.
  • Introduce a deterministic e2e regression test that pauses the index search, appends concurrently, and asserts the search retries and returns consistent results across SQLite and OceanBase and across FTS/vector/hybrid modes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/e2e/test_memory_search_concurrency.py New deterministic concurrency regression test covering SQLite/OceanBase and FTS/vector/hybrid search modes.
src/powercontext/builtin/runtime/application.py Adds retry + conflict behavior to prevent stale-head inconsistencies during Memory search.
src/powercontext/builtin/persistence/memory.py Validates requested heads before/after index search to detect head advancement during projection reads.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/powercontext/builtin/runtime/application.py Outdated
@thunguo
thunguo marked this pull request as draft August 17, 2026 10:09
@thunguo
thunguo marked this pull request as ready for review August 17, 2026 10:32

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice fix overall. I verified the race against a real OceanBase instance in FTS, vector, and hybrid modes. There is one API contract update needed before this lands.

if not _is_stale_memory_search(error) or latest is None or latest.as_ref() == current.as_ref():
raise
if attempt == _MEMORY_SEARCH_ATTEMPTS:
raise RevisionConflictError(current, latest) from error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we update the API contract for this new failure mode? Once the retries are exhausted, this becomes a RevisionConflictError, and the server returns 409 revision_conflict. POST /v1/memory/search still does not declare a 409 response in openapi/powercontext.yaml, so generated clients and the docs will not know it can happen. Please add the Conflict response, regenerate the API artifacts, and add a server-level test that checks the 409 response. The PR description should call this out as a user-facing change too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll fix it later

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: concurrent Memory append silently drops valid FTS search hits

3 participants