Files
boxmaker/docs/plans/M1/08-proto-strictness.md
T
kyleandClaude Fable 5.1 dcdcf65d83 Review M1: accept with two follow-up tasks
The branch passes the checklist: seven commits, copied files unchanged,
gate and audit green. Reading and probing found that AuditRecord and
ToolCall accept unknown fields, that large Timestamps panic when
formatted, and that the dependency-direction scripts miss table-form
dependencies and pass when their inputs are missing. The last two
families were gaps in the tasks, not only in the code.

Tasks 08 and 09 carry the fixes, defined by an exhaustive unknown-field
test, a bounded-Timestamp test and an extended gate-script self-test.
All three were checked against the reference implementation and fail on
the current branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-17 09:14:29 -07:00

3.3 KiB

M1 task 08: close two gaps in proto (review follow-up)

Branch: m1 (run git switch m1; git status --short must be empty, otherwise stop) Commit subject: Reject unknown fields in every proto struct and bound Timestamp

Goal

The M1 review found two defects in proto. Fix both. Each is defined by a test file that you copy in and must not edit.

  1. AuditRecord and ToolCall accept unknown JSON fields. The rule is that unknown fields are rejected everywhere. The reviewer's probe showed an audit line with an extra "forged":true field decoding without error.
  2. Timestamp::from_unix_millis accepts any u64, but to_rfc3339 can only spell times up to the year 9999. For a larger value, humantime's formatter returns an error, to_string() panics, and so does serializing the value. Library code must never panic. This one was a mistake in the task you were given, not in your code.

Files

  • Copy (replacing the old one): crates/proto/tests/ids.rs
  • Copy (new): crates/proto/tests/strict.rs
  • Modify: crates/proto/src/ids.rs, crates/proto/src/audit.rs, crates/proto/src/log.rs, docs/implementer-log.md
  • Check, and modify only if the test says so: every other file in crates/proto/src/

Interfaces

Changes to Timestamp in crates/proto/src/ids.rs. Everything else keeps its signature.

impl Timestamp {
    /// 9999-12-31T23:59:59.999Z, the last instant RFC 3339 can spell with a four-digit year.
    pub const MAX: Timestamp;                                    // 253_402_300_799_999 ms

    // Was: -> Self. Now fails with ValueError::Timestamp when ms > MAX.
    pub fn from_unix_millis(ms: u64) -> Result<Self, ValueError>;
}

Rules:

  • No Timestamp value above MAX can exist. from_unix_millis and parse reject such values, and now() never produces one (clamp it to MAX; a clock that far off is not worth an error).
  • With that guarantee to_rfc3339 cannot fail. Keep it returning String.
  • strict.rs takes every fixture, adds one unknown key to one JSON object at a time, at every depth, and requires decoding to fail. If it reports a type other than AuditRecord or ToolCall, fix that type too and say so in your log row.

Steps

  • 1. Copy the tests.
git switch m1
cp docs/plans/M1/files/crates/proto/tests/ids.rs docs/plans/M1/files/crates/proto/tests/strict.rs crates/proto/tests/
  • 2. See them fail. cargo test -p proto --test strict. Expected: 2 of 4 tests fail with accepted an unknown key. cargo test -p proto --test ids. Expected: it does not compile, because Timestamp::MAX does not exist and from_unix_millis does not return a Result.

  • 3. Fix the code.

  • 4. See them pass. cargo test -p proto. Expected: ids 12 passed, strict 4 passed, and 45 passed in total across the six test files.

  • 5. Format, then run the gate. cargo fmt --all, then make gate. Expected last line: gate: ok.

  • 6. Log and commit.

git add crates/proto docs/implementer-log.md
git commit

Done when

  • cargo test -p proto reports 45 passed.
  • make gate prints gate: ok.
  • diff -r crates/proto/tests docs/plans/M1/files/crates/proto/tests prints nothing.

Stop and report if

  • strict.rs fails for a type after you have added deny_unknown_fields to it.