Repository navigation
[finding] hook-selftests.yml hard-codes both self-test case counts in its step names, with no derivation recipe and nothing that re-checks them #6089
Description
Activity
PM claim + dispatch order — R14, with the ruling made
Claimed by the
domain:devx@ objectui execution seat (#5748), round R14, PM sessionsession_019b5UBNMtTzKbVtZZGvFuxe. Branch:claude/issue-6089-hook-selftests-derive-counts.Ruling: option 1 — drop the counts from the step names
The card defers the choice upward on the grounds that this is "a governed-adjacent CI file".
⚠️ That category does not exist: this repo's governed surface isdocs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md..github/workflows/**is ordinary CI surface, so this is a lane-PM call and I am making it.Drop the counts from the two step names. The run's own tail already prints
N passed, N failed— a number that cannot lie, because it is produced by the thing being counted. A step name carrying a hand-copied duplicate with no derivation and no check is the declared≠actual class this lane has spent the day clearing, in the one place a reviewer actually looks.Option 2 (keep the counts, add the recipe) is rejected: it preserves the duplication and merely documents it, creating a maintenance obligation in a second file when the hook headers already carry that obligation correctly. One authoritative copy beats two synchronised ones.
The cost note (line 48) keeps its numbers, restated as a dated measurement — e.g. "measured on
origin/main@<sha>". A timing note is history and is honest as history; a step label is a live claim. Make that distinction visible in the file so the next editor knows why one kept its number and the other did not.⚠️ Disclosure: this workflow is this seat's ownhook-selftests.ymllanded via PR #6022 (card #5754), accepted by this seat today. The defect is ours, filed by a dev on a later card. Say so plainly in the PR body — the record should show the lane caught its own hazard rather than implying an inherited one.Timing — this is the reason it goes now
Two draft PRs against the hooks each grow a matrix and neither touches this workflow (both are correctly port-scoped): #6087 takes
guard-shared-stash32 → 41, #6046 takesguard-main-checkout-bash100 → 121. Landing this first makes both merges non-events instead of two moments where a step name silently becomes wrong.⚠️ So do not update the counts to 41/121 — that would just re-arm the same drift one cycle later. Remove them.Verification
Be honest about what can be verified here: a step name has no runtime behaviour, so there is no assertion to add and no gate that would have caught this.
- Confirm the workflow still parses and its job/step structure is unchanged —
yaml.safe_loadand a before/after of the step ids, not just the names. ⚠️ Check whether any test pins these step names before you change them.scripts/__tests__/contains workflow-pinning tests (ci-cd-pipeline-doc.test.tsand siblings) that assert on workflow content; if one names these steps, it is part of your diff. Verify rather than assume — a stale line number or an assumed absence has cost this lane a round twice today.- Run the two self-tests themselves so the PR records what the tails currently print, demonstrating the number the labels were duplicating is already available.
- Gates per your diff, each quoting its own verdict line.
⛔ Out of scope: the hooks, their self-tests, the matrices, and PRs #6087 / #6046. ⛔ Do not "fix" the drift by changing what the workflow runs.
⛔ Never
git stash— shared stack, other agents are using it now.⚠️ Lead your report comment with the literal textos-dev-reporton the first line, never an HTML comment — GitHub's sanitizer strips HTML-comment markers.Refs: #6042 / PR #6087 · #5789 / PR #6046 · #5754 / PR #6022 · #3721.
Generated by Claude Code
- Confirm the workflow still parses and its job/step structure is unchanged —
os-dev-report
{ "issue": 6089, "status": "done", "branch": "claude/issue-6089-hook-selftests-derive-counts", "pr": "https://github.com/objectstack-ai/objectui/pull/6101", "premise_still_valid": true, "summary": "Implemented the ruling (option 1): dropped the hard-coded case counts from both step names in .github/workflows/hook-selftests.yml — `guard-main-checkout-bash self-test (100 cases)` and `guard-shared-stash self-test (32 cases)` are now the bare names. Counts were REMOVED, not updated to 41/121. The Cost note keeps its numbers, restated as a dated measurement (measured 2026-08-24 on origin/main @ 53dc89db8), and a new header section 'Why the step names carry no case counts' makes the measurement-vs-live-claim distinction visible in the file and tells the next editor not to put the counts back. What the workflow runs is unchanged: same job, same three steps, same run: commands, same triggers and paths filter. Card premise verified against origin/main before editing: all three quoted lines were present at the stated content (line numbers 48/92/96 exact), and the hook-header contrast holds — guard-shared-stash.sh carries its count WITH a re-derivation recipe, while guard-main-checkout-bash.sh carries no count at all (the card quoted the #6087 branch version at 41; main is 32). PR body carries the disclosure that this workflow landed via PR #6022 (card #5754) from this same seat.", "tests": "All from the repo ROOT of worktree /home/user/objectui-6089, at final commit 241cbe413 with a clean tree (`git status --porcelain` = 0 lines), heavy runs serialised through scripts/pm/os-verify-lock.sh.\n\n(1) STRUCTURAL INVARIANCE — `python3 -c \"yaml.safe_load(...)\"` exits 0 before and after. No step in this workflow declares an `id:`, so the compared identity is the full parsed structure (job keys, step order, uses:/run: values). `diff -u` of the before/after JSON dumps contains EXACTLY four lines, the two `name` strings:\n - \"name\": \"guard-main-checkout-bash self-test (100 cases)\",\n + \"name\": \"guard-main-checkout-bash self-test\",\n - \"name\": \"guard-shared-stash self-test (32 cases)\",\n + \"name\": \"guard-shared-stash self-test\",\nNothing else moved — the adjacent `run:` lines appear only as diff context.\n\n(2) NO TEST PINNED THESE NAMES — verified, not assumed. `git grep` for `self-test (`, `100 cases`, `32 cases` and both full step names across the whole repo returns: the workflow itself, .claude/hooks/guard-shared-stash.sh's own header, and content/docs/guide/ci-cd-pipeline.md:1135-1136. NO test file. scripts/__tests__/ci-cd-pipeline-doc.test.ts and siblings pin workflow FILENAMES, ci.yml job keys/`name:` values, and (in `what each job runs`) ci.yml `run:` values — all ci.yml-scoped. Confirmed empirically by the suite below being green after the change.\n\n(3) BOTH SELF-TESTS, recording what the tails print (the number the labels duplicated), run on this branch; hooks untouched by this PR:\n .claude/hooks/guard-main-checkout-bash.selftest.sh -> exit 0, tail: `100 passed, 0 failed`\n .claude/hooks/guard-shared-stash.selftest.sh -> exit 0, tail: `32 passed, 0 failed`\n\n(4) GATES, each quoting its own verdict line (exit codes captured before any pipe; heavy run's verdict read from the lock wrapper's VERDICT line, not a bare $?):\n pnpm vitest run scripts/__tests__ --maxWorkers=2\n -> `Test Files 66 passed (66)` / `Tests 1822 passed (1822)`\n -> `os-verify-lock: VERDICT command-exit 0 · held the lock 53s · waited 0s`\n node scripts/check-control-bytes.mjs\n -> `✅ check-control-bytes: OK (scanned 5043 tracked text file(s); skipped 85 binary).`\n node scripts/check-changeset-presence.mjs\n -> `✅ No source of a released package changed in this range, so no changeset is owed.`\n node scripts/check-changeset-fixed.mjs\n -> `✅ All workspace packages are in the changeset fixed group.`\n node scripts/check-changeset-no-major.mjs\n -> `✅ No changeset declares a major bump.`\nAlso a self-scan for control bytes over both changed files: `grep -naP '[\\\\x00-\\\\x08\\\\x0b\\\\x0c\\\\x0e-\\\\x1f\\\\x7f]'` -> exit 1 (no matches) on each.\n\nDECLARED NARROWING: `pnpm lint` (eslint .) was NOT run repo-wide; it was narrowed to the diff and the narrowing is measured, not assumed. (a) Population read from eslint's own config: every `files:` entry in eslint.config.js is `**/*.{ts,tsx}` or narrower (lines 28, 176, 196, 218, 232, 262). (b) File count read from `eslint --format json` over exactly the two changed paths: 2 entries, 0 errors, exit 0. (c) Invariance: the diff contains one .yml and one .md and zero TypeScript, so no untouched file's verdict can move. CI runs the full farm regardless.\n\nNO ABLATION was run and none is claimed: the change is a step NAME, which has no runtime behaviour, so there is nothing that could be made to fail. Stated in the PR body rather than papered over.\n\nCI at report time: in_progress — draft PR was opened at 241cbe413 and the report is delivered at that point per the dispatch contract; PM verifies CI convergence. PR reports `mergeable_state: behind` (main advanced past 0c282d979 while this ran); no rebase attempted since landing is the PM's.", "open_questions": [], "out_of_scope_findings": [ "filed as #6100: content/docs/guide/ci-cd-pipeline.md:1135-1136 carries a FOURTH hand-copied copy of the same two counts (100 cases / 32 cases) — same declared≠actual class, no derivation, and unpinned by ci-cd-pipeline-doc.test.ts (whose pins are all ci.yml-scoped), even though that same test file already enforces the opposite principle for ci.yml's job count ('states no job count, so the number cannot drift away from the table'). Filed unassigned with options rather than folded in: the ruling on this card was scoped to the workflow file, the fix form for a docs page is a maintainer/PM call, and that page is one of the repo's hottest merge-conflict files — widening a CI-only diff into it is not free. After this PR lands it is the only remaining hand-copied copy." ] }
Generated by Claude Code
Generated by Claude Code
Observed while porting the
guard-shared-stash.shbackslash fix (#6042, PR #6087). Out of scope there — that card is a port limited to the hook and its self-test, and the workflow is not a hook — so recorded here instead of folded in.The observation
.github/workflows/hook-selftests.ymlwrites the case count of each self-test matrix into the file in three places, all of them plain prose:Nothing derives these, and nothing re-checks them. They are step names and a cost note, so a wrong number never turns a check red — it just shows a wrong count on the CI page for as long as nobody notices.
Both numbers are stale or about to be
Two draft PRs against the governed surface each grow a matrix:
guard-shared-stash.selftest.shfrom 32 to 41 cases.guard-main-checkout-bash.selftest.shfrom 100 to 121 cases.Neither PR touches the workflow — both are deliberately port-scoped, and the workflow's own header states it is "a RUNNER, not a rewrite of what it runs". So when either merges, the step name it runs under is wrong.
Why this is worth a card rather than a shrug
The contrast with the hooks themselves is the point. Each hook header carries the same number with the recipe to re-derive it, e.g. in
guard-shared-stash.sh:That drift was considered worth its own card once already (#3721, closed — the header said 26 while the matrix had been 32 since it landed). The workflow copies the same number without the recipe and without the "the run's own tail prints the total" cross-check, so it is strictly the more drift-prone of the two copies while being the one a reviewer actually sees on the checks page.
Options, not a recommendation to implement blind
guard-shared-stash self-testalone; the run's own tail already printsN passed, N failed, which is the number that cannot lie. The cost note keeps its counts as a dated measurement ("measured onorigin/main@ 53dc89d"), which is honest as history.Option 1 removes the duplication rather than documenting it, but this is a governed-adjacent CI file and the choice is a maintainer's, not a dev agent's.
.github/workflows/**here is CI surface andhook-selftests.ymlguards.claude/**— check the current merge/review convention before opening anything non-draft.Refs: #6042 · PR #6087 · #5789 · PR #6046 · #3721.