diff --git a/agents/darkwing/work/slice1-sr-review-r2-2026-10-04.md b/agents/darkwing/work/slice1-sr-review-r2-2026-10-04.md new file mode 100644 index 00000000..2a9f5215 --- /dev/null +++ b/agents/darkwing/work/slice1-sr-review-r2-2026-10-04.md @@ -0,0 +1,57 @@ +# Queue row 35 (SR), round 2 review (#1517) + +Darkwing, 2026-10-04. Request: #1517 comment 26710. Candidate: commit +104cf4f3, `docs/guides/slice-1-identities.md`. Round 1 record: +`agents/darkwing/work/slice1-sr-review-r1-2026-10-04.md`. + +Verdict: changes. One defect, a one-line reorder. Every round 1 item is +in, and I checked each against `git diff c9c1699a 104cf4f3`. The scope +tables are unchanged and still right. + +## The defect + +R1. The Vikunja rotation removes the owner header before the step that +needs it (lines 295-299). It says to finish with section 4's `rm -f`, +"Then revoke the old token" with `api -X DELETE`. `api` sends +`-H @"$S/vikunja-owner.hdr"`, and that file is gone by then. curl stops +with "option -H: error encountered when reading a file" and exit 26 (I +ran it with curl 8.22), so the revoke fails on every rotation. Jason +would see the error, but the step as written can't work, and the +leftover old token stays live until he works out why. Revoke first, +then remove the header: + +> Mint a new token for the same bot, update `expires` in the business +> file, and restart the broker. Then revoke the old token with +> `api -X DELETE "$VK/api/v2/tokens/"`, and finish with +> section 4's `rm -f`. + +## Optional, not blocking + +1. Rotation writes over the live token file (line 245). The shell + truncates `$S/$r-vikunja.token` before `jq` runs, so a mint response + without a `token` field empties the file the broker was using. The + broker then refuses to start, which fails closed, but the old value + is lost from disk. Writing to `"$S/.$r-vikunja.token.new"` and + renaming it with `mv` only after `jq -je` succeeds avoids that. +2. The Gitea rotation points at the section 1 loop, which prompts for + all four roles. When rotating one, say to run the loop body once with + `r` set to that role. +3. `api` (line 190) still uses `curl -sf`, so a failed bot, share or + delete call prints nothing. The `jq -c` line then prints nothing + either, and a missing line is the only sign. A sentence saying "each + call prints one line; a missing line means it failed" would do. +4. The business file example (line 283) has `"botId": 0`. Row S1's + validator refuses 0, because a bot id is a positive integer, so a + pasted placeholder can't pass by accident. A comment there saying to + put the real id would help. + +## Checked and right + +- D1 and D2: the `mint` body is mine verbatim, and the guide says the + error path prints only `code` and `message`. +- Reviewer-bot text, the approvals ruling (advisory, no allowlist), the + login text, the Gitea names `mosaic-stack--bot`, the `read -rs` + loop, `expires` as the date part of `EXP`, the Path B text on `-it` + and the two mounts, and the labels note. +- Token file names from the loop (`$S/-gitea.token`) match the + business file example and section 4's count of nine.