Skip to content

ci(lint): add a lint guardrail for floating promises, any and dead code - #109

Open
anned20 wants to merge 7 commits into
mainfrom
fix/floating-promises
Open

anned20 wants to merge 7 commits into
mainfrom
fix/floating-promises

Conversation

@anned20

@anned20 anned20 commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Summary

A lint guardrail for the defect classes tsc and the tests can't see: floating promises, any at the untrusted boundaries, and dead exports. Formatting stays out — it catches nothing and rewrites all 140 files.

 check
-  tsc
-  vitest
+  tsc
+  oxlint   --type-aware   # floating promises, any, correctness
+  knip                    # unused exports, types, files
+  vitest

oxlint rather than ESLint because typescript-eslint declares typescript <6.1.0 and this repo is on 7.0.2; oxlint parses with its own engine and reads types through oxlint-tsgolint. Knip rather than a hand-rolled scan because it resolves the import graph and finds the 48 symbols nothing outside their module touches.

.oxlintrc.json          # the rule set: two rules, correctness on, scripts/** exempt from any
knip.json               # entry points: boot.ts, scripts/*, test/**
.githooks/pre-commit    # installed by npm prepare, runs the same command as CI
scripts/install-hooks.sh# sets core.hooksPath without clobbering a hand-set path

Floating promises are resolved by triage, not suppression: void where the callee handles its own failure, .catch with a reason where a rejection leaves a visibly broken screen.

Evidence

  • Before: npx oxlint src/ts test scripts → 77 findings, exit 1.
    After: the same command is silent, and npm run lint (oxlint + knip) exits 0.
npm run precommit
  typecheck   0 errors
  oxlint      clean
  knip        clean
  vitest      53 files, 431 passed

npm run brand  # was broken mid-branch, green now
Brand module already current: scripts/brand.sh

CI before   test ×2 per branch push   (push + pull_request both fired)
CI after    test ×1, via npm run precommit

No screenshot: the behaviour changes are error paths and hooks, not pixels.

Merge Danger

Door: two-way.

Every change is reversible by dropping the two new devDependencies and the config. The 48 floating-promise markers and the 9 new .catch handlers change failure handling, not the success path; the deleted symbols were referenced nowhere, and one of them (BRAND_SHELL_PATH) was restored after review when it turned out a build script reaches it through a cast.

Blast Radius: developer workflow.

The pre-commit hook now runs typecheck, lint and tests on every commit — around ten seconds, and it blocks the commit on failure until --no-verify. A fresh clone picks it up on npm install. If a void site turns out to reject on real hardware it surfaces as an unhandled rejection, the same as today.

main only moves through a PR in this repo, and the last implement run committed straight to it because the skill says to commit to the current branch.
typescript-eslint declares typescript <6.1.0 and this repo is on 7.0.2, so the type-aware check runs on oxlint, which parses with its own engine. The baseline is 48 floating promises and 29 any at this commit.

Closes #104
Every site is fire-and-forget by design: event handlers, repaints, probe chains that already handle their own failures, and the video/screensaver transitions that count their own retries. The void marker makes that intent explicit so the linter keeps surfacing the accidental ones.

Closes #105
The two declared untyped boundaries (LunaResponse, RawApp) keep any behind a one-line disable with its reason; every other site gets a real type, a generic deepMerge matching config.ts, or the new errorMessage helper instead of reaching into an unknown catch binding. Config's deepMerge signature is now the one profiles.ts uses too.

The three type-aware rules that only ride along with type-aware mode (unbound-method, no-base-to-string, no-redundant-type-constituents) are off in the config, so the 31 findings they carry stay out of scope as the spec says.

Closes #106
knip is configured with the repo's real entry points (boot.ts, the manual scripts, the tests) instead of guessing, which is why its zero-config output called boot.ts and both manual scripts dead. The 48 unused exports and types are un-exported where their module still uses them and deleted where nothing does; getBuiltinImageUrl, getBuiltinTrackUrl, resolveBuiltinTrack, ForegroundInfo and BRAND_SHELL_PATH were referenced nowhere at all.

npm run lint now runs oxlint and knip, and npm run precommit chains typecheck, lint and test.

Closes #107
The hook lives in .githooks and installs itself through an npm prepare script, so a fresh clone gets it on npm install without husky. It runs the same precommit script CI runs, and skips with a note before node_modules exists because oxlint --type-aware cannot find its tsgolint binary without it.

test.yml also stops running the suite twice per branch push: push is now limited to main, and pull_request still covers every branch.

Closes #108
Restores BRAND_SHELL_PATH: scripts/generate-brand.ts requires it through a cast, so neither tsc nor knip saw the deletion, and npm run brand was broken. It carries @public for the same reason manifestFileName does.

errorName/errorMessage now read name and message off any object: webOS rejects video.play() and autoplay with a DOMException, which is not an Error on older firmware, so instanceof checks would have misread an AbortError as a decode failure.

Nine sites where a rejection leaves a visibly broken screen (screensaver content, refreshAll after a settings save, the settings app lists, the all-apps grid) now catch with a reason instead of sitting behind a bare void.

CI runs npm run precommit, so the local and CI command cannot drift, and the prepare script no longer overwrites a hooksPath the developer set by hand.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant