Files

3.3 KiB

Filbert review: row 112 round 1 (#1606): changes

Candidate manifest sha256 ac34ebd094fb3bb17d1dd14b8f6e0ca53d661e176ef42463dd5f5c8ecf683d32 (5 files), as named in comment 28376. I rebuilt it in my own worktree at BASE 96656dd21 with build.patch from packet dbf72a2a3. All 5 hashes match, git diff --check is clean, no manifest path has drifted from 96656dd21 to HEAD, and the patch's added lines have no dashes, bidi or control characters.

The brief, item by item

  1. Tool text. recommendation now says exactly one option key, as written, with nothing added, and that the reason goes in question. question asks for the reason. approvalChoice names the yes default and says to pass it when no key is yes. tools.test.mjs asserts each of these.
  2. Broker. The shared line is split. Duplicate keys refuse invalid-options first, a recommendation miss then refuses invalid-recommendation, and invalid-approval-choice is unchanged. Count and per-option checks still run before both. There is no schema change.
  3. README. See C1.
  4. Tests. The new broker test covers "yes: it matches the direction", maybe and a missing recommendation (all invalid-recommendation), duplicate keys with a good and a bad recommendation (both invalid-options), and a single option (invalid-options). Decision and message counts don't change and the inbox stays empty. A plain yes is accepted and stored as yes. The existing duplicate-key case is unchanged.
  5. Mutants. I reran m1 (shared line restored) and m2 (recommendation check before the duplicate check) from mutants.py against broker.test.mjs alone. Each gave 15 pass and 1 fail on the new test. I restored the file each time and rechecked all 5 hashes.

Change requested

  • C1. The invalid-options row names the wrong code for a malformed option. The row says an option "that isn't {key,text}" refuses invalid-options. In the broker, keys(o,['key','text'],['key','text']) runs first and refuses invalid-request for that case. I ran a probe against the rebuilt broker with recommendation: 'yes':

    options refusal
    an option with an extra note field invalid-request
    an option with no text invalid-request
    a bare string 'yes' as an option invalid-request
    key Yes! invalid-options
    empty text invalid-options
    one option invalid-options

    This row exists so a caller can tell which field to fix, so it shouldn't name a code the broker doesn't return. The lead-in, "each refusal names one field", also doesn't hold for this case. Fix the README text, not the broker. For example: "fewer than 2 or more than 9 options, an option key that isn't a valid id, empty or over-long text, or duplicate keys", plus a sentence saying that an option that isn't exactly {key,text} refuses invalid-request. Widening the broker's codes is out of scope under the brief. No test is needed for a README fix.

What I ran (single files, no slot)

bus: broker 16/16, decisions 7/7, review 8/8. harness: tools 11/11. I also ran the mutant reruns and the option probe described above. I didn't rerun the full gate and relied on gate-run.log for it: 14 packages and 9 scripts all pass, GATE_PASSED. The worktree is removed and no process is left in it.

Verdict: changes (C1, README only).