Skip to content

fix(memory): resolve QMD Windows cmd shims#73674

Open
knightplat-blip wants to merge 26 commits intoopenclaw:mainfrom
knightplat-blip:fix/qmd-windows-cmd-shim
Open

fix(memory): resolve QMD Windows cmd shims#73674
knightplat-blip wants to merge 26 commits intoopenclaw:mainfrom
knightplat-blip:fix/qmd-windows-cmd-shim

Conversation

@knightplat-blip
Copy link
Copy Markdown

@knightplat-blip knightplat-blip commented Apr 28, 2026 

## Summary

Fix QMD binary resolution on Windows when `memory.qmd.command` resolves to a `.cmd` shim instead of a native executable.

This was found while using QMD installed via:

```bash
npm install -g @tobilu/qmd

In the affected Windows environment, QMD was not operated through a standalone .exe. Instead, the working command was provided by a generated/custom qmd.cmd wrapper with content similar to:

@echo off
node "E:\software\encoding\common\nodeJs\node_global\node_modules\@tobilu\qmd\dist\cli\qmd.js" %*

Running QMD through this wrapper works normally from the shell, but OpenClaw's Windows wrapper resolver previously only handled %~dp0-relative shim targets. As a result, QMD availability checks failed closed with:

qmd.CMD wrapper resolved, but no executable/Node entrypoint could be resolved without shell execution

This change allows Windows .cmd wrappers to resolve quoted absolute JS/EXE entrypoints without using shell execution.

It also updates QMD package fallback resolution from qmd to the actual npm package name @tobilu/qmd.

Changes

  • Support quoted absolute Windows entrypoints in .cmd shim parsing
  • Apply the same fix to both plugin SDK and memory host SDK wrapper resolvers
  • Use @tobilu/qmd for QMD package fallback resolution
  • Add unit coverage for .cmd shims that quote an absolute JavaScript entrypoint

Testing

  • pnpm exec vitest run src/plugin-sdk/windows-spawn.test.ts
  • Local Windows runtime validation:
    • memory.qmd.command = "qmd"
    • QMD installed via npm install -g @tobilu/qmd
    • qmd.cmd resolves to: 
      @echo off
node "E:\software\encoding\common\nodeJs\node_global\node_modules\@tobilu\qmd\dist\cli\qmd.js" %*
    • QMD search provider reports provider: "qmd"
    • Search for 京能 successfully returns QMD-backed memory results

Platform notes

This has been validated on Windows.

macOS was not tested. The changed wrapper-resolution path is Windows-only (platform === "win32"), and non-Windows platforms continue to use the existing direct command path.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026 •
edited
Loading

Greptile Summary

This PR fixes Windows .cmd shim resolution for the QMD memory provider by adding support for quoted absolute JS/EXE entrypoints (e.g. node "E:\path\qmd.js" %*) alongside the existing %~dp0-relative resolution. It also corrects the npm package fallback name from qmd to @tobilu/qmd and adds a unit test for the new resolution path. Both the src/plugin-sdk and packages/memory-host-sdk copies of windows-spawn.ts are updated identically, preserving consistency between the two implementations.

Confidence Score: 5/5

Safe to merge — changes are Windows-only, well-scoped, and covered by a new unit test.

No P0 or P1 findings. The logic change is minimal and correct: the ternary in resolveEntrypointFromCmdShim preserves the existing %~dp0-relative path for the common shim format while correctly falling back to isAbsoluteEntrypointPath for the new quoted-absolute-path case. The path.isAbsolute inclusion in isAbsoluteEntrypointPath also means the new test passes on Linux CI. The @tobilu/qmd package name update is correct; resolveBinEntry gracefully degrades to Object.values iteration when the scoped key is absent from the bin field.

No files require special attention.

Reviews (2): Last reviewed commit: "fix(memory): resolve qmd Windows cmd shi..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Apr 28, 2026
View reviewed changes
Comment thread src/plugin-sdk/windows-spawn.test.ts
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026 •
edited
Loading

Codex review: needs real behavior proof before merge.

Summary
The PR updates Windows QMD command resolution to handle quoted absolute .cmd shim entrypoints, switches QMD fallback lookup to @tobilu/qmd, and adds focused resolver/memory-manager tests.

Reproducibility: yes. by source inspection: current main only resolves quoted %~dp0-relative .cmd targets and then throws the fail-closed wrapper error when no package fallback entrypoint is found. I did not run a live Windows shell.

Real behavior proof
Needs real behavior proof before merge: The PR body describes local Windows validation but provides no inspectable after-fix output, logs, screenshot, recording, or artifact; please add redacted proof and update the PR body so ClawSweeper can re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun.

Next step before merge
External PR needs contributor-supplied redacted Windows proof and a small rebase/code correction to preserve the host SDK bridge; ClawSweeper should not queue a repair because automation cannot supply the contributor's real setup proof.

Security
Cleared: No concrete security or supply-chain regression found; the intended resolver change keeps shell execution disabled and adds no dependencies, workflows, or secret handling.

Review findings

  • [P2] Keep the host SDK resolver bridged — packages/memory-host-sdk/src/host/windows-spawn.ts:1
Review details

Best possible solution:

Keep the host SDK file as a re-export through openclaw-runtime-io, apply the absolute-entrypoint parsing in the central plugin SDK resolver, keep the @tobilu/qmd fallback updates, and merge after redacted Windows proof is added.

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

Yes, by source inspection: current main only resolves quoted %~dp0-relative .cmd targets and then throws the fail-closed wrapper error when no package fallback entrypoint is found. I did not run a live Windows shell.

Is this the best way to solve the issue?

No, not as currently diffed: the central parser change is the right narrow fix, but the host SDK should stay bridged to the central resolver instead of carrying a copied implementation.

Full review comments:

  • [P2] Keep the host SDK resolver bridged — packages/memory-host-sdk/src/host/windows-spawn.ts:1
    Current main keeps this file as a thin re-export through openclaw-runtime-io, which carries the central src/plugin-sdk/windows-spawn.ts policy into the host SDK. Replacing it with a local resolver copy would undo that bridge and let future wrapper-policy fixes drift, so keep this file as a re-export and make the resolver change in the central SDK path.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.82

Acceptance criteria:

  • pnpm test src/plugin-sdk/windows-spawn.test.ts packages/memory-host-sdk/src/host/qmd-process.test.ts extensions/memory-core/src/memory/qmd-manager.test.ts -- --runInBand

What I checked:

Likely related people:

  • steipete: Current blame on the Windows resolver, QMD manager call site, QMD availability probe, and host SDK bridge points to the recent bridge/refactor commit; nearby log history also shows QMD process runner extraction by Peter Steinberger. (role: recent area contributor and bridge refactor owner; confidence: high; commits: e384932497f9, c5095153b0b7; files: src/plugin-sdk/windows-spawn.ts, packages/memory-host-sdk/src/host/windows-spawn.ts, packages/memory-host-sdk/src/host/qmd-process.ts)
  • Takhoffman: Git log identifies the earlier non-core spawn unification across ACP, QMD, and Docker as related Windows process-spawn work. (role: prior Windows spawn behavior contributor; confidence: medium; commits: cd653c55d7ce; files: src/plugin-sdk/windows-spawn.ts, packages/memory-host-sdk/src/host/qmd-process.ts)
  • vignesh07: Git log identifies a prior fix(memory): resolve qmd Windows shim commands commit in the same QMD Windows shim area. (role: prior QMD Windows shim contributor; confidence: medium; commits: 1ad9f9af5a37; files: src/plugin-sdk/windows-spawn.ts, packages/memory-host-sdk/src/host/qmd-process.ts)

Remaining risk / open question:

  • No inspectable Windows runtime proof has been provided for the contributor's reported setup.
  • Merging the current diff would undo the current host SDK re-export bridge and create duplicated Windows resolver logic.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4e5980eab439.

@knightplat-blip knightplat-blip force-pushed the fix/qmd-windows-cmd-shim branch from 9cd7180 to b6ca38b Compare April 28, 2026 16:49
@knightplat-blip
Copy link
Copy Markdown
Author

knightplat-blip commented Apr 28, 2026

@greptileai review

@BradGroux BradGroux mentioned this pull request Apr 29, 2026
Draft
@openclaw-barnacle openclaw-barnacle Bot added the extensions: memory-core Extension: memory-core label Apr 29, 2026
@knightplat-blip
Copy link
Copy Markdown
Author

knightplat-blip commented Apr 30, 2026

@greptileai review

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Assignees

No one assigned

Labels

extensions: memory-core Extension: memory-core size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@knightplat-blip @heiqishi666