Skip to content

Make every sg_report call outside its grammar a caller bug (issue #211) - #233

Merged
tkphd merged 2 commits into
mainfrom
fix/211-sg-report-validation
Oct 9, 2026
Merged

tkphd merged 2 commits into
mainfrom
fix/211-sg-report-validation

Conversation

@tkphd

@tkphd tkphd commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Ruling

Trevor decided issue #211 on 2026-10-08: each case below is a caller bug, recorded at user.warning as ADR-0033 (amended by issue #189) sets for every caller-bug record. A call with fewer than two arguments writes a caller-bug record and does not abort under set -u.

What changed

  • node/shim/install.sh: sg_report writes caller-bug for

    • a long-form state other than expensive, covered or unmounted;
    • a long-form reasserted marker after any state but expensive. The short form's marker is unchanged: coverage_change unwrapped-N reasserted stays valid;
    • six or more arguments, which it used to read as the short form, ignoring the extra arguments;
    • fewer than two arguments. $1 and $2 are read as ${1-} and ${2-}, so no path reads an unset one, and the existing character check refuses the empty action or state.

    These follow PR Make long-form misuse of sg_report a caller bug, and every caller bug a warning (issue #189) #215's path: _rep_bug=1, then the existing reset to the caller-bug record at warning. The header comment lists the new cases. sg_report is the reconcile's sink in install.sh, not the guard's audit path. The checks are case and [ only, so it gains no fork.

  • docs/operating.md: the journal section now describes the caller-bug record: what it means, its priority, and that it is a defect to report.

  • tests/test_install.py, under dash and bash:

    • each of the four cases, with several variants, writes exactly one caller-bug record at warning, and the call returns 0 under set -eu;
    • a marked short-form coverage_change keeps its record and priority. The existing tests already pin the valid short and long forms, unchanged.
  • examples/payload rebuilt.

No caller changes. ADR-0030, ADR-0031 and ADR-0033 stay true as written: none says the long form accepts any other state, a marker after another state, or another argument count. So no ADR amendment is needed.

Tested

  • Full suite: 3606 passed.
  • Mutations: against main's install.sh, all 24 new grammar cases fail. Dropping the count check fails 6 (the six-or-more cases). Dropping the state checks fails 12.
  • walk-blocker build --check, shellcheck on the payload shims, dash -n, pymarkdown, pyflakes on deploy.py, and the IP gate (--require-terms, 0 findings) all pass.

Closes #211

🤖 Generated with Claude Code

tkphd and others added 2 commits October 9, 2026 00:26
Trevor decided issue #211 on 2026-10-08: each of these is a caller bug,
recorded at user.warning per ADR-0033 as amended by issue #189:

- a long-form state outside expensive|covered|unmounted;
- a reasserted marker after any state but expensive;
- six or more arguments, no longer read as the short form;
- fewer than two arguments, which now write a caller-bug record instead
  of aborting the caller under set -u.

No caller makes any of these calls; none changes. docs/operating.md now
describes the caller-bug record. examples/payload rebuilt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
auto-review round 1 on PR #233 found two things:

- The sg_report header and docs/operating.md said a marker after any
  state but `expensive` is a caller bug. That holds only for the long
  form: `coverage_change unwrapped-N reasserted` is valid and is
  re-asserted (ADR-0031). Both now say so.
- `[ $# -lt 2 ]` changed no outcome: a missing action or state reads as
  empty through `${1-}`/`${2-}`, and the character check already refuses
  an empty one. Removing it passed every test. It is gone, and the
  comment names the check that does the work. The fewer-than-two tests
  remain the oracle: reading `$1` or `$2` bare fails them.

examples/payload rebuilt. Full suite: 3606 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tkphd

tkphd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

auto-review round 1

Reviewed PR #233 at 7c4b3e9 against its body (issue #211's four caller-bug cases, the header comment, operating.md, tests).
Probe engine: claude (sonnet), session verdict forced; health check n/a; prose-truth → claude, test-oracle → claude.

Confirmed and fixed (in scope)

  • node/shim/install.sh header, docs/operating.md: both said a marker after any state but expensive is a caller bug. That is true only of the long form. coverage_change unwrapped-1 reasserted is valid (ADR-0031), and the PR's own test pins it. Both now scope the rule to uncovered_mount's long form. c2d1d3e
  • node/shim/install.sh [ $# -lt 2 ]: redundant. ${1-}/${2-} read empty, and the character check already refuses an empty action or state, so removing the guard passed every test. The guard is gone, and the comment names the check that does the work. The fewer-than-two tests remain its oracle: reading $1/$2 bare fails 6 of them. c2d1d3e

Filed, not fixed (out of scope)

Refuted

  • none

Unverified

  • none

Architect ruling

  • not consulted: both findings are unambiguously in scope, and each fix is minimal (prose scoping, plus removing a guard that changes no outcome).

3 findings, 3 confirmed, 0 refuted. Every isolated revert of the remaining guards fails the new tests (probe-verified). Valid forms are byte-identical to main under dash and bash (20 lines compared).

@tkphd

tkphd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

auto-review round 2

Reviewed PR #233 at c2d1d3e, round 1's fix first: the header comment, the operating.md paragraph, the PR body, the commit messages, and the payload.
Probe engine: claude (sonnet), session verdict forced; health check n/a; prose-truth → claude.

Confirmed and fixed (in scope)

  • PR body, "What changed": still said a marker after any state but expensive is a caller bug, without the long-form scoping that c2d1d3e gave the header and operating.md. Body edited: it now scopes the rule to the long form, says the short form's marker is unchanged, and names the character check as what refuses a missing action or state. No code change.

Filed, not fixed (out of scope)

  • none

Refuted

  • none

Unverified

  • none

Architect ruling

  • not consulted: a body-text correction, in scope.

1 finding, 1 confirmed, 0 refuted. The probe confirmed four things under dash and bash: every header sentence matches the code; operating.md lists exactly the paths that set _rep_bug; the round 1 comment and the c2d1d3e message are true; and examples/payload equals a fresh build.

@tkphd

tkphd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

auto-review round 3

Reviewed PR #233 at c2d1d3e (code unchanged since round 2; the body was edited) on a dimension not yet spent: semantics against ADR-0030, ADR-0031, ADR-0033 and the issue #211 decision, plus the edited body.
Probe engine: claude (sonnet), session verdict forced; health check n/a; semantics → claude.

Confirmed and fixed (in scope)

  • none

Filed, not fixed (out of scope)

  • none

Refuted

  • none

Unverified

  • none

Architect ruling

  • not consulted: nothing to rule on.

Round 3 found nothing. The probe ran a matrix of 33 calls under dash and bash with set -eu; the outputs were identical in both shells and every call returned 0. Valid forms kept their records and priorities: unknown at err, unwrapped-N at warning, uncovered_mount at notice. Every call outside the grammar logged caller-bug at warning, including 6+ argument coverage_change unknown, which never reaches the err raise. Every real caller fits the grammar, and the body is true.

@tkphd

tkphd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

auto-review round 4

Confirming round. Reviewed PR #233 at c2d1d3e, unchanged since round 3, on a dimension not yet spent: grammar/divergence between sg_report and the tests that parse its calls (the census in tests/test_install.py, the regexes in tests/test_runbook_matches_code.py), the header comment and operating.md.
Probe engine: claude (sonnet), session verdict forced; health check n/a; grammar → claude.

Confirmed and fixed (in scope)

  • none

Filed, not fixed (out of scope)

Refuted

  • none

Unverified

  • none

Architect ruling

  • not consulted: one finding, and the probe and I agree it is out of scope. No current caller is affected.

1 finding, 1 confirmed, 0 refuted, 0 in scope. Every valid shape the census pins matches what sg_report enforces, and the census and runbook tests pass unmodified at HEAD.

@tkphd

tkphd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

auto-review summary — 4 rounds

Round Dimension (engine) Findings In scope Outcome
1 prose-truth, test-oracle (claude) 3 2 fixed c2d1d3e; filed issue #234
2 prose-truth (claude) 1 1 PR body corrected
3 semantics (claude) 0 0 clean
4 grammar (claude) 1 0 filed issue #239

Fixed in this PR: the marker rule in the header comment, operating.md and the PR body is scoped to the long form (c2d1d3e plus a body edit); the redundant [ $# -lt 2 ] guard is removed (c2d1d3e).
Filed, not fixed: issue #234: a short-form uncovered_mount call is accepted, which is outside issue #211's decision. Issue #239: the call census pins a long-form call's count but not its state or marker; that is PR #215's mechanism and is untouched here.
Refuted: none.
Unverified / left open: none.
Stopped because: converged after 4. Round 3 was clean, and round 4, on the same SHA, confirmed it with no finding in scope.
Probe engines: claude (sonnet) throughout, session verdict forced; no halt.
What a human still has to decide: whether a short-form uncovered_mount call should be a caller bug (issue #234).

@tkphd
tkphd merged commit 7c2738d into main Oct 9, 2026
11 checks passed
@tkphd
tkphd deleted the fix/211-sg-report-validation branch October 9, 2026 04:57
tkphd added a commit that referenced this pull request Oct 9, 2026
Brings in PR #229 and PR #233. The only conflict was the generated
examples/payload/site.lock.json, resolved by rebuilding the payload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tkphd added a commit that referenced this pull request Oct 9, 2026
…ing (issue #234) (#241)

* Record a short-form uncovered_mount call as a caller bug at user.warning (issue #234)

As Trevor decided on issue #234, uncovered_mount takes the long form only,
in both directions. PR #215 (issue #189) made a long-form call naming any
other action a caller bug; the converse was never checked, so
`sg_report uncovered_mount expensive` wrote an uncovered_mount record with
no mount or fstype, at user.warning, a priority the action never otherwise
has. sg_report in node/shim/install.sh now writes the caller-bug record at
user.warning for a two- or three-argument uncovered_mount call, the shape
PR #233 (issue #211) gave every other misuse.

The check is one [ test; nothing forks. No caller changes: every
uncovered_mount call in install.sh already uses the long form.
docs/operating.md names the short form among the conditions that produce
a caller-bug record. ADR-0033 already says the long form is
uncovered_mount's alone and that uncovered_mount records are at notice, so
it needs no line.

Closes #234

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Name uncovered_mount's exclusion in sg_report's short-form grammar line (issue #234)

auto-review round 1 on PR #241 found that the grammar at the head of
sg_report still listed `sg_report ACTION STATE` for any action, while the
new branch makes a short-form uncovered_mount call a caller bug. Verified
by reading the comment block. Comment only; examples/payload rebuilt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* State uncovered_mount's exclusion for both short forms in sg_report's grammar (issue #234)

auto-review round 2 on PR #241 found that round 1's qualifier sat on the
two-argument grammar line only, while a three-argument
`uncovered_mount STATE reasserted` call is a caller bug too (the
`expensive-reasserted` case of the new test). The exclusion is now one
sentence under the grammar, covering both short forms. Comment only;
examples/payload rebuilt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <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.

sg_report: the long form's state and the argument count are not validated

1 participant