fix(memory): honor qmd search timeout and bound one-shot CLI cleanup by jason-allen-oneal · Pull Request #95347 · openclaw/openclaw

jason-allen-oneal · 2026-06-20T14:34:45Z

What problem this solves

memory_search historically imposed a fixed 15 second outer deadline. QMD searches can legitimately require a longer whole-operation budget when memory.qmd.limits.timeoutMs is raised, so the wrapper could terminate the tool before the configured QMD operation completed.

Fixes #91947.

Summary

Lets a memory manager expose an optional whole-search timeout budget.
Keeps the historical 15 second guard for builtin memory and default/short QMD configurations.
Computes a bounded QMD whole-search budget from command, collection, sync, embed, and retry phases.
Applies one shared deadline to the active QMD search lifecycle.
Keeps QMD subprocess cancellation through AbortSignal.
Keeps forced-sync retry out of long-lived QMD tool hot paths; one-shot CLI QMD managers may still bootstrap and retry within the same deadline.
Bounds one-shot QMD cleanup with a 5 second grace period so pending update work cannot hold CLI return indefinitely.
Preserves the active search-mode override through cached fallback managers.

User impact

Raised QMD timeout configurations can complete beyond the old 15 second wrapper limit. Builtin memory and short QMD configurations retain the existing fast-failure behavior.

If QMD falls back to builtin search during the same call, the builtin fallback receives its own bounded 15 second deadline rather than inheriting the longer QMD budget.

No storage migration or data-model change is required.

Real behavior proof

A live QMD-backed harness run used the repository development entrypoint, a temporary QMD workspace, and the real @tobilu/qmd CLI. Search-like commands were delayed by 16 seconds, beyond the former fixed outer deadline.

[proof] memory.qmd.limits.timeoutMs=60000
[proof] old memory_search outer deadline=15000ms
[qmd-delay-wrapper] delayed search command by 16000ms
[proof] elapsedMs=28878
[proof] resultCount=1
[proof] first hit path=memory/qmd-proof.md
[proof] PASS elapsedMs=28878 > oldOuterDeadlineMs=15000

The proof was captured on proven ancestor 9104253dabcb2d4f4b101f51e672edb81a8e2b15. The clean current-main rebuild preserves the same timeout implementation and tests while incorporating current memory-search behavior.

Review findings addressed

QMD-to-builtin fallback has a separate bounded fallback deadline.
Cached fallback managers forward qmdSearchModeOverride into timeout resolution.
Pending and queued QMD update waits share one bounded cleanup timeout.
Cleanup timers are cleared when update work settles first.
Long-lived QMD searches do not force update work into the tool hot path.
One-shot QMD bootstrap, sync, and retry remain inside one whole-operation deadline.

Validation

Current head: 16e2484891d06543da79f2a13a9b99e33c7be836

The clean current-main repair runner completed successfully before pushing the rebuilt branch:

217 focused memory tests passed
pnpm tsgo:extensions:test passed
formatting, lint, and git diff --check passed

The current head adds only contract documentation clarifying the optional whole-search and bounded-cleanup hooks. Fresh full GitHub Actions are running on this exact head.

Scope and risk

The optional MemorySearchManager.getSearchTimeoutMs and bounded close(timeoutMs) hooks extend the memory-host contract. Maintainers should explicitly accept that interface direction and the availability tradeoff of allowing intentionally configured QMD searches to run longer than 15 seconds.

QMD subprocess cancellation remains shared behavior from #93394 and is not reimplemented here.

Tests

AI-assisted: yes.

clawsweeper · 2026-06-20T14:36:04Z

Codex review: needs maintainer review before merge. Reviewed July 10, 2026, 2:27 PM ET / 18:27 UTC.

Summary
This PR computes a configurable whole-search deadline for QMD-backed memory searches, bounds one-shot manager cleanup, preserves bounded builtin fallback behavior, and adds optional timeout and cleanup-budget hooks to the memory-host manager interface.

PR surface: Source +303, Tests +385. Total +688 across 8 files.

Reproducibility: yes. Current main visibly applies a 15-second whole-tool deadline while QMD commands use a separately configured timeout, and the PR supplies a real delayed-QMD run that succeeds after 28.878 seconds; I did not execute the branch because this review is read-only.

Review metrics: 1 noteworthy metric.

Memory-host contract: 2 optional hooks added or changed. The PR introduces manager-selected search deadlines and caller-supplied close budgets, making API direction the central pre-merge decision.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/memory-tool-manager.test-mocks.ts, unknown-data-model-change: extensions/memory-core/src/memory/qmd-manager.test.ts, unknown-data-model-change: extensions/memory-core/src/memory/qmd-manager.ts, unknown-data-model-change: extensions/memory-core/src/memory/search-manager.test.ts, unknown-data-model-change: extensions/memory-core/src/memory/search-manager.ts, unknown-data-model-change: extensions/memory-core/src/tools.test.ts, and 9 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #91947
Summary: The canonical issue remains open with two competing open fix candidates; this PR is viable but broader, while the maintainer-repaired alternative is proof-positive but currently conflicting.

