fix(lint): ratchet the CLI's style debt so a release stops dying on it, and fix the rule that was wrong - #600
Merged
Conversation
…t, 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.
Signed-off-by: aarroyo <beyondnet.peru@gmail.com>
|
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 |
beyondnetPeru
added a commit
that referenced
this pull request
Aug 17, 2026
…604) fix(ci): the CLI release type-checks against siblings it never built THIRD FAILURE ON THE SAME JOB, THIRD DIFFERENT CAUSE. `build-and-test` in sdk-cli-release has now failed on every tag it ever ran on: v1 / v1.3.0 Generate SBOM (CycloneDX) — npm ls exit code (fixed, #568) v1.3.3 Architecture Boundary Lint — pre-existing debt (fixed, #600) v1.3.4 Type Check and Build — this Each fix uncovered the next, because the job only ever executes on a tag: it is gated on `release_created`, which `release-gate` sets only for `refs/tags/v*`. On every branch push it is SKIPPED and the workflow reports success. Five green runs on `main` tonight told us nothing about it. WHAT FAILED `Type Check and Build` runs `npm run build` inside `src/sdk/cli`, which compiles the CLI and nothing else. In a workspace, `@beyondnet/*` resolve to the local packages by symlink, but a SUBPATH import needs that package's `dist/` to exist, and it does not after a bare `npm ci`. 100+ errors, all the same shape: src/app.module.ts(12,29): error TS2307: Cannot find module '@beyondnet/evolith-core-domain/application/sync/sync.service' src/commands/agents/agents.command.ts(5,40): error TS2307: Cannot find module '@beyondnet/evolith-infra-providers' THE FIX ALREADY EXISTS, ONE WORKFLOW OVER `npm-release.yml` has a step called "Build every workspace" that runs the root `npm run build` (`tsc -b tsconfig.json` over all projects). sdk-cli-release never had it. Added, immediately after `npm ci` and before the lint and type-check that depend on it. VERIFIED, INCLUDING THE PART THAT LOOKED LIKE A PROBLEM Locally, the root build exits 2 — but for a reason that does not exist on the runner, and the distinction was measured rather than assumed: root `npm run build` here -> exit 2, 4 × TS2307, all '@nestjs/cache-manager' is it declared? -> yes, in src/packages/mcp-server/package.json is it in the lockfile? -> yes, @3.1.3 under src/packages/mcp-server/node_modules and src/apps/core-api/node_modules so `npm ci` installs it -> and the SAME step in npm-release.yml ran tonight on main with conclusion=success and 0 TS2307 errors (run 31991739387) My local tree has a symlinked node_modules without those nested installs. On the runner the root build passes, which is the only place this step will run. And after a root build, the CLI's own type-check is clean: cli `npm run build` -> exit 0, TS2307 errors: 0 NOT FIXED The reason all three of these survived: `build-and-test` never runs on a pull request. Fixing that is a larger change than a release-night fix and is not attempted here — but it is the actual defect, and each of these three is a symptom of it. CI: 30 SUCCESS, 2 SKIPPED, 0 failing.
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.
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-releaseat "Architecture Boundary Lint", soupload-assetswasskipped and no GitHub Release exists. Diagnosing it turned up something bigger than the lint:
build-and-testis gated onrelease_created, whichrelease-gatesets only forrefs/tags/v*. So it never runs on a branch push, the workflow reports success anyway, and thejob 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 passclean. The failures are style and complexity debt:
THE THREE eqeqeq WERE NOT DEBT. THE RULE WAS WRONG.
All three are the deliberate
!= nullidiom, which tests null AND undefined in one comparison.That is correct code flagged by a config that did not match it.
eqeqeqis configured in ablock 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+), sono 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.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
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
37. Nothing here refactors them; the baseline makes them visible and stops them growing.
build-and-teststill only runs on a tag. That is the reason this debt reached a releaseinstead of a pull request, and it is not addressed here.
src/packages/infra-providershas 7 of its own errors (4 max-params, 3 max-lines), measuredidentical before and after this change, and is linted by NO CI step at all.
🤖 Generated with Claude Code