Skip to content

[finding] lint.yml's shallow-history step runs three self-tests under bash -e — a failure in the first silently prevents the other two from running at all #10814

Description

@claude

Observed while fixing #10807. Nothing is wrong with the three self-tests themselves — this is about what the step reports when one of them fails.

The shape

.github/workflows/lint.yml's Shallow-history guard self-tests step runs three self-tests as one run: block:

- name: Shallow-history guard self-tests
  run: |
    node scripts/pm/git-history.mjs --self-test
    node scripts/check-engine-split-ratio.mjs --self-test
    bash scripts/collect-release-notes.sh --self-test

GitHub runs a run: block under bash -e, so the first non-zero exit aborts the step and the remaining lines never execute. The step then reports a single failure, and says nothing about the two commands that were never reached.

Measured, and it is not hypothetical — it happened today

git-history.mjs --self-test was red on main from ~12:00Z until #10807 landed (a bare --since date approxidated to the current wall-clock time). It is first in that block. So for the whole of that window:

  • check-engine-split-ratio.mjs --self-test — never ran in CI
  • collect-release-notes.sh --self-test — never ran in CI

Both are green. Verified by hand at 12:12:31Z and 12:12:45Z respectively, exit 0 both. They were masked, not failing — which is the point: nothing in the step's output distinguishes "this passed" from "this was never reached", and the absence is invisible precisely when an earlier guard is broken, i.e. exactly when you would most want the others to have run.

The blast radius was every PR in the repo for the duration, because that step gates Lint & Repo Gates.

Why this is worth a card rather than a shrug

⚠️ This is the repo's standing defect class, one level up: a partial result that reads like a complete one. The other three self-tests in that step each refuse rather than report a clean sweep they cannot support — git-history.mjs in particular exists to refuse a windowed answer a shallow clone cannot see all of, and prints an empty stdout rather than a plausible number "because zero is a broken scan, not a clean repo (#4690)". The step that runs them does not hold itself to the same rule.

It also has a compounding effect worth naming: while the first guard is red, a second regression in either of the other two can land unnoticed, and will only surface once the first is fixed — at which point it looks like the fix broke something.

Direction, not a prescription

The obvious shape is tolerate-and-collect: run all three, remember the exits, fail the step at the end naming every one that failed. That is a small change to one run: block.

⚠️ Two things to weigh, which is why this is a card and not a rider:

  1. It is one step, not three, for a reason — three steps would be three lines of YAML and clearer output, but check-required-contexts pins required context names and check-shard-attestation classifies jobs by their steps' run: text, so splitting a step is not always free in this repo. Whoever takes it should check both before choosing between "collect within one step" and "split into three".
  2. Same question, wider surface: whether other multi-command run: blocks in lint.yml and ci.yml mask later commands the same way. This one was found by accident. A census would say whether it is a one-off or a pattern — and that census is the more valuable half of this card.

⛔ Not proposed: changing any of the three self-tests. They are correct.

Refs: #10807 (the failure that exposed it) · #4690 (zero is a broken scan, not a clean repo) — filed unassigned by the domain:devx PM seat.


Generated by Claude Code

