Repository navigation
Manage llama.cpp models with a verified Go download cache - #62
Merged
Merged
Conversation
Build the model catalog in Go with validated single-file Q4_K_M entries, curated publishers and bounded cached public search. Resolve exact LFS metadata separately for transfer. Keep desktop and TUI catalog screens for the next PR; retain existing unreleased minor service bumps relative to v0.9.0. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Download public single-file GGUF models in Go using pinned LFS metadata, bounded transfers, checksum and size verification, exclusive cache staging and atomic no-replace commit. Reuse operation-ID progress, asynchronous cancellation and verified-peer control. Persist ownership for deletion and pin cache mutations with no-follow directory descriptors. Keep model hub screens for PR 4 and retain the existing engine-manager 1.5.0 and broker 2.6.0 unreleased minor bumps relative to v0.9.0. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Persist descriptor-derived file identity after model commit and refuse deletion or installed inventory promotion after a same-size file replacement. Parent-swap and replacement regressions fail with the guards disabled and pass with them restored. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Exercise the curated-publisher limits through JSON-RPC and confirm worker replacement interrupts llama.cpp downloads while fencing old events. Normalize bridge type formatting and Go import groups. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Do not confirm successful cancellation if removal of the operation-owned temporary file fails. Add a permission-failure regression, verified failing with the cleanup guard disabled and passing after restoration. Limit POSIX cache integration fixtures to non-Windows platforms while retaining portable catalog coverage. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Qualify the existing cancellation and shutdown guidance for engine-managed caches and document the Go adapter exception. Correct the llama.cpp settings and readiness summary to match the preceding routing work. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Honor cancellation and a five-second scan context between bounded GGUF metadata reads, with a ten-second broker envelope. Reject invalid model IDs before registering progress. The new inventory cancellation regression fails with caller context propagation disabled and passes after restoration. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Derive ownership from the exact verified transfer descriptor after atomic rename, never from a freshly reopened destination pathname. Do not roll back a replacement file after an ownership-manifest write failure. Cover descriptor identity across final-path replacement. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Inject the commit primitive in fixtures so the destination can be replaced between rename and ownership recording. The regression fails if ownership is taken from a reopened path and passes when it stays on the verified descriptor; replacement files remain undeletable. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Replace literal Unicode escape notation in Markdown and use the same literal apostrophe in the cancellation message without changing its compiled value. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Check the record count before transfer and before manifest commit, so adding a model cannot create a manifest the bounded reader rejects. Keep the ownership store within its existing 4096-record limit. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Check the published model inode and size after no-replace rename before recording ownership. Reserve generated filenames so rejected commits never become manual selections, and validate recorded identities against the descriptor inherited at launch. Document the same-user threat model and unavoidable OS check-to-use window. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Use no-follow fstatat directly before unlinkat for staging cleanup, failed manifest rollback, and completed-model deletion. Leave substituted entries untouched and surface skipped cleanup as an error rather than confirmed cancellation. Keep the staging descriptor open through cleanup and document the OS-minimum unlink window. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Recheck fstatfs on the open transfer descriptor every 256 MiB or five seconds of streaming, requiring room for the remaining bytes and safety reserve. Abort through the existing progress error state and identity-checked temp cleanup when space drops. Cover both interval bounds with mid-transfer statfs injection. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Treat generated filenames and recorded model matches case-insensitively, as required by default macOS filesystem semantics. A regression reproduces a replaced model selected through an uppercase filename. This closes the selection alias found while verifying the commit-identity fix. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Duplicate the pinned directory atomically with F_DUPFD_CLOEXEC so concurrent child spawns cannot inherit the transient directory used for launch identity checks. Preserve the PR 2 descriptor-inheritance boundary. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Normalize errors observed after an operation-ID cancellation to context cancellation before identity-checked cleanup, so the shared pull tracker returns the same cancelled result as Ollama without reporting a pull failure. Keep earlier primary failures and independent cleanup failures visible. Fence completion after the post-rename identity check and ownership manifest write. Earlier cancellation rolls back the operation's manifest entry and published file; later cancellation cannot undo a completed model. Cover metadata, staging, streaming, verification and commit cancellation, plus failure and completion ordering. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Review follow-up. Deferred calls run last-in first-out, so the response body was closed before the outcome was classified, and a cancel arriving while Close blocked could relabel an already observed failure, such as a checksum mismatch, as a clean cancellation. Classify first, then close. The regression gates the body's Close on a channel and cancels while it is blocked; it fails on 4113fff. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Review follow-up. Early failures returned through the cache release and directory close defers before the top-level classify ran, so a cancel during that teardown could still relabel an observed failure. Both cleanups now classify first, so every exit path takes the outcome fence before any teardown. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Review follow-up. Two early failure returns closed a file descriptor before setting the error and taking the outcome fence, so a cancel during that close could relabel the failure. Set the error and classify first, then close. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Use the shared pull tracker cancellation-cause rule instead of fencing primary failures before teardown. Preserve context cancellation in metadata and transfer I/O and filesystem helper errors without exposing request URLs. Keep the completion fence after verified cache ownership commits. Cover wrapped and unwrapped transport cancellation, real filesystem failures with concurrent cancel, and checksum failures during nested teardown. Cancellation fixtures now return their context cause. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Owner
Author
|
Merge note: the live re-run's step f was recorded as a failure only because the check expected the surviving model cache to be completely empty. Uninstall deliberately keeps the model cache (PR 1's rule), and what remained was PAIR's own empty ownership record ( Found during the run and fixed separately: llama-server reports a model launched from the verified descriptor as |
8 tasks done
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.
Description
llama.cpp PR 3 of 4: a Go-owned model catalog, verified downloads with progress and asynchronous cancel, installed inventory, and owned-model delete. Desktop and TUI hub screens remain PR 4.
engine:model-catalog {query?: string}RPC: curated ggml-org, bartowski and unsloth (50 repos each); public search at most 50 results, 100 query characters, 20 cached queries, six-hour TTL and 20-second aggregate request deadline (broker 25 seconds; future clients must allow 30 seconds). Only public, ungated text-generation GGUF repositories with one unsplit Q4_K_M candidate; IDsowner/repo:Q4_K_M. Strict bounded ASCII identifiers/filenames and bounded metadata bodies.engine:actiongains llama.cpppull_model,list_downloaded, anddelete_model; existing pull/cancel/remote RPCs and progress streams are reused. Installed inventory combines the ownership manifest with a bounded no-follow regular-GGUF scan; manually supplied files outside the reserved generated-filename namespace are selectable but never PAIR-deletable. Catalog IDs also resolve for--model. Installed scans honor a five-second context between bounded per-file metadata reads (broker ten seconds; future clients fifteen seconds).openatstaging records dev/ino immediately. After no-replace rename,fstatat(AT_SYMLINK_NOFOLLOW)must match the verified descriptor's dev/ino and expected size before directory fsync and manifest commit. A mismatch reports an error, writes no ownership record, remains unselectable, and is left untouched. The manifest records dev/ino, size and SHA-256. Selection and the actual inherited launch descriptor must match the recorded dev/ino/size; post-commit replacements are refused without re-hashing up to 64 GiB. Generated names and record matches are case-insensitive to prevent filename-case aliases on default macOS filesystems.unlinkat, no-followfstatatmust match the operation's recorded dev/ino (plus recorded size for completed models). Substituted temp/completed entries are left untouched. Skipped cleanup produces a clear error through Show model download progress and allow confirmed cancellation #50 progress/state, never successful cancellation. Manifest staging and rollback use the same checked cleanup. The temp descriptor stays open through cleanup.fstatfson the open transfer fd every 256 MiB or five seconds, whichever occurs first at a streaming boundary. Space must cover the remaining file bytes plus the 256 MiB margin; a drop aborts with “not enough disk space” and identity-checked temp cleanup.Intentional difference from NVIDIA/Personal-AI-Router@6d2d67f: PAIR downloads in Go into
{install_dir}-models, not through llama-server's router download API. No blocked download launch option is enabled. Only the operation's identity-matched temp is eligible for cleanup; transfer URLs and bodies are never logged.Cancellation-cause follow-up (current head)
Signed-off commit
1613ca01081d6273fb94b54e18bf5e632f35c14breplaces order-based failure fencing with the shared pull tracker's cause-based rule: only an operation's requested user cancel plus an error wrappingcontext.Canceledreportscancelledwithout a pull-failed engine error. Unrelated checksum/size, HTTP, disk, identity and manifest failures remain errors regardless of cancellation during teardown. Metadata/download request and body-read cancellation causes are preserved without exposing URLs; plain read errors after cancellation teardown use the cancelled operation context. Filesystem helper causes remain wrapped. Successful committed downloads still disable further cancellation.New tables:
TestLlamaMetadataPreservesCancellationCause,TestLlamaDownloadClassifiesByCause,TestLlamaDownloadCancellationCausePhases, andTestLlamaDownloadChecksumCancelDuringTeardown. Metadata cause loss and real failures incorrectly relabelled as cancellation were reproduced against the previous code before the fix. The existing cancellation-phase fixtures now wrap their simulated cancellation causes rather than return unrelated plain errors. Existing llama.cpp regression tests pass.Local validation: engine-manager built with
go test -race -c -o /tmp/pair-cause-final.test .; the full binary passed twice with-test.timeout=300s. Darwin/Linux/Windows vet, gofmt, diff checks and SPDX (1548 checked, zero missing) pass. No fake-engine/llama-server survivors; the implementation worktree is removed. No additional version bump or CHANGELOG change.Final-head CI 42412304611, head
1613ca01081d6273fb94b54e18bf5e632f35c14b: all six jobs pass, without a rerun. Downloaded the Linux cross-process artifact withgh run downloadinto new empty/var/folders/sb/5bf7gsds34v599d8y8x4pfb00000gn/T/pair-ci62-final-96otanlq;python3 -I /tmp/pair-review/scripts/noskip.py <dir>/cross-process.jsonlreports 127 passing tests, zero skips/failures. These source/CI results do not recertify the earlier packaged live run below.Scope
Services, generated desktop contracts and a bridge parameter type/overload in the original PR. Review fixes touch only engine-manager code/tests and
LLAMACPP.md. No desktop/TUI screens, real engine installation, real model download, or CHANGELOG edits.Version decision: retain the existing unreleased engine-manager 1.5.0 and broker 2.6.0 additive bumps. No additional version bump; product/installer and other binaries are unchanged.
Validation
Latest review fixes: macOS arm64, isolated
/tmp/pair-work/llamacpp-review, based on06d5565; signed-off commits pushed normally:c15d066a: commit and launch identity.d8806cff: identity-checked cleanup/deletion.8babe5f3: streaming disk-space checks.2c98dbf0: small follow-up for a filename-case selection bypass found during verification of the first fix.590690c5: preserve close-on-exec atomically when duplicating the pinned directory for launch identity checks.Evidence on the final source:
go test -race -c -o <tmpfile> .and run directly with-test.timeout=300s; broker nodemetrics, relay and workloadstore race binaries also pass.go vet ./...pass for both modules; gofmt andgit diff --checkare clean. SPDX: 1546 checked, zero missing.TestLlamaDownloadTempSwapAtCommit,TestLlamaRecordedReplacementRefusedAtLaunch,TestLlamaDownloadCleanupLeavesSubstitutedTemp(failure/cancel), andTestLlamaDownloadFreeSpaceDropsDuringStreaming(byte/time interval) all fail against06d5565and pass after the fixes. Commit mismatch checks no ownership record, no selection/launch, and preservation of the mismatching file.TestLlamaDeleteLeavesReplacedCompletedModelandTestLlamaRecordedIdentityCannotBeBypassedByFilenameCasepass; the case regression also failed before its guard was added.590690c5b295e33385fc9b6a67f4a098873e25d2: all six jobs pass, including both Go-module/race jobs, both cross-process jobs, desktop and SPDX. Downloaded the Linux artifact withgh run downloadinto new empty/tmp/pair-llamacpp-review-ci.1zI46g;python3 -I /tmp/pair-review/scripts/noskip.py /tmp/pair-llamacpp-review-ci.1zI46g/cross-process.jsonlreports 127 passing tests, zero skips/failures, and the package terminal event ispass. No rerun was needed.Earlier PR validation (before these review fixes): desktop typecheck, zero-warning lint, 945 unit tests, dead-code and service-contract checks passed. Prior CI run 42399660708 passed all six jobs on attempt 2, with 127 Linux cross-process tests and zero skips; attempt 1 had only the unchanged
TestModelsHTTPEnrichmentdiscovery flake. Those earlier results are not claimed as final-head certification.Risk
The cache lives in the user's own app data: entry swaps require another process running as the same user. macOS has no rename-by-descriptor or unlink-by-descriptor, so perfect atomicity is unavailable. Post-rename identity checks fail closed, and cleanup checks occur immediately before unlinkat; a same-user replacement after the last check leaves the minimum OS-allowed check-to-use/unlink window. This is documented in code and
LLAMACPP.md, not presented as an atomicity guarantee. Launch validates the descriptor inherited by the child, so later pathname replacement cannot redirect its opens. Identity checks detect replacement, not writes in place by an already-authorized same-user process.No live model/inference or packaged-asset certification. The engine remains macOS arm64-only. A crash between model rename and manifest commit may leave an unrecorded reserved-name file, which PAIR will neither select nor delete; abandoned crash temps are not automatically reclaimed. Inventory checks metadata and recorded identity/size, not a fresh full-file checksum. Streaming checks happen at read/write boundaries; a blocked read writes no additional bytes. Filesystem syscall latency itself is not context-preemptible. Desktop/TUI consumers must use a 30-second catalog deadline in PR 4.
Checklist
git commit -s).Live check
2026-11-07 rerun, head
1613ca01081d6273fb94b54e18bf5e632f35c14b, packaged macOS arm64 build on the owner's macOS 27.2 arm64 machine (Python 3.9.6). a–e PASS; f FAIL only on the strict empty-cache requirement: after successful model deletion and engine uninstall, the surviving cache contained the empty ownership manifest. Tests stopped at that finding; the owner's published install was restored and verified.The previous live run at
590690cfound cancellation during Hugging Face metadata resolution incorrectly ending aserror(model metadata request failed). That cancellation-classification bug is fixed in1613ca0and passed this packaged rerun: immediate cancel was accepted and terminal progress/result werecancelled, with no pull-failed engine error.Built in detached
/tmp/pair-work/live62busingnpm ci --no-audit --no-fund && npm run -s build:mac:arm64; no node_modules symlink and no build in the main checkout. ZIP size 200,591,852 bytes; local and remoteshasum -a 256matched1f64f066683c3cc091eb4f9a7225691d4caaa0ed9c26cf59c4699e3f60ca1eea. Reused the previous remote driver/control/phase/tree-verification scripts in fresh~/pair-live62b, without modifying previous runs or backups. Fresh app/app-data backups made withditto, checked withdiff -rq(Singleton*/logs excluded for app data) and bytewise regular-file, symlink, mode and structure verification. The packaged broker ran standalone with the flags the new desktop passes./propsHTTP 200 with rolerouter. Catalog returned 90 entries: ggml-org 7, bartowski 50, unsloth 33; selected model present.pullingprogress event;{accepted:true}acknowledged 0.665 ms after that event. Terminal stagecancelled, result{"status":"cancelled"}; engine errors empty; cache listing empty before and after, no temp file.done. Cached file 101,016,128 bytes, remote SHA-256 matchede3131339bf4e8065265593d4fd8f7bb7ff2d3abff1edb5618aa1197b89cad9f5; read-only ownership manifest and installed-models inventory agreed.--model ggml-org/SmolLM2-135M-GGUF:Q4_K_M; settings succeeded and engine restarted healthy. Proxy 1237/v1/modelsHTTP 200 listed ID/dev/fd/3. Non-streaming and streaming chat completions and/v1/messageswithanthropic-version: 2023-06-01all HTTP 200. Stream emitted 14 events and[DONE]; recorded only 12 content characters per response.launchText:""), healthy router restart confirmed. PAIR requires stopped-engine deletion, so stopped it beforedelete_model. Delete returned{"ok":true}, inventory became[], GGUF disappeared; stop/uninstall succeeded and install directory disappeared. Survivingllamacpp-modelswas not empty: only.pair-models.json(13 bytes), content{"models":[]}.Exact failed check:
Expected empty surviving model cache; found [{"name": ".pair-models.json", "size": 13, "content": "{\"models\":[]}"}]. This is a checker assertion about observed PAIR residue, not a JSON-RPC error: PAIR returned success for delete and uninstall. No GGUF or download temp remained. No further product tests or remediation were attempted after the finding.The only model used was the prior run's selection,
ggml-org/SmolLM2-135M-GGUF:Q4_K_M(SmolLM2-135M-Q4_K_M.gguf), 101,016,128 bytes, expected SHA-256 above; exactly two pull attempts (one cancelled during metadata, one completed). The committed cache filename was12926887e1d5c71725345eed940cb84b214e23fad6dc6cbe8e2063d4edbdf18e.gguf; manifest recorded matching size/hash and file identity. Only PAIR executed llama.cpp, installed/uninstalled only llama.cpp, and managed the model lifecycle. No owner engine directories or OS security settings were changed.Restoration: clean broker shutdown returned null and left no nvpair/PAIR service or llama-server processes. Moved current app data aside, restored
/Applications/PAIR.appand app data from~/pair-live62b/backupusingditto, verified diffs, then relaunched withopen -a /Applications/PAIR.app. Final state: published 0.9.0, broker + 12 workers alive as broker children, oMLX proxy 1236/v1/modelsHTTP 200 (7 models), original saved settings and revisions restored, no.gguffiles or llama.cpp paths under app data, no llama.cpp survivors. Removed~/pair-live62b/incomingand~/pair-live62b/replaced-appdata; kept the fresh backup and all older backups. Local/tmp/pair-work/live62bremoved.Evidence:
/tmp/pair-review/live-llamacpp/evidence-62b.jsonl(incremental checks plus archived driver requests/responses/progress). Checker-only corrections: the initial listener probe wrongly required an inventory HTTP 200 before engine availability; replaced it with TCP/listener verification. Cancellation verification initially referencedstateinstead of the wire fieldstage; corrected verification without repeating that pull. Neither script error is a PAIR failure or an additional download attempt. This run verifies only the tested local macOS arm64 package and flows, not other platforms or untested scenarios.