fix: green the `make lint` baseline on master #427

Merged

joshtronic merged 1 commit from agent/423-green-lint-baseline into master

2026-07-26 16:35:45 +00:00

igor commented

2026-07-26 02:09:25 +00:00

Collaborator

What this PR does

fix: green the make lint baseline on master

Both halves of make lint were failing on a clean master. Every agent run was spending turns re-deriving whether a warning was its own or pre-existing (two runs did exactly that on 2026-07-24, with extra git stash/re-run cycles), and any genuine new regression was invisible under the standing noise.

shellcheck

bin/test-automerge.sh:451 SC2034 -- AUTOMERGE_BLOCK_COOLDOWN_SECS is read by lib/automerge.sh, which this file sources; shellcheck cannot see across the source boundary. Narrowly-scoped disable with the reason inline, rather than deleting a line the test actually needs.
bin/tick.sh:4634 SC2015 -- [ cond ] && log ... || true rewritten as a plain if. The || true existed only to keep errexit off the false branch, which an if does not need, so the rewrite is equivalent and removes the ambiguous A && B || C shape instead of suppressing the warning.

markdownlint

Not named in the issue, but the other half of the same red baseline -- fixing only shellcheck still left make lint exiting 2. 16 MD022/MD032 violations, all in one design spec, all from the same cause: ATX headers with no blank line after them, which also pulled the lists following those headers into MD032. Blank lines inserted around headers; no prose changed.

Test plan

make lint exits 0 on this branch (was exit 2 on master)
make test passes -- 24 suites via bin/check-sync.sh
shellcheck bin/*.sh lib/*.sh clean
mdl . reports 0 violations (was 16)
Verified the two shellcheck sites are behaviour-preserving: the SC2034 line is a test fixture consumed by the sourced lib, and the SC2015 rewrite has identical semantics under set -e

Note for the reviewer

CI currently runs make test only, so lint is not gated and this baseline can rot again. I deliberately did not add a lint job in this PR -- the runner would need shellcheck/mdl installed, and a CI change that fails unattended overnight is worse than the problem it fixes. Filed separately as a follow-up.

Closes #423

## What this PR does - [x] fix: green the `make lint` baseline on master Both halves of `make lint` were failing on a clean `master`. Every agent run was spending turns re-deriving whether a warning was its own or pre-existing (two runs did exactly that on 2026-07-24, with extra `git stash`/re-run cycles), and any genuine new regression was invisible under the standing noise. ### shellcheck - `bin/test-automerge.sh:451` **SC2034** -- `AUTOMERGE_BLOCK_COOLDOWN_SECS` is read by `lib/automerge.sh`, which this file sources; shellcheck cannot see across the source boundary. Narrowly-scoped `disable` with the reason inline, rather than deleting a line the test actually needs. - `bin/tick.sh:4634` **SC2015** -- `[ cond ] && log ... || true` rewritten as a plain `if`. The `|| true` existed only to keep errexit off the false branch, which an `if` does not need, so the rewrite is equivalent and removes the ambiguous `A && B || C` shape instead of suppressing the warning. ### markdownlint Not named in the issue, but the other half of the same red baseline -- fixing only shellcheck still left `make lint` exiting 2. 16 MD022/MD032 violations, all in one design spec, all from the same cause: ATX headers with no blank line after them, which also pulled the lists following those headers into MD032. Blank lines inserted around headers; **no prose changed**. ## Test plan - [x] `make lint` exits 0 on this branch (was exit 2 on master) - [x] `make test` passes -- 24 suites via `bin/check-sync.sh` - [x] `shellcheck bin/*.sh lib/*.sh` clean - [x] `mdl .` reports 0 violations (was 16) - [x] Verified the two shellcheck sites are behaviour-preserving: the SC2034 line is a test fixture consumed by the sourced lib, and the SC2015 rewrite has identical semantics under `set -e` ## Note for the reviewer CI currently runs `make test` only, so lint is not gated and this baseline can rot again. I deliberately did **not** add a lint job in this PR -- the runner would need `shellcheck`/`mdl` installed, and a CI change that fails unattended overnight is worse than the problem it fixes. Filed separately as a follow-up. Closes #423

igor added 1 commit

2026-07-26 02:09:25 +00:00

fix: green the make lint baseline on master

Lint / check-sync (push) Successful in 5s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

5212f91462

Both halves of `make lint` were failing on a clean master, so every
agent run burned turns re-deriving "is this warning mine or
pre-existing?" and any genuine new regression was invisible under the
noise (igor#423).

shellcheck:
- bin/test-automerge.sh:451 SC2034 -- AUTOMERGE_BLOCK_COOLDOWN_SECS is
  read by lib/automerge.sh, which this file sources; shellcheck can't
  see across the source boundary. Narrowly-scoped disable with the
  reason, rather than deleting a line the test needs.
- bin/tick.sh:4634 SC2015 -- `[ cond ] && log ... || true` rewritten as
  a plain `if`. The `|| true` existed only to keep errexit off the
  false branch, which an `if` doesn't need, so this is equivalent and
  drops the ambiguous A && B || C shape rather than suppressing it.

markdownlint (not mentioned in the issue, but the other half of the
same red baseline -- fixing only shellcheck still left `make lint`
exiting 2):
- 16 MD022/MD032 violations, all in one design spec, all the same
  cause: ATX headers with no blank line after them, which also pulled
  the lists following those headers into MD032. Blank lines inserted
  around headers; no prose changed.

`make lint` now exits 0 and `make test` still passes (24 suites).

Closes #423

igor commented

2026-07-26 02:10:21 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for 5212f914: success

Lint-baseline cleanup: two narrowly-scoped shellcheck fixes plus blank lines around headers in one design spec. In scope, behavior-preserving, CI green — approving.

Findings

bin/tick.sh:4634 — the A && B || true → if rewrite is equivalent for the condition (a non-numeric SHOT_N makes [ -gt ] exit 2, which an if treats as false, not fatal under errexit). One hairline difference worth knowing: in the old form || true also swallowed a failure of log itself; in the new form a failing log propagates out of the if body and would abort under set -e. log is a write-to-stderr/file helper so this is effectively unreachable, and the rewrite is the right shape — noting it only so nobody is surprised later.
bin/test-automerge.sh:451 — disable=SC2034 with the cross-source-boundary reason inline is the correct call here; the variable is genuinely consumed by the sourced lib/automerge.sh. No test coverage was removed to appease the linter.
Spec doc — 10 headers get a blank line inserted, no prose touched. That reconciles exactly with the claimed 16 violations: 10 × MD022 (no blank below) + 6 × MD032 for the headers immediately followed by a list (Rollout order, Contents, Mechanics, Testing, Non-goals, Rollout sequence). Checklist matches the diff.

Coverage / follow-up notes

The central claim (make lint now exits 0) is not verifiable from CI, which runs make test only — so my confidence here comes from the diff plus the violation-count arithmetic above, not from a green lint run. The description is upfront about this rather than implying CI proved it, which is the honest framing.
Deliberately not adding a lint CI job is a defensible scope call (runner would need shellcheck/mdl), but it means this baseline can silently rot again next week. Worth making sure the filed follow-up actually lands, otherwise this PR is a one-shot cleanup rather than a fix.
No security surface, no workflow changes, no test deletions, diff well under budget.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `5212f914`: **success** Lint-baseline cleanup: two narrowly-scoped shellcheck fixes plus blank lines around headers in one design spec. In scope, behavior-preserving, CI green — approving. **Findings** - `bin/tick.sh:4634` — the `A && B || true` → `if` rewrite is equivalent for the condition (a non-numeric `SHOT_N` makes `[ -gt ]` exit 2, which an `if` treats as false, not fatal under errexit). One hairline difference worth knowing: in the old form `|| true` also swallowed a failure of `log` itself; in the new form a failing `log` propagates out of the `if` body and would abort under `set -e`. `log` is a write-to-stderr/file helper so this is effectively unreachable, and the rewrite is the right shape — noting it only so nobody is surprised later. - `bin/test-automerge.sh:451` — `disable=SC2034` with the cross-source-boundary reason inline is the correct call here; the variable is genuinely consumed by the sourced `lib/automerge.sh`. No test coverage was removed to appease the linter. - Spec doc — 10 headers get a blank line inserted, no prose touched. That reconciles exactly with the claimed 16 violations: 10 × MD022 (no blank below) + 6 × MD032 for the headers immediately followed by a list (Rollout order, Contents, Mechanics, Testing, Non-goals, Rollout sequence). Checklist matches the diff. **Coverage / follow-up notes** - The central claim (`make lint` now exits 0) is *not* verifiable from CI, which runs `make test` only — so my confidence here comes from the diff plus the violation-count arithmetic above, not from a green lint run. The description is upfront about this rather than implying CI proved it, which is the honest framing. - Deliberately not adding a lint CI job is a defensible scope call (runner would need `shellcheck`/`mdl`), but it means this baseline can silently rot again next week. Worth making sure the filed follow-up actually lands, otherwise this PR is a one-shot cleanup rather than a fix. - No security surface, no workflow changes, no test deletions, diff well under budget. --- <sub>Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-26 02:10:21 +00:00

41 seconds