fix(web-ui): upgrade next to 16.3 to clear four high-severity advisories (#1124)

[frankbria]

Closes #1124.

What was actually exposed

Four high-severity advisories were live. The PR that would have fixed them
(#1087) was closed by Dependabot itself — "updatable in another way" — and
its replacement #1123 bumped only top-level postcss, which was never the
exposure. The vulnerable copy was nested at next/node_modules/postcss, pinned
by next 16.2.12.

next 16.3.0 clears all four:

Advisory	Package
GHSA-r28c-9q8g-f849	postcss	path traversal via previous-source-map loading
GHSA-6g55-p6wh-862q	postcss	arbitrary file read via sourceMappingURL
GHSA-qx2v-qp2m-jg93	postcss	XSS via unescaped </style> in stringify
GHSA-f88m-g3jw-g9cj	sharp	inherited libvips CVEs (< 0.35.0)

npm audit:  7 vulnerabilities (6 high)  ->  4 (3 high)
sharp:      0.34.5 -> 0.35.3
nested postcss: now 8.5.23, past every advisory's patched version

Each of the four verified individually as absent from the post-upgrade audit.

The 27 type errors — fixed, not silenced

The upgrade brings stricter bundled typings. All 27 were in src/__tests__,
none in production source. No as any was added anywhere — two pre-existing
ones were narrowed to as never — and all 1276 tests across 104 suites still
pass.

Two of them were real defects rather than typing noise:

reviewApi.getFileDiff does not exist. The test declared it in the module
factory and stubbed it in beforeEach. There is no such method on reviewApi;
the as jest.Mock cast hid it. Removed as dead setup — its 13 tests still pass.

TaskBoardView mocked with the wrong types. It called .mockResolvedValue
on the real typed tasksApi. Switching to jest.mocked() keeps the true
signatures, so the toHaveBeenCalledWith assertions still type-check against
real argument shapes — and that immediately surfaced two more:
updateStatus.mockResolvedValue({}) where a Task is required. Now uses the
file's own makeTask factory.

The rest were genuine drift or stricter typings:

• ProofRequirement.scope became a required (nullable) field with the [Phase 3.5C] Glitch Capture: entry point and capture form #568 scope
  work; five fixtures predated it
• Blocker.created_by is required; one fixture omitted it
• The WebSocket mock declared onclose as taking no arguments while calling it
  with a CloseEvent, and passed Uint8Array.buffer (ArrayBufferLike, i.e.
  possibly a SharedArrayBuffer) where a real binary message carries an
  ArrayBuffer
• Two @ts-expect-error directives are now unused because axios types
  handlers; removed, reason kept as a comment
• axios.isAxiosError was being reassigned, which fights jest.Mocked's
  intersection type; now drives the existing auto-mock
• Remaining partial mocks of SWRResponse / AppRouterInstance assert through
  unknown, which keeps the target type asserted rather than erasing it

Evidence

	before	after
npm run build	exit 0	exit 0
tsc --noEmit errors	0 (on 16.2) / 27 (on 16.3)	0
npm test	1276 passed	1276 passed
npm audit	7 (6 high)	4 (3 high)
the four target advisories	live	all RESOLVED
as any added	—	0

Acceptance criteria

• next ≥ 16.3.0 in package.json and the lockfile
• All 27 type errors resolved; npm run build exits 0
• npm test passes, with the fixes preserving what the tests assert
• The three postcss advisories and the sharp advisory are gone
• chore(deps-dev): bump postcss from 8.5.23 to 8.5.24 in /web-ui #1123 — already merged; harmless and unrelated to this fix

The two "also worth deciding" items → #1131

The remaining four advisories (brace-expansion, js-yaml, nanoid,
@babel/core — all dev/build-time) and raising deploy.yml's
--audit-level=critical are coupled, so I filed them together as
#1131 (P2.35) rather than half-doing either here: raising the gate to high
today would immediately block every deploy on three findings nothing has decided
about yet. The right order is decide, then raise.

Worth remembering

Dependabot closed a security PR and substituted a materially weaker one that
looked like the same fix. An open Dependabot PR is not evidence that a class of
advisory is being handled — #1123 was green, plausible, and did not touch the
vulnerable dependency.

[github-actions]

Claude finished @frankbria's task in 3m 49s —— View job

---

🔍 Precision bug-hunt review complete

• Gather PR diff and context (gh pr view, gh pr diff)
• Check existing comments to avoid duplicates
• Inspect each suspicious hunk against surrounding code
• Post inline findings (defects only — no style/coverage)
• Post summary comment

Verdict: no defects found. Formal summary posted as a separate comment below.

What I verified against the real source (not just the diff): the diff is next 16.2.12→16.3.0 + lockfile resolution clearing the four advisories, plus test-only typing fixes. No production source changed. reviewApi.getFileDiff confirmed absent (src/lib/api.ts:740); updateStatus({})→makeTask() and isAxiosError reassignment→.mockReturnValue() preserve runtime behavior and assertion semantics; the WS mock's onclose/simulateBinaryMessage signatures match their call sites.

[claude]

Reviewed the diff (package.json/lockfile bump + 13 test-file fixes, no production source changes).

Verified claims

• next ^16.2.12 → ^16.3.0 in both package.json and package-lock.json; nested next/node_modules/postcss goes 8.4.31 → 8.5.23, and sharp ^0.34.5 → ^0.35.3 — matches the advisory versions cited in the PR description.
• Grepped the diff for as any: all three occurrences are removals (in app/page.test.tsx), none added. Consistent with the "0 as any added" claim.
• The two "real defects" callouts check out: reviewApi.getFileDiff genuinely isn't mocked/used elsewhere in review/page.test.tsx after removal, and TaskBoardView.test.tsx's mockedTasksApi.updateStatus.mockResolvedValue(makeTask(...)) correctly reuses the file's existing makeTask factory instead of the old {} stub.
• Scope is honest: no production src/ files changed, everything outside the lockfile/package.json is under src/__tests__/.

Minor, non-blocking nits

• CaptureGlitchModal.test.tsx and ProofPage.test.tsx both add scope: null onto the same line as related_reqs: [] (related_reqs: [], scope: null,) rather than its own line — cosmetic only, doesn't affect behavior.
• TaskBoardView.test.tsx inserts const mockedTasksApi = jest.mocked(tasksApi); between two import statements rather than after all imports. Valid TS/JS and harmless, but unusual placement that a linter with import/first could flag; worth moving below the imports if that rule is ever turned on.

Not addressed here (and correctly deferred): the four remaining dev/build-time advisories and raising deploy.yml's --audit-level are pushed to #1131 per the PR description — that's a reasonable split given they're coupled decisions, not something this PR should half-do.

No bugs, security concerns, or missing test coverage found. The typing fixes look like genuine drift/defect fixes rather than suppression, and the evidence table in the description (build, tsc, test, audit counts) is consistent with the diff. LGTM.

[github-actions]

✅ GLM precision review: no defects found

severity	file:line	finding
—	—	no concrete defects (logic / security / data-loss / race / contract / runtime-type) found

Scope checked — every hunk read against surrounding source, not the diff alone:

• package.json + lockfile: next ^16.2.12→^16.3.0 with transitive sharp/postcss resolution — clears the four target advisories. No version conflict or stray pin; caret range correct.
• No production source changed — only the generated next-env.d.ts line and src/__tests__/**.
• review/page.test.tsx: confirmed reviewApi (src/lib/api.ts:740) exposes only getDiff/getPatch/generateCommitMessage; getFileDiff exists nowhere under src/ — the removed factory entry + beforeEach stub were dead, the as jest.Mock cast had hidden it.
• TaskBoardView.test.tsx: jest.mocked(tasksApi) resolves at module load after the hoisted jest.mock('@/lib/api') installs the factory; updateStatus.mockResolvedValue({})→makeTask(...) only makes the resolved value type-correct — these tests assert toHaveBeenCalledWith(...) + mutate, which don't read the response shape, so behavior is unchanged.
• auth.test.ts: isAxiosError = jest.fn(()=>true) → .mockReturnValue(true). jest.clearAllMocks() (beforeEach) does not reset either form, so persistence semantics are identical; and no later test relies on the falsy default (the resolve-path tests never call isAxiosError).
• useTerminalSocket.test.ts: the onclose: (event:{ code:number })=>void signature matches both call sites (close() and simulateClose() each pass { code }); Uint8Array<ArrayBuffer>→.buffer yielding ArrayBuffer is type-only.
• page.test.tsx: key typed unknown + typeof === 'string' guards are more correct than string | null; the as never/as unknown as T escapes return real {data,error,isLoading,mutate} objects whose consumed members are all present.

Nothing to fix. Security-wise this is a net improvement (four high advisories cleared).

[frankbria]

Took both nits — they were fair:

• const mockedTasksApi = jest.mocked(tasksApi) moved below the imports. Valid where it was, but an import/first rule would flag it and it read oddly.
• scope: null now gets its own line instead of being appended to related_reqs: [], matching the surrounding fixture style.

No behaviour change: tsc clean, eslint clean, 108 tests in the touched suites still pass.

Also — thanks for independently verifying rather than taking the description on trust, particularly grepping the diff to confirm the as any count and checking that getFileDiff exists nowhere under src/. Those were exactly the two claims most worth not believing on my say-so.

[github-actions]

Claude finished @frankbria's task in 2m 57s —— View job

---

✅ GLM precision review: no defects found (re-run on synchronize eec2792)

severity	file:line	finding
—	—	no concrete defects (logic / security / data-loss / race / contract / runtime-type) found

Re-ran on the new nit-fix commit and re-verified the full diff against the checked-out source (not the diff alone). The new commit eec2792 is purely cosmetic — scope: null moved to its own line in the two proof fixtures, and import type { Task, TaskListResponse } moved above the jest.mocked() comment — zero runtime surface, no behavior change.

What I confirmed against real code this pass:

• auth.test.ts — isAxiosError.mockReturnValue(true) vs the old property reassignment. beforeEach uses jest.clearAllMocks() (lines 59, 103), which clears call history but does not reset mockReturnValue or undo a = reassignment — so once set to true, both forms persist identically across subsequent tests. No test orders a "needs-falsy-isAxiosError" case after the point it's first set true: the login/register success-path tests (lines 62, 106/118/129) never reject, so the catch branch that calls isAxiosError is never reached. Behavior is equivalent; no regression introduced.
• TaskBoardView.test.tsx — updateStatus.mockResolvedValue(makeTask(...)) vs {}. Both call sites (lines 262–276, 369–394) assert only toHaveBeenCalledWith('/test', 't6', 'READY') and mockMutate.toHaveBeenCalled() — neither reads the resolved value. makeTask (line 41) yields a more complete Task than {}, so this can only be safer. jest.mocked(tasksApi) resolves against the hoisted jest.mock('@/lib/api') factory; pure typing, same mock objects at runtime.
• useTerminalSocket.test.ts — signature changes. Both onclose call sites (close() line 43, simulateClose() line 62) pass { code } matching (event: { code: number }) => void; the lone simulateBinaryMessage caller (line 152) passes new Uint8Array([...]) whose inferred type is Uint8Array<ArrayBuffer>, and bytes.buffer is an ArrayBuffer matching onmessage's { data: string | ArrayBuffer }. Runtime unchanged.
• ResolvedBlockersSection + CaptureGlitchModal/ProofPage fixtures. Verified against src/types/index.ts: Blocker.created_by: BlockerOrigin (line 140, required) and ProofRequirement.scope: ProofScope | null (line 381, required-nullable) — both real fields, both satisfied by the added values.
• page.test.tsx — key: unknown + typeof === 'string' is a stricter guard than the old key: string | null + truthiness check (rejects non-string truthy keys too); as never/as unknown as T escapes return real {data,error,isLoading,mutate} objects whose consumed members are all present. Type-only.
• package.json + lockfile — next ^16.2.12→^16.3.0, transitive sharp→0.35.x and nested postcss→8.5.23 clear the four target advisories. No stray pin or version conflict.

Net security change is positive (four high advisories cleared); no production src/ changed. Nothing to fix.

[claude]

Re-reviewed after the eec2792f nit-fix commit — consistent with my earlier pass and the other automated reviews on this thread.

Confirmed fixed:

• mockedTasksApi = jest.mocked(tasksApi) now sits below all imports in TaskBoardView.test.tsx.
• scope: null is on its own line in both CaptureGlitchModal.test.tsx and ProofPage.test.tsx, matching surrounding fixture style.

Re-verified independently:

• next is ^16.3.0 in both package.json and package-lock.json; the nested next/node_modules/postcss and sharp bumps land at the versions the advisories require.
• getFileDiff genuinely doesn't exist anywhere under web-ui/src/ — the removed mock/stub in review/page.test.tsx was dead code, not a coverage loss.
• No production src/ files changed — everything outside package.json/package-lock.json/next-env.d.ts is under src/__tests__/.

Nothing new to flag. This is a clean, well-scoped dependency bump with test-only typing fixes — no behavior change, no as any added, scope of the remaining advisories correctly deferred to #1131. LGTM.