Members:

canonical: memory_search 工具硬编码 15s timeout 不够 qmd query 跑完，建议可配置 #91947 - It reports the exact fixed 15-second wrapper versus configured QMD timeout mismatch addressed by both PRs.
fixed_by_candidate: fix(memory): respect QMD timeout for memory_search #95757 - It implements the maintainer-repaired command-boundary alternative for the same issue, but live GitHub currently reports it conflicting.
superseded: fix(memory): honor configured qmd search timeouts #92065 - It was an earlier closed-unmerged attempt for the same issue and is not a viable landing candidate.
adjacent_distinct: fix(memory): abort orphaned qmd search subprocess when memory_search times out #93394 - Its merged AbortSignal propagation is required for safe deadline cancellation but does not solve configurable timeout ownership.
adjacent_distinct: fix(memory-core): skip qmd zero-hit search sync #90030 - It separately removed synchronous zero-hit QMD maintenance from long-lived search hot paths and defines adjacent lifecycle behavior.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Get the maintainer timeout-ownership decision and consolidate the fix onto one clean current-main branch.
Preserve the existing live QMD proof while adding exact-head proof if the chosen implementation changes materially.

Risk before merge

[P1] Merging this PR would establish two new optional memory-host manager semantics that external or future managers may implement inconsistently: manager-selected outer deadlines and caller-supplied close budgets.
[P2] A large configured QMD timeout can expand the outer budget across collection attempts, missing-collection repair, dirty sync, embedding, and retries, allowing an interactive memory operation to run substantially longer than the active QMD command alone.
[P1] The preferred narrower design is documented and strongly proven but currently lives on a conflicting PR, so maintainers must consolidate the implementation onto one current-main branch rather than treating either open branch as automatically canonical.

Maintainer options:

Port the command-boundary repair (recommended)
Resolve current-main drift by moving the proven narrower timeout design onto one clean branch without adding the manager timeout and close hooks.
Accept the broader manager contract
Merge this branch only after maintainers explicitly approve the SDK expansion and the longer whole-lifecycle availability tradeoff.
Pause both competing branches
Keep both PRs unmerged until a single owner-approved implementation is prepared against current main, then close the unused candidate.

Next step before merge

[P2] A maintainer must select the permanent timeout/API model and consolidate the two competing branches; there is no narrow mechanical defect for an automated repair worker to resolve without that choice.

Maintainer decision needed

Question: Should the permanent memory timeout contract extend only active QMD command execution, or allow each memory manager to select a whole-operation deadline and bounded close behavior?
Rationale: Both implementations can fix the observed 15-second failure, but they create materially different plugin API and availability contracts; repository owner-boundary guidance and the existing maintainer repair favor the narrower command-owned model, while the branch implementing that model currently conflicts with main.
Likely owner: steipete — The clearest current intent signal is the maintainer repair and proof summary authored on the competing timeout implementation.
Options:
- Keep timeout at QMD command boundary (recommended): Port or rebase the narrower maintainer repair so configured QMD time applies only during active QMD commands and all surrounding work remains bounded by the existing default.
- Adopt manager-owned lifecycle budgets: Accept this PR's new SDK hooks and explicitly make whole-search and bounded-close timing part of the memory manager contract.

Security
Cleared: The diff is limited to memory runtime logic, SDK types, and focused tests; it adds no dependency source, workflow permission, secret handling, download, lifecycle hook, or supply-chain execution surface.

Review details

Best possible solution:

Consolidate onto one current-main branch that keeps the memory-host manager contract unchanged, pauses or hands off the default deadline only for active QMD command execution, restores bounded deadlines for setup and builtin fallback, and proves direct, grouped, mcporter, cancellation, one-shot cleanup, and fresh-fallback behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main visibly applies a 15-second whole-tool deadline while QMD commands use a separately configured timeout, and the PR supplies a real delayed-QMD run that succeeds after 28.878 seconds; I did not execute the branch because this review is read-only.

Is this the best way to solve the issue?

No. The patch plausibly fixes the bug, but the narrower command-boundary design better preserves owner boundaries and the existing manager contract; that design must be repaired or ported because its current PR is conflicting.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b56bad33f2fb.

Label changes

Label changes:

