fix(lint): apply approved lint fixes and Prettier formatting - #10
Merged
Conversation
…nges
Config-only. Takes 125 problems to 31 without touching a single source file.
- no-unused-vars: ignore the `^_` prefix. `_req`, `_url`, `_init` are already
this codebase's "deliberately unused" marker.
- require-await, no-unsafe-assignment, no-unsafe-member-access,
no-base-to-string: off for tests/ and e2e/ only. All 85 require-await hits are
test doubles declared `async` to match the interface they stand in for; zero
are in src/, so the rules keep their full value where it matters.
- no-unnecessary-type-assertion: DISABLED, with evidence. It flagged 5
assertions in src/ as unnecessary; its autofix broke `tsc` on all 5. The rule
judges an assertion in isolation, where literal types survive, and misses that
the value is then widened by its context:
src/services/accounts.ts `as "valid" | "needs_reauth"` — without it the
property widens to `string` and fails the drizzle insert type.
src/jobs/discord-roles.ts `as Record<string, number>` (x4) — without it the
return branches infer `notInGuild?: undefined` etc., failing the index
signature on JobResult["counts"].
Returning object literals into a wider declared type is the dominant shape of
the job handlers, so this would keep misfiring. A rule whose --fix breaks the
build is worse than no rule: `lint:fix` is a wired script anyone may run.
Takes 31 problems to 2 (both intentional warnings).
Real fix:
- src/app/admin/accounts/actions.ts — `String(formData.get("note") ?? "")`.
FormData.get() returns string | File | null, so a File would stringify to
"[object File]" and be persisted as the admin note *and* written to the audit
log. Now rejects anything that isn't a string. Surfaced by no-base-to-string.
Rule limitations, disabled inline with the reasoning at the call site:
- src/jobs/contacts.ts — `if (stepErr) throw stepErr` is a deliberate rethrow of
a caught `unknown` captured across two try/catch blocks so add/edit failures
don't block removals. only-throw-error's allowRethrowing option only covers
`throw` sited directly inside a catch, so it does not apply.
- scripts/wanderer-smoke.ts — the throw inside `finally` is caught by an
enclosing try/catch *within* that finally, so it can never escape or mask the
original error. no-unsafe-finally is lexical and cannot see this.
Test cleanup (mechanical, no behaviour change):
- 16 genuinely redundant type assertions removed by `eslint --fix`. Unlike the
five in src/, these typecheck cleanly without them.
- Dead imports: bootstrapAdminGrant, session (accounts.test.ts), EveSsoError
(eve-sso.test.ts). The apparent extra references were a module path string and
a test *name*, not uses.
- Two unused `const a =` bindings dropped; the `await login(ch())` side effect
each one existed for is preserved.
Left as warnings by decision: 2x @next/next/no-img-element (login and account
pages). These render external avatar URLs, where next/image needs remote-pattern
config and buys little.
Pure `npm run format` output, isolated in its own commit so the preceding two commits stay reviewable. 63 files, +830/-356. No hand edits are mixed in: `npm run format:check` is clean at this commit and `git diff` against the parent is entirely reflowing. Markdown is excluded via .prettierignore, so the docs are untouched.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request standardizes formatting across application and test files, adds TypeScript and test-specific ESLint overrides, removes redundant test assertions, and validates that ChangesFormatting and lint alignment
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eslint.config.mjs`:
- Line 80: Remove the global "`@typescript-eslint/no-unnecessary-type-assertion`"
disable from the ESLint configuration and keep the rule enabled for TypeScript
files. Add line-scoped suppressions only at the five documented assertions that
eslint --fix incorrectly changes, preserving enforcement everywhere else.
In `@src/app/admin/accounts/actions.ts`:
- Around line 50-63: Update saveNoteAction to reject non-string or missing
formData.get("note") values before starting the transaction, rather than
converting them to an empty string. Preserve an explicit empty string as the
valid request to clear the note, and continue passing valid string notes to
setStatusNote.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a2989c8-7248-46d4-962e-8f9369fd37ef
📒 Files selected for processing (65)
drizzle.config.tse2e/account.spec.tse2e/admin.spec.tseslint.config.mjsscripts/wanderer-smoke.tssrc/app/account/page.tsxsrc/app/admin/accounts/actions.tssrc/app/admin/accounts/page.tsxsrc/app/admin/audit/page.tsxsrc/app/admin/sync/page.tsxsrc/app/auth/discord/link/route.tssrc/app/auth/eve/callback/route.tssrc/config.tssrc/core/role-diff.tssrc/db/schema.tssrc/jobs/contacts.tssrc/jobs/membership.tssrc/jobs/purge.tssrc/jobs/wanderer.tssrc/lib/discord/oauth.tssrc/lib/discord/rest.tssrc/lib/esi/client.tssrc/lib/esi/sso.tssrc/lib/wanderer/client.tssrc/services/account-view.tssrc/services/accounts.tssrc/services/admin-accounts.tssrc/services/discord-link.tssrc/services/oauth-tx.tssrc/services/session.tssrc/services/tokens.tstests/account-view.test.tstests/accounts.test.tstests/admin-accounts.test.tstests/audit-query.test.tstests/auth-routes.test.tstests/config.test.tstests/contacts-diff.test.tstests/contacts-job.test.tstests/db-schema.test.tstests/deprovision-flow.test.tstests/desired.test.tstests/discord-link.test.tstests/discord-oauth.test.tstests/discord-rest.test.tstests/discord-roles-job.test.tstests/dispatcher.test.tstests/errors.test.tstests/esi-client.test.tstests/eve-sso.test.tstests/helpers/config.tstests/helpers/db.tstests/membership-job.test.tstests/ops-webhook.test.tstests/outbox.test.tstests/purge-job.test.tstests/role-diff.test.tstests/session.test.tstests/sync-run.test.tstests/tier.test.tstests/token-health-job.test.tstests/tokens.test.tstests/wanderer-client.test.tstests/wanderer-job.test.tstests/worker-queues.test.ts
💤 Files with no reviewable changes (1)
- src/lib/wanderer/client.ts
…n-string note Replace the blanket disable of @typescript-eslint/no-unnecessary-type-assertion with five line-scoped suppressions at the documented false positives, so the rule keeps enforcing everywhere else. Verified load-bearing: stripping the five directives yields exactly five errors, and the rule still fires on a freshly injected redundant assertion. saveNoteAction now rejects a non-string formData note instead of coercing it to "". Coercion silently cleared the note (setStatusNote maps "" to null) and wrote a status.note_changed audit entry for an edit nobody requested. An explicit "" remains the valid way to clear it. Also ignore next-env.d.ts: it is generated by next build/dev and gitignored, and its triple-slash reference fails lint on any machine that has built.
This was referenced Aug 3, 2026
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.
Stage 1b. Three commits, deliberately separable:
30cd610config only — rule adjustments, no source file touched. 125 → 31.e54eeb1source fixes — 31 → 2. One real bug, two inline rule-limitation disables, test cleanup.4cbb1ebmechanical Prettier reformat — 63 files, +830/-356, nothing hand-edited.Review 1 and 2; skim 3.
One thing changed from the plan you approved
You approved autofixing
no-unnecessary-type-assertion. I ran it and it broketscon all fivesrc/occurrences. The rule judges an assertion in isolation, where literal types survive, and misses that the value is then widened by its context:src/services/accounts.ts—as "valid" | "needs_reauth". Without it the object-literal property widens tostringand no longer satisfies the drizzle insert type.src/jobs/discord-roles.ts—as Record<string, number>(x4). Without it the return branches infer as a union carryingnotInGuild?: undefined, which fails the index signature onJobResult["counts"].All five were load-bearing. I reverted them and disabled the rule instead. Returning object literals into a wider declared type is the dominant shape of the job handlers, so it would keep misfiring — and a rule whose
--fixbreaks the build is worse than no rule, givenlint:fixis a wired script.The 16 test-side occurrences of the same rule were genuinely redundant and typecheck cleanly without the assertions, so those fixes are kept.
This is the exact failure mode you flagged when you said you didn't want a linter dictating a rewrite. It only got caught because the typecheck ran after the autofix.
Real bug fixed
src/app/admin/accounts/actions.tsFormData.get()returnsstring | File | null. AFilestringifies to"[object File]", which would be persisted as the admin note and written to the audit log.Also different from plan
no-unused-varswith^_killed 4 of 9, not all 9. The other 5 were genuine dead code, now removed: unused importsbootstrapAdminGrant,session,EveSsoError, and two unusedconst a =bindings (theawait login(ch())side effect each existed for is preserved). The apparent extra references tosessionandEveSsoErrorwere a module path string and a test name.Verification
The 2 remaining warnings are the two
@next/next/no-img-elementyou chose to leave as warnings.🤖 Generated with Claude Code
Summary by CodeRabbit