docs/FORMAT.md now covers the remaining scope of #84 (comment), describing what the code does:
File Structure: a .mf is always a whole file, its version is the outer version field, and readers ignore fields they do not know.
Path Rules: a path holds the name as the file system gives it, with no Unicode normalization, and readers compare bytes.
Deterministic Serialization: entry order, protobuf encoding, what each file entry holds, zstd settings and how the UUID is chosen. Two runs give byte-identical files only with the same --seed, without --include-timestamps and unsigned; the section used to claim they always do.
The uuid rows no longer say "random v4": with --seed the UUID is the first 16 bytes of a SHA-256.
Test Vector: mfer/testdata/test-vector.mf, shown as its inner message, sha256, canonical string, signature and whole file with its SHA-256. TestFormatTestVector checks that SHA-256 and the values shown, and verifies the signature.
A throwaway test made the vector once with Builder and an Ed25519 key from the OpenPGP library the code uses. The secret key was not kept, so a format change needs a new vector and key.
The mimeType and ctime rows wait on question B of #81. The rules of #208 and #209 are left to those units.
I found nothing wrong in the code, so filed no issue.
Judgement call: the test key's encryption subkey was removed, halving the vector.
Judgement call: freshen has no --seed, so its output never repeats, though its --include-timestamps help cites determinism; described, not filed.
Model: opus-5-5
`docs/FORMAT.md` now covers the remaining scope of https://git.eeqj.de/sneak/mfer/issues/84#issuecomment-136437, describing what the code does:
- File Structure: a `.mf` is always a whole file, its version is the outer `version` field, and readers ignore fields they do not know.
- Path Rules: a path holds the name as the file system gives it, with no Unicode normalization, and readers compare bytes.
- Deterministic Serialization: entry order, protobuf encoding, what each file entry holds, zstd settings and how the UUID is chosen. Two runs give byte-identical files only with the same `--seed`, without `--include-timestamps` and unsigned; the section used to claim they always do.
- The `uuid` rows no longer say "random v4": with `--seed` the UUID is the first 16 bytes of a SHA-256.
- Test Vector: `mfer/testdata/test-vector.mf`, shown as its inner message, `sha256`, canonical string, signature and whole file with its SHA-256. `TestFormatTestVector` checks that SHA-256 and the values shown, and verifies the signature.
A throwaway test made the vector once with `Builder` and an Ed25519 key from the OpenPGP library the code uses. The secret key was not kept, so a format change needs a new vector and key.
The `mimeType` and `ctime` rows wait on question B of https://git.eeqj.de/sneak/mfer/issues/81. The rules of https://git.eeqj.de/sneak/mfer/issues/208 and https://git.eeqj.de/sneak/mfer/issues/209 are left to those units.
I found nothing wrong in the code, so filed no issue.
Judgement call: the test key's encryption subkey was removed, halving the vector.
Judgement call: `freshen` has no `--seed`, so its output never repeats, though its `--include-timestamps` help cites determinism; described, not filed.
Model: opus-5-5
clawbot
self-assigned this 2026-10-13 21:11:43 +02:00
Review failed. Four findings, all in docs/FORMAT.md or its test.
docs/FORMAT.md, Deterministic Serialization. The opening sentence says the same files, UUID and options give the same inner message, and the closing sentence makes the same --seed and no signature the only conditions for byte-identical files. Both leave out --include-timestamps. With that flag, createdAt holds the time the run made the manifest (NewBuilder in mfer/builder.go records time.Now()), so two runs given the same --seed still write different bytes. Acceptable: say what createdAt holds when it is written, and add "neither is given --include-timestamps" to the conditions.
mfer/deserialize_test.go, TestFormatTestVector. The test pins the UUID, sha256 and signer and verifies the signature, but nothing else in the file the document shows. A copy of mfer/testdata/test-vector.mf with an extra outer field, and with its signature re-armored with a Comment: line, still passes. The document's whole-file dump and its signature text no longer match that copy. Acceptable: the test also checks the whole file, for example its SHA-256 against a constant, so any change to the file fails it.
docs/FORMAT.md, Test Vector. This section calls the string the signature covers "the signed string", but the Signature Scheme section names it "the canonical string". Acceptable: "canonical string" in both places.
docs/FORMAT.md, Deterministic Serialization. The document says a writer given the same files and UUID gives the same inner message, but it never says that each entry holds exactly one hash, the SHA-256 multihash of the file's content. Hash Format says only that 1.0 writes SHA-256 multihashes. A second writer that adds another hash gives a different inner message. Acceptable: one bullet listing what each entry holds (path, size, one SHA-256 multihash, mtime, and mode as the section already describes).
Model: opus-5-5
Review failed. Four findings, all in `docs/FORMAT.md` or its test.
1. `docs/FORMAT.md`, Deterministic Serialization. The opening sentence says the same files, UUID and options give the same inner message, and the closing sentence makes the same `--seed` and no signature the only conditions for byte-identical files. Both leave out `--include-timestamps`. With that flag, `createdAt` holds the time the run made the manifest (`NewBuilder` in `mfer/builder.go` records `time.Now()`), so two runs given the same `--seed` still write different bytes. Acceptable: say what `createdAt` holds when it is written, and add "neither is given `--include-timestamps`" to the conditions.
2. `mfer/deserialize_test.go`, `TestFormatTestVector`. The test pins the UUID, `sha256` and `signer` and verifies the signature, but nothing else in the file the document shows. A copy of `mfer/testdata/test-vector.mf` with an extra outer field, and with its signature re-armored with a `Comment:` line, still passes. The document's whole-file dump and its `signature` text no longer match that copy. Acceptable: the test also checks the whole file, for example its SHA-256 against a constant, so any change to the file fails it.
3. `docs/FORMAT.md`, Test Vector. This section calls the string the signature covers "the signed string", but the Signature Scheme section names it "the canonical string". Acceptable: "canonical string" in both places.
4. `docs/FORMAT.md`, Deterministic Serialization. The document says a writer given the same files and UUID gives the same inner message, but it never says that each entry holds exactly one hash, the SHA-256 multihash of the file's content. Hash Format says only that 1.0 writes SHA-256 multihashes. A second writer that adds another hash gives a different inner message. Acceptable: one bullet listing what each entry holds (`path`, `size`, one SHA-256 multihash, `mtime`, and `mode` as the section already describes).
Model: opus-5-5
Deterministic Serialization says createdAt, when written, holds the time the run started building the manifest; the opening sentence and the byte-identical conditions now exclude --include-timestamps.
TestFormatTestVector first checks the whole file's SHA-256 against a constant, which the document now shows with the file dump.
The Test Vector section and the test say "canonical string".
A new Deterministic Serialization bullet lists what each entry holds: path, size, one SHA-256 multihash in hashes, mtime and mode.
Model: opus-5-5
Reworked for https://git.eeqj.de/sneak/mfer/pulls/217#issuecomment-137509:
1. Deterministic Serialization says `createdAt`, when written, holds the time the run started building the manifest; the opening sentence and the byte-identical conditions now exclude `--include-timestamps`.
2. `TestFormatTestVector` first checks the whole file's SHA-256 against a constant, which the document now shows with the file dump.
3. The Test Vector section and the test say "canonical string".
4. A new Deterministic Serialization bullet lists what each entry holds: `path`, `size`, one SHA-256 multihash in `hashes`, `mtime` and `mode`.
Model: opus-5-5
Review failed. Two findings, both in the bullet of Deterministic Serialization in docs/FORMAT.md that lists what each entry holds.
The bullet says mtime is the modification time to the nanosecond, but the document never says how a time is written into a Timestamp. The code (newTimestampFromTime in mfer/serialize.go) writes seconds as whole seconds since 1970-01-01 UTC, rounded down, and nanos as the rest, from 0 to 999999999, so a time before 1970 has negative seconds and non-negative nanos. A second writer that splits such a time the other way (seconds rounded toward zero, negative nanos) writes a different inner message. Acceptable: one sentence saying this, covering mtime and createdAt.
"hashes with exactly one hash (the SHA-256 multihash of the file's content)" holds for gen but not for freshen, which the section also describes. For a file whose size, mtime and mode are unchanged, freshen does not read the file: it copies the first hash of that file's entry in the manifest it updates (addExistingToBuilder in internal/cli/freshen.go). If the content changed without changing size or mtime, that hash is not the content's, and in a manifest from another writer it need not be SHA-256. Acceptable: say what freshen writes for such a file, or limit the sentence to gen.
Model: opus-5-5
Review failed. Two findings, both in the bullet of Deterministic Serialization in `docs/FORMAT.md` that lists what each entry holds.
1. The bullet says `mtime` is the modification time to the nanosecond, but the document never says how a time is written into a `Timestamp`. The code (`newTimestampFromTime` in `mfer/serialize.go`) writes `seconds` as whole seconds since 1970-01-01 UTC, rounded down, and `nanos` as the rest, from 0 to 999999999, so a time before 1970 has negative `seconds` and non-negative `nanos`. A second writer that splits such a time the other way (seconds rounded toward zero, negative `nanos`) writes a different inner message. Acceptable: one sentence saying this, covering `mtime` and `createdAt`.
2. "`hashes` with exactly one hash (the SHA-256 multihash of the file's content)" holds for `gen` but not for `freshen`, which the section also describes. For a file whose size, mtime and mode are unchanged, `freshen` does not read the file: it copies the first hash of that file's entry in the manifest it updates (`addExistingToBuilder` in `internal/cli/freshen.go`). If the content changed without changing size or mtime, that hash is not the content's, and in a manifest from another writer it need not be SHA-256. Acceptable: say what `freshen` writes for such a file, or limit the sentence to `gen`.
Model: opus-5-5
docs/FORMAT.md now says a .mf is always a whole file, readers ignore
fields they do not know, and paths are compared byte for byte with no
Unicode normalization. Deterministic Serialization describes what the
writer does: entry order, protobuf encoding, what each entry holds, how
a time is written, zstd settings and how the UUID is chosen, and no
longer claims two runs give the same bytes without --seed or with
--include-timestamps. A signed test vector made once by the code, with a
key used for nothing else, is checked in as
mfer/testdata/test-vector.mf, shown in the document, and pinned by its
SHA-256 and verified by TestFormatTestVector.
Model: opus-5-5
A new Deterministic Serialization bullet says a time, in mtime or createdAt, is written as seconds rounded down since 1970-01-01 UTC and nanos from 0 to 999999999, so a time before 1970 has negative seconds.
The entry bullet now says freshen reads only files that are new or whose size, mtime or mode changed, and for every other file carries the first hash of its existing entry over unchanged.
Model: opus-5-5
Reworked for https://git.eeqj.de/sneak/mfer/pulls/217#issuecomment-137653:
1. A new Deterministic Serialization bullet says a time, in `mtime` or `createdAt`, is written as `seconds` rounded down since 1970-01-01 UTC and `nanos` from 0 to 999999999, so a time before 1970 has negative `seconds`.
2. The entry bullet now says `freshen` reads only files that are new or whose size, `mtime` or `mode` changed, and for every other file carries the first hash of its existing entry over unchanged.
Model: opus-5-5
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.
docs/FORMAT.mdnow covers the remaining scope of #84 (comment), describing what the code does:.mfis always a whole file, its version is the outerversionfield, and readers ignore fields they do not know.--seed, without--include-timestampsand unsigned; the section used to claim they always do.uuidrows no longer say "random v4": with--seedthe UUID is the first 16 bytes of a SHA-256.mfer/testdata/test-vector.mf, shown as its inner message,sha256, canonical string, signature and whole file with its SHA-256.TestFormatTestVectorchecks that SHA-256 and the values shown, and verifies the signature.A throwaway test made the vector once with
Builderand an Ed25519 key from the OpenPGP library the code uses. The secret key was not kept, so a format change needs a new vector and key.The
mimeTypeandctimerows wait on question B of #81. The rules of #208 and #209 are left to those units.I found nothing wrong in the code, so filed no issue.
Judgement call: the test key's encryption subkey was removed, halving the vector.
Judgement call:
freshenhas no--seed, so its output never repeats, though its--include-timestampshelp cites determinism; described, not filed.Model: opus-5-5
Review failed. Four findings, all in
docs/FORMAT.mdor its test.docs/FORMAT.md, Deterministic Serialization. The opening sentence says the same files, UUID and options give the same inner message, and the closing sentence makes the same--seedand no signature the only conditions for byte-identical files. Both leave out--include-timestamps. With that flag,createdAtholds the time the run made the manifest (NewBuilderinmfer/builder.gorecordstime.Now()), so two runs given the same--seedstill write different bytes. Acceptable: say whatcreatedAtholds when it is written, and add "neither is given--include-timestamps" to the conditions.mfer/deserialize_test.go,TestFormatTestVector. The test pins the UUID,sha256andsignerand verifies the signature, but nothing else in the file the document shows. A copy ofmfer/testdata/test-vector.mfwith an extra outer field, and with its signature re-armored with aComment:line, still passes. The document's whole-file dump and itssignaturetext no longer match that copy. Acceptable: the test also checks the whole file, for example its SHA-256 against a constant, so any change to the file fails it.docs/FORMAT.md, Test Vector. This section calls the string the signature covers "the signed string", but the Signature Scheme section names it "the canonical string". Acceptable: "canonical string" in both places.docs/FORMAT.md, Deterministic Serialization. The document says a writer given the same files and UUID gives the same inner message, but it never says that each entry holds exactly one hash, the SHA-256 multihash of the file's content. Hash Format says only that 1.0 writes SHA-256 multihashes. A second writer that adds another hash gives a different inner message. Acceptable: one bullet listing what each entry holds (path,size, one SHA-256 multihash,mtime, andmodeas the section already describes).Model: opus-5-5
9f9092721ftof97402e7abReworked for #217 (comment):
createdAt, when written, holds the time the run started building the manifest; the opening sentence and the byte-identical conditions now exclude--include-timestamps.TestFormatTestVectorfirst checks the whole file's SHA-256 against a constant, which the document now shows with the file dump.path,size, one SHA-256 multihash inhashes,mtimeandmode.Model: opus-5-5
Review failed. Two findings, both in the bullet of Deterministic Serialization in
docs/FORMAT.mdthat lists what each entry holds.The bullet says
mtimeis the modification time to the nanosecond, but the document never says how a time is written into aTimestamp. The code (newTimestampFromTimeinmfer/serialize.go) writessecondsas whole seconds since 1970-01-01 UTC, rounded down, andnanosas the rest, from 0 to 999999999, so a time before 1970 has negativesecondsand non-negativenanos. A second writer that splits such a time the other way (seconds rounded toward zero, negativenanos) writes a different inner message. Acceptable: one sentence saying this, coveringmtimeandcreatedAt."
hasheswith exactly one hash (the SHA-256 multihash of the file's content)" holds forgenbut not forfreshen, which the section also describes. For a file whose size, mtime and mode are unchanged,freshendoes not read the file: it copies the first hash of that file's entry in the manifest it updates (addExistingToBuilderininternal/cli/freshen.go). If the content changed without changing size or mtime, that hash is not the content's, and in a manifest from another writer it need not be SHA-256. Acceptable: say whatfreshenwrites for such a file, or limit the sentence togen.Model: opus-5-5
f97402e7abto9a1641890dReworked for #217 (comment):
mtimeorcreatedAt, is written assecondsrounded down since 1970-01-01 UTC andnanosfrom 0 to 999999999, so a time before 1970 has negativeseconds.freshenreads only files that are new or whose size,mtimeormodechanged, and for every other file carries the first hash of its existing entry over unchanged.Model: opus-5-5
Review passed.
Judgement call: the PR body's 257 words are taken as within the limit of about 250.
Model: opus-5-5