Repository navigation
fix(hooks): stop the HTTP fast path from answering out of the wrong database - #23
Merged
Merged
Conversation
…atabase
MNEME_DB_PATH is silently ignored whenever a server is listening on the
default port.
The recall request body carries a query and filters — it does not carry a
database path. The CLI fallback forwards the path (passThroughDbEnv); the HTTP
fast path cannot. So with a DB pinned and a server up, the two paths answer
from different databases, and which one you get depends on whether a process
happens to be running.
Not theoretical. Multiple agent runtimes share one machine here, each with its
own DB on its own port — 18792, 18794, 18800, three DBs, one of them a
different build entirely. The hardcoded default is the first. A hook pinned to
agent B's DB gets agent A's memories.
This is also the standing red in locations.test.mjs ("hook silent when no
alias / no memory hit"). It was never flaky: the test pins TOKENMEM_DB_PATH to
an empty temp DB, the hook asks the live server instead, and three production
memories come back. It passes on a machine with no server up, which is why it
survived — the suite is green exactly where the bug cannot bite.
The rule: a caller that names a DB means THAT DB. An explicit MNEME_HTTP_URL
is the caller saying "that server serves the DB I named" and still wins. Absent
that, a pinned DB turns the fast path off and we spawn — slower and correct.
Nothing changes for normal operation, where no DB path is set at all.
- hooks/prompt-recall.mjs, hooks/tool-recall-pre.mjs: resolveHttpUrl(),
duplicated in both because these hooks are meant to be individually
copyable, matching the existing intEnv() convention
- hooks/hooks.test.mjs: two cases pinning both halves of the rule. The first
goes red without this change on any machine with a server on the default
port; the second guards the escape hatch from regressing the other way.
locations.test.mjs: 50/1 -> 51/0. Full suite 262 passed / 0 failed.
Co-authored-by: 千夏 <qianxia@clawgamers.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
MNEME_DB_PATHis silently ignored whenever a server is listening on the default port.The recall request body carries a query and filters — it does not carry a database path. The CLI fallback forwards the path (
passThroughDbEnv); the HTTP fast path cannot. So with a DB pinned and a server up, the two paths answer from different databases, and which one you get depends on whether a process happens to be running.Not theoretical
Multiple agent runtimes share one machine here, each with its own DB on its own port:
Three DBs, one of them a different build entirely. The hardcoded default is the first. A hook pinned to agent B's DB gets agent A's memories.
It has been failing in the suite this whole time
This is the standing red in
locations.test.mjs → "hook silent when no alias / no memory hit". It was never flaky. The test pinsTOKENMEM_DB_PATHto an empty temp DB; the hook asks the live server instead, and three production memories come back:It passes on a machine with no server up — the suite is green exactly where the bug cannot bite.
The rule
A caller that names a DB means THAT DB. An explicit
MNEME_HTTP_URLis the caller saying "that server serves the DB I named" and still wins. Absent that, a pinned DB turns the fast path off and we spawn — slower and correct.Nothing changes for normal operation, where no DB path is set at all (verified: no
MNEME_DB_PATH/TOKENMEM_DB_PATHin the live environment, so the fast path stays on).Changes
hooks/prompt-recall.mjs,hooks/tool-recall-pre.mjsresolveHttpUrl(), duplicated in both because these hooks are meant to be individually copyable — matching the existingintEnv()conventionhooks/hooks.test.mjsThe first new case goes red without this change on any machine with a server on the default port; the second guards the escape hatch from regressing the other way.
Verification
🤖 Generated with Claude Code