feat(inference): make llama.cpp receipt publication recoverable - #8422
Conversation
…receipt-clean-8414 # Conflicts: # src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts
…receipt-clean-8414 # Conflicts: # src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.ts
📝 WalkthroughWalkthroughThe Docker llama.cpp lifecycle now uses operation-scoped receipt writers. The host-local journal records canonical receipt bytes and publication state. Recovery revalidates authority, replays prepared receipts exactly, and finalizes or rolls back unfinished operations. ChangesLlama.cpp receipt recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DockerLlamaCppManagedLifecycle
participant HostLocalCreateJournalStore
participant HostLocalInferenceReceiptWriter
DockerLlamaCppManagedLifecycle->>HostLocalCreateJournalStore: prepare canonical receipt
DockerLlamaCppManagedLifecycle->>HostLocalInferenceReceiptWriter: writeExact canonical receipt
HostLocalInferenceReceiptWriter-->>DockerLlamaCppManagedLifecycle: exact committed bytes
DockerLlamaCppManagedLifecycle->>HostLocalCreateJournalStore: finalize receipt publication
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 46de11b in the TypeScript / code-coverage/cliThe overall coverage in commit 46de11b in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
5 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
…-clean-8414 # Conflicts: # src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts # src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.ts # src/lib/onboard/runtime-provider/host-local-create-journal.test.ts # src/lib/onboard/runtime-provider/host-local-create-journal.ts # src/lib/onboard/runtime-provider/host-local-inference.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.ts (1)
1355-1371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the prepare-outcome classification into a named helper.
The nested
try/catchinside the maintrydecides one thing: whether receipt publication may already be durable. It uses a nestedtry, an emptycatch, and three assignments toreceiptPublicationPossibleto express that decision.startalready spans about 160 lines, and the coding guidelines require low function complexity.Extract the classification so the sequencing in
startstays linear and the fail-safe direction is stated once.♻️ Proposed extraction
Add a module-level helper near
writePreparedReceipt:/** Reports whether a failed prepare may have left publication authority durable. */ function preparePublicationPossible( store: HostLocalCreateJournalStore, transactionId: string, ): { possible: boolean; journal: HostLocalCreateJournalRecord | null } { try { const persisted = store.load(transactionId); if (persisted === null) return { possible: true, journal: null }; const journal = normalizeHostLocalCreateJournalRecord(persisted); return { possible: journal.phase !== "started", journal }; } catch { // The durable outcome is unknown, so treat publication as possible. return { possible: true, journal: null }; } }Then simplify the call site:
try { journal = options.journalStore.prepareReceipt(transactionId, serialized); receiptPublicationPossible = true; } catch (error) { - try { - const persisted = options.journalStore.load(transactionId); - if (persisted === null) { - receiptPublicationPossible = true; - } else { - journal = normalizeHostLocalCreateJournalRecord(persisted); - receiptPublicationPossible = journal.phase !== "started"; - } - } catch { - receiptPublicationPossible = true; - } + const outcome = preparePublicationPossible(options.journalStore, transactionId); + receiptPublicationPossible = outcome.possible; + if (outcome.journal !== null) journal = outcome.journal; throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.ts` around lines 1355 - 1371, Extract the nested prepare-outcome classification from start into a module-level preparePublicationPossible helper near writePreparedReceipt. Have it load and normalize the persisted journal, return both the fail-safe possible flag and journal value, and treat load or normalization failures as possible with a null journal; update start to use this result before rethrowing the original prepare error.Source: Coding guidelines
src/lib/onboard/runtime-provider/host-local-create-journal.test.ts (1)
168-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a receipt that is canonical but bound to a different authority.
The new cases reach three distinct rejection points: the phase invariant, the parser, and the digest recompute. None reach the authority-binding branch that raises
prepared receipt differs from create authorityinnormalizeHostLocalCreateJournalRecord. That branch checksproviderId,service,runtime.runtimeId,runtime.specSha256,model.generation, andengineAuthority.Rejecting receipt drift is a stated objective of this PR. Add a case that stores a canonical receipt with a correct digest but a mismatched
generationorruntimeId.💚 Proposed additional case
it("rejects a canonical receipt bound to another transaction (`#8414`)", () => { const store = createHostLocalCreateJournalStore(stateDirectory); store.create(prepared()); store.recordCreating(TRANSACTION_ID, CREATE_INTENT_UNIX_MS); store.recordCreated(TRANSACTION_ID, RUNTIME_ID); store.recordStarted(TRANSACTION_ID); const foreign = serializeHostLocalInferenceReceipt({ ...JSON.parse(serializedReceipt()), runtime: { ...JSON.parse(serializedReceipt()).runtime, model: { ...JSON.parse(serializedReceipt()).runtime.model, generation: "c".repeat(64) }, }, }); expect(() => store.prepareReceipt(TRANSACTION_ID, foreign)).toThrow( "prepared receipt differs from create authority", ); expect(store.load(TRANSACTION_ID)?.phase).toBe("started"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/runtime-provider/host-local-create-journal.test.ts` around lines 168 - 192, Add a test near the existing receipt tampering coverage that creates the journal through the started phase, serializes a canonical receipt with a mismatched authority field such as model generation or runtime ID while preserving a valid digest, and verifies prepareReceipt rejects it with “prepared receipt differs from create authority” while the journal remains in the started phase. Reuse the existing helpers and constants, including createHostLocalCreateJournalStore, prepared, serializedReceipt, TRANSACTION_ID, and RUNTIME_ID.src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts (1)
978-1009: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a malformed receipt writer.
requireReceiptWriteris a new validation at both public entrypoints. It raisesDocker llama.cpp receipt writer authority is malformed.whentransactionIdortargetSha256is not a SHA-256 string, or whenwriteExactis not a function. The drift test covers only well-formed writers with mismatched identities, so that branch stays uncovered.Assert that
startrejects the malformed writer before it touches the engine or the journal.💚 Proposed additional case
it("rejects a malformed receipt writer before any mutation (`#8414`)", () => { const fixture = dockerFixture(); const store = journalStore(); const lifecycle = controller(fixture, store); expect(() => lifecycle.start({ ...receiptWriter(), targetSha256: "not-a-digest" }), ).toThrow("receipt writer authority is malformed"); expect(fixture.capture).not.toHaveBeenCalled(); expect(store.list()).toEqual([]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts` around lines 978 - 1009, Add a test beside the existing receipt-writer drift coverage that passes a malformed writer, such as one returned by receiptWriter with an invalid targetSha256, to lifecycle.start. Assert it throws the receipt-writer authority malformed error and verify fixture.capture was not called and journalStore.list() remains empty, proving validation occurs before engine or journal mutation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.ts`:
- Around line 978-1009: Add a test beside the existing receipt-writer drift
coverage that passes a malformed writer, such as one returned by receiptWriter
with an invalid targetSha256, to lifecycle.start. Assert it throws the
receipt-writer authority malformed error and verify fixture.capture was not
called and journalStore.list() remains empty, proving validation occurs before
engine or journal mutation.
In `@src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.ts`:
- Around line 1355-1371: Extract the nested prepare-outcome classification from
start into a module-level preparePublicationPossible helper near
writePreparedReceipt. Have it load and normalize the persisted journal, return
both the fail-safe possible flag and journal value, and treat load or
normalization failures as possible with a null journal; update start to use this
result before rethrowing the original prepare error.
In `@src/lib/onboard/runtime-provider/host-local-create-journal.test.ts`:
- Around line 168-192: Add a test near the existing receipt tampering coverage
that creates the journal through the started phase, serializes a canonical
receipt with a mismatched authority field such as model generation or runtime ID
while preserving a valid digest, and verifies prepareReceipt rejects it with
“prepared receipt differs from create authority” while the journal remains in
the started phase. Reuse the existing helpers and constants, including
createHostLocalCreateJournalStore, prepared, serializedReceipt, TRANSACTION_ID,
and RUNTIME_ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d60a3dcb-dd1d-4c94-a1c1-a4a79f5f25e0
📒 Files selected for processing (5)
src/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.test.tssrc/lib/onboard/runtime-provider/docker-llama-cpp-managed-lifecycle.tssrc/lib/onboard/runtime-provider/host-local-create-journal.test.tssrc/lib/onboard/runtime-provider/host-local-create-journal.tssrc/lib/onboard/runtime-provider/host-local-inference.ts
1 participant
Summary
receipt-preparedjournal phase containing exact canonical, secret-free receipt bytes and their digestThis PR is stacked only on the clean dormant lifecycle base in #8418. It remains unregistered and changes no production onboarding, routing, YAML schema, or support claim.
Validation
git diff --checkSigned-off-by: Aaron Erickson aerickson@nvidia.com
Closes #8414
Part of #8144
Summary by CodeRabbit
Reliability
Bug Fixes