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>
94 lines
4.0 KiB
Markdown
94 lines
4.0 KiB
Markdown
# M1 task 09: make the gate scripts fail closed (review follow-up)
|
|
|
|
**Branch:** `m1` (run `git switch m1`; `git status --short` must be empty, otherwise stop)
|
|
**Commit subject:** `Make gate scripts handle table-form dependencies and fail closed`
|
|
|
|
## Goal
|
|
|
|
The M1 review found that the dependency checks can be bypassed by accident, and that all three
|
|
scripts pass when they cannot find what they are supposed to check. Fix the scripts so the
|
|
extended self-test passes. The self-test is copied in and must not be edited.
|
|
|
|
The first problem was a gap in the task you were given: it did not mention that TOML lets a
|
|
dependency be written as its own table. The reviewer's probe added this to `crates/loopd/Cargo.toml`
|
|
(with `brokerd` also listed in `[workspace.dependencies]`), and both scripts passed although
|
|
`loopd` then depended on another role:
|
|
|
|
```toml
|
|
[dependencies.brokerd]
|
|
workspace = true
|
|
```
|
|
|
|
## Files
|
|
|
|
- Copy (replacing the old one): `scripts/test-gate-scripts.sh`
|
|
- Modify: `scripts/check-lines.sh`, `scripts/check-crate-deps.sh`, `scripts/check-dep-docs.sh`,
|
|
`docs/implementer-log.md`
|
|
|
|
## Required behaviour
|
|
|
|
Everything from task 01 still holds. In addition:
|
|
|
|
1. **A dependency's name can appear in three places.** All three count, in any section whose name
|
|
ends in `dependencies` (`[dependencies]`, `[dev-dependencies]`, `[build-dependencies]`,
|
|
`[target.'cfg(unix)'.dependencies]`):
|
|
- a plain key: `brokerd = { path = "../brokerd" }` or `brokerd = "1"`
|
|
- a dotted key: `brokerd.workspace = true` or `brokerd.path = "../brokerd"`
|
|
- a table header whose last part is the name: `[dependencies.brokerd]` or
|
|
`[dev-dependencies.brokerd]`. The lines inside that table (`path = …`, `workspace = true`,
|
|
`features = […]`) are settings of that dependency, not dependencies themselves.
|
|
2. **`check-crate-deps.sh`** applies its rule to names found in all three places.
|
|
3. **`check-dep-docs.sh`**, second rule: a plain or dotted dependency must have `workspace = true`
|
|
on its line, as before. A table-form dependency must have a `workspace = true` line inside its
|
|
table. `[dependencies.serde]` followed by `workspace = true` and `features = ["derive"]` is
|
|
fine; `[dependencies.rand]` followed by `version = "0.9"` is an error.
|
|
4. **Fail closed.** Each script exits 1 with a message if `ROOT/crates` does not exist.
|
|
`check-dep-docs.sh` also exits 1 if `ROOT/Cargo.toml` or `ROOT/docs/dependencies.md` does not
|
|
exist. Do not hide errors from `find`, `awk` or `grep` with `2>/dev/null`: if a tool fails, the
|
|
script must not report success.
|
|
5. `check-lines.sh` prints the line count of each file that is too long, for example
|
|
`check-lines: crates/x/src/a.rs has 612 lines (limit 500)`.
|
|
|
|
Still POSIX `sh` only: no bash features, no Python, no `jq`.
|
|
|
|
## Steps
|
|
|
|
- [ ] **1. Copy the self-test.**
|
|
|
|
```sh
|
|
git switch m1
|
|
cp docs/plans/M1/files/scripts/test-gate-scripts.sh scripts/
|
|
```
|
|
|
|
- [ ] **2. See it fail.** `sh scripts/test-gate-scripts.sh`. Expected: 7 failures, among them
|
|
`role depends on another role, written as a table` and
|
|
`check-lines without a crates directory`.
|
|
|
|
- [ ] **3. Fix the three scripts.** Read the new cases in `scripts/test-gate-scripts.sh` first; each
|
|
one has a comment or a name that says what it checks.
|
|
|
|
- [ ] **4. See it pass.** `sh scripts/test-gate-scripts.sh`. Expected: `test-gate-scripts: ok`.
|
|
Then run each script on the real tree: `sh scripts/check-lines.sh`,
|
|
`sh scripts/check-crate-deps.sh`, `sh scripts/check-dep-docs.sh`. Expected: no output, exit 0.
|
|
|
|
- [ ] **5. Run the gate.** `make gate`. Expected last line: `gate: ok`.
|
|
|
|
- [ ] **6. Log and commit.**
|
|
|
|
```sh
|
|
git add scripts docs/implementer-log.md
|
|
git commit
|
|
```
|
|
|
|
## Done when
|
|
|
|
- `sh scripts/test-gate-scripts.sh` prints `test-gate-scripts: ok`.
|
|
- `make gate` prints `gate: ok`.
|
|
- `cmp scripts/test-gate-scripts.sh docs/plans/M1/files/scripts/test-gate-scripts.sh` prints
|
|
nothing.
|
|
- `grep -n '2>/dev/null' scripts/check-*.sh` prints nothing.
|
|
|
|
## Stop and report if
|
|
|
|
- A case in the self-test contradicts the rules above.
|