Skip to content

Fix failing CI: lint, tests, accidentally-deleted types, audit finding - #347

Merged
chonilius merged 7 commits into
MergeFi:mainfrom
gideononiru:fix/ci-lint-tests-and-type-restoration
Sep 7, 2026
Merged

Fix failing CI: lint, tests, accidentally-deleted types, audit finding#347
chonilius merged 7 commits into
MergeFi:mainfrom
gideononiru:fix/ci-lint-tests-and-type-restoration

Conversation

@gideononiru

Copy link
Copy Markdown
Contributor

Summary

CI on main has been red across the board (lint, tests, verify:env/build type-checking, and npm audit). This PR walks every stage of .github/workflows/ci.yml's build-and-lint job in order and fixes everything found at each stage.

  • Lint — 9 ESLint errors across 4 files: require()-based jest mock overrides and any casts replaced with proper ES imports typed against jest.Mock (CallbackClient.test.tsx, DashboardShell.test.tsx, Navbar.test.tsx, including a leftover usePathname.mockReturnValue that should have read mockedUsePathname), and the react-hooks/set-state-in-effect violation in WalletContext.tsx fixed via a ref kept current every render instead of peeking at previous state from inside an effect.
  • Tests — 4 failing suites: two were stale test expectations (Tabs.test.tsx querying the wrong ARIA role, BountyCard.test.tsx expecting an abbreviated deadline string the component never produces), one was an incomplete mock (WalletContext.test.tsx missing getActiveFreighterAddress/checkNetworkMismatch on @/lib/wallet), and one was a real bug: AuthContext.refresh() never cleared loading on a 401/403, leaving the UI stuck showing "loading" forever after a session became invalid.
  • Types — the big one: commit 1717eee accidentally replaced the entire src/types/index.ts with a single re-export line as a side effect of an unrelated feature commit, deleting UserRole, Difficulty, TeamSplit, Milestone, ReputationProfile, MaintenancePool, and AuthUser. This doesn't fail Jest (no type-checking there) but fails tsc/next build/npm run verify:env across adapters.ts, api.ts, mock-data.ts, and AuthContext.tsx. Restored the 7 types verbatim (recovered via git show 1717eee^:src/types/index.ts) into a new src/types/shared.ts, re-exported from the barrel, and tightened Bounty.difficulty/teamSplits to use the restored Difficulty/TeamSplit types instead of the widened string/inline shapes they'd quietly regressed to — every real consumer (DifficultyBadge, etc.) was already written against the narrower types. Also fixed the resulting useBountyStatus.ts type mismatch (fetchBounty can resolve with data: undefined).
  • Once the restored types made tsc actually check BountyStatus.tsx/ClaimButton.tsx, both turned out to be written against a status vocabulary ('in-progress', 'completed', 'cancelled') that has never matched the real BountyStatus union. Fixed both to use the real union, and dropped a bounty.updatedAt reference — that field doesn't exist anywhere in Bounty.
  • Auditnpm audit --audit-level=high was failing on a high-severity browserslist advisory; resolved via npm audit fix (transitive bump only).

Verified locally end-to-end after each stage: lint clean (0 errors, 2 pre-existing unrelated warnings), 291/291 Jest tests passing across 21 suites, verify:env all 5 scenarios pass, next build succeeds, verify:headers passes, npm audit --audit-level=high clean.

Test plan

  • npm run lint — 0 errors
  • NEXT_PUBLIC_STELLAR_NETWORK=TESTNET npx jest — 291/291 passing
  • NEXT_PUBLIC_STELLAR_NETWORK=TESTNET npm run verify:env — all scenarios pass
  • NEXT_PUBLIC_STELLAR_NETWORK=TESTNET npm run build — succeeds
  • NEXT_PUBLIC_STELLAR_NETWORK=TESTNET npm run verify:headers — passes
  • npm audit --audit-level=high — 0 vulnerabilities

- CallbackClient.test.tsx / DashboardShell.test.tsx / Navbar.test.tsx:
  replace require()-based jest mock overrides and `any` casts with
  proper ES imports typed against jest.Mock, and fix a leftover
  `usePathname.mockReturnValue` call that should have been
  `mockedUsePathname.mockReturnValue`.
