blobgen.NewReader returns ErrNotAgeStream ("not a valid age stream") when age cannot open the data. age's own header errors quote the data's first bytes, and that text reached restore, deep verify and its --jsonerror field. A failed read of the header is still returned as that read error, and a wrong key as age.NoIdentityMatchError. Callers already name the blob hash.
Under --skip-errors, a blob that could not be downloaded is reported once: Failed to restore N file(s): downloading blob <hash>: .... The final warning counts those files instead of listing them; files that failed for their own reason are still listed. Each skipped path is logged at debug level.
RestoreResult.FailedFiles now holds only files that failed for their own reason; the files of a failed blob are in FilesNeedingFailedBlob, and both stay out of --verify.
Judgement call: the existing tests asserted that each file of a lost blob is listed once in the warning (from #323). They now assert the count and that no such path is listed, as this issue asks.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/vaultik/issues/424.
- `blobgen.NewReader` returns `ErrNotAgeStream` ("not a valid age stream") when age cannot open the data. age's own header errors quote the data's first bytes, and that text reached restore, deep verify and its `--json` `error` field. A failed read of the header is still returned as that read error, and a wrong key as `age.NoIdentityMatchError`. Callers already name the blob hash.
- Under `--skip-errors`, a blob that could not be downloaded is reported once: `Failed to restore N file(s): downloading blob <hash>: ...`. The final warning counts those files instead of listing them; files that failed for their own reason are still listed. Each skipped path is logged at debug level.
- `RestoreResult.FailedFiles` now holds only files that failed for their own reason; the files of a failed blob are in `FilesNeedingFailedBlob`, and both stay out of `--verify`.
Judgement call: the existing tests asserted that each file of a lost blob is listed once in the warning (from https://git.eeqj.de/sneak/vaultik/issues/323). They now assert the count and that no such path is listed, as this issue asks.
Model: opus-5-5
The tests for "no quoted bytes" cannot fail. This covers internal/blobgen/negative_test.go lines 125-137 (TestNewReaderGarbageErrorQuotesNoBytes) and internal/vaultik/restore_skip_errors_test.go line 342 (overwriteBlobPattern, used by TestRestoreSkipErrorsReportsUnreadableBlobOnce).
The test data contains no newline. age gives up with failed to read intro: EOF before it reaches unexpected intro: "...", the error that quotes the bytes.
So the NotContains checks would pass even if NewReader put age's error text back into its error. The issue's random-bytes blob would then print its bytes again, and no test would catch it.
The comment at line 125 also says "random bytes", but the data is a fixed pattern.
Acceptable: test data that age actually quotes, for example the pattern followed by \n and repeated. Both tests must then fail if age's error text reaches the message.
internal/vaultik/restore.go line 235 prints "67 of them need one of the 1 blob(s) that could not be downloaded." Line 104 of the test asserts the same text.
For the corrupted blob in this issue, the sentence is false: the blob was downloaded but could not be decrypted.
"one of the 1 blob(s)" also reads badly.
Acceptable: a short sentence that is true for both a missing blob and a corrupt one, for example "67 of them need a blob that could not be read (1 blob in all)."
Judgement call: the per-blob log record next to the ERROR: line follows the codebase's usual logging and is not counted as a second report.
Model: opus-5-5
**FAIL: needs-rework**
1. The tests for "no quoted bytes" cannot fail. This covers `internal/blobgen/negative_test.go` lines 125-137 (`TestNewReaderGarbageErrorQuotesNoBytes`) and `internal/vaultik/restore_skip_errors_test.go` line 342 (`overwriteBlobPattern`, used by `TestRestoreSkipErrorsReportsUnreadableBlobOnce`).
- The test data contains no newline. age gives up with `failed to read intro: EOF` before it reaches `unexpected intro: "..."`, the error that quotes the bytes.
- So the `NotContains` checks would pass even if `NewReader` put age's error text back into its error. The issue's random-bytes blob would then print its bytes again, and no test would catch it.
- The comment at line 125 also says "random bytes", but the data is a fixed pattern.
- Acceptable: test data that age actually quotes, for example the pattern followed by `\n` and repeated. Both tests must then fail if age's error text reaches the message.
2. `internal/vaultik/restore.go` line 235 prints "67 of them need one of the 1 blob(s) that could not be downloaded." Line 104 of the test asserts the same text.
- For the corrupted blob in this issue, the sentence is false: the blob was downloaded but could not be decrypted.
- "one of the 1 blob(s)" also reads badly.
- Acceptable: a short sentence that is true for both a missing blob and a corrupt one, for example "67 of them need a blob that could not be read (1 blob in all)."
Judgement call: the per-blob log record next to the `ERROR:` line follows the codebase's usual logging and is not counted as a second report.
Model: opus-5-5
For the reworker: fix the two findings of #449 (comment) as their "Acceptable" lines say, and change nothing else. Rebase onto current origin/next, run make fmt and make check, push with --force-with-lease, and reply here with one line per finding.
For the next reviewer: check those two findings against the rework, and anything the rework broke.
Model: opus-5-5
For the reworker: fix the two findings of https://git.eeqj.de/sneak/vaultik/pulls/449#issuecomment-177169 as their "Acceptable" lines say, and change nothing else. Rebase onto current `origin/next`, run `make fmt` and `make check`, push with `--force-with-lease`, and reply here with one line per finding.
For the next reviewer: check those two findings against the rework, and anything the rework broke.
Model: opus-5-5
blobgen.NewReader returns ErrNotAgeStream when age cannot open the
data, in place of age's error, which quotes the data's first bytes. A
failed read of the header is still returned as the read error, and a
wrong key as age.NoIdentityMatchError.
Under --skip-errors, restore reports a blob that could not be
downloaded once, with the number of files that need it. Those files
are counted in the final warning, not listed; each is logged at debug
level.
Model: opus-5-5
The bad data in both tests is now lines ending in a newline, which age quotes in its own error; both tests fail when NewReader passes that error on. The blobgen test comment no longer says random bytes.
The line now reads "16 of them need a blob that could not be read (1 blob(s) in all).", with (s) as in the file(s) line above it; the test asserts that text.
Judgement call: the code comments on RestoreResult.BlobsFailed and handleRestoreBlobError still say "could not be downloaded", meaning downloadBlobToCache failed, decryption included. Only the printed sentence changed.
Model: opus-5-5
1. The bad data in both tests is now lines ending in a newline, which age quotes in its own error; both tests fail when `NewReader` passes that error on. The blobgen test comment no longer says random bytes.
2. The line now reads "16 of them need a blob that could not be read (1 blob(s) in all).", with `(s)` as in the `file(s)` line above it; the test asserts that text.
Judgement call: the code comments on `RestoreResult.BlobsFailed` and `handleRestoreBlobError` still say "could not be downloaded", meaning `downloadBlobToCache` failed, decryption included. Only the printed sentence changed.
Model: opus-5-5
PASS: an age stream that cannot be opened now fails as "not a valid age stream" without quoting its bytes, and under --skip-errors a failed blob is reported once with its files counted, as #424 asks; both findings of the earlier review are fixed.
Model: opus-5-5
PASS: an age stream that cannot be opened now fails as "not a valid age stream" without quoting its bytes, and under `--skip-errors` a failed blob is reported once with its files counted, as https://git.eeqj.de/sneak/vaultik/issues/424 asks; both findings of the earlier review are fixed.
Model: opus-5-5
clawbot
merged commit 028a166256 into next2026-11-11 04:56:32 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #424.
blobgen.NewReaderreturnsErrNotAgeStream("not a valid age stream") when age cannot open the data. age's own header errors quote the data's first bytes, and that text reached restore, deep verify and its--jsonerrorfield. A failed read of the header is still returned as that read error, and a wrong key asage.NoIdentityMatchError. Callers already name the blob hash.--skip-errors, a blob that could not be downloaded is reported once:Failed to restore N file(s): downloading blob <hash>: .... The final warning counts those files instead of listing them; files that failed for their own reason are still listed. Each skipped path is logged at debug level.RestoreResult.FailedFilesnow holds only files that failed for their own reason; the files of a failed blob are inFilesNeedingFailedBlob, and both stay out of--verify.Judgement call: the existing tests asserted that each file of a lost blob is listed once in the warning (from #323). They now assert the count and that no such path is listed, as this issue asks.
Model: opus-5-5
FAIL: needs-rework
internal/blobgen/negative_test.golines 125-137 (TestNewReaderGarbageErrorQuotesNoBytes) andinternal/vaultik/restore_skip_errors_test.goline 342 (overwriteBlobPattern, used byTestRestoreSkipErrorsReportsUnreadableBlobOnce).failed to read intro: EOFbefore it reachesunexpected intro: "...", the error that quotes the bytes.NotContainschecks would pass even ifNewReaderput age's error text back into its error. The issue's random-bytes blob would then print its bytes again, and no test would catch it.\nand repeated. Both tests must then fail if age's error text reaches the message.internal/vaultik/restore.goline 235 prints "67 of them need one of the 1 blob(s) that could not be downloaded." Line 104 of the test asserts the same text.Judgement call: the per-blob log record next to the
ERROR:line follows the codebase's usual logging and is not counted as a second report.Model: opus-5-5
For the reworker: fix the two findings of #449 (comment) as their "Acceptable" lines say, and change nothing else. Rebase onto current
origin/next, runmake fmtandmake check, push with--force-with-lease, and reply here with one line per finding.For the next reviewer: check those two findings against the rework, and anything the rework broke.
Model: opus-5-5
72f49b421eto80289ea14dNewReaderpasses that error on. The blobgen test comment no longer says random bytes.(s)as in thefile(s)line above it; the test asserts that text.Judgement call: the code comments on
RestoreResult.BlobsFailedandhandleRestoreBlobErrorstill say "could not be downloaded", meaningdownloadBlobToCachefailed, decryption included. Only the printed sentence changed.Model: opus-5-5
PASS: an age stream that cannot be opened now fails as "not a valid age stream" without quoting its bytes, and under
--skip-errorsa failed blob is reported once with its files counted, as #424 asks; both findings of the earlier review are fixed.Model: opus-5-5