Skip to content

docs+harness: M14-02 marker field, the ui rule's confirm step, M14-03 notes, CharBindsReport fields, M13-70, M13-68 run 2 - #289

Merged
sandgraal merged 8 commits into
mainfrom
docs/m14-02-notes
Oct 19, 2026
Merged

sandgraal merged 8 commits into
mainfrom
docs/m14-02-notes

Conversation

@sandgraal

@sandgraal sandgraal commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Ticket

Conductor docs and harness change under the ADR-0013 lane (nothing under lab/*/src/): corrections and notes from the M14-02, M14-05 and M14-06 reviews, one ticket, and the M13-68 measurements.

Acceptance — with proof

  • docs/LAB_PLAN.md §16.1 and the M14-02 ticket: the doctor plan marker reads first_run.chosen, not first_run.flavor (dated correction; the merged code uses chosen).
  • .claude/rules/lab-load-bearing.md, ui bullet: every --yes command is refused except through the ADR-0029 confirm step (M14-02, feat(ui): the confirm flow on snap gc and doctor --yes (M14-02) #288); the leaves not yet in the step stay refused.
  • .claude/agents/pr-shepherd.md: a note that gh api graphql with $-variables is refused under worktree isolation (inline the values or -F query=@file), and that REST POSTs are blocked.
  • docs/LAB_PLAN.md §16.2: dated amendment with the final CharBindsReport fields (M14-06, feat(char): wowlab_core.keymap and wowlab char binds (M14-06) #287).
  • M14-03 ticket: dated notes from the M14-02 reviews (the _CONFIRM_READY seam, --expect needs the report, the decline shape exits 1, the 120 s run limit versus a guard transaction).
  • New ticket M13-70 (labaddon.ActionBars admits duplicate and out-of-range slots), found 2026-10-19 in the code review of feat(char): wowlab_core.keymap and wowlab char binds (M14-06) #287, with size, dependency, provenance, checkable acceptance and a reviewer line.
  • M13-68 run 2 (lab/): feat(char): wowlab char gear, and the lossless item-link parser (M14-05) #286 M14-05 at 9f79ef9 hit the 30-turn cap (36 turns, failure, no verdict; 137 KB diff); feat(ui): the confirm flow on snap gc and doctor --yes (M14-02) #288 M14-02 at d73111e CLEAN in 2 min 17 s. The raise waits for a third lab/ run.
  • .claude/hooks/guard_bash.py reads a query=@<file> and applies the mutation allowlist to it; harness test added; make hooks-test 185 passed on 3.12 and 3.9
  • scripts/check_frontmatter.py 0 problems; the frontier hook parses M13-70.
  • luadata.py, guard.py or another parser changed? n/a
  • New external-format parsing? n/a
  • New runtime dependency? none
  • User-facing surface: n/a (docs and harness prose)

Reviewed by: code-reviewer CLEAN (findings fixed in 70f6868, c52b308 and 69a3a8a).

session: 284d1153-c891-4b84-8e47-150842fac4ba

🤖 Generated with Claude Code

sandgraal and others added 6 commits October 19, 2026 00:22
…e names the confirm step; shepherd note on graphql variables (M14-02 notes)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rs admits duplicate slots)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hit); #288 M14-02 CLEAN within the cap

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n inline query; #289 review fixes

The shell guard reads a query passed as -F/-f query=@<file> and applies
the mutation allowlist to its text, refusing a file it cannot read, so
the file route the shepherd definition now recommends cannot carry a
mutation past the guard (harness test added, 183 pass on 3.12 and 3.9).
The shepherd note says why the inline form is refused and gives the
file form, and the thread-query example uses it. The M14-03 note names
#288 and says confirmed() takes nothing from the report and a killed run
gets no in-process rollback; the M13-68 run-2 note separates job wall
time from the model's duration and records the diff size; M13-70 cites
main's lines and puts the decision in the report; the ui rule says what
the confirmed argv is and where the token travels.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sandgraal
sandgraal marked this pull request as ready for review October 19, 2026 06:40
@claude

claude Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Claude finished @sandgraal's task in 1m 10s —— View job


Independent review: PR #289

One finding, low, in the shell guard's new query-file check. The docs and rule changes match the code on main. CI was still running when I finished, so this verdict does not cover it.

  • Read AGENTS.md and wrote down what must hold. This PR is docs and harness only; nothing under lab/*/src/ changes.
  • Read .pr-review/pr.diff in full: 6 files, +78/−4, not truncated.
  • Checked the claims against the code.
  • Checked CI.
  • Posted findings and this summary.

What should be true

  • Nothing under lab/*/src/ changes. This is the ADR-0013 conductor lane: docs, harness and tests.
  • L1–L8 are untouched. There is no fixture, parser, guard.py or luadata.py change.
  • Every correction in the docs matches the code on main.
  • The guard change only makes the hook stricter, never looser.

What I checked

  • docs/LAB_PLAN.md §16.1 and the M14-02 ticket (first_run.chosen): matches the code. ui/__init__.py:626-629 reads first.get("chosen"), and the module docstring at line 40 says first_run.chosen.
  • .claude/rules/lab-load-bearing.md, the ui bullet: the diff and .claude-pr/ agree.
    • The rule says --yes goes after --json and before any argument. That matches _Argv.confirmed() at ui/__init__.py:421-426 ([*head, *json, --yes, *tail]).
    • _CONFIRM_READY at line 146 holds snap gc and doctor, so the rule's "leaves not yet in the step stay refused" is accurate.
    • ui itself, looks page --out and log tail --follow are still always refused.
  • M14-03 note: the names it cites are real. _CONFIRM_READY, confirmed() at line 421 and report() at line 586 all exist.
  • M13-70: the anchors are accurate. class ActionBars is at labaddon.py:1176 (cited as ~1174), and the keep-the-last dict is at bindscoach.py:536. The ticket has a size, dependencies, provenance, checkable acceptance and a reviewer line.
  • §16.2 CharBindsReport amendment: it is dated and additive prose. I did not check it field by field against feat(char): wowlab_core.keymap and wowlab char binds (M14-06) #287's code; feat(char): wowlab_core.keymap and wowlab char binds (M14-06) #287 is not on this branch.
  • M13-68 run 2: the backlog gives run URLs, turn counts and outcomes.
  • pr-shepherd.md: I read the PR's copy from .claude-pr/. The -F query=@file recipe is correct: gh's -F reads @file and -f does not.
  • guard_bash.py: I compared it with .claude-pr/.claude/hooks/guard_bash.py. The change is only the 20-line block. It fails closed on an unreadable file, -, or ~. It treats -f query=@x as a file too, which is harmless because it only refuses more.
  • Test: test_guard_bash_reads_a_graphql_query_file_for_mutations covers a blocked mutation, an allowed mutation, a plain query and a missing file.
  • L1–L8, fixtures, secrets, workflows: nothing in the diff touches them. There are no identifiers or secrets.
  • User-facing surface: the PR body says n/a, which is correct; the diff has no CLI, --json output, page or addon change.
  • CI: the CI run (39211096715) was in_progress when I checked, so I could not confirm make ci and make hooks-test green for this commit. The PR body reports hooks-test 183 passed.

Finding

  1. Low (guard_bash.py:152): the query file is read from the wrong place, and before the command runs.

verdict: FINDINGS(1)

Comment thread .claude/hooks/guard_bash.py Outdated
sandgraal and others added 2 commits October 19, 2026 00:45
…d cwd, and a file the same command line writes is refused

From the automated review of #289: _check_gh now takes the cwd the
inspection tracks through cd, so a relative query file is read where
the shell will read it; and every write target of the command line is
collected first, so a query file written earlier in the same line (the
hook reads the file before the shell runs) is refused with the advice
to write it in its own command. Two harness tests added; 184 pass on
3.12 and 3.9.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hecks a graphql query file

From the #289 re-check: _inspect passes its collected write set into the
recursive call for bash -c / sh -c, so a query file written on the outer
line is refused inside the nested shell too. One harness test added;
185 pass on 3.12 and 3.9.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sandgraal
sandgraal merged commit 7415e07 into main Oct 19, 2026
12 checks passed
@sandgraal
sandgraal deleted the docs/m14-02-notes branch October 19, 2026 10:22
sandgraal added a commit that referenced this pull request Oct 19, 2026
…-72; M14-09 pointers (#293)

## What

Conductor docs lane (ADR-0013 amendment, 2026-10-17): `docs/BACKLOG.md`
only.

- Ticks M14-02 (#288), M14-05 (#286), M14-06 (#287), M14-08 (#290).
- M14-02: a dated note on the Windows stdin follow-up (#291, 826ce3d):
why `DEVNULL` made the child's `isatty()` True on Windows, what changed,
and that Windows on `main` at 826ce3d is green.
- M14-08: a dated note from the #290 reviews (shipped-table colours vs
the ticket's "from the model" wording, which is an owner item; the
`aria-live` region; the `page.css` exemption in the colour grep test).
- M14-09: pointers from the #290 code review (the `loadGear` refactor,
the intro paragraph and allowlist test that change, the default-flavor
question, the `aria-live` narrowing).
- New tickets: M13-71 (`guard_bash.py`: `--input` bodies and the
substring allowlist match, from the #289 review; conductor lane) and
M13-72 (`char list --json` carries every full record under the Character
tab, and the colour grep's `page.css` exemption; from the #290 code
review, off that branch).

## Proof

- `grep -c '^## \[' docs/BACKLOG.md` → 87 headings; the four ticks are
the only `[ ]` → `[x]` changes.
- Pre-commit hooks green on the commit (merge-conflict check,
end-of-file).
- Rebased on `main` at 7415e07 (#289); the one conflict (M13-70 and
M13-71 both appended after M13-69) was resolved by keeping both.

## Reviewed by

`code-reviewer` CLEAN at cec1ea3 (every claim checked against main and
the CI history; the one nit, a blank line before M13-72, fixed in the
follow-up commit).

session: 284d1153-c891-4b84-8e47-150842fac4ba

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant