fix(memory-core): honor qmd search timeouts#92624
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 11:19 AM ET / 15:19 UTC. Summary PR surface: Source +60, Tests +90. Total +150 across 3 files. Reproducibility: yes. Current main's fixed 15s outer deadline can fire before a QMD search using a higher configured memory.qmd.limits.timeoutMs completes; the linked issue's 46.7s reranking trace matches that source path. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land one canonical QMD timeout fix that derives the wrapper deadline from the resolved active QMD backend budget while preserving 15 seconds for non-QMD and wiki-only paths. Do we have a high-confidence way to reproduce the issue? Yes. Current main's fixed 15s outer deadline can fire before a QMD search using a higher configured memory.qmd.limits.timeoutMs completes; the linked issue's 46.7s reranking trace matches that source path. Is this the best way to solve the issue? Yes, this is the best fix shape among the inspected timeout PRs: it reuses canonical resolved QMD configuration, avoids a new config key, and keeps wiki-only, builtin, and default QMD paths on the existing 15s guard. Maintainers should still choose this or another branch as the canonical PR before merging overlapping fixes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 45056a463ab2. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +60, Tests +90. Total +150 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
7a1b29d to
ca55d54
Compare
ca55d54 to
81f41f6
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
81f41f6 to
1407fef
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
maintainer deep review found the reported bug is real, but this PR's fix is not safe to land. The proposed outer deadline is derived from one I am closing this PR rather than broadening the patch into a risky timeout rewrite. Issue #91947 remains open for a canonical fix: QMD should own and expose a defensible whole-search budget (or fully enforce its own finite lifecycle), while runtime loading and wiki supplements retain their separate bounded deadlines. Reviewed against current |
Closes #91947
Summary
memory_searchouter deadline from the canonical resolved QMD backend config when QMD is configured above the default 15s tool guard.Real behavior proof
memory_searchhad a fixed 15s wrapper deadline, so QMD searches withmemory.qmd.limits.timeoutMsset higher could still fail at 15s before QMD's configured search budget was honored.codex/memory-search-qmd-deadline, running OpenClaw's realmemory_searchtool against the real QMD backend. QMD was executed through a temporary wrapper that delays onlysearch/query/vsearchcommands for 16s, thenexecs the live@tobilu/qmdCLI. This isolates the old outer-deadline failure without relying on a naturally slow reranker.memory_searchtool completed successfully after 25.5s with QMD backend debug metadata and two live QMD results. This exceeds the old fixed 15s wrapper deadline and stays below the patched 65s wrapper deadline derived from canonicalmemory.qmd.limits.timeoutMs: 60000.5plus bounded overhead.@tobilu/qmdCLI so the outermemory_searchdeadline behavior is reproducible without external model latency.Validation