Promotion: develop → main (pinned 8eecb4b5 — the lint ratchet that unblocks the Release) - #601
Merged
Merged
Conversation
…t, and fix the rule that was wrong (#600) fix(lint): ratchet the CLI's style debt so a release stops dying on it, and fix the rule that was wrong WHY THE RELEASE DIED, AND THE PART NOBODY HAD SEEN The v1.3.3 tag failed `sdk-cli-release` at "Architecture Boundary Lint", so `upload-assets` was skipped and no GitHub Release exists. Diagnosing it turned up something bigger than the lint: push to main -> build-and-test: SKIPPED (5 runs tonight, all "success") push of a tag -> build-and-test: FAILED (v1, v1.3.0, v1.3.3 — every tag ever) `build-and-test` is gated on `release_created`, which `release-gate` sets only for `refs/tags/v*`. So it never runs on a branch push, the workflow reports success anyway, and the job has failed on 100% of the occasions it actually executed. That is why v1.3.0 never produced a Release either. And the SBOM fix (#568) was correct AND unmasked the next failure in the same job: v1/v1.3.0 died at "Generate SBOM", v1.3.3 got past it and died at the lint. The job was red for two reasons; we had removed one. WHAT THE LINT ACTUALLY REPORTED 87 errors, and NOT ONE of them from `boundaries/*` — the rules the step is named after pass clean. The failures are style and complexity debt: 34 max-lines 25 max-params 20 complexity 5 default-case 3 eqeqeq THE THREE eqeqeq WERE NOT DEBT. THE RULE WAS WRONG. output-formatter.service.ts:152 const title = i.title != null ? String(i.title) : ''; output-formatter.service.ts:153 const description = i.description != null ? ... output-formatter.service.ts:165 if (remediation != null && String(remediation).length > 0) All three are the deliberate `!= null` idiom, which tests null AND undefined in one comparison. That is correct code flagged by a config that did not match it. `eqeqeq` is configured in a block of CWE-mapped rules (CWE-597), so it is set to `['error', 'always', { null: 'ignore' }]` — ESLint's documented option for exactly this — rather than either editing correct code or recording it as debt. Baselining it would have put correct code in a file that later readers will treat as a to-do list. THE RATCHET The remaining 84 go into `eslint-suppressions.json`, ESLint 9's native suppressions (9.24+), so no new script and no new guard — the freeze holds. 80 violations across 51 files under `src`, which is exactly what `npm run lint` (`eslint src --ext .ts`) checks. suppressed: 31 max-lines, 24 max-params, 20 complexity, 5 default-case boundaries/* suppressed: 0 That last line is the point: the architecture gate is untouched. What is silenced is debt in rules that were riding along under a step named for boundaries. EXERCISED IN ALL THREE DIRECTIONS current tree -> exit 0 NEW file with a max-params violation -> exit 1, names it an ALREADY-SUPPRESSED file gaining one more -> exit 1, names it probe removed, tree restored -> exit 0 A baseline that only made things pass would be worth nothing; the second and third cases are what make it a ratchet. NOT FIXED, AND WORTH KNOWING - The 84 suppressions are real debt: files of 9052 lines, methods with 8 parameters, complexity 37. Nothing here refactors them; the baseline makes them visible and stops them growing. - `build-and-test` still only runs on a tag. That is the reason this debt reached a release instead of a pull request, and it is not addressed here. - `src/packages/infra-providers` has 7 of its own errors (4 max-params, 3 max-lines), measured identical before and after this change, and is linted by NO CI step at all. CI: 32 SUCCESS, 2 SKIPPED, 0 failing.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📊 Bilingual Coverage ImpactPR Changes
Repository Coverage
✅ Good: All EN changes have ES counterparts. Generated by GitHub Actions |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pinned to
8eecb4b5.What this unblocks
The
v1.3.3tag failedsdk-cli-releaseat the boundary-lint step, soupload-assetswas skipped and no GitHub Release exists — the same wayv1.3.0failed before it, at the SBOM step.Diagnosing it found the reason nobody had noticed:
build-and-testis gated onrelease_created, whichrelease-gatesets only forrefs/tags/v*. On a branch push it is skipped and the workflow reports success anyway. It has failed on every tag it ever ran on —v1,v1.3.0,v1.3.3.What is in here
eslint-suppressions.json— ESLint 9's native suppressions, 80 violations across 51 files. Zeroboundaries/*suppressed; the architecture gate is untouched. What was failing were style rules riding along under a step named for boundaries.eqeqeqreconfigured to['error', 'always', { null: 'ignore' }]. Its three violations were all the deliberate!= nullidiom — correct code flagged by a config that did not match it. Baselining those would have filed correct code as debt.Exercised in all three directions: clean tree exit 0; a new file with a violation exit 1; an already-suppressed file gaining one more exit 1.
After this
Cut
v1.3.4. Thev1.3.3tag points at6ac72c05and workflow runs are pinned to their commit, so re-running it would use the tree without this baseline.Every package is already level with the registry (
cli1.3.1,mcp1.3.3,core-domain1.3.1,infra-providers1.2.1,contracts1.2.0), so the new tag publishes nothing — it exists to produce the GitHub Release that two tags have now failed to produce.🤖 Generated with Claude Code