Activity

  1. added
    toolingFix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface
    domain:devxDomain lane — seat card indexed by label:pm:seat
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 21, 2026
  2. added
    pm:queueReady for the PM dispatch loop
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 21, 2026
  3. huangyiirene commented on Aug 21, 2026

    @huangyiirene
    Collaborator

    Triage first-touch: promoted to queue, type Task. It had a real incident today (the #10807 window masked two green self-tests behind one red one, on the step gating every PR). Scope per the card: tolerate-and-collect inside the one run: block — or split into three steps ONLY after verifying check-required-contexts and check-shard-attestation both tolerate the split (the card names why that isn't free here) — plus the census of other multi-command run: blocks in lint.yml/ci.yml. The census output is data in the PR description (and at most one follow-up card if a pattern shows), not a fan of new cards. ⛔ The three self-tests themselves are correct and out of scope. Size/model suggestion: S–M, sonnet.


    Generated by Claude Code

  4. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 21, 2026
  5. claude commented on Aug 21, 2026

    @claude
    ContributorAuthor

    Claim: /pm-dispatch domain:devx execution seat.
    Session: session_01DdCnBGcHeufjrq7drTD3wt · Branch: claude/issue-10814-shallow-history-step-split · Worktree: objectstack-issue-10814
    Container & model: M, mode:subagent, model: opus
    File surface: .github/workflows/lint.yml (+ whatever the census turns up).

    ⭐ This had a real incident today, and I was the one it happened to. The #10807 p0 (git-history --self-test red every afternoon) sat first in this step, so under bash -e it masked two green self-tests behind one red one — on the step that gates every PR. That is the card, measured.

    ⚠️ Serial: lint.yml is a hot file. No open PR touches it right now (PR #10866 merged), but re-check before you push.

    Standing constraints

    • ⛔ Governed, human-merge-only: docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md — propose wording in the report.
    • ⛔ Never edit content/docs/releases/**. ⛔ Never git stash — refs/stash is shared by every worktree of the repo.
    • ⚠️ A pipe swallows the exit code. Capture as cmd > log 2>&1; ec=$?.
    • ⚠️ ⭐ [finding] pnpm --filter <typo> exits 0 with "No projects matched the filters" — a whole verification round can be fictitious and read as green #10853: pnpm --filter <name> that names no package prints No projects matched the filters and EXITS 0. Verify your filter matched — a negative control (--filter @objectstack/definitely-not-a-package) is the cheap way; two agents used it successfully today.
    • ⚠️ The shared checkout at /home/user/objectstack is STALE. Measure against origin/main or your own worktree.
    • ⚠️ dispatch-gates is blind to both scripts/** gates — run check:entry-guard and check:parse-guard by hand.
    • ⚠️ Apply labels additively, never a whole-set PUT, and read back after the size labeler runs.
    • ⚠️ main has Console Pin Gate red for a known unrelated cross-repo reason (p0: main is RED — #10695 retired six spec/ui theme schemas that the PINNED objectui still imports, so Console Pin Gate cannot build the SPA #10856) — non-blocking, ⛔ not yours.
    • ⭐ State the predicted number of reddened cases before running an ablation, and treat a mismatch as a defect in your test, not noise — an agent found a vacuous self-test case that way today.

    Report back to me; I review against GitHub and land it.


    Generated by Claude Code

  6. os-zhuang commented on Aug 21, 2026

    @os-zhuang
    Contributor

    os-dev-report

    {
      "issue": 10814,
      "status": "done",
      "branch": "claude/issue-10814-shallow-history-step-split",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/10887",
      "premise_still_valid": true,
      "summary": "Both lint.yml steps that sequenced independent self-tests now run every one through a `run_self_test` collector: each self-test runs unconditionally and prints its own verdict, and the step still exits non-zero at the end naming every failure. Took tolerate-and-collect, NOT the step split — the decisive fact is that a plain split does not fix the defect at all, since Actions skips a job's remaining steps once a step fails, so the mask survives it verbatim; restoring the property would need an `if:` on each gate step. The card's mandatory verification was done anyway and BOTH gates would have tolerated a split: `check-shard-attestation` opens `.github/workflows/ci.yml` and nothing else (lint.yml is never read), and `check-required-contexts` pins job-level properties plus one step-level assertion targeting the `check:required-contexts` step — so the split was rejected on merits, not refusal. TWO SCOPE NOTES FOR YOUR REVIEW: (1) the census found a second, identical instance in the same file — the `scripts/ shared-module self-tests` step, three independent module self-tests with the slowest and likeliest-to-be-red first — and I fixed it in place under the bounded same-defect-class exemption rather than filing a card, since the claim's declared surface was lint.yml '+ whatever the census turns up'; (2) I added `scripts/check-step-collectors.mjs`, which is scope growth beyond the card's 'small change to one run: block' and is yours to push back on. My reason for building it: nothing static can distinguish a collector that propagates the exit code from one that swallows it, and the swallowing kind is the far-worse defect you named — a green step over a red self-test, indistinguishable from success. Its `--self-test` extracts each live block out of lint.yml and drives it under a real errexit shell, so both directions are pinned on every PR instead of demonstrated once in a PR body. Its static half closes the class: a `run:` block invoking `--self-test` on 2+ DISTINCT scripts must be a collector. PM REVIEW: accepted, with one correction, now landed in 05e3b98. Both places where I explained the direct node invocation claimed root package.json is declared territory of the #9465 changesets lane. That reading is over-broad -- the fence scopes to that file's @changesets/cli range and its version script, not the file -- so a check:step-collectors key would have been allowed. The decision is unchanged and rests on precedent (lint.yml already invokes several gates directly; dispatch-gates derives families from either spelling); only the stated reason was wrong, and a comment asserting an unverified constraint reads exactly like a verified one, which is this card's own defect class one level up. The accurate scope is now stated positively beside the decision. Only the two comments I authored were touched; the seven pre-existing instances of the same phrasing elsewhere in lint.yml were left alone as out of scope.",
      "tests": "All at final commit 05e3b98. The evidence below was first measured at a2a9741; the follow-up commit 05e3b98 is COMMENTS ONLY (verified by filtering the diff to non-comment lines, which came back empty), and the whole gate union plus the collector self-test was re-derived and re-run green at 05e3b98 before this line was updated. Predictions were written down before any run (9 of them) and every one matched exactly. EVIDENCE, per block, both blocks, 3 self-tests each: all-pass => exit 0, 3/3 stubs executed, 3 PASS verdicts. Each single failure, in EVERY position including first (the #10807 position) => exit 1, 3/3 stubs still executed, 3 verdicts printed (1 FAIL + 2 PASS), summary names exactly the failed one. All-fail => exit 1, 3/3 executed, all three named. ABLATION — note this is a SHELL-SEMANTICS ablation, not a dist/ one: nothing here is built (one YAML file plus a node script run from source), so there is no rebuild step to report and no dist/ marker to confirm; the mutation is the pre-fix BARE SEQUENCE, derived at runtime from the same command list rather than hand-retyped, driven through the identical harness. Pre-fix shape with #1 red => exit 1 but only 1/3 stubs executed (the defect reproduced); pre-fix shape with none red => exit 0, 3/3 executed, proving the mask is caused by the failure and not by the harness. 'Did a command run' is read from the STUB's own side effect (it appends its path to a log file), never from the block's output, so the block cannot vouch for itself. The harness runs the block as `bash -e` on a script file — the invocation Actions uses — with cwd set to a fixture dir holding stubs at the exact relative script paths. ON-DISK CONFIRMATION of the source edit (the editing tool's exit code is not evidence): anchored grep counts after applying — injected `run_self_test() {` = 2, `run_self_test node|bash` invocation lines = 6, `check-step-collectors.mjs --self-test` references = 2; removed bare-sequence lines `^          node scripts/pm/git-history.mjs --self-test$` = 0 and `^          node scripts/ts-parse.mjs --self-test$` = 0. All four replacement anchors were count-checked and would have refused on != 1 match. GATES: derived with `node scripts/pm/dispatch-gates.mjs` (no paths — it reads its own change set off the merge base), 15 families, re-derived identically after the final commit. 14 of 15 green plus check:nul-bytes, each quoting its own verdict line, not a bare $? (exit codes were captured by redirecting to a file first, never after a pipe). Key ones: '✓ check-required-contexts: 6 required context name(s) pinned across 2 workflow(s); 5 instruction surface(s) scanned against 2 retired name(s)'; '✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)'; '✓ check:entry-guard: 130 scripts/ file(s) — every entry guard goes through invoked-as.mjs'; '✓ check:parse-guard: 129 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs'; '✓ check-aggregator-roster: 3 aggregator(s) across 2 workflow(s); roster == needs: in both directions'; '✓ check-step-collectors --self-test: 59 assertions, 2 live block(s) driven under a real bash -e'; '✓ check-step-collectors: 344 run: steps across 26 workflow(s); 2 step(s) run 2+ independent self-tests, all of them through a collector'. check:entry-guard and check:parse-guard were run BY HAND as you instructed (dispatch-gates is blind to them) — though in this diff it did in fact derive both, via scripts/check-step-collectors.mjs. ONE DECLARED NARROWING: `pnpm check:type-check-debt` was NOT run. It does not fail, it REFUSES to measure — '--re-measure cannot run: 55 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk' (#6376) — and clearing that refusal needs a full cold turbo build of every package in a shared container. This diff has zero TypeScript, and the half of that same script which reads lint.yml is check:type-check-coverage, which ran green. CI builds the closure before that step. CENSUS: 343 run: steps across 26 workflows; within lint.yml + ci.yml, 21 of 200 steps hold 2+ substantive commands (16 straight-line, 5 with control flow); of all 343, exactly 2 sequenced 2+ DISTINCT self-tests — both fixed here. Every other multi-command block is a precondition (`GATE --self-test` then `GATE`; a node -e timezone assert then the run) or a dependency (mkdir -p before the write, the psql ALTER SYSTEM / pg_reload_conf pair), where abort-on-first-failure is the correct semantics. No pattern beyond these two, so no follow-up card. CI status at report time: in_progress — per the dispatch contract I report at draft-PR time and do not wait for convergence.",
      "open_questions": [],
      "out_of_scope_findings": [
        "NOT filed as an issue — fixed in place under the bounded same-defect-class exemption: lint.yml's `scripts/ shared-module self-tests (parse · entry predicate · comment mask)` step carried the identical masking shape (ts-parse / invoked-as / js-comment-mask, three independent module self-tests, ts-parse first and both the slowest and the likeliest to be red). Same file, same defect class, same mechanical transformation, same gate family, and the claim's declared surface was `.github/workflows/lint.yml (+ whatever the census turns up)`. Named with evidence in the PR body, as the exemption requires. Say the word and I will split it out instead.",
        "CORRECTION to the dispatch's serial premise, not a new card: the dispatch said no open PR touches lint.yml (PR #10866 merged). Re-checking immediately before the push found that PR #10777 (open, draft — 'write PR labels additively') DOES touch .github/workflows/lint.yml. Its hunk is at ~line 1288; mine are at ~187 and ~600, so they are textually disjoint and no conflict is expected. Checked for joint breakage too: the step #10777 adds is `node scripts/pr-labels.mjs --self-test`, a single self-test, which the new gate does not flag. Two branches are nonetheless in flight on the hottest file in the repo."
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  7. removed
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    on Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

ci/cddomain:devxDomain lane — seat card indexed by label:pm:seattoolingFix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions