Repository navigation
fix(cli): persist SHA-256 when recording artifacts - #236
Conversation
Normal download leaves artifact_files.sha256 empty, so the first --reuse-verified run fills whatever digest is on disk. These tests lock the intended record-time digest and same-size hash_mismatch fallback before the production fix. Co-authored-by: rabess <rabesss@users.noreply.github.com>
recordManifestTx hashes any file whose digest is still empty so CLI download, CompleteJob, and recovery store a reference digest. Fill-when-empty stays for libraries recorded before this change. Co-authored-by: rabess <rabesss@users.noreply.github.com>
Recording now stores a digest, so no test left a real row with an empty sha256. This clears one row through the store's own database and proves verify --hash fills it from disk and persists it. Co-authored-by: rabess <rabesss@users.noreply.github.com>
The store opens SQLite with _txlock=immediate, so hashing inside recordManifestTx held the write lock for the whole digest. Measured on a 4-core sha_ni VM with in-memory files: a 2 GiB record made a concurrent writer wait 2.3 s, and a 10 GiB record failed it with SQLITE_BUSY after the 5 s busy timeout. RecordManifests and completeValidatedJob now digest before BeginTx, which brought the concurrent writer's wait to 5 ms. The job expectation still matches the caller's manifest, so a pinned digest must come from the caller. Co-authored-by: rabess <rabesss@users.noreply.github.com>
manifest_json and artifact_files.sha256 come from the same digested manifest. Pin the manifest side too. Co-authored-by: rabess <rabesss@users.noreply.github.com>
download --json still emits the downloader's manifest without the field. Library reads carry the stored digest. Rewrap the architecture paragraph that was left with an orphaned line. Co-authored-by: rabess <rabesss@users.noreply.github.com>
TestCompleteJobEnforcesExpectedSHA256 already requires a pinned digest on the caller's manifest. The digest-then-BeginTx order is visible without restating it. Co-authored-by: rabess <rabesss@users.noreply.github.com>
hashStoredFile hashed whatever the path named at record time without the stability proof the artifact package requires of completed-file reads, and never checked that the hashed bytes matched the size the validated manifest recorded. A file replaced mid-hash persisted a digest that never described a durable on-disk state. Mirror verifyArtifactFile: Lstat before open, reject a swapped or non-regular descriptor, require the recorded size, then validate stability across the read before publishing the digest. Wrap batch digest failures with the artifact index and document the digest-before-transaction trade-off on completeValidatedJob. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Greptile SummaryThis PR records SHA-256 digests when completed artifact manifests enter the library, preventing the first reuse verification from trusting modified bytes.
Confidence Score: 5/5The PR appears safe to merge; the new recording path consistently persists stable reference digests without an identified correctness or security regression. The hashing flow validates file identity, regular-file status, expected size, and stability before persisting the digest, while preserving pinned digests and legacy-library behavior. The tests cover the principal corruption and replacement scenarios introduced by the change.
|
| Filename | Overview |
|---|---|
| internal/library/verify.go | Adds stability-checked hashing for manifest files whose SHA-256 is empty. |
| internal/library/artifact.go | Digests validated manifests before opening the artifact-recording transaction. |
| internal/library/jobs.go | Digests completed-job manifests before beginning the write transaction. |
| internal/library/digest_internal_test.go | Covers size drift rejection and preservation of caller-pinned digests. |
| internal/library/verify_internal_test.go | Covers rejection of a same-size path replacement before digest publication. |
| internal/cli/cli_download_reuse_test.go | Verifies that a same-size content substitution triggers fallback downloading instead of reuse. |
Sequence Diagram
sequenceDiagram
participant Caller
participant Library
participant File
participant SQLite
Caller->>Library: Record/complete validated manifest
loop Each file with empty SHA-256
Library->>File: Lstat and open completed file
Library->>File: Verify regular file, identity, and size
Library->>File: Read and compute SHA-256
Library->>File: Validate post-read stability
end
Library->>SQLite: Begin write transaction
Library->>SQLite: Persist manifest and file digests
SQLite-->>Library: Commit
Library-->>Caller: Success
Reviews (1): Last reviewed commit: "fix(library): prove record-time digest r..." | Re-trigger Greptile
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
- Record-time digest fill at the two recording seams:
RecordManifestsandcompleteValidatedJobfill empty per-file digests viadigestManifestFilesbefore the write transaction begins; these are the only tworecordManifestTxcallers, so one-shot CLI, watcher, API, and startup recovery are all covered, and caller-pinned digests pass through untouched. - Stability-checked digest read:
hashStoredFilemirrorsverifyArtifactFile(pre-openLstat, no-follow open,SameFile/regular check, size equality against the validated manifest, post-readValidateStableCompletedFile), so a swapped or growing file fails the record instead of persisting a bogus reference digest. - Legacy fill retained:
verify --hashand the first--reuse-verifiedstill fill empty rows from older libraries, pinned byTestVerifyArtifactFillsLegacyEmptySHA256. - Tests and docs: the acceptance criteria from #227 are pinned exactly (recorded rows carry the digest; a same-size valid-header swap now ends in
hash_mismatch+ fallback rather thanreused; drift/swap rejections; pinned-digest passthrough), and the README/api-reference/architecture claims match the code. - Verified locally:
go test -count=1 ./...passes andgo vetis clean on the touched packages.
openai-compatible/glm-5.3 ｜ 𝕏
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // transaction begins, so the full-file read never holds the library write | ||
| // lock; a job that turns out to be already completed pays that read anyway. | ||
| func (store *Store) completeValidatedJob(ctx context.Context, jobID string, validated artifact.Manifest) error { | ||
| digested, digestErr := digestManifestFiles(validated) |
There was a problem hiding this comment.
WARNING: A digest failure during recovery aborts RecoverInterruptedJobs entirely and can wedge watch startup with no per-job escape hatch
completeValidatedJob now performs a stability-checked full-file read before the write transaction. RecoverInterruptedJobs (jobs.go:392-397) skips only ErrJobTerminal/ErrJobTransition/ErrJobNotFound, so any digest error — a path swap or size drift in the window after buildManifest validated the outputs, a mid-read mutation of a multi-GB file, or an EIO on the first full read — returns RecoveryResult{}, err for the whole loop. prepareWatch (internal/cli/cli_watch.go:253-260) then exits the command with watch.ErrDurableState before polling starts: remaining recoverable jobs are not processed this run, and a deterministic cause (e.g. persistent I/O error) makes every subsequent watch start fail identically. Consider treating a digest error like buildErr — record a sanitized summary, park the job as pending/recoverable, and continue to the next job.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return fmt.Errorf("validate artifact %d: %w", index+1, err) | ||
| } | ||
| validated[index] = rebuilt | ||
| validated[index], err = digestManifestFiles(rebuilt) |
There was a problem hiding this comment.
WARNING: A record-time digest failure fails the whole batch and, in --events mode, reports fully published media as a failed job
RecordManifests digests every manifest before BeginTx, so one file that cannot be hashed (swap race in the window after the manifest was built, size drift, or EIO during the first full read) now leaves the entire run unrecorded: applyLibraryRecording (internal/cli/cli_download.go:271-279) sets LibraryRecorded=false for all lectures, and downloadEventStream.finish (internal/cli/cli_download_events.go:47-49) converts that into errDownloadLibraryCommit — a terminal job.failed event and nonzero exit even though all media was published. Before this change, recording never read file bytes, so post-download filesystem state could not fail the commit (only DB errors could). If failing the record is the intended semantics (it prevents persisting a bogus reference digest), consider distinguishing "media published, library commit failed" from a download failure in events mode, or degrading per-manifest so one file does not suppress recording of the other lectures.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return nil | ||
| } | ||
| if recordErr := recordManifestTx(ctx, tx, validated); recordErr != nil { | ||
| if recordErr := recordManifestTx(ctx, tx, digested); recordErr != nil { |
There was a problem hiding this comment.
WARNING: No test asserts the digest persisted through completeValidatedJob
The acceptance criterion ("recording stores SHA-256") is covered only for the RecordManifests seam (TestRecordManifestPersistsSHA256). TestCompleteJobIdempotencyStillValidatesManifestContract completes a hashless manifest but asserts only the path (jobs_test.go:394), and TestCompleteJobEnforcesExpectedSHA256 exercises only the caller-pinned path. Passing validated instead of digested here would pass the entire suite, so the job-path digest behavior (watcher, API, startup recovery) is effectively untested. Add an assertion that GetArtifact after CompleteJob with a hashless manifest returns the file's SHA-256, mirroring the store-path test.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if err != nil { | ||
| t.Fatalf("digestManifestFiles() error = %v", err) | ||
| } | ||
| if len(digested.Files) != 1 || digested.Files[0].SHA256 != pinnedSHA256 { |
There was a problem hiding this comment.
WARNING: This test cannot detect removal of the pinned-digest skip
artifact.Build already rejects a pinned digest that does not match the file contents, so the only pin that reaches digestManifestFiles is the file's true digest. If digestManifestFiles were changed to re-hash pinned files, the recomputed value would equal pinnedSHA256 and this assertion would still pass — the test verifies the output value, not that the pin was preserved without a second full-file read. To make it meaningful, monkeypatch validateStableArtifactFile to fail (as TestDigestManifestFilesRejectsPathSwapBeforePublication does): the pinned path must return without ever invoking it, so any re-hash fails the test. Note this also means there is currently no effective coverage anywhere that pinned digests pass through unmodified (the same holds for a store-level RecordManifests test).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if openedInfo.Size() != expectedBytes { | ||
| return "", fmt.Errorf("size changed between validation and recording: got %d bytes, want %d", openedInfo.Size(), expectedBytes) | ||
| } | ||
| digest, err := hashFile(opened) |
There was a problem hiding this comment.
SUGGESTION: Assert the hashed byte count matches expectedBytes
hashFile discards io.Copy's byte count. If the file is appended to mid-read and truncated back to expectedBytes within the filesystem's modtime granularity, both the size check above and the post-read stability proof can pass while the persisted digest describes a transient content that never existed at a single instant. Comparing the number of bytes actually read to expectedBytes here closes that window at zero cost and makes the persisted digest provably describe exactly manifest.Bytes bytes.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| verification pass calculates it. When supplied to the manifest builder, it is | ||
| checked against the complete file before being published. | ||
| The optional lowercase `files[].sha256` field is the SHA-256 of the completed | ||
| file. `download --json` emits the manifest the downloader built, which omits |
There was a problem hiding this comment.
SUGGESTION: "download --json emits the manifest the downloader built, which omits the field" is overbroad
With download --reuse-verified --json, the same artifacts array mixes downloader-built manifests with library-read manifests for reused lectures, and the latter carry sha256 (internal/cli/cli_download_reuse.go:101,217-218 — verified.Manifest flows into result.Artifacts). As written, the sentence contradicts the "manifests read back from the library carry it" statement that follows. Consider scoping it to newly downloaded lectures.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| and manifests read back from the library carry it. When the manifest builder | ||
| is given a digest, it checks the complete file before publishing. Rows | ||
| recorded by older releases may still have an empty digest. | ||
| `library verify --hash` and the first `download --reuse-verified` fill those |
There was a problem hiding this comment.
SUGGESTION: Reuse fills only legacy rows named in the latest manifest
download --reuse-verified verifies with LatestMaterialization: true (internal/cli/cli_download_reuse.go:207-209), so it fills only empty rows whose paths appear in the latest manifest; historical artifact_files rows outside it stay empty (internal/library/verify.go:101-133). library verify --hash fills all rows. The sentence lumps both paths together and overstates the reuse path — worth a qualifier such as "for rows in the latest manifest".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit f2d12dc)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f2d12dc)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 1b32005)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit efeb4df)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (11 files)
Reviewed by glm-5.3 · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
rabesss
changed the title
Review on #236 found that one job's digest failure aborted the whole RecoverInterruptedJobs loop and could wedge watch startup. Completion failures are now sorted: job-state errors skip the job, ErrJobOutputUnstable parks it recoverable with a summary (the same containment the buildErr path applies), and only durable-store failures abort the pass. Also assert the hashed byte count matches the recorded size so the persisted digest provably covers exactly manifest.Bytes bytes, cover digest persistence through CompleteJob, and prove the pinned-digest skip by failing the test if the stability seam is invoked. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Review on #236 noted download --json emits library-read manifests for reused lectures, which carry the digest, and that the first --reuse-verified fills only empty rows named in the latest manifest while library verify --hash fills all rows. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Review feedback addressed in Fixed
Left as intended behavior
Also retitled the PR to |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Contained record-time digest failures per job during recovery: digest errors from
completeValidatedJobare now wrapped in the newErrJobOutputUnstablesentinel, andRecoverInterruptedJobsroutes completion failures throughcontainRecoveryError, which parks an unstable job inPendingwith a recoverable summary and moves on — the same containment thebuildErrpath applies — so one unreadable output can no longer wedge watch startup. Durable-store failures still abort the batch. - Rejected mid-read size drift:
hashFilenow reports how many bytes it hashed, andhashStoredFilefails the record when that count differs from the manifest'sbytes, so the persisted digest provably covers exactly the recorded size even if the file changes length during the read. - Hardened the new tests to be able to fail: the pinned-digest passthrough test now injects a failing stability proof, so re-hashing a pinned file would fail it;
TestRecoverInterruptedJobsSkipsJobWhoseOutputsCannotBeDigestedpins the containment (unstable job pending, stable job still recovered, no batch error) and fails against the pre-fix batch abort. - Scoped the SHA-256 documentation:
download --jsonis now documented as emitting the downloader's hashless manifest for newly downloaded lectures but the library-read manifest (which carries the digest) for reused lectures, and the legacy-fill wording distinguishesverify --hash(any empty row) from the first--reuse-verified(rows named in the latest manifest). Checked against the actual reuse path — the emitted reuse manifest isverified.ManifestfromGetArtifact, and reuse verifies withHash: true+LatestMaterialization: true. - Verified locally:
go test -count=1 ./...passes;go vetandgofmt -l internal/are clean.
openai-compatible/glm-5.3 ｜ 𝕏
| } | ||
| if options.Hash { | ||
| actual, hashErr := hashFile(opened) | ||
| actual, _, hashErr := hashFile(opened) |
There was a problem hiding this comment.
SUGGESTION: The verify path still discards hashFile's byte count
hashStoredFile now rejects a digest that covered more bytes than the validated size (verify.go:285-287), but finishArtifactFileVerification drops the count here, and this hash has the same persistence consequences: recordVerification fills legacy rows whose stored sha256 is empty with exactly this value (verify.go:307), and it also decides hash_mismatch against the stored digest. A file appended to mid-read and truncated back within the stability proof's granularity can therefore persist a reference digest that never described a durable on-disk state — the same race this PR closes at the recording seam. file.Bytes is in scope; treat read != file.Bytes like the size-mismatch case instead of trusting the digest.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
The verification hash path discarded the byte count the same way the record path did, so an append-and-truncate inside the modtime granularity could publish or fill a digest that never described a stable state. finishArtifactFileVerification now requires the hashed byte count to equal the recorded size before publishing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Droid encountered an error —— View job |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior review at 1b32005 — a single commit extending the stability contract to the verification hash path.
- Extended the mid-read size-drift rejection to the verification hash path:
finishArtifactFileVerificationnow requires the hashed byte count to equal the recordedbytesbefore the digest is used, mirroring the checkhashStoredFilealready applies at record time — the two completed-file hash paths stay symmetric, so an append inside the modtime granularity can no longer publish or fill a digest covering a different byte count than the row records. - Kept the drifted digest unpublished: the drift branch returns before
result.SHA256 = actual, andrecordVerificationonly fills onFilePresent, so a racy digest can never reach the database; theFileNotRegularstatus matches the adjacent stability-proof branch's convention for racy-instability rejections, and all failure statuses share one downstream case (cli_download_reuse.go:339), so the choice is behaviorally inert. - Added a test that fails without the fix:
TestFinishArtifactFileVerificationRejectsSizeDriftDuringHashdrives the finish step directly with a recorded size one byte behind the file — without the check it would returnFilePresentwith a published digest, so both assertions genuinely pin the rejection. - Verified locally:
go test -count=1 ./internal/library/...,go vet, andgofmt -lare clean;hashFile's only two callers both consume the read count.
openai-compatible/glm-5.3 ｜ 𝕏
| if read != file.Bytes { | ||
| // The file changed size mid-read; the digest never described a | ||
| // stable state, so it must not be published or filled. | ||
| result.Status = FileNotRegular |
There was a problem hiding this comment.
SUGGESTION: Mid-read size drift is reported as not_regular, but FileSizeMismatch is the precise status for this branch
FileNotRegular is documented as "exists but is not a regular file" (verify.go:25), yet the file hashed here was already proven regular and identical to the Lstat target two checks earlier (verify.go:183-190) — the only new fact is that the byte count differs from the record, which is exactly what FileSizeMismatch ("differs from its recorded byte count") names. The token is user-facing: it lands in the verify JSON status field and flows verbatim into the reuse outcome Reason (internal/cli/cli_download_reuse.go:335-339 maps the status string directly), so anyone diagnosing a mid-read growth or truncation sees not_regular and is pointed at symlinks/special files. Behavior is otherwise identical — every failure status is non-reusable and never fills a legacy row — so this is classification accuracy only. If adopted, the new test's want mid-read growth rejection assertion should follow. Consider also setting result.ActualBytes = read here so the JSON reports the observed byte count instead of the pre-hash stat, which still equals expectedBytes.
| result.Status = FileNotRegular | |
| result.Status = FileSizeMismatch |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
FileSizeMismatch is the precise status when the hashed byte count differs from the recorded size; not_regular describes a type change. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Droid finished @rabesss's task —— View job The PR persists SHA-256 digests at record time with thorough TOCTOU defenses (pre-open Lstat, SameFile/regular-file checks, size equality, mid-read byte-count verification, and a post-read stability proof), and caller-pinned digests pass through untouched after artifact.Build verification. Pass 1 produced no candidates, and my independent read of the full diff against the codebase and prior review threads (including the kilo status-classification note, now fixed by c6f8223 reporting mid-read drift as size_mismatch) found no high-confidence, actionable issues to add. 0 inline comments posted |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior review at f2d12dc — a single commit tightening the failure taxonomy of the mid-read size-drift rejection.
- Relabeled mid-read drift as
size_mismatch— the verification hash path's drift branch now returnsFileSizeMismatchinstead ofFileNotRegular, matching the pre-open size check two branches above it (internal/library/verify.go:187), so a regular file that grew mid-read reports the status its error message already described and--jsonreasons stay consistent across both rejection points. - Kept the relabel behaviorally inert downstream —
FileNotRegularandFileSizeMismatchshare one case inisVerificationFailureReason(internal/cli/cli_download_reuse.go:339),recordVerificationstill fills only onFilePresent, and no docs referenced the old label; the drift test was updated to pin the new status and still asserts the digest stays unpublished. - Verified locally:
go test -count=1 ./internal/library/ ./internal/cli/,go veton both packages, andgofmt -l internal/are clean.
openai-compatible/glm-5.3 ｜ 𝕏
Pull Request
Description
What does this PR do?
download --reuse-verified(#226) only compares SHA-256 whenartifact_files.sha256is already set. A normaldownloadrecorded files with an empty digest, so the first reuse hashed whatever was on disk and adopted it as the reference: a same-size swap with a valid container header was reported asreused.This PR persists the SHA-256 when the artifact is recorded, so
artifact_files.sha256is non-empty from creation:RecordManifestsandcompleteValidatedJobfill empty digests viadigestManifestFiles/hashStoredFilebefore the write transaction begins. The store opens SQLite with_txlock=immediate, so hashing inside the transaction would hold the write lock for the full file read. This single seam covers every recording path: one-shot CLI download, watcher, API, and startup recovery.artifact.Buildalready verified them.Supersedes #232. This branch contains #232 in full, plus a review pass that found and fixed one correctness gap in its central new function:
hashStoredFilenow honors the completed-file stability contract documented ininternal/artifact/manifest.go("callers that inspect or hash an opened completed file must invokeValidateStableCompletedFileimmediately before publishing their result"). fix(library): persist SHA-256 when recording artifacts #232 hashed whatever the path named at record time with no pre-openLstat, noSameFile/regular-file check, no size consistency check against the validated manifest, and no stability proof after the read. A file replaced mid-hash would have persisted a digest that never described a durable on-disk state, and a file replaced between validation and the digest could persist a row whosebytesandsha256describe two different contents. The digest read now mirrorsverifyArtifactFile: pre-openLstat, regular-file/SameFilecheck, size must equal the manifest's recorded bytes, hash, thenValidateStableCompletedFile. A swapped or growing file now fails the record instead of persisting a bogus reference digest.RecordManifestsnow carry the artifact index (digest artifact %d), matching the validate step.completeValidatedJobdoc comment: why digesting happens before the transaction, and that an already-completed job pays the read anyway.Related Issue(s):
Type of Change
Testing Done
Unit Tests:
go test ./...)New coverage:
TestRecordManifestPersistsSHA256— recorded row and stored manifest carry the digest (acceptance criterion 1).TestReuseVerifiedFallsBackAfterSameSizeHashMismatch— same-size swap with a validftypheader after download now ends in a fallback download with reasonhash_mismatch, notreused(acceptance criterion 2).TestVerifyArtifactFillsLegacyEmptySHA256— legacy empty rows still fill on first hash verification (acceptance criterion 3).TestDigestManifestFilesRejectsSizeDrift— a file that changes size between validation and recording fails the record.TestDigestManifestFilesRejectsPathSwapBeforePublication— a same-size path swap mid-hash is caught by the stability proof.TestDigestManifestFilesKeepsCallerPinnedDigest— caller-pinned digests pass through unmodified.Test Coverage:
Trade-offs
--reuse-verified/verify --hash, so total work across a typical lifecycle is unchanged.BeginTxmeans an idempotentCompleteJobretry on an already-completed job hashes the files before the completion check discovers the job state. Moving the digest inside the transaction would avoid that but would hold the write lock during the read; the write-lock-free order is kept and documented.download --jsonstill emits the downloader's hashless manifest; manifests read back from the library carry the stored digest (documented indocs/api-reference.md).Code Quality Checklist
golangci-lint runpasses)Documentation
architecture.md,api-reference.mddescribe record-time digests and the legacy fill)Additional Context
Review thread on #226 ("Require a preexisting digest before reusing an artifact") deferred this gap to #227; this closes it at the recording seam as the issue's fix direction preferred.
Reviewer Checklist: