feat(contracts/org): versioned canonical encoder and digest

[itsHabib]

First code of the org substrate. Deliberately first: a chain's digests outlive every refactor that follows them, so the bytes that produce a hash must be specified before any record type exists. Unfixable after the first chain is written.

Closes task p1-t0-canonical-encoder. Design: docs/features/org/spec.md §4.10 (PR #245, not yet locked — this piece is settled independently of the open questions there).

Why not json.Marshal

All four bakeoff kernels this package draws from — branchroom, mandate, obligation, proofline — canonicalize with json.Marshal. That ties the byte sequence to Go declaration order, HTML-escapes &, <, >, and carries no scheme version. The digests are stable Go-to-Go, reproducible nowhere else, and a field reorder during a refactor silently invalidates every historical digest.

Here a type states its canonical shape by building a Value, so the encoding is a specification a second implementation can follow rather than an artifact of this one.

The encoding

No insignificant whitespace. Map fields in sorted-name order — build order cannot reach the bytes. Shortest-decimal integers. Exactly one escape rule: JSON's two required escapes plus C0 controls; everything else literal UTF-8, including &, <, >, DEL, and U+2028/9.

No floats. Their decimal representation is the single richest source of cross-language digest disagreement, and nothing in an org record needs one — sequences are integers, deadlines are RFC 3339 strings, sizes are integers.

Ambiguous input is refused rather than resolved: duplicate field names, empty names, invalid UTF-8. "Last wins" would be a rule a second implementation has to reproduce exactly; refusal needs no agreement.

The scheme version is hashed in, not recorded beside the digest — a reader that ignores a beside-the-digest field would silently accept bytes produced under different rules; hashing it in makes that an ordinary mismatch.

Tests

The authoritative tests are hand-written expected bytes, not round-trips — a round-trip passes for any self-consistent encoder, including a wrong one.

Golden digests are reproducible with standard tools, which is the portability claim as an executable check rather than an assertion:

printf 'canon/v1\x00{}' | shasum -a 256

Verified to agree with this implementation. TestEncodingDiffersFromEncodingJSON pins the divergence so nobody "simplifies" this back into the thing it exists to replace.

97.7% coverage, -race clean, go list -deps confirms it imports nothing in the module.

🤖 Generated with Claude Code

[itsHabib]

@codex review

[itsHabib]

@claude review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @itsHabib's task in 2m 25s —— View job

---

Code Review — PR #246

• Read changed files
• Analyze correctness, design, and encoding spec
• Post complete review

---

This is a well-designed, well-motivated piece of foundational infrastructure. The encoding decisions are sound, the test strategy is exactly right (hand-written expected bytes rather than round-trips), and the reasoning is documented at the right level of detail. The notes below are mostly minor polish.

---

Correctness

appendString byte-by-byte iteration (canon.go:257–265) — Safe and correct. The loop walks bytes, not runes; multi-byte UTF-8 sequences are preserved intact because all continuation bytes (0x80–0xBF) and leading bytes (0xC2–0xFF) are ≥ 0x20 and are neither " (0x22) nor \ (0x5C), so they pass through unmodified one byte at a time. utf8.ValidString at the top of the function guarantees there are no ill-formed sequences. This is correct even though it looks unusual.

sortFields validation order (canon.go:224–237) — The empty-name check fires before the duplicate check (both empty names sort to positions 0 and 1; the check at i=0 catches the first one). This means two empty field names return ErrEmptyFieldName rather than ErrDuplicateField. Not wrong — both are hard refusals — but worth knowing.

Kind zero value (canon.go:48–55) — The deliberate gap at 0 means Value{} refuses at encode time. This is tested. Good defensive design.

Scheme separator (digest.go:37–39) — Using \x00 as a separator between the scheme string and the canonical bytes is correct defensive practice. Since Scheme = "canon/v1" is a fixed constant (no null bytes possible), the separator is technically redundant today but protects against a future scheme that is a prefix of another.

---

Minor Issues

1. Garbled comment on hexDigits (canon.go:240–241)

// hexDigits is the lowercase alphabet for \u escapes. Case is part of the
// encoding:  and  are different bytes.

Something was elided after encoding: — the two examples that were supposed to appear there (likely A and a, or "A" and "a") are missing. The comment reads as a sentence fragment. Fix this →

2. Duplicate comment block above TestGoldenDigests (canon_test.go:195–201)

// Golden digests. These pin the byte-to-digest mapping including the scheme
// version, which is hashed in rather than merely recorded beside the digest.
// A change to the encoder, the scheme string, or the hash construction breaks
// these — which is the intent: they are the chain's compatibility contract.
// Golden digests, pinning the byte-to-digest mapping including the scheme
// version that is hashed in rather than recorded beside it. A change to the
// encoder, the scheme string, or the hash construction breaks these — the
// intent, since they are the chain's compatibility contract.

The comment block says the same thing twice, slightly rephrased — looks like an edit artifact where a revised version was appended without removing the original. Fix this →

3. Redundant intermediate variable in Encode (canon.go:154–161)

func Encode(v Value) ([]byte, error) {
    var buf []byte
    buf, err := appendValue(buf, v)
    if err != nil {
        return nil, err
    }
    return buf, nil
}

This allocates buf, then immediately passes it to appendValue where the same nil slice would have been the correct starting point. The simplest form:

func Encode(v Value) ([]byte, error) {
    return appendValue(nil, v)
}

is idiomatic, eliminates the intermediate variable, and behaves identically since appendValue uses append internally. Fix this →

4. List aliases its input slice (canon.go:105–107)

func List(items ...Value) Value {
    return Value{kind: KindList, list: items}
}

The variadic items slice is stored directly. A caller holding a reference to the underlying array can mutate the list's contents after construction (before encoding). Map's pairs field has the same issue, but sortFields defensively copies before sorting, so the original is never mutated. List has no such protection. This is unlikely to matter in practice since Values are typically built and immediately encoded, but it is an asymmetry worth noting.

---

Design Observations (no action required)

• The scheme-inside-hash design (DigestBytes prepends Scheme + \x00 before hashing) is exactly right. It makes a scheme mismatch manifest as a digest mismatch, which every consumer already handles, rather than requiring a parallel field-check.
• Refusing invalid inputs (ErrDuplicateField, ErrEmptyFieldName, ErrInvalidUTF8) rather than resolving them is the only defensible choice for a durable hash substrate — "last wins" is a protocol two independent implementations have to agree on, while "refuse" is not.
• The test TestEncodingDiffersFromEncodingJSON pinning the HTML-escape divergence is important defensive infrastructure. If someone replaces this encoder with json.Marshal for "simplicity," the test breaks loudly.
• DigestBytes is exposed publicly. The docstring says "Prefer Digest" but doesn't stop a caller from hashing non-canonical bytes and getting a digest for them. Whether to export it is a judgment call; the test helper TestSchemeIsInsideTheHash uses it legitimately. Fine as-is.

---

Summary: Correctness is solid. The three fixable items are a garbled comment, a duplicate comment block (edit artifact), and a one-line simplification to Encode. The List aliasing note is a heads-up, not a blocker. Ready to merge after the comment cleanup.

[github-actions]

Reviewer: claude
Reviewed commit: 7c4988250cf1dbd42363512d98d9a02df92a81f3

[itsHabib]

@claude review

[claude]

Claude finished @itsHabib's task in 3m 49s —— View job

---

Code Review — PR #246

• Read changed files (contracts/org/canon.go, canon_test.go, digest.go)
• Analyze correctness, design, and encoding spec
• Post complete review

---

Reviewed at HEAD c7834f7 (round-1 fix commit). Round-1 addressed four of the prior findings — Encode simplification, List aliasing, the duplicate comment above TestGoldenDigests, and the garbled hexDigits comment. Two issues remain.

---

Issue 1 — hexDigits comment still effectively garbled (canon.go:240–241)

The round-1 fix inserted raw bytes into the comment source to serve as the "examples," but the bytes are C0 control characters (0x1F, the unit separator) that render as invisible in GitHub, most editors, and cat. Inspecting the file with grep … | cat -v shows ^_ at both example positions — the same character twice, which does not illustrate the case distinction the comment is trying to make.

The intent is to show that lowercase hex matters for the encoding. A readable version:

// hexDigits is the lowercase alphabet for \u escapes. Case is part of the
// encoding: the escape for 0x1F is ``, not ``.
const hexDigits = "0123456789abcdef"

Fix this →

---

Issue 2 — Map has the same aliasing gap List just fixed (canon.go:117)

List was fixed to use slices.Clone and now documents why:

A variadic call cannot alias, but List(slice...) can, and a caller that mutated its slice after building the Value would change what a later Encode produced — a digest that depends on when it was taken.

Map has the identical exposure and got no equivalent fix:

func Map(fields ...Field) Value {
    return Value{kind: KindMap, pairs: fields}  // aliases the caller's backing array
}

sortFields makes a defensive copy before sorting, so the encoder itself never mutates pairs — but a caller who holds the original []Field and mutates an element between two Encode calls gets different digests from the same Value. The List docstring and fix both argue this is unacceptable ("a digest that depends on when it was taken"); Map should be consistent:

func Map(fields ...Field) Value {
    return Value{kind: KindMap, pairs: slices.Clone(fields)}
}

Fix this →

---

Round-1 fixes confirmed

Finding	Status
Encode simplification to return appendValue(nil, v)	✅ applied
List aliasing → slices.Clone	✅ applied
Duplicate comment above TestGoldenDigests	✅ removed
Garbled hexDigits comment	⚠️ attempted but still broken (see Issue 1)

---

Design notes (no action required)

• The byte-by-byte appendString loop is correct: multi-byte UTF-8 continuation bytes (0x80–0xBF) and leading bytes (0xC2–0xFF) are all ≥ 0x20, not " or \, so they pass through intact. utf8.ValidString at the top ensures no ill-formed sequences reach the loop.
• TestDistinctShapesDigestDistinctly is excellent: the collision check across ten structurally different values (list-of-two vs concatenated string, int-1 vs string-"1", etc.) catches the framing accidents that golden-digest tests alone would miss.
• TestEncodingDiffersFromEncodingJSON is load-bearing defensive infrastructure. If this package gets "simplified" back to json.Marshal, the test breaks loudly — exactly right for foundational hash substrate.
• The scheme-inside-hash construction in DigestBytes is correct. Hashing Scheme + \x00 + canonical means a scheme mismatch presents as a digest mismatch, which every consumer already handles, rather than requiring a parallel field check.

---

Summary: Two issues remain — the hexDigits comment still has invisible control characters where its examples should be, and Map has the same aliasing exposure List was just fixed for. Both are one-liners. Ready to merge after those.

[github-actions]

Reviewer: claude
Reviewed commit: c7834f7ea99d34d2433038026cf1601b48606696

[itsHabib]

@claude review

[claude]

Claude finished @itsHabib's task in 1m 42s —— View job

---

Code Review — PR #246 (Round 3)

• Read changed files (contracts/org/canon.go, canon_test.go, digest.go)
• Analyze round-2 fixes at HEAD 98ce4af
• Post complete review

---

Reviewed at HEAD 98ce4af. Both round-2 findings are addressed.

Round-2 fixes confirmed

Finding	Status
hexDigits comment — invisible 0x1F bytes where examples should be	✅ fixed: now reads the escape "" and the escape "" are different byte sequences for the same rune
Map aliasing — pairs: fields stores caller's backing array directly	✅ fixed: pairs: slices.Clone(fields)

A sweep confirms no C0 control bytes survive anywhere in the package. The comment is now instructive and readable; Map and List are now symmetric on the aliasing guarantee.

Cumulative findings across all rounds

Round	Finding	Status
1	Encode simplification → return appendValue(nil, v)	✅
1	List aliasing → slices.Clone	✅
1	Duplicate comment above TestGoldenDigests	✅
1→2	Garbled hexDigits comment	✅ (required a second attempt)
2	Map aliasing → slices.Clone	✅

Design observations (no action required)

• The byte-by-byte appendString loop is correct. Multi-byte UTF-8 continuation bytes (0x80–0xBF) and leading bytes (0xC2–0xFF) are all ≥ 0x20 and neither " nor \, so they pass through intact. utf8.ValidString at the entry point ensures no ill-formed sequences reach the loop.
• The scheme-inside-hash construction (Scheme + \x00 + canonical before hashing) is correct. A scheme mismatch surfaces as a digest mismatch — which every consumer already handles — rather than requiring a parallel field check.
• TestEncodingDiffersFromEncodingJSON and TestDistinctShapesDigestDistinctly are load-bearing defensive tests. The first breaks loudly if this encoder is ever "simplified" back to json.Marshal; the second catches framing accidents that golden-digest tests alone miss.
• sortFields makes its own defensive copy before sorting (make + copy), so Map's slices.Clone and sortFields's internal copy are two separate layers — neither is redundant. sortFields is an internal helper that a future caller might pass a slice it still holds; the clone in Map closes the public constructor, sortFields's own copy closes the helper.

Ready to merge. All verified findings from rounds 1 and 2 are addressed; the code is correct, the tests are authoritative, and the encoding decisions are sound.

[github-actions]

Reviewer: claude
Reviewed commit: 98ce4af793379356928fcbaa0b7133b2b024397f