Skip to content

fix(scripts): drop unnecessary type assertions in stage-publish runner (unblocks Check)#1436

Closed
John-David Dalton (jdalton) wants to merge 1 commit into
mainfrom
fix/stage-publish-oxlint
Closed

fix(scripts): drop unnecessary type assertions in stage-publish runner (unblocks Check)#1436
John-David Dalton (jdalton) wants to merge 1 commit into
mainfrom
fix/stage-publish-oxlint

Conversation

@jdalton

@jdalton John-David Dalton (jdalton) commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What

The 🔎 Check CI job (pnpm run check --all) is red on main — and therefore on every open socket-cli PR — because the type-aware oxlint gate flags five unnecessary type assertions in scripts/repo/stage-publish-cli-exe.mts:

##[error]This assertion is unnecessary since it does not change the type of the expression.  (×5)
Found 0 warnings and 5 errors.

This removes those five assertions. No rule was disabled and no oxlint-disable was added — the code is fixed so the type-aware rule passes on its own.

Why unblocking this matters

Check gates the whole PR fleet. While it's red on main, every PR inherits a red Check regardless of its contents, which hides real regressions. Fixing the root cause greens Check for all socket-cli PRs (same fleet-wide win as the cmd-ci snapshot fix).

The findings (all type-aware no-unnecessary-type-assertion)

  • arr[i]! in indexed for loops (lines ~103, ~270, ~285, ~292). The tsconfig does not enable noUncheckedIndexedAccess, so arr[i] is already the non-nullable element type — the ! changes nothing. Dropped the !.
  • (e as { code: unknown }).code (line ~193) inside an e && typeof e === 'object' && 'code' in e branch. After that narrowing, e.code is already accessible as unknown — the cast changes nothing. Replaced with e.code.

Removing an assertion the type-checker itself deems redundant is type-safe by definition (the assertion did not alter the type).

Verification (proof)

scripts/repo/stage-publish-cli-exe.mts is a build/release script with no unit tests; the type-aware lint gate is the relevant check.

  • Before (this file, on main): the type-aware pass reports 5 errors — see the Check job on the latest main run.
  • After (this branch): pnpm run lint:all (the same whole-tree gate that runs the --type-aware oxlint leg) passes clean:
$ pnpm run lint:all
Lint scope: all
Running oxlint on all files…
Formatting all files…
Checking formatting...
All matched files use the correct format.
Finished in 3711ms on 1387 files using 14 threads.
Lint passed
# exit 0
Note on the other red checks

The 🧪 Test job still fails on the unrelated cmd-ci.test.mts branch-name banner snapshot, which fails on every non-main PR (tracked separately). This PR only addresses the 🔎 Check failure.


Note

Low Risk
Lint-only cleanup in a release script; assertions were provably redundant and runtime behavior is unchanged.

Overview
Clears five type-aware oxlint no-unnecessary-type-assertion errors in scripts/repo/stage-publish-cli-exe.mts so the Check CI job can pass again.

In indexed for loops over parts and targets, arr[i]! becomes arr[i] because the indexed element type is already non-nullable under the script’s tsconfig. In the spawn error handler, (e as { code: unknown }).code becomes e.code after the existing typeof / 'code' in e narrowing.

No publish or staging behavior changes—only redundant assertions removed so pnpm run lint:all / check stays green.

Reviewed by Cursor Bugbot for commit c63d556. Configure here.

The type-aware oxlint gate (`no-unnecessary-type-assertion`, run via
`pnpm run check --all` in CI) flagged five assertions that don't change
the expression type:

- four `arr[i]!` non-null assertions in indexed for-loops (the tsconfig
  does not enable `noUncheckedIndexedAccess`, so the element type is
  already non-nullable)
- one `(e as { code: unknown }).code` cast inside a `'code' in e`
  narrowed branch, where `e.code` is already accessible

Removing them clears the findings. These errors currently red the
`Check` job on socket-cli `main` and therefore on every open PR, so this
unblocks Check across the whole PR fleet.
@jdalton

Copy link
Copy Markdown
Collaborator Author

Closing — this fix is wrong, and CI proved it.

Removing the assertions clears the type-aware oxlint finding but breaks the tsc gate in the same 🔎 Check job:

Argument of type 'StageTarget | undefined' is not assignable to parameter of type 'StageTarget'.
'target' is possibly 'undefined'.

Root cause is a fleet tooling contradiction, not a code smell in this file:

  • tsc gatetsconfig.check.jsontsconfig.check.base.jsontsconfig.base.json sets noUncheckedIndexedAccess: true and includes scripts/**/*.mts, so arr[i] is T | undefined and the ! is required.
  • socket/prefer-cached-for-loop (error)mandates the exact for (let i = 0, { length } = arr; …) { const item = arr[i]! } shape for .mts files (its own docs say the ! "suppresses TS18048 under noUncheckedIndexedAccess"). for...of is auto-reverted to this shape, so it's not an escape.
  • type-aware no-unnecessary-type-assertion (tsgolint) — finds no tsconfig.json at or above scripts/ (there is none; the fleet tsconfigs live under .config/fleet/), so it type-checks with defaults where noUncheckedIndexedAccess is off, sees arr[i] as T, and flags the mandated ! as unnecessary.

So the same ! is simultaneously required (tsc + prefer-cached-for-loop) and forbidden (type-aware). It can't be resolved in this file without breaking a gate or adding a banned oxlint-disable.

The correct fix is at the config boundary — point the --type-aware oxlint pass at the strict tsconfig (or disable no-unnecessary-type-assertion, which structurally conflicts with prefer-cached-for-loop + noUncheckedIndexedAccess). That's a fleet/wheelhouse-template change (related to the oxlint-plugin reconciliation), so it needs an owner call rather than a per-repo patch.

@jdalton
John-David Dalton (jdalton) deleted the fix/stage-publish-oxlint branch July 24, 2026 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant