Skip to content

feat(cli): os lint --strict — warning-severity findings fail the run - #15967

Merged
os-steve merged 2 commits into
mainfrom
claude/issue-15935-os-lint-strict
Sep 5, 2026
Merged

feat(cli): os lint --strict — warning-severity findings fail the run#15967
os-steve merged 2 commits into
mainfrom
claude/issue-15935-os-lint-strict

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15935

Epic hotcrm#1579, step 1 (dispatched by the epic PM). Adds the public flag os lint --strict: with it, a run with one or more warning-severity findings exits 1 exactly as an error does; suggestion stays advisory; the default is unchanged.

What changed

  • packages/cli/src/commands/lint.ts — the flag, and one verdict computed above the two faces: failing = errors + (strict ? warnings : 0). The exit code (both faces) reads failing, so --json and the console cannot disagree. The console names the count and the flag when the flag alone decided the exit: N warning(s) fail this run under --strict (a warning is advisory without the flag). The pre-existing --i18n-strict (which re-grades i18n coverage findings) is untouched and composes with this one.
  • --json face, two keys unconditionally present on every project-lint payload, flag or no flag: strict (was the flag in effect) and failing (the count the exit was read from). passed is now failing === 0 — the same statement the exit code makes. Without the flag failing === errors, so passed is byte-identical to before; with the flag a warning-only stack reads passed: false beside exit 1 rather than passed: true next to a failing exit.
  • content/docs/deployment/cli.mdx — the flag in the os lint block plus a "What fails the run" paragraph (the flag, the default, the three JSON keys).
  • .changeset/cli-lint-strict-warnings-fail.md@objectstack/cli minor.
  • packages/cli/test/lint-strict-warnings.e2e.test.ts — the pins below.

Re-measured on this branch's base a2051fa (the card measured 6c439f2): packages/lint still ships 161 error / 119 warning / 5 info / 6 suggestion (git grep -h -oE "severity: '(error|warning|info|suggestion)'" -- packages/lint/src | sort | uniq -c), and lint.ts:719 was still if (errors.length > 0) process.exit(1); — the card's premise holds.

Contract spelling, for the downstream card that pins it: the flag is exactly --strict; the JSON keys are strict (boolean) and failing (integer).

What each pin establishes

Eight e2e tests spawn the SOURCE CLI (bin/run-dev.js via tsx) over four generated fixtures — warns (2 warnings, 0 errors, 0 suggestions), suggests (2 suggestions only), errs (1 error + 1 warning), clean (nothing):

  1. warns really warns — errors === 0, warnings > 0, and warnings === 2 — read off the --json face of the run under test BEFORE either half of the pair, so the pair cannot pass over an empty set.
  2. warns without the flag: exit 0, passed: true, strict: false, failing: 0 — a regression that promoted warnings by default reds here.
  3. warns with the flag: exit 1, passed: false, strict: true, failing === warnings — a regression that made the flag a no-op reds here; the count is the number of warnings, not "1 if any".
  4. Console face with the flag: exit 1 and the reason sentence, whose number is asserted equal to the warning count the --json run measured.
  5. Console face without the flag: exit 0, warnings still printed, no strict sentence.
  6. suggests with the flag: exit 0, failing: 0 — the severity boundary (suggestions stay advisory).
  7. errs: exit 1 with and without the flag; failing === errors without, failing === errors + warnings with.
  8. clean with the flag: exit 0 in both faces — the flag does not invent a failure.

Which rule produces which finding is deliberately not pinned (a rule's severity is packages/lint's decision); the count preconditions go red loudly if one moves.

Reverse verification (ablation), both directions, restore proven by blob hash

Committed first (b239599), then mutated on disk, run, restored with git checkout HEAD -- packages/cli/src/commands/lint.ts, and the restore proven by git hash-object equal to the HEAD blob (a0bb8e20…) plus git diff HEAD empty — never by exit code. Anchor and marker counts were read before and after each mutation (anchor 1→0, marker 0→1; after restore marker 0). No dist/ sits on the measured path (the tests run lint.ts from source), so no rebuild leg applies.

  • A — flag made a no-op (failing = errors.length): 3 failed / 5 passed — exactly pins 3, 4 and the strict half of 7. Predicted direction: red on the strict half; observed as predicted.
  • B — flag made always-on (strict = true): 3 failed / 5 passed — exactly pins 2, 5 and the default half of 7. Predicted: red on the default half; observed as predicted.

Verification, all at b239599c (tree content identical to the gate runs)

Every build/test below went through scripts/pm/os-verify-lock.sh; every result is the wrapper's VERDICT command-exit line.

  • pnpm exec turbo run build --filter='@objectstack/cli^...': 56/56, exit 0. pnpm --filter @objectstack/cli build: exit 0.
  • pnpm --filter @objectstack/cli exec vitest run --project integration test/lint-strict-warnings.e2e.test.ts: 1 file, 8/8 passed, exit 0.
  • pnpm --filter @objectstack/cli exec vitest run --project unit: 177 files — 176 passed, 1 refused as a prerequisite (published-subpath-hook-body.pin.test.ts: "packages/cli is not built"); after pnpm --filter @objectstack/cli build that file reruns 13/13, exit 0. Net: 177/177.
  • pnpm --filter @objectstack/cli typecheck (tsc + check:test-typecheck over tsconfig.test.json, which compiles the new test file): exit 0.
  • Affected packages per TURBO_SCM_BASE=a2051fa pnpm exec turbo ls --affected: cli, dogfood, downstream-contract, example-crm, example-multi-package (no test script), example-showcase, example-todo. Run: downstream-contract 3 files/31 tests, example-todo 4/106, example-crm 5/45, example-showcase 27/375 — all exit 0.
  • Declared narrowing — dogfood: 133 files do not fit the foreground cap; the 5 files that reach the CLI (grep -l 'build-shaped-artifact|@objectstack/cli|run-dev.js|lint' packages/qa/dogfood/test) ran: 5 files / 45 tests, exit 0. The rest is CI's (Dogfood Regression Gate).
  • Declared narrowing — integration tier of packages/cli: only the new file and the pin rerun ran locally (the diff touches no spawn entry, no driver, no kernel boot path); the tier is declared to CI.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands derived 85 families from the real changed set (untracked included) at this tree; all 85 ran; --ran reconciles "85 derived, 85 run, 0 NOT-MEASURED, 0 UNRUN". Three needed a real rerun to reach a verdict, none for a reason connected to this diff: check:skill-examples (needed client-react/client dist; rerun: 257 examples type-check, exit 0), check:dual-build-cjs-loads and check:i18n-coverage (needed packages outside cli's closure built; after turbo run build --filter='!@objectstack/docs' — 71/72 from the shared cache — both exit 0: 13 configs, none new; dual-build population 103/66/619/1 at floors), and check-plugin-teardown-shape --self-test (refused on the shallow clone; after git fetch --depth=1 origin 621a4876… it passes 47 cases). Everything else exit 0 on first run, check:nul-bytes included.
  • ESLint, narrowed and measured: population read from eslint.config.mjs (the packages/cli/src/**/*.{ts,tsx,mts,cts} block; packages/cli/test not ignored); eslint --no-inline-config --format json over the two TS files changed: 2 files, 0 errors / 0 warnings; invariance: the config runs no typed linting (line 328: no parserOptions.project, no typed rules), so this diff cannot move any untouched file's verdict. The repo-wide pnpm lint is CI's.

Not in this change

  • The default is not flipped; no rule's severity moves; no per-rule severity configuration; --eval keeps its own pass bar (--eval-min), and --strict has no effect in that mode, matching how --score behaves there.
  • skills/** is untouched: no generator emits the os lint flag list (the --score/--eval lines in skills/objectstack-data/SKILL.md are hand-written prose on a governed surface), so the card's "follows through the generator" has nothing to run — flagged in the report for the PM.
  • hotcrm is untouched.

Generated by Claude Code

Only an error failed the run; with the flag a warning does too, suggestions
stay advisory and the default is unchanged. The --json face publishes the
verdict as `strict` + `failing` beside `passed`, and the console names the
count and the flag when the flag alone decided the exit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DuzfS5chho38Yx1jxx9DEj
Eight e2e pins over four fixtures: the stack REALLY warns (2 warnings, 0
errors) before either half is read; without the flag exit 0 / passed:true,
with it exit 1 / passed:false / failing === warnings; the console names the
count and the flag; suggestions stay advisory; errors still fail and
`failing` sums under the flag; a clean stack stays 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DuzfS5chho38Yx1jxx9DEj
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 1 documentable anchor(s).

20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json ceb48776534dc6ac5923cdabb717f6de6b135ae0.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ceb48776534dc6ac5923cdabb717f6de6b135ae0packageMentionDocs.

Which tree this was computed on

This run read content/docs from b6f1d3246153e7b005f31d13f4720f1c43fd4e9b — the merge of head b239599c619ce2b430a8cd039142af910f02f4af into base ceb48776534dc6ac5923cdabb717f6de6b135ae0, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b6f1d3246153e7b005f31d13f4720f1c43fd4e9b && git checkout b6f1d3246153e7b005f31d13f4720f1c43fd4e9b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ceb48776534dc6ac5923cdabb717f6de6b135ae0 b239599c619ce2b430a8cd039142af910f02f4af && git checkout -B drift-repro ceb48776534dc6ac5923cdabb717f6de6b135ae0 && git merge --no-ff b239599c619ce2b430a8cd039142af910f02f4af

node scripts/docs-audit/affected-docs.mjs --json ceb48776534dc6ac5923cdabb717f6de6b135ae0

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ceb48776534dc6ac5923cdabb717f6de6b135ae0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Standing down on the merge-queue red — it is not this PR's, and it is blocking the whole queue

Epic PM for hotcrm#1579, session_01DuzfS5chho38Yx1jxx9DEj, 2026-09-05T18:1xZ. Posted because standing down on a red is never silent.

The failure

Queue branch gh-readonly-queue/main/pr-15967-f7db8f4f…, head 05b9f636: Lint & Type CheckLint & Repo Gates → step Merge-driver wiring gate, conclusion: failure, 17:51:18Z.

✗ self-test failed -- 1 failure(s) (cases and floor).
 ELIFECYCLE  Command failed with exit code 1.

Why it is not this PR's — measured, not asserted

The same check is red on main itself. lint.yml, event: push, branch: main:

main head conclusion time
f7db8f4f — the exact base this queue branch was rebased onto failure 17:51:18Z
aa6ba062 failure 17:26:44Z

⚠️ Every earlier run in that window is cancelled, ⛔ not green — a cancelled run is unmeasured, so this reading establishes "red on the base" and deliberately does not claim which commit broke it.

And on this PR's own head b239599c the same workflow was success at 17:06:39Z, alongside the other 9 green checks and the contract-review PASS. ⇒ The check passes on the PR and fails on the base.

Root cause already diagnosed by other seats — ⛔ no third card filed, this is a dedupe hit with a working control:

⇒ An environment move (the pnpm version Corepack resolves), surfacing through an unpinned invocation in a gate fixture. Nothing in this PR touches scripts/check-regen-pending.mjs, any merge=os-regen artifact, or that gate — the diff is packages/cli/src/commands/lint.ts, one new test, content/docs/deployment/cli.mdx and a changeset.

No fix ported, because none exists yet

Searched open PRs for a repair (regen-pending / self-test / pnpm / corepack / merge-driver): zero. ⛔ A zero is not a zero without a control — the control query (lint strict warning severity) returns this PR, so the search reaches. ⇒ There is nothing to port.

Deliberately not repairing it here. The fix belongs in shared tooling that two seats already hold cards on, it is outside this PR's declared file surface, and adding an unreviewed tooling change to a PR that has just passed contract review would put a second, unreviewed change into a contract-surface PR. The queue is blocked for everyone, so this lands centrally once, not per-PR.

The one permitted re-run is not spent, and will not be spent on this. Re-submitting into a queue whose base is red reproduces the same failure by construction — it is not a flake, and "flake" is not a root cause. The re-run stays unspent for a case that could actually be one.

What happens next

This PR holds. When the queue is unblocked — #15992 / #15994 landing on main — it needs to be re-queued, and this seat will verify auto-merge is still armed (a queue ejection can clear it) before re-arming. ⛔ Nothing about the contract review, the CI on b239599c, or the ACCEPT is re-opened by this: the review verdict stands (objectstack#15935 comment 5553454472).


Generated by Claude Code

@os-steve
os-steve added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 2da2901 Sep 5, 2026
43 checks passed
@os-steve
os-steve deleted the claude/issue-15935-os-lint-strict branch September 5, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants