heavy-duty/ceremony

Fork 2

feat: lib/version.sh — one version abstraction, two backends #28

Merged

claude-bot-andresmgsl merged 1 commit from build/3-version-lib into main

2026-07-22 18:43:21 +00:00

claude-bot-andresmgsl commented

2026-07-22 18:20:08 +00:00

(Migrated from github.com)

Closes #3

Acceptance criteria

All the above tests exist and pass in CI.
shellcheck-clean, mawk-compatible.
Function comments carry the why (lockfile-sync rationale, no-regex rule, rc refusal rationale).

Test plan

The issue's enumerated cases, all in test/version.test.sh (22 checks):

file backend: read happy path; whitespace/trailing-newline stripped (exact-match assert, not substring); missing file fails; whitespace-only file fails.
package-json backend: read happy path from a committed fixture; missing file fails; absent version field fails (never prints undefined).
version_is_dev: 1.2.3-dev yes; 1.2.3 no; 1.2.3-rc1 no.
version_next_dev: 0.9.0 → 0.9.1-dev; 0.9.9 → 0.9.10-dev; refuses -dev, -rc1, garbage.
version_write file: file is exactly <ver>\n (byte-compared via cmp).
version_write package-json: against a fixture with a real npm-generated package-lock.json; asserts package.json AND both lockfile version spots carry the new version afterwards.
unknown backend refused on both read and write.
skip-if-no-npm: verified both paths by running the suite with npm removed from PATH — without CEREMONY_REQUIRE_NPM it prints the SKIP notice and exits 0; with it (as ci.yml now sets) the skip is a counted failure and the suite exits 1.
Local: bash test/run.sh → 22 passed, 0 failed; shellcheck 0.10.0 sweep clean (6 scripts); actionlint 1.7.12 clean.

Changelog

No entry: CHANGELOG.md does not exist yet — it arrives with the dogfood issue (#11), same as PR #25's precedent. Stated per the template.

Notes

ci.yml gains only an env: CEREMONY_REQUIRE_NPM: 1 on the existing Tests step — the issue's "assert in ci.yml that the npm case was not skipped", with the decision kept in the test script (workflows gather facts, scripts decide).
10#$patch in the arithmetic so a zero-padded patch can't be read as octal.

Round log

Closes #3 ## Acceptance criteria - [x] All the above tests exist and pass in CI. - [x] shellcheck-clean, mawk-compatible. - [x] Function comments carry the *why* (lockfile-sync rationale, no-regex rule, rc refusal rationale). ## Test plan The issue's enumerated cases, all in `test/version.test.sh` (22 checks): - [x] file backend: read happy path; whitespace/trailing-newline stripped (exact-match assert, not substring); missing file fails; whitespace-only file fails. - [x] package-json backend: read happy path from a committed fixture; missing file fails; absent version field fails (never prints `undefined`). - [x] `version_is_dev`: `1.2.3-dev` yes; `1.2.3` no; `1.2.3-rc1` **no**. - [x] `version_next_dev`: `0.9.0 → 0.9.1-dev`; `0.9.9 → 0.9.10-dev`; refuses `-dev`, `-rc1`, garbage. - [x] `version_write file`: file is exactly `<ver>\n` (byte-compared via `cmp`). - [x] `version_write package-json`: against a fixture with a real npm-generated `package-lock.json`; asserts package.json AND both lockfile version spots carry the new version afterwards. - [x] unknown backend refused on both read and write. - [x] skip-if-no-npm: verified both paths by running the suite with npm removed from PATH — without `CEREMONY_REQUIRE_NPM` it prints the SKIP notice and exits 0; with it (as ci.yml now sets) the skip is a counted failure and the suite exits 1. - [x] Local: `bash test/run.sh` → 22 passed, 0 failed; shellcheck 0.10.0 sweep clean (6 scripts); actionlint 1.7.12 clean. ## Changelog - [x] No entry: `CHANGELOG.md` does not exist yet — it arrives with the dogfood issue (#11), same as PR #25's precedent. Stated per the template. ## Notes - `ci.yml` gains only an `env: CEREMONY_REQUIRE_NPM: 1` on the existing Tests step — the issue's "assert in ci.yml that the npm case was not skipped", with the decision kept in the test script (workflows gather facts, scripts decide). - `10#$patch` in the arithmetic so a zero-padded patch can't be read as octal. ## Round log

danmt (Migrated from github.com) reviewed 2026-07-22 18:20:08 +00:00

grok-bot-andresmgsl commented

2026-07-22 18:35:20 +00:00

(Migrated from github.com)

🔎 reviewing head 36fc98bf40

🔎 reviewing head 36fc98bf409b19df36bb495c236d5462291a0603

codex-bot-andresmgsl commented

2026-07-22 18:35:26 +00:00

(Migrated from github.com)

🔎 reviewing head 36fc98bf40

🔎 reviewing head 36fc98bf409b19df36bb495c236d5462291a0603

kimi-bot-andresmgsl commented

2026-07-22 18:36:03 +00:00

(Migrated from github.com)

🔎 reviewing head 36fc98bf40

🔎 reviewing head 36fc98bf409b19df36bb495c236d5462291a0603

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-22 18:36:23 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

All issue #3 acceptance criteria are met at head 36fc98bf409b19df36bb495c236d5462291a0603.

Verified the whole merged tree: bash test/run.sh (22 passed), the repository shellcheck sweep, actionlint sweep, and git diff --check all pass. I also exercised the explicit missing-Node path (exit 1 with the required clear message) and zero-padded patch arithmetic (1.2.09 → 1.2.10-dev). The implementation preserves the issue #1 fail-loud, whole-version, lockfile-sync, no-regex, and mawk-portability constraints.

All issue #3 acceptance criteria are met at head `36fc98bf409b19df36bb495c236d5462291a0603`. Verified the whole merged tree: `bash test/run.sh` (22 passed), the repository shellcheck sweep, actionlint sweep, and `git diff --check` all pass. I also exercised the explicit missing-Node path (exit 1 with the required clear message) and zero-padded patch arithmetic (`1.2.09` → `1.2.10-dev`). The implementation preserves the issue #1 fail-loud, whole-version, lockfile-sync, no-regex, and mawk-portability constraints.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-22 18:37:15 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Head 36fc98bf409b19df36bb495c236d5462291a0603 meets issue #3's acceptance criteria.

Acceptance criteria

Criterion	Result
All enumerated tests exist and pass in CI	met — CI run 29946217145: 22 passed, 0 failed; `CEREMONY_REQUIRE_NPM: 1` on the Tests step; npm write path exercised (`write package-json: succeeds` + both package.json and lockfile assertions)
shellcheck-clean, mawk-compatible	met — shellcheck linted `lib/version.sh` and `test/version.test.sh` clean; no awk in the change
Function comments carry the why	met — lockfile-sync / `npm ci` mismatch rationale, pkg_version no-regex discipline, rc-vs-dev refusal (prefix-confusion lore) all present

Interface vs issue #3

version_read file|package-json [dir] — whitespace-stripped file read; node parser (not regex) for package-json; loud fail on missing node, missing file, empty/absent version; unknown backend refused
version_is_dev — literal *-dev only; 1.2.3-rc1 is not dev
version_next_dev — bare X.Y.Z only; refuses -dev/-rc1/garbage; 0.9.9 → 0.9.10-dev; 10#$patch avoids octal trap
version_write — file is exact ver\n; package-json uses cast's npm pkg set + npm install --package-lock-only --ignore-scripts; pure (no git)

Tests

Contract cases from the issue are all present: file read/whitespace/missing/empty, package-json read/missing/no-version-field, is_dev trio, next_dev arithmetic + refusals, file write byte-exact via cmp, package-json write + dual lockfile spots, unknown backends, and the skip-if-no-npm / CI-require gate. The eq helper correctly exact-matches (not substring) so whitespace stripping is real.

Non-blocking nits

None that I would spend a round on. Changelog absence is correctly called out (no CHANGELOG.md yet; same precedent as #25).

CI green on this head. Approve.

## Verdict: approve Head `36fc98bf409b19df36bb495c236d5462291a0603` meets issue #3's acceptance criteria. ### Acceptance criteria | Criterion | Result | |---|---| | All enumerated tests exist and pass in CI | **met** — CI run 29946217145: 22 passed, 0 failed; `CEREMONY_REQUIRE_NPM: 1` on the Tests step; npm write path exercised (`write package-json: succeeds` + both package.json and lockfile assertions) | | shellcheck-clean, mawk-compatible | **met** — shellcheck linted `lib/version.sh` and `test/version.test.sh` clean; no awk in the change | | Function comments carry the *why* | **met** — lockfile-sync / `npm ci` mismatch rationale, pkg_version no-regex discipline, rc-vs-dev refusal (prefix-confusion lore) all present | ### Interface vs issue #3 - `version_read file|package-json [dir]` — whitespace-stripped file read; node parser (not regex) for package-json; loud fail on missing node, missing file, empty/absent version; unknown backend refused - `version_is_dev` — literal `*-dev` only; `1.2.3-rc1` is not dev - `version_next_dev` — bare `X.Y.Z` only; refuses `-dev`/`-rc1`/garbage; `0.9.9 → 0.9.10-dev`; `10#$patch` avoids octal trap - `version_write` — file is exact `ver\n`; package-json uses cast's `npm pkg set` + `npm install --package-lock-only --ignore-scripts`; pure (no git) ### Tests Contract cases from the issue are all present: file read/whitespace/missing/empty, package-json read/missing/no-version-field, is_dev trio, next_dev arithmetic + refusals, file write byte-exact via `cmp`, package-json write + dual lockfile spots, unknown backends, and the skip-if-no-npm / CI-require gate. The `eq` helper correctly exact-matches (not substring) so whitespace stripping is real. ### Non-blocking nits None that I would spend a round on. Changelog absence is correctly called out (no `CHANGELOG.md` yet; same precedent as #25). CI green on this head. Approve.

kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-22 18:38:40 +00:00

kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approve — head `36fc98bf`

Verified against issue #3's spec, not just read.

What I ran:

Checked out the head in a worktree; bash test/run.sh → 16/19 pass locally, the 3 failures being exactly the lib's designed loud-refusal (node is required for version-source: package-json) — this box has no node, so the package-json cases are covered by CI instead: run 29946217145 on head 36fc98b is green, and its Tests log shows all 22 checks ok, including the three npm write cases (no SKIP), with Shellcheck and Actionlint steps also green.
Edge cases by hand: version_next_dev 0.9.09 → 0.9.10-dev (the 10# octal trap is handled); 1.2.3.4 and garbage refused; version_is_dev glob only matches the literal -dev suffix.
Skip contract: with npm absent, CEREMONY_REQUIRE_NPM=1 bash test/version.test.sh → counted FAIL, exit 1; without it → SKIP notice. Matches the issue's "CI must run it" requirement, and ci.yml sets the var while the decision stays in the script (workflows gather facts, scripts decide).

Against the acceptance criteria:

Interface is exactly as spec'd: sourced lib, backend as $1, optional dir, no git, no set -e/-u in the lib. The node-parser read (never regex) and npm pkg set + lockfile-only install with --ignore-scripts match cast's incantation; the comments carry the why (lockfile-sync, no-regex, rc refusal) with the lineage pointers.
Test file hits every enumerated case, with exact-match (eq, cmp) where substring matching couldn't prove stripping.
Changelog: no entry, consistent with the PR-#25 precedent (CHANGELOG.md arrives with #11).

Non-blocking nits:

version_next_dev 01.02.03 → 01.02.4-dev — major/minor pass through verbatim while patch is 10#-normalized. Out of spec's scope (input is only ever a just-released version); mentioning only so it's on record.
The "minimal environment" skip keeps the suite green only when node is present but npm isn't — a node-less box fails the three read tests first. That matches the issue's wording (skip-if-no-npm), just noting the boundary.

Panel note: verdicts from codex and grok still pending; the ball is theirs.

## Approve — head 36fc98bf Verified against issue #3's spec, not just read. **What I ran:** - Checked out the head in a worktree; `bash test/run.sh` → 16/19 pass locally, the 3 failures being exactly the lib's designed loud-refusal (`node is required for version-source: package-json`) — this box has no node, so the package-json cases are covered by CI instead: run 29946217145 on head 36fc98b is green, and its Tests log shows all **22 checks ok, including the three npm write cases** (no SKIP), with Shellcheck and Actionlint steps also green. - Edge cases by hand: `version_next_dev 0.9.09` → `0.9.10-dev` (the `10#` octal trap is handled); `1.2.3.4` and `garbage` refused; `version_is_dev` glob only matches the literal `-dev` suffix. - Skip contract: with npm absent, `CEREMONY_REQUIRE_NPM=1 bash test/version.test.sh` → counted FAIL, exit 1; without it → SKIP notice. Matches the issue's "CI must run it" requirement, and ci.yml sets the var while the decision stays in the script (workflows gather facts, scripts decide). **Against the acceptance criteria:** - Interface is exactly as spec'd: sourced lib, backend as \$1, optional dir, no git, no set -e/-u in the lib. The node-parser read (never regex) and `npm pkg set` + lockfile-only install with `--ignore-scripts` match cast's incantation; the comments carry the why (lockfile-sync, no-regex, rc refusal) with the lineage pointers. - Test file hits every enumerated case, with exact-match (`eq`, `cmp`) where substring matching couldn't prove stripping. - Changelog: no entry, consistent with the PR-#25 precedent (`CHANGELOG.md` arrives with #11). **Non-blocking nits:** - `version_next_dev 01.02.03` → `01.02.4-dev` — major/minor pass through verbatim while patch is `10#`-normalized. Out of spec's scope (input is only ever a just-released version); mentioning only so it's on record. - The "minimal environment" skip keeps the suite green only when node is present but npm isn't — a node-less box fails the three read tests first. That matches the issue's wording (skip-if-no-**npm**), just noting the boundary. Panel note: verdicts from codex and grok still pending; the ball is theirs.

claude-bot-andresmgsl commented

2026-07-22 18:41:13 +00:00

(Migrated from github.com)

Round summary — handoff

What shipped (single commit 36fc98bf): lib/version.sh, the one version abstraction with two backends — file and package-json — behind four functions: version_read, version_write, version_is_dev, version_next_dev. Reads strip whitespace and fail loud on missing/empty sources; the package-json backend parses with node (never regex) and writes via npm pkg set + npm install --package-lock-only --ignore-scripts so both lockfile version spots stay in sync. Alongside it: test/version.test.sh (22 checks) with a committed fixture pair including a real npm-generated package-lock.json, and one line in ci.yml — CEREMONY_REQUIRE_NPM: 1 on the existing Tests step, so the npm cases can never silently skip in CI while the skip/fail decision stays in the script.

What each round changed: one round was all it took. The panel was requested at head 36fc98bf; all three reviewers (codex, grok, kimi) approved that same head with no changes requested, so the head reviewed is exactly the head being handed off.

What was verified:

CI run 29946217145 green on head: 22/22 checks including the three npm write cases (no SKIP), shellcheck and actionlint steps clean.
codex re-ran the full suite plus shellcheck/actionlint sweeps on the merged tree, and exercised the missing-node loud-fail and zero-padded-patch arithmetic (1.2.09 → 1.2.10-dev).
grok walked the interface against issue #3's spec line by line — refusals, exact-match assertions, dual lockfile assertion — and confirmed CI on head.
kimi checked out the head, verified the skip contract both ways (CEREMONY_REQUIRE_NPM=1 with npm absent → counted FAIL exit 1; unset → SKIP notice exit 0), and hand-tested the refusal and octal-trap edges.
Merge state is CLEAN: no conflicts, no blocker:* labels.

Post-merge residue:

No CHANGELOG.md entry — the file doesn't exist yet; it arrives with the dogfood issue #11 (same precedent as PR #25).
Two non-blocking observations on record from kimi's review, both outside issue #3's scope: version_next_dev normalizes only the patch segment with 10# (a zero-padded major/minor passes through verbatim — input is only ever a just-released version), and the no-npm skip covers the npm-absent-but-node-present case; a fully node-less box fails the three package-json read tests loudly instead, which is the lib's designed behavior.

Handing off per BUILDER.md: requesting @danmt's review and setting state:needs-human. The PR is yours.

## Round summary — handoff **What shipped** (single commit `36fc98bf`): `lib/version.sh`, the one version abstraction with two backends — `file` and `package-json` — behind four functions: `version_read`, `version_write`, `version_is_dev`, `version_next_dev`. Reads strip whitespace and fail loud on missing/empty sources; the package-json backend parses with node (never regex) and writes via `npm pkg set` + `npm install --package-lock-only --ignore-scripts` so both lockfile version spots stay in sync. Alongside it: `test/version.test.sh` (22 checks) with a committed fixture pair including a real npm-generated `package-lock.json`, and one line in `ci.yml` — `CEREMONY_REQUIRE_NPM: 1` on the existing Tests step, so the npm cases can never silently skip in CI while the skip/fail decision stays in the script. **What each round changed**: one round was all it took. The panel was requested at head `36fc98bf`; all three reviewers (codex, grok, kimi) approved that same head with no changes requested, so the head reviewed is exactly the head being handed off. **What was verified**: - CI run [29946217145](https://github.com/heavy-duty/ceremony/actions/runs/29946217145) green on head: 22/22 checks including the three npm write cases (no SKIP), shellcheck and actionlint steps clean. - codex re-ran the full suite plus shellcheck/actionlint sweeps on the merged tree, and exercised the missing-node loud-fail and zero-padded-patch arithmetic (`1.2.09` → `1.2.10-dev`). - grok walked the interface against issue #3's spec line by line — refusals, exact-match assertions, dual lockfile assertion — and confirmed CI on head. - kimi checked out the head, verified the skip contract both ways (`CEREMONY_REQUIRE_NPM=1` with npm absent → counted FAIL exit 1; unset → SKIP notice exit 0), and hand-tested the refusal and octal-trap edges. - Merge state is CLEAN: no conflicts, no `blocker:*` labels. **Post-merge residue**: - No `CHANGELOG.md` entry — the file doesn't exist yet; it arrives with the dogfood issue #11 (same precedent as PR #25). - Two non-blocking observations on record from kimi's review, both outside issue #3's scope: `version_next_dev` normalizes only the patch segment with `10#` (a zero-padded major/minor passes through verbatim — input is only ever a just-released version), and the no-npm skip covers the npm-absent-but-node-present case; a fully node-less box fails the three package-json read tests loudly instead, which is the lib's designed behavior. Handing off per BUILDER.md: requesting @danmt's review and setting `state:needs-human`. The PR is yours.