- WalletContext.tsx: replace the setState(prev => ...) functional-updater
  read of previous state inside the logout-clearing effect (flagged by
  react-hooks/set-state-in-effect) with a ref kept current after every
  render, preserving the original behavior without peeking at previous
  state from inside an effect.
- Tabs.test.tsx: query by role "tab", not "button" — Tabs.tsx renders
  <button role="tab">, and the explicit ARIA role overrides the
  implicit button role.
- BountyCard.test.tsx: expect "5 days left", matching formatDaysUntil's
  actual pluralized output, not the "5d left" abbreviation the test
  incorrectly expected.
- WalletContext.test.tsx: add the missing getActiveFreighterAddress
  and checkNetworkMismatch mocks to the @/lib/wallet mock — any test
  path reaching the mount-hydration effect was throwing
  "is not a function" without them.
AuthContext.refresh() never called setLoading(false) in its 401/403
error branch, so the UI stayed stuck showing "loading" indefinitely
once a session became invalid — the only way out was a full reload.
Commit 1717eee ("feat: add smart polling with claim-race detection
for bounty status") replaced the entire src/types/index.ts with a
single `export * from './bounty'` line as an apparent unrelated side
effect, deleting UserRole, Difficulty, TeamSplit, Milestone,
ReputationProfile, MaintenancePool, and AuthUser. tsc/next build (but
not Jest, which doesn't type-check) have been failing on the missing
imports ever since across src/lib/adapters.ts, src/lib/api.ts,
src/lib/mock-data.ts, and src/context/AuthContext.tsx.

- Add src/types/shared.ts with the 7 recovered types (content restored
  verbatim from the pre-1717eee index.ts via git history).
- Re-export it from the src/types barrel alongside the existing
  ./bounty export.
- Tighten Bounty.difficulty from a bare `string` to the restored
  Difficulty union, and Bounty.teamSplits to use the restored TeamSplit
  interface instead of an inline duplicate — both had quietly widened
  in the same commit, and every real usage (DifficultyBadge, etc.) was
  already written against the narrower types.
- Fix the resulting real type mismatch in useBountyStatus.ts: fetchBounty
  can resolve with `data: undefined` (no fallback bounty, live fetch
  failed), which the hook's useSmartPolling<> generic didn't allow for.
Both components were written against a status vocabulary
('in-progress', 'completed', 'cancelled') that has never matched the
actual BountyStatus union (open/funded/claimed/in_review/merged/paid/
refunded/expired) used everywhere else (BountyCard, StatusBadge). This
only surfaced once the type restoration made tsc check these files at
all — status/style lookups keyed by the wrong strings, and dead
`status === 'completed'` comparisons that could never be true.

- BountyStatus.tsx: rebuild statusColors/statusLabels against the real
  union, and drop the "Updated: ..." line, which read a bounty.updatedAt
  field that doesn't exist anywhere in the Bounty type.
- ClaimButton.tsx: drop the unreachable `status === 'completed'` checks;
  'claimed' already covers "already claimed" for this union.
npm audit --audit-level=high (run in CI) was failing on GHSA-c83g-rgw3-j3cx
/ GHSA-73wf-gq98-2v4g in browserslist <=4.28.6. Resolved via npm audit fix
(transitive bump only, no direct dependency changes).
@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

@gideononiru is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

CI's own PR run failed at "npm test" — Jest couldn't load
jest.config.ts: "'ts-node' is required for the TypeScript configuration
files... Cannot find package 'ts-node'". jest-config tries a native
require/import of the .ts config first and only falls back to ts-node
(or esbuild-register) if that fails; it never showed up locally because
this sandbox's Node version natively strips TypeScript syntax well
enough for the native path to succeed, silently masking the gap. CI's
pinned Node 24 does not, and falls through to the ts-node path, which
was never added as a dependency. Verified with a from-scratch `rm -rf
node_modules && npm ci` plus the full lint/test/verify:env/build/
verify:headers/audit pipeline.
@chonilius
chonilius merged commit 0100e58 into MergeFi:main Sep 7, 2026
1 of 2 checks passed
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.

2 participants