[WIP] Add llama.cpp serving when local hardware can support it - #1174
Open
larroy wants to merge 7 commits into
Open
[WIP] Add llama.cpp serving when local hardware can support it#1174larroy wants to merge 7 commits into
larroy wants to merge 7 commits into
Conversation
Wraps the existing scripts/setup-dev.ps1 rather than reimplementing prerequisite detection. Documents the winget package IDs and the PATH refresh step needed before re-verifying, since freshly installed tools are not visible to an already-open shell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…alog First phase of local llama.cpp inference: the pure decision layer that maps a host's hardware onto a llama.cpp build and a model it can run. No download, process, or UI code yet. New in src/OpenClaw.Shared/Inference/: - HardwareProbe / NvidiaSmiParser / HostHardwareInfo detect CPU architecture, installed RAM, and graphics adapters. NVIDIA VRAM and CUDA version come from nvidia-smi via the injectable ICommandRunner seam, so tests need no GPU. The probe never throws; every failure degrades to unknown so an unclassifiable host lands on the CPU build. - LlamaBackendCatalog pins llama.cpp release b10472 and its six Windows variants (CUDA 12.4/13.3 x64, CUDA 13.4 arm64, Vulkan x64, CPU x64/arm64), each with a verified SHA-256 and size. - BackendSelector maps hardware to a preferred variant plus an ordered fallback chain, since a CUDA build can fail to launch for reasons the probe cannot observe. - LocalModelCatalog holds Qwen3.6-35B-A3B, DeepSeek V4 Flash 0731, and a placeholder for the unreleased Qwen3.8-27B, each with its tuned llama-server run recipe stored as a structured argument list. - ModelRecommender assesses fit against VRAM then system RAM and returns no recommendation rather than guessing when nothing fits. Notable decisions: - Unknown CUDA version degrades to the CUDA 12 build. A CUDA 12 runtime works on newer drivers; a CUDA 13 runtime does not work on older ones. - Win32_VideoController.AdapterRAM is never used as a VRAM number. It is a 32-bit field that wraps above 4 GB, exactly the range that matters, so the non-NVIDIA fallback reports vendor and name only. - Vulkan is preferred only when a Vulkan loader is present. Shipping a Vulkan build to a host without one turns "no acceleration" into "the server will not start". - DeepSeek V4 Flash is never auto-recommended. A 155 GB download must be an explicit choice. PhysicalMemoryProbe takes ownership of the GlobalMemoryStatusEx interop that DeviceStatusProvider held; DeviceStatusProvider now calls it, with identical behavior, so the two copies cannot drift. Asset integrity follows the existing audio-asset policy: an entry without a pinned hash is not downloadable. AssetHashPinningTests is extended to guard both new catalogs. GGUF hashes are the HuggingFace LFS object ids; llama.cpp hashes were computed from the downloaded archives with sizes cross-checked against the releases API. See docs/LOCAL_INFERENCE_ASSETS.md for provenance and its limits. Validation: ./build.ps1 (all projects, 49 docs), OpenClaw.Shared.Tests 3759 passed, OpenClaw.Tray.Tests 2469 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
larroy
changed the title
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed August 19, 2026, 12:55 PM ET / 16:55 UTC. ClawSweeper reviewWhat this changesThe branch adds local llama.cpp hardware detection, verified model/runtime downloads, a WinUI settings page, and a local OpenAI-compatible server. Merge readiness⛔ Blocked until real behavior proof is added - 14 items remain Keep open, but the feature is not merge-ready: its normal entry point, disable lifecycle, gateway integration, and autostart promise are incomplete, and no real-behavior proof is present. Priority: P2 Review scores
Verification
How this fits togetherThe tray app would select local hardware, download a pinned runtime and model, then launch a local inference server. To affect normal chat, that endpoint must be reachable through tray navigation and registered with the gateway model flow. flowchart LR
A[User settings] --> B[Hardware detection]
B --> C[Verified runtime and model]
C --> D[Local model server]
D --> E[Gateway registration]
E --> F[Chat model picker]
Decision needed
Why: Gateway registration changes the product’s provider configuration and lifecycle contract, while the current branch exposes the setting but implements neither that integration nor its recovery behavior. Before merge
Findings
Agent review detailsSecurityNeeds attention: The branch can leave an unauthenticated local-inference server active after the user disables its master setting. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Either complete the integrated feature with reachable navigation, safe stop and autostart semantics, gateway registration, tests, and isolated runtime proof, or narrow the PR to an explicitly local-only endpoint and remove unsupported controls. Do we have a high-confidence way to reproduce the issue? Yes. Current-head source shows the page is absent from normal navigation, disabling only saves state, and the gateway and autostart settings have no implementation consumer. Is this the best way to solve the issue? No. The intended integrated solution needs its missing lifecycle and gateway behavior completed, or the PR must stop advertising those behaviors. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b37307cbd51f. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
Second phase of local inference: fetch and install what the Phase 1 decision layer selected. Still no process launch or UI. New in src/OpenClaw.Shared/Inference/: - VerifiedFileDownloader stages to a .part file, verifies SHA-256, and only then moves into place. Shared by both managers. - SafeZipExtractor extracts with an explicit path-traversal guard. - LlamaRuntimeManager installs a backend variant into llama/runtimes/<runtime-key>/ and resolves llama-server.exe, or resolves a user-supplied custom build. - GgufModelManager downloads multi-shard checkpoints into llama/models/<model-id>/, preserving upstream shard names. Notable decisions: - Nothing unverified reaches a final path. A missing pinned hash fails before any network traffic; a hash mismatch, a length disagreement with the catalog, or a truncated response deletes the partial file and throws. The error never echoes the computed hash, which would be a confirmation oracle. - Resume is opt-in per request. GGUF shards run to tens of gigabytes, so a dropped connection has to be recoverable. A server that ignores Range and answers 200 triggers a clean restart rather than appending a full body onto an existing prefix. A partial file at or past the expected size is discarded, since it is the residue of an attempt that already failed verification and resuming from its end would loop. - A runtime directory is only trusted with its completion marker. A CUDA variant has two archives, and an interrupted install can leave a directory holding llama-server.exe but none of its CUDA DLLs, which would look installed and then fail at launch with a missing-DLL error. - Archives are deleted as they extract; a CUDA pair is close to 800 MB and keeping both would double peak disk use. - Free space is checked before starting, counting only missing shards so resuming a mostly-complete model is not blocked by the full size. Unknown free space proceeds rather than refusing. - A custom build skips download and hashing by design, and is reported through LlamaRuntime.IsUnverified so the UI can never show the bypass silently. Progress reporting uses a new InlineProgress<T> rather than System.Progress<T>. Progress<T> posts each report to the thread pool with no ordering guarantee, so the per-file to aggregate translation could deliver counts out of order and make a bound progress bar rewind. This surfaced as an intermittent test failure and is a real defect, not a test artifact. Tests run against an in-memory HttpMessageHandler: tampered bodies and length disagreements rejected with no residue, resume via Range, clean restart when Range is ignored, truncated-then-retried downloads, monotonic aggregate progress, zip traversal and sibling-prefix rejection, interrupted-install rebuild, and the free-space precheck. Validation: ./build.ps1 (all projects, 49 docs), OpenClaw.Shared.Tests 3805 passed across five consecutive runs, OpenClaw.Tray.Tests 2469 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
clawsweeper
Bot
added
the
merge-risk: 🚨 security-boundary
Third phase of local inference: launch and supervise the server, and give it a settings surface. Local inference is now usable end to end except for gateway registration. New in src/OpenClaw.Shared/Inference/: - LlamaServerArguments builds the command line and the health/base URLs. - ProcessJobObject wraps a kill-on-close Win32 job object. - LlamaServerProcess launches, health-polls, tails stderr, and stops. - LocalInferenceService sequences probe, selector, runtime, model, and server, and is the seam the UI talks to. New in the tray: Pages/LocalInferencePage (hardware, model, server, and advanced sections), Services/DisplayAdapterEnumerator (registry adapter fallback injected into the probe), eight LocalInference* settings with SettingsManager passthroughs, HubPageRegistry and HubWindow wiring, a lazily-built service on App, and a shutdown step that stops the server. Notable decisions: - The child runs inside a kill-on-close job object. Without it a tray crash leaves llama-server holding tens of gigabytes of VRAM with no UI left to stop it, and the next launch fails on a port conflict or an allocation error. A test proves an assigned process really dies when the job handle closes. - The health poll checks for child exit on each tick. A rejected recipe flag makes llama-server exit at once; without that check the user would wait out the full ready timeout for an error known in a second. - stderr is tailed for diagnostics; stdout is drained but discarded, because it is request logging and would put prompt content into our diagnostics. - Loopback unless explicitly widened, with its own opt-in and warning. - Start never begins a download. - Confirmation is an inline bar rather than a ContentDialog. REACTOR_DIALOG_001 keeps new surfaces off imperative dialogs and the per-file suppressions in .editorconfig are deliberately not extended. - Progress repaints are throttled to 150 ms; a 22 GB model produces roughly 280,000 progress callbacks. Also fixes a real BackendSelector gap found while testing on an ARM64 host with an NVIDIA GPU. The pinned release ships no CUDA 12 ARM64 build, so such a host lands on the CUDA 13 runtime, which may not load with a CUDA 12 driver. The plan now says so instead of reporting a confident choice the driver may reject. Strings are seeded English-only across all five locales using the repo's deferred-translation pattern and registered in LocalizationValidationTests. Validation: ./build.ps1 (all projects, 49 docs), OpenClaw.Shared.Tests 3848 passed across three consecutive runs, OpenClaw.Tray.Tests 2469 passed. Readiness against a real llama-server remains manual proof. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Probing the real dev host (a GB10 DGX Spark: ARM64 Windows, RTX Spark N1X, driver 616.29) surfaced two parser bugs that made the hardware probe wrong on that exact hardware. 1. The CUDA version banner. Older drivers print "CUDA Version: 12.8"; 616.29 prints "CUDA UMD Version: 13.4", which does not contain the older marker as a substring. The probe therefore reported an unknown CUDA version and, per the deliberate degrade-downward rule, preferred the CUDA 12 build. Both spellings are now matched. 2. The NVIDIA NPU. --query-gpu lists it alongside the GPU because it shares the driver, but it is not a CUDA device. It was being counted as a second adapter with unknown VRAM, which would show a phantom entry in the UI and imply an accelerator llama.cpp cannot use. Rows whose name matches NPU as a whole word are skipped. Both are pinned by tests using output captured verbatim from that host. Verified against real hardware after the fix: architecture Arm64, CUDA 13 detected, one GPU at 25,702,694,912 bytes VRAM, backend b10472-cuda13-arm64 with both the llama.cpp and cudart archives. Installing that runtime for real downloaded and SHA-256 verified both pinned archives in 15 seconds, extracted 43 files, and llama-server.exe --version reported "version: 0.1.1-dev (build 10472, commit 60eeeb608)", confirming the pinned tag and that the CUDA dependencies resolve. Known limitation, not addressed here: the recommender describes a model that exceeds VRAM as running "from system RAM" and being slow. On a unified-memory host like this one that framing is misleading, because the GPU carve-out and system RAM come from the same physical pool. There is no reliable way to detect unified memory from the probe's current sources, so the wording is left accurate for discrete GPUs and flagged rather than guessed at. Validation: ./build.ps1 (all projects, 49 docs), OpenClaw.Shared.Tests 3854 passed across six consecutive full runs plus ten runs of the process-spawning subset, OpenClaw.Tray.Tests 2469 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A real end-to-end run on the GB10 host found two defects that no unit test had reached. 1. A dropped connection destroyed the download. The 21 GB model transfer died at 50% on a socket read, and the catch-all cleanup deleted the .part file, discarding 10.6 GB of perfectly good bytes. That is precisely the case resume was built for. Cleanup is now scoped to verification failures, where the bytes really are known bad, and a transport failure keeps the partial file and retries with a resumed range request (bounded, with backoff). Cancellation also keeps the file so the user can resume later. The retry delay is injectable so tests exercise the path without paying wall-clock time. 2. A deliberate stop was reported as a crash. Killing the child raised Exited while the status was still Ready, so every normal Stop emitted "the server stopped unexpectedly" before Stopped, flashing a failure in the UI. The exit handler is now detached before the kill, guarded by an explicit stop-requested flag, and the decision is a pure testable policy. Tests reproduce the real failure: a fake transport that delivers part of a body and then throws, asserting the transfer resumes, that the partial file survives an exhausted retry budget, and that retries stay bounded. End-to-end proof on this host, now recorded in the plan doc: hardware probe, backend selection to b10472-cuda13-arm64, runtime install with both archives SHA-256 verified, 22,663,387,424 bytes of Qwen3.6-35B-A3B downloaded and verified in 11.9 min, server ready in 78 s, HTTP 200 from /v1/chat/completions answering "What is 2+2?" with "4", speculative decoding active at 0.88 draft acceptance, 40.4 tokens/second, and a clean Starting -> Ready -> Stopped with no spurious failure. A second run skipped both downloads. Also corrects a stale deferral: the arm64 CUDA path is now the proven one, so x64 is what remains unverified. Validation: ./build.ps1 (all projects, 49 docs), OpenClaw.Shared.Tests 3862 passed across three consecutive runs, OpenClaw.Tray.Tests 2469 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…heck fails Modern WSL distributes as an MSIX package (Program Files\WSL / WindowsApps), which doesn't embed per-file Authenticode/catalog signatures on its EXEs -- trust is established at the package level instead. This caused genuine Microsoft wslrelay.exe binaries to be rejected during setup's loopback listener provenance check with "WSL relay Authenticode verification failed (Unsigned)". Add a fallback in WindowsAuthenticodeVerifier: when the classic Authenticode check fails and the file is wslrelay.exe, corroborate trust via the installed AppX package instead. Requires an exact match on the known WSL package family name (MicrosoftCorporationII.WindowsSubsystemForLinux_8wekyb3d8bbwe, whose suffix is derived from the publisher's signing cert), a real SignatureKind, and a Microsoft publisher (reusing the existing HasMicrosoftPublisherIdentity check). Looked up via a PowerShell Get-AppxPackage shell-out, matching this file's existing pattern for invoking schtasks.exe/wsl.exe. The primary Authenticode/catalog check is unchanged and always consulted first; the fallback narrows to wslrelay.exe specifically and never weakens the trust bar for any other binary.
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
2 participants
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.
Additional instructions
MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.
What Problem This Solves
Why This Change Was Made
User Impact
Evidence
Change Type
Scope
winnodeValidation
Real Behavior Proof
Yes/No/N/A)Security Impact
Yes/No)Yes/No)Yes/No)Yes/No)Yes/No)Yes, explain the risk and mitigation:Compatibility and Migration
Yes/No)Yes/No)Yes/No)Review Conversations