add merge-risk: 🚨 compatibility: The branch changes the plugin-facing MemorySearchManager contract with new timeout and cleanup semantics that other implementations may inherit.
add merge-risk: 🚨 availability: The computed whole-operation deadline can keep an interactive memory turn active across setup, synchronization, retries, and multiple collection attempts far beyond the historical bound.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix live output from the repository development entrypoint with a real QMD workspace and CLI, showing one result after 28.878 seconds despite a 16-second injected command delay; private data should remain redacted in any further proof.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix live output from the repository development entrypoint with a real QMD workspace and CLI, showing one result after 28.878 seconds despite a 16-second injected command delay; private data should remain redacted in any further proof.

Label justifications:

P2: The PR fixes a reproducible QMD memory-search failure affecting users with intentionally longer configured search operations, but the blast radius is limited to that backend and configuration.
merge-risk: 🚨 compatibility: The branch changes the plugin-facing MemorySearchManager contract with new timeout and cleanup semantics that other implementations may inherit.
merge-risk: 🚨 availability: The computed whole-operation deadline can keep an interactive memory turn active across setup, synchronization, retries, and multiple collection attempts far beyond the historical bound.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix live output from the repository development entrypoint with a real QMD workspace and CLI, showing one result after 28.878 seconds despite a 16-second injected command delay; private data should remain redacted in any further proof.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix live output from the repository development entrypoint with a real QMD workspace and CLI, showing one result after 28.878 seconds despite a 16-second injected command delay; private data should remain redacted in any further proof.

Evidence reviewed

PR surface:

Source +303, Tests +385. Total +688 across 8 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	5	561	258	+303
Tests	3	387	2	+385
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	8	948	260	+688

What I checked:

Current-main timeout mismatch: Current main wraps the full memory_search operation in a fixed 15-second deadline even though QMD command execution separately uses memory.qmd.limits.timeoutMs, so a valid configured slow QMD query can be aborted early. (extensions/memory-core/src/tools.ts:129, b56bad33f2fb)
Whole-lifecycle budget: The branch derives the outer deadline from command attempts, collection count, optional dirty sync, embedding, and overhead, which can extend more than the active QMD command itself. (extensions/memory-core/src/memory/qmd-manager.ts:260, 16e2484891d0)
Plugin-facing contract change: The PR adds getSearchTimeoutMs and changes close to accept an optional timeout, creating two new optional semantics at the memory-host SDK boundary. (packages/memory-host-sdk/src/host/types.ts:165, 16e2484891d0)
Alternative maintainer direction: The competing branch contains a maintainer-authored repair that pauses the default deadline only while the actual QMD command runs, preserving the default budget for setup, maintenance, fallback, and cleanup without adding manager hooks. (extensions/memory-core/src/memory/search-deadline.ts:22, cb42cc599321)
Alternative is not a safe superseding target: Live GitHub reports the competing PR as CONFLICTING/DIRTY, so it cannot justify closing this mergeable PR as superseded under the conservative PR-to-PR rule. (cb42cc599321)
Live behavior proof: The PR body reports a real repository-entrypoint QMD run with a 16-second injected search delay that completed in 28.878 seconds and returned a result, demonstrating the central behavior beyond the former outer deadline. (extensions/memory-core/src/tools.ts:477, 16e2484891d0)

Likely related people:

steipete: Authored the maintainer repair on the competing implementation and documented the command-boundary timeout ownership and validation expectations. (role: recent repair author; confidence: high; commits: cb42cc599321; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/search-deadline.ts, extensions/memory-core/src/memory/qmd-manager.ts)
Alix-007: Authored the merged QMD AbortSignal propagation that the proposed deadline behavior relies on to stop active subprocesses safely. (role: adjacent behavior introducer; confidence: high; commits: c9ddf2eca6e8; files: extensions/memory-core/src/memory/qmd-manager.ts, packages/memory-host-sdk/src/host/qmd-process.ts)
sahibzada-allahyar: Authored the merged lifecycle-specific zero-hit QMD repair that distinguishes long-lived and one-shot memory manager behavior adjacent to this PR. (role: recent area contributor; confidence: high; commits: 90e31be388b9; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.test.ts)
dreamhunter2333: Authored the merged builtin-memory cancellation work that introduced caller AbortSignal semantics across the memory manager contract. (role: adjacent contract contributor; confidence: medium; commits: 8d72cb9401e5; files: extensions/memory-core/src/tools.ts, packages/memory-host-sdk/src/host/types.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (1 earlier review cycle)

reviewed 2026-07-10T18:07:11.756Z sha 306b26d :: needs real behavior proof before merge. :: none

jason-allen-oneal · 2026-06-20T15:12:21Z

@clawsweeper re-review

clawsweeper · 2026-06-20T15:12:24Z

🦞🧹
ClawSweeper re-review requested.