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>
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.
AuditRecordandToolCallaccept unknown JSON fields. The rule is that unknown fields are rejected everywhere. The reviewer's probe showed an audit line with an extra"forged":truefield decoding without error.Timestamp::from_unix_millisaccepts anyu64, butto_rfc3339can 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
Timestampvalue aboveMAXcan exist.from_unix_millisandparsereject such values, andnow()never produces one (clamp it toMAX; a clock that far off is not worth an error). - With that guarantee
to_rfc3339cannot fail. Keep it returningString. strict.rstakes 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 thanAuditRecordorToolCall, 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 withaccepted an unknown key.cargo test -p proto --test ids. Expected: it does not compile, becauseTimestamp::MAXdoes not exist andfrom_unix_millisdoes not return aResult. -
3. Fix the code.
-
4. See them pass.
cargo test -p proto. Expected:ids12 passed,strict4 passed, and 45 passed in total across the six test files. -
5. Format, then run the gate.
cargo fmt --all, thenmake 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 protoreports 45 passed.make gateprintsgate: ok.diff -r crates/proto/tests docs/plans/M1/files/crates/proto/testsprints nothing.
Stop and report if
strict.rsfails for a type after you have addeddeny_unknown_fieldsto it.