kyleandClaude Fable 5.1
dcdcf65d83
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>
87 lines
3.3 KiB
Markdown
87 lines
3.3 KiB
Markdown
# 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.
|
|
|
|
```rust
|
|
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.**
|
|
|
|
```sh
|
|
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.**
|
|
|
|
```sh
|
|
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.
|