Homelab web replatform: self-hosted multi-user firearm & magazine inventory (The Machined Console)

[unclesp1d3r]

Overview

Replatforms MagStacker from a single-user desktop app into a self-hosted, multi-user web application for firearm & magazine inventory: compatibility mapping, per-caliber/per-firearm summaries, CSV export, and per-item view/edit sharing with owner-scoped data.

Stack: TypeScript · Next.js 16 (App Router) · React 19 · Bun · Biome · Tailwind v4 · Postgres + Drizzle · Better Auth · Docker.

29 commits, 133 files. The branch builds the platform bottom-up (U1–U16), then a full design/UX pass over the inventory surface.

What's included

Platform & infrastructure

• Postgres + Drizzle, Docker / docker-compose, Bun test harness (U1).
• Write-safety: idempotency store, mutation rate limiting, DB health checks (U12).
• Dockerfile builds with placeholder env; ships auth.ts + scripts/seed:admin for first-admin bootstrap.

Auth & authorization

• Better Auth: email+password, operator-created accounts (no public sign-up), DB-backed sessions, rate-limited sign-in, optimistic cookie proxy gate (U2).
• Scoping & authorization layer: viewer-relative visible-set resolution, grants, write gate (U4).
• Sharing UI: grant/revoke per-item view/edit access + "create-on-behalf" toggle (U16).

Data model & domain

• Core schema: firearms, magazines, compatibility join + ordinal, grants, idempotency (U3).
• Firearms: all-failures validation + visibility-scoped CRUD (U5).
• Magazines + compatibility: ordinal ordering, dedup, FK-visible scoping, atomic replacement (U6).
• Summary: viewer-relative aggregation (U7); CSV export with RFC-4180 + injection guard (U8); search & filter with LIKE escaping (U9); bulk-add label algorithm (U10); curated reference data (U11).

UI — "The Machined Console" design system + UX pass

• App shell + auth UI: login, gated layout, nav, admin accounts (U13).
• Inventory UI: firearms & magazines forms/lists (U14); insight UI: summary, CSV export, search/filter (U15); sharing UI (U16).
• Two-mode theme (dark "Field Console" default / light "Machined Instrument"), system-aware.
• This session's design pass (delight → audit → polish → harden → distill → onboard, commit 07a3e62):
  • Machined "readout" toast feedback on create/bulk/edit/export/delete; "armed gauge" row flash; tactile button press; console maker's mark.
  • Accessible delete confirmation (role="alertdialog", focus trap, Escape, focus return) replacing native confirm().
  • WCAG AA contrast across both themes; reduced-motion honored for Motion; semantic z-index scale.
  • Firearm-first cold-start onboarding; controls hidden until inventory exists so the empty-state CTA is the single path.
  • Extracted useRowFlash + useDeleteConfirmation hooks, de-duplicating the two inventory views.

Docs

• PRODUCT.md, DESIGN.md (Impeccable context), README (user-first), deployment guide, replatform implementation plan, AI_POLICY.md.

Test plan

• Unit/integration: bun test — 152 pass / 0 fail across 21 files (auth scoping, grants, domain validation, compatibility, CSV, summary, bulk-add, idempotency, DB health).
• Typecheck & lint: bun tsc --noEmit and bun biome check clean.
• Manual UI verification (this session): login, cold-start onboarding (empty account → firearm-first guidance → /firearms), create/edit/bulk + toasts, delete confirmation dialog (open / Escape / confirm / focus return), controls-gating (hidden when empty, restored with inventory), CSV export, light-mode contrast, theme toggle — all verified in-browser against an ephemeral dev DB.
• Automated e2e: none yet — tracked in Add Playwright e2e test suite (Testcontainers-backed) for inventory UI flows #6 (Playwright + Testcontainers).

Follow-ups (filed)

• Add Playwright e2e test suite (Testcontainers-backed) for inventory UI flows #6 — Playwright e2e suite (Testcontainers-backed).
• Add ammo inventory tracking with low-stock alerts and summary rollups #7–Attach documents to firearms (receipts, warranties, ATF forms) #12 — feature backlog: ammo inventory, accessories tracker, firearm photos, service intervals + reminders, shot count, document attachments.

Notes for reviewers

• Data is owner-scoped / multi-tenant; sharing is per-item and reversible. Serials are never exported to CSV.
• The pre-commit git hook has a stale hardcoded Python path (regenerate with pre-commit install); there's no .pre-commit-config.yaml, so it currently no-ops.

Summary by CodeRabbit

• New Features

  • Added sign-in, admin account management, inventory screens, sharing controls, summaries, and CSV export.
  • Introduced theme switching and a more polished app shell for navigation and user actions.

• Bug Fixes

  • Improved access control so items, shares, and admin pages are only available to the right users.
  • Added safer delete confirmations, better form validation, and clearer error handling.

• Documentation

  • Added setup, deployment, product, design, and AI usage guidance for the new experience.

[coderabbitai]

Walkthrough

Introduces MagStacker, a complete self-hosted Next.js/Bun/PostgreSQL firearm and magazine inventory application. The PR adds Dockerfile, Docker Compose, Drizzle ORM schema (three migrations), Better Auth with admin and rate-limiting plugins, a framework-agnostic visibility/grant authorization system, domain services for firearms/magazines/CSV/summary/bulk-add, a Machined Console design-system component library, and all Next.js route surfaces.

Changes

MagStacker Web Application

Layer / File(s)	Summary
Platform, Docker, and project configuration Dockerfile, docker-compose.yml, .dockerignore, .env.example, .gitignore, package.json, drizzle.config.ts, proxy.ts, scripts/seed-admin.ts, public/.gitkeep	Multi-stage Dockerfile and Compose stack (db→migrate→app), optimistic session-cookie middleware, admin bootstrap script, and full dependency additions.
Database schema, migrations, client, and health src/db/env.ts, src/db/client.ts, src/db/health.ts, src/db/idempotency.ts, src/db/auth-schema.ts, src/db/inventory-schema.ts, src/db/schema.ts, src/db/migrate.ts, src/db/migrations/*, src/db/__tests__/*, src/test-support/factories.ts	Drizzle auth and inventory schemas with cascade deletes, CHECK constraints, grant-cleanup triggers, three SQL migrations plus snapshots/journal, lazy pg.Pool/Drizzle client, health utilities, and a 5-minute idempotency dedup mechanism. Tests verify schema constraints, cascade behavior, connectivity, migration idempotency, and idempotency replay/concurrency.
Better Auth configuration, session helpers, and auth client auth.ts, src/auth/session.ts, app/api/auth/[...all]/route.ts, lib/auth-client.ts, src/auth/__tests__/gating.test.ts	DB-backed betterAuth with email/password, disabled public sign-up, admin plugin, DB rate limiting with stricter /sign-in/email rule, server-side getCurrentUser/isAdmin helpers, and browser authClient with adminClient plugin. Tests cover proxy redirect, sign-in/session resolution, and rate-limit enforcement.
Authorization: visibility, grants, authorize, and rate limiting src/auth/errors.ts, src/auth/visibility.ts, src/auth/grants.ts, src/auth/authorize.ts, src/auth/rate-limit.ts, src/auth/users.ts, src/auth/__tests__/*	NotFoundError/NotAuthorizedError; getVisibleIds/resolvePermission; owner-only createGrant/revokeGrant/listGrantsForItem with allowCreateOnBehalf normalization; resolveCreateOwner/authorizeUpdate/authorizeDelete/authorizeAndDeleteParent; in-memory RateLimiterMemory with bulkAddCost; listShareableUsers. Full grant, visibility, authorize, and rate-limit test suites.
Domain errors, ActionResult, and validation messages src/domain/errors.ts, src/domain/action-result.ts, src/domain/validation-messages.ts	ValidationError with codes[], ActionResult discriminated union, toActionError mapper, and VALIDATION_MESSAGES with messageForCode/firstMessage.
Firearms domain: validation and service src/domain/firearms/validate.ts, src/domain/firearms/service.ts, src/domain/firearms/__tests__/*	Pure validateFirearm returning all failure codes, and visibility-scoped CRUD service (createFirearm, updateFirearm, deleteFirearm, getFirearm, listFirearms). Tests cover validation, round-trips, grant-based listing, delete cascade, and NotFoundError.
Magazines domain: validation, compatibility, filter, and service src/domain/magazines/validate.ts, src/domain/magazines/compatibility.ts, src/domain/magazines/filter.ts, src/domain/magazines/service.ts, src/domain/magazines/__tests__/*	validateMagazine/effectiveCapacity; dedupeFirearmIds/replaceCompatibility/loadCompatibility/loadCompatibilityBatch; escapeLike/listMagazinesFiltered with ILIKE/exact/join; full CRUD service with viewer-relative compatibility attachment. Tests cover ordinal ordering, rollback atomicity, visibility scoping, and filtering.
Bulk add, CSV export, inventory summary, and reference data src/domain/bulkadd/*, src/domain/csv/*, src/domain/summary/*, src/domain/reference/*, src/data/*	generateLabels/nextLabelStart; bulkAddMagazines with rate limiting and optional idempotency; RFC-4180 CSV serializer with formula-injection guard and viewer-relative buildInventoryCsv; computeSummary/inventorySummary in-memory aggregation; embedded caliber/manufacturer reference lists. All areas have parity tests.
UI component library and hooks components/ui/*, components/theme-provider.tsx, hooks/use-delete-confirmation.ts, hooks/use-row-flash.ts	cn, Button, Input, Textarea, Select, Field, Spinner, EmptyState, Badge, Callout, Card, PageHeader, Stat, DataTable/THead/TH/TRow/TD, accessible ConfirmDialog, ToastProvider/useToast, ThemeToggle, ThemeProvider, ConsoleSignature, useDeleteConfirmation, and useRowFlash.
App shell, global CSS, layouts, and login app/globals.css, app/layout.tsx, app/(app)/app-shell.tsx, app/(app)/layout.tsx, app/(auth)/login/*, app/(app)/page.tsx	Machined Console dark/light CSS tokens, arm-row flash keyframes, tabular numerals, reduced-motion override; AppShell with admin-conditional nav and sign-out; authenticated AppLayout; LoginForm with 429-aware error handling; root redirect to /magazines.
Firearms routes and UI app/(app)/firearms/actions.ts, app/(app)/firearms/firearm-form.tsx, app/(app)/firearms/firearms-view.tsx, app/(app)/firearms/page.tsx	Server actions (create/update/delete) with requireUserId; FirearmForm with datalist suggestions and toast feedback; FirearmsView with row flash, delete confirmation, ownership-gated share/edit/delete, and conditional Serial column; FirearmsPage loading data in parallel.
Magazines routes and UI app/(app)/magazines/actions.ts, app/(app)/magazines/magazine-form.tsx, app/(app)/magazines/magazines-view.tsx, app/(app)/magazines/filter-bar.tsx, app/(app)/magazines/export-button.tsx, app/(app)/magazines/page.tsx, app/api/export/route.ts	Magazine server actions including bulkAdd; MagazineForm with single/bulk mode, bulk label preview, and firearm checkboxes; MagazinesView with multiple empty-state variants; FilterBar with debounced search and / accelerator; ExportButton streaming CSV download; authenticated export route.
Grants and sharing UI app/(app)/grants/actions.ts, app/(app)/grants/grants-list.tsx, app/(app)/grants/share-control.tsx	loadShareState/shareItemAction/revokeGrantAction server actions with owner gate and revalidation; GrantsList with permission/allowCreateOnBehalf badges; ShareControl modal with grantee/permission selectors, edit-only allowCreateOnBehalf checkbox, and Escape dismissal.
Admin user management and inventory summary app/(admin)/layout.tsx, app/(admin)/users/actions.ts, app/(admin)/users/admin-users.tsx, app/(admin)/users/page.tsx, app/(app)/summary/page.tsx	Admin layout with role gate; createAccountAction/setAccountDisabledAction using Better Auth admin API; AdminUsers create-account form and enable/disable table; UsersPage via auth.api.listUsers; SummaryPage with Stat cards and by-caliber/by-firearm tables.
Documentation, design system, and project config README.md, DESIGN.md, PRODUCT.md, AI_POLICY.md, AGENTS.md, docs/deployment.md, docs/plans/*, .impeccable/design.json, skills-lock.json, .claude/settings.json	Full product, design system, deployment, and AI policy documentation; detailed implementation plan; Impeccable design tokens/component catalog; Claude plugin and skills lock config.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~180 minutes

Poem

🐇 A burrow of code, from schema to screen,
Firearms and mags, a console so keen!
Auth gates the door, grants share the prize,
CSV exports beneath digital skies.
The rabbit hops proud — MagStacker's alive! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.28% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: a self-hosted multi-user web replatform for firearm and magazine inventory.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch full_setup

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

🟠 Major comments (24)

app/(app)/grants/share-control.tsx-110-116 (1)

110-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

This modal is missing keyboard focus management.

Opening the dialog does not move focus inside it, and Tab can still escape to the page behind the overlay, so aria-modal="true" is not actually honored for keyboard users. Please reuse the existing focus-trapping dialog primitive here instead of hand-rolling another modal.

🤖 Prompt for 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.

In `@app/`(app)/grants/share-control.tsx around lines 110 - 116, The dialog in
ShareControl is hand-rolled and lacks keyboard focus management, so focus can
escape behind the overlay. Replace the current modal markup in the ShareControl
component with the existing focus-trapping dialog primitive used elsewhere, and
wire its open/close behavior through the same open state so focus is moved into
the dialog and trapped there automatically.

app/(admin)/users/actions.ts-55-66 (1)

55-66: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Enforce the “admins cannot be disabled” rule on the server too.

app/(admin)/users/admin-users.tsx Line 134 disables the button for admin rows, but this action never validates the target account before calling banUser / unbanUser. A forged request can bypass the UI check and disable an admin account.

🤖 Prompt for 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.

In `@app/`(admin)/users/actions.ts around lines 55 - 66, The
setAccountDisabledAction flow currently trusts the client and can ban or unban
any user without verifying whether the target is an admin. Add a server-side
guard in setAccountDisabledAction before calling auth.api.banUser or
auth.api.unbanUser to load the target user and reject the request if that user
has admin privileges, mirroring the protection already implied by
admin-users.tsx. Keep the check inside the action so forged requests cannot
bypass the UI restriction.

app/(admin)/users/actions.ts-26-29 (1)

26-29: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Keep auth failures inside the ActionResult contract.

requireAdmin() runs before the try, so an expired session or direct server-action POST rejects the action instead of returning { ok: false }. Both client callers await an ActionResult, so this turns a normal permission failure into an unhandled action error.

Suggested fix

 export async function createAccountAction(
   formData: FormData,
 ): Promise<ActionResult> {
-  await requireAdmin();
-  const email = String(formData.get("email") ?? "").trim();
-  const name = String(formData.get("name") ?? "").trim() || email;
-  const password = String(formData.get("password") ?? "");
-  if (!email || password.length < 8) {
-    return {
-      ok: false,
-      error: "Email is required and password must be at least 8 characters.",
-    };
-  }
   try {
+    await requireAdmin();
+    const email = String(formData.get("email") ?? "").trim();
+    const name = String(formData.get("name") ?? "").trim() || email;
+    const password = String(formData.get("password") ?? "");
+    if (!email || password.length < 8) {
+      return {
+        ok: false,
+        error: "Email is required and password must be at least 8 characters.",
+      };
+    }
     await auth.api.createUser({
       body: { email, password, name, role: "user" },
       headers: await headers(),
@@
 export async function setAccountDisabledAction(
   userId: string,
   disabled: boolean,
 ): Promise<ActionResult> {
-  await requireAdmin();
   try {
+    await requireAdmin();
     const h = await headers();
     if (disabled) {
       await auth.api.banUser({ body: { userId }, headers: h });

Also applies to: 55-59

🤖 Prompt for 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.

In `@app/`(admin)/users/actions.ts around lines 26 - 29, Move the authorization
check in createAccountAction and the other affected action so requireAdmin() is
executed inside the existing try/catch path rather than before it, ensuring
expired sessions and unauthorized posts are converted into a failed ActionResult
instead of throwing. Keep the same ActionResult contract by catching the auth
failure in createAccountAction and the related action at the referenced
locations, then return { ok: false } with the appropriate error handling already
used by these server actions.

app/(app)/firearms/firearms-view.tsx-120-126 (1)

120-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate Edit on actual update permission.

This renders for every visible firearm, but the sharing model has per-grantee permissions. Read-only shares can open the form and only fail on submit. Pass a resolved canEdit capability into the row model and hide this control when update is not allowed.

🤖 Prompt for 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.

In `@app/`(app)/firearms/firearms-view.tsx around lines 120 - 126, The Edit button
is always shown even for read-only shares, so the row action needs to respect
per-grantee update permission. Update the firearms row model and the
`firearms-view.tsx` rendering path to pass a resolved `canEdit` capability for
each item, then conditionally hide the `Edit` control when `canEdit` is false.
Use the existing `setForm` action and the row/item shape in this view to wire
the permission check without changing the submit flow.

app/api/export/route.ts-16-20 (1)

16-20: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Mark the CSV download as non-cacheable.

This response contains authenticated inventory data. Without explicit cache headers, a browser or intermediary can retain and replay a stale/private export. Add at least Cache-Control: private, no-store here, and ideally Vary: Cookie as well.

🤖 Prompt for 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.

In `@app/api/export/route.ts` around lines 16 - 20, The CSV export response
currently lacks cache directives, so update the response headers in the export
route handler that returns the new Response(csv) to mark it non-cacheable for
authenticated data. Add a Cache-Control header such as private, no-store, and
include Vary: Cookie if possible so browsers and intermediaries do not reuse or
replay stale inventory exports.

app/(app)/magazines/filter-bar.tsx-24-42 (1)

24-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep query synchronized with useSearchParams().

query is only initialized from params once. If the user navigates Back/Forward or otherwise changes q outside this input, the debounce sees query !== params.get("q") and pushes the stale value back into the URL after 250ms. Add a URL→state sync path before running the debounced write.

🤖 Prompt for 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.

In `@app/`(app)/magazines/filter-bar.tsx around lines 24 - 42, The `query` state
in `filter-bar.tsx` is only seeded from `useSearchParams()` once, so external
URL changes can leave it stale and the debounced `pushParam` will write the old
value back. Update the `query` state whenever `params.get("q")` changes, using a
URL→state sync path in `FilterBar` before the debounce effect runs, and keep the
existing `pushParam`/`useEffect` logic so back/forward navigation stays in sync.

app/(app)/magazines/page.tsx-50-64 (1)

50-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Don't collapse compatible firearms to bare names here.

You compute duplicate-name hints above, then drop that identity and pass only string[] into MagazinesView. When two compatible firearms share a name, the UI gets indistinguishable badges and duplicate React keys downstream. Preserve an id-backed display shape here instead of flattening to plain names.

🤖 Prompt for 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.

In `@app/`(app)/magazines/page.tsx around lines 50 - 64, The `magazines/page.tsx`
mapping is flattening compatible firearms into plain `compatibleFirearmNames`,
which loses identity and causes duplicate-name collisions in `MagazinesView`.
Update the `items` shape in this mapping to preserve an id-backed compatible
firearm display structure instead of converting `compatibleFirearmIds` to
`string[]`, and make sure the downstream `MagazinesView`/badge rendering uses
the preserved id plus name so duplicate names remain distinguishable.

app/(app)/magazines/magazines-view.tsx-177-185 (1)

177-185: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate magazine editing on update permission.

This Edit button is shown for every visible magazine even though sharing is permissioned. Read-only grantees will hit an edit flow that can only fail server-side. Feed a resolved canEdit flag into the row data and conditionally render the control from that.

🤖 Prompt for 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.

In `@app/`(app)/magazines/magazines-view.tsx around lines 177 - 185, The Edit
control in the magazine row is always rendered even for users without update
access. Update the row data flow in magazines-view.tsx so the item passed into
the list includes a resolved canEdit flag, then use that flag in the Edit
button’s rendering logic around the setForm/toFormValues(item) action. This
should gate the edit entry point on update permission rather than allowing all
visible magazines to open the edit flow.

src/auth/grants.ts-47-69 (1)

47-69: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Close the share/delete race before inserting grants.

createGrant checks ownership and then upserts the grant without locking the parent row. A concurrent owner delete between Line 47 and Line 56 can run the grant-cleanup trigger first, then this insert leaves a dangling grant that resolvePermission will trust. Run the permission check and upsert in a transaction that locks/rechecks the parent row before writing the grant.

🤖 Prompt for 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.

In `@src/auth/grants.ts` around lines 47 - 69, The createGrant flow in
src/auth/grants.ts has a share/delete race because it checks ownership via
resolvePermission and then upserts grant without locking the parent row. Move
the permission check and insert into a transaction, and lock or recheck the
parent record before the db.insert(grant).values(...).onConflictDoUpdate(...)
step so a concurrent delete cannot leave a dangling grant that resolvePermission
would later trust.

src/auth/rate-limit.ts-42-49 (1)

42-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Don’t map non-limiter errors to throttling. rate-limiter-flexible rejects with a RateLimiterRes on real rate-limit hits, but store/infrastructure failures reject with an Error; this catch turns both into RateLimitError, hiding outages on mutation paths. Re-throw non-RateLimiterRes rejections and only wrap the over-limit case.

🤖 Prompt for 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.

In `@src/auth/rate-limit.ts` around lines 42 - 49, In the rate-limit handling
inside the catch block, only convert genuine rate-limit rejections from
rate-limiter-flexible into RateLimitError and re-throw everything else. Update
the logic around the rejection check in the rate limit function so it
specifically recognizes a RateLimiterRes-like object with msBeforeNext, while
non-limiter Error failures from the limiter/store path are propagated unchanged
to avoid masking outages.

src/domain/firearms/service.ts-68-75 (1)

68-75: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Push the permission predicate into the read/write query itself.

authorizeUpdate, resolvePermission, and getVisibleIds run before a later UPDATE/SELECT that no longer constrains visibility. If a grant is revoked between those statements, the caller can still get one last read or write. Make the UPDATE/SELECT visibility-scoped so revokes take effect atomically.

Also applies to: 93-112

🤖 Prompt for 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.

In `@src/domain/firearms/service.ts` around lines 68 - 75, The permission check in
the firearm service is being done separately from the actual database
mutation/read, which leaves a race window after revocation. Update the
authorization flow in service.ts so the visibility predicate from
authorizeUpdate, resolvePermission, and getVisibleIds is embedded directly into
the UPDATE/SELECT query built on tx, rather than checked first and applied
later. Make the affected methods return or use a scoped query that includes the
permission filter atomically, so revokes take effect before the write/read
completes.

src/domain/magazines/__tests__/compatibility.test.ts-4-12 (1)

4-12: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Lazy-load the DB fixtures behind the DATABASE_URL gate.

describe.skip will not protect this suite because @/src/db/client and @/src/test-support/factories are evaluated before Line 27 runs. Since the DB client validates DATABASE_URL at import time, this file can still fail in no-DB runs instead of being skipped. Mirror the lazy beforeAll import pattern already used in src/domain/reference/__tests__/reference.test.ts.

♻️ Suggested shape

-import { db } from "`@/src/db/client`";
-import { magazineFirearm } from "`@/src/db/schema`";
-import {
-  createUser,
-  deleteUsers,
-  makeFirearm,
-  makeMagazine,
-} from "`@/src/test-support/factories`";
+import type { Database } from "`@/src/db/client`";
+import type * as FactoriesType from "`@/src/test-support/factories`";
+
+let db: Database;
+let magazineFirearm: typeof import("`@/src/db/schema`").magazineFirearm;
+let createUser: typeof FactoriesType.createUser;
+let deleteUsers: typeof FactoriesType.deleteUsers;
+let makeFirearm: typeof FactoriesType.makeFirearm;
+let makeMagazine: typeof FactoriesType.makeMagazine;

beforeAll(async () => {
  const clientMod = await import("`@/src/db/client`");
  db = clientMod.db;
  const schemaMod = await import("`@/src/db/schema`");
  magazineFirearm = schemaMod.magazineFirearm;
  const factMod = await import("`@/src/test-support/factories`");
  createUser = factMod.createUser;
  deleteUsers = factMod.deleteUsers;
  makeFirearm = factMod.makeFirearm;
  makeMagazine = factMod.makeMagazine;
});

Also applies to: 27-27

🤖 Prompt for 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.

In `@src/domain/magazines/__tests__/compatibility.test.ts` around lines 4 - 12,
Lazy-load the DB-dependent imports in compatibility.test.ts so the suite can
safely skip when DATABASE_URL is absent. Move the top-level imports for db,
magazineFirearm, and the factory helpers behind a beforeAll dynamic import
pattern, similar to the reference.test.ts approach, and keep the suite gated by
the existing describe.skip logic. Use the imported symbols db, magazineFirearm,
createUser, deleteUsers, makeFirearm, and makeMagazine as mutable bindings
initialized inside beforeAll.

src/domain/magazines/filter.ts-43-48 (1)

43-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Scope compatibleFirearmId to the actor's visible firearms.

Line 43 lets any firearm UUID constrain the query before loadCompatibilityBatch() strips unseen firearm IDs, so a caller can probe hidden firearm↔magazine links by observing which visible magazines remain. Return [] for an unseen firearm ID (or intersect the subquery with the visible-firearm set) and reuse that same set when attaching compatibility.

Suggested fix

 export async function listMagazinesFiltered(
   actorId: string,
   filter: MagazineFilter,
 ): Promise<MagazineWithCompatibility[]> {
-  const visibleMagazines = await getVisibleIds(db, actorId, "magazine");
+  const [visibleMagazines, visibleFirearms] = await Promise.all([
+    getVisibleIds(db, actorId, "magazine"),
+    getVisibleIds(db, actorId, "firearm"),
+  ]);
   if (visibleMagazines.size === 0) return [];
 
   const conditions: SQL[] = [inArray(magazine.id, [...visibleMagazines])];
@@
   if (filter.compatibleFirearmId) {
+    if (!visibleFirearms.has(filter.compatibleFirearmId)) return [];
     const linked = db
       .select({ id: magazineFirearm.magazineId })
       .from(magazineFirearm)
       .where(eq(magazineFirearm.firearmId, filter.compatibleFirearmId));
     conditions.push(inArray(magazine.id, linked));
   }
@@
-  const visibleFirearms = await getVisibleIds(db, actorId, "firearm");
   const byMag = await loadCompatibilityBatch(
     db,
     visibleFirearms,
     rows.map((r) => r.id),
   );

🤖 Prompt for 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.

In `@src/domain/magazines/filter.ts` around lines 43 - 48, The compatibleFirearmId
filter in filterMagazines currently uses any firearm UUID before visibility is
enforced, which can leak hidden firearm↔magazine relationships. Update
filterMagazines to first scope compatibleFirearmId against the actor-visible
firearms set used by loadCompatibilityBatch(), and if the firearm is not visible
return no results or intersect the magazineFirearm subquery with that visible
set. Reuse the same visible-firearms set when attaching compatibility so the
filter and batch loading stay consistent.

src/domain/bulkadd/service.ts-44-46 (1)

44-46: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject non-integer count values at the validation boundary.

validateMagazine() only bounds the range today, so values like 1.5 or NaN can reach generateLabels(). That either creates the wrong number of rows (1.5 becomes 2 for non-empty prefixes) or throws on blank/whitespace prefixes via new Array(count). Please require a finite integer here and add a regression test.

🤖 Prompt for 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.

In `@src/domain/bulkadd/service.ts` around lines 44 - 46, The bulk-add validation
path in service.ts currently only checks range via validateMagazine(), so
non-integer counts like 1.5 or NaN can still reach generateLabels() and produce
incorrect output or throw. Update the validation boundary in the bulk-add
service to require a finite integer count before calling validateMagazine(), and
keep rejecting invalid counts with ValidationError. Add a regression test
covering fractional and NaN count inputs to ensure generateLabels() is never
reached for these cases.

src/domain/bulkadd/service.ts-48-49 (1)

48-49: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Resolve idempotency before charging the mutation budget.

A replay with the same idempotencyKey still calls mutationLimiter.consume(). Once the caller is near the limit, a legitimate retry can be rejected before withIdempotency() returns the stored result, which breaks the double-submit contract. Short-circuit idempotent replays first, or charge quota only inside the first execution path.

Also applies to: 114-116

🤖 Prompt for 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.

In `@src/domain/bulkadd/service.ts` around lines 48 - 49, In bulkadd/service.ts,
the bulk add flow is charging the mutation budget too early:
`mutationLimiter.consume()` runs before `withIdempotency()` can return a stored
replay result, so retries may be rejected even when they should short-circuit.
Update the `bulkAdd` path to resolve idempotent replays first using
`withIdempotency()` and only call `mutationLimiter.consume()` on the first
execution path, or otherwise ensure the quota charge happens after replay
detection; apply the same ordering fix anywhere else the same pattern appears.

src/domain/bulkadd/service.ts-69-94 (1)

69-94: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Sequence continuation still races for non-empty prefixes.

Two concurrent transactions for the same owner/prefix can both read the same highest label, compute the same start, and insert overlapping labels. That defeats the advertised collision-avoidance behavior; depending on the schema it either persists duplicates or turns into intermittent failures. Please serialize this section per (owner, labelPrefix) or back it with a DB uniqueness-plus-retry strategy.

🤖 Prompt for 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.

In `@src/domain/bulkadd/service.ts` around lines 69 - 94, The label sequence logic
in bulkadd service can still race when two transactions hit the same owner and
prefix at once, because the read of existing labels and the subsequent insert
are not protected. Update the create path around the existing label lookup and
label generation in the bulk add service to serialize per (owner, labelPrefix)
or rely on a uniqueness constraint with retry handling, so concurrent calls
cannot compute the same start value and insert overlapping labels.

components/ui/confirm-dialog.tsx-55-61 (1)

55-61: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Block Escape/backdrop dismissal while the confirm action is pending.

Lines 59-61 and 89-90 still call onCancel() during an in-flight destructive action. That closes the modal even though the mutation is still running, which defeats the pending guard you already apply to the footer buttons and can expose stale row actions before the first delete settles.

Suggested fix

      if (event.key === "Escape") {
        event.preventDefault();
-       onCancel();
+       if (!pending) onCancel();
        return;
      }
...
      onMouseDown={(event) => {
-       if (event.target === event.currentTarget) onCancel();
+       if (!pending && event.target === event.currentTarget) onCancel();
      }}

Also applies to: 89-90, 113-123

🤖 Prompt for 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.

In `@components/ui/confirm-dialog.tsx` around lines 55 - 61, In ConfirmDialog,
Escape key handling and backdrop dismissal still call onCancel() while the
destructive action is pending, which can close the modal too early. Update the
confirm-dialog logic in the useEffect onKey handler and the backdrop/cancel
paths to no-op when the pending flag is true, matching the disabled-state guard
already used by the footer buttons. Keep the modal open until the in-flight
action finishes, and ensure onCancel is only reachable when the confirm action
is not pending.

components/ui/field.tsx-5-8 (1)

5-8: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Expose the generated aria-describedby ids from Field.

Lines 29-30 create *-hint and *-error ids, but children only receives a raw ReactNode, so callers cannot discover those ids when wiring the control. That makes the accessibility contract described on Lines 5-8 impossible to satisfy and leaves downstream forms without a reliable way to associate help/error text with the focused input. Please switch this primitive to a render prop (or clone a single control child) so Field owns the aria-describedby/aria-invalid wiring.

Also applies to: 20-30, 37-51

🤖 Prompt for 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.

In `@components/ui/field.tsx` around lines 5 - 8, The Field primitive currently
generates hint/error ids internally but only accepts a raw ReactNode child, so
consumers cannot wire those ids into the control. Update Field to use a render
prop or clone a single control child so it can pass the generated describedBy
ids and apply the aria-invalid/aria-describedby wiring itself. Keep the
accessibility contract aligned with the Field component and its generated
controlId, hintId, and errorId values.

hooks/use-delete-confirmation.ts-40-56 (1)

40-56: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Catch thrown delete failures before cleanup.

Line 44 assumes remove() always resolves, but a server action can still reject on transport or unexpected server errors. When that happens, this shared delete flow skips the toast and never reaches Lines 54-55, so the dialog state and refresh path are left inconsistent.

Suggested fix

   function confirm() {
     const item = target;
     if (!item) return;
     startTransition(async () => {
-      const result = await remove(item.id);
-      if (result.ok) {
-        toast({
-          message: `${entityLabel} removed`,
-          detail: getName(item),
-          tone: "neutral",
-        });
-      } else {
-        toast({ message: result.error ?? "Could not delete.", tone: "danger" });
-      }
-      setTarget(null);
-      router.refresh();
+      try {
+        const result = await remove(item.id);
+        if (result.ok) {
+          toast({
+            message: `${entityLabel} removed`,
+            detail: getName(item),
+            tone: "neutral",
+          });
+          router.refresh();
+        } else {
+          toast({ message: result.error ?? "Could not delete.", tone: "danger" });
+        }
+      } catch {
+        toast({ message: "Could not delete.", tone: "danger" });
+      } finally {
+        setTarget(null);
+      }
     });
   }

🤖 Prompt for 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.

In `@hooks/use-delete-confirmation.ts` around lines 40 - 56, The confirm flow in
useDeleteConfirmation assumes remove(item.id) always resolves, but a rejected
server action can skip the toast and leave cleanup unfinished. Update confirm()
to catch failures around the remove call inside startTransition, using the
existing remove, toast, setTarget, and router.refresh flow so both success and
thrown errors still clear the target and refresh appropriately. Keep the error
handling within confirm() consistent with the current result.ok branch, and
ensure unexpected exceptions produce a danger toast before cleanup.

docker-compose.yml-35-36 (1)

35-36: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Don't build the internal DATABASE_URL from a raw password.

These URLs break as soon as POSTGRES_PASSWORD contains reserved URI characters like @, :, /, ?, #, or %, which is common for generated secrets. That turns a valid password into a startup failure for both migrate and app. Prefer a pre-encoded connection string or discrete PG env vars over assembling the URI here.

Also applies to: 47-48

🤖 Prompt for 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.

In `@docker-compose.yml` around lines 35 - 36, The DATABASE_URL assembly in the
environment block is using POSTGRES_PASSWORD directly, which breaks when the
password contains URI-reserved characters. Update the docker-compose
configuration for the app and migrate services to stop constructing the
connection string inline from raw credentials; instead use a pre-encoded
DATABASE_URL or pass discrete PostgreSQL env vars and let the client build the
connection safely. Use the existing DATABASE_URL entries in the compose service
definitions to locate and replace this pattern consistently.

docker-compose.yml-20-23 (1)

20-23: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Bind Postgres to loopback instead of every host interface.

This publishes the database on 0.0.0.0, so it is reachable from the LAN even though only the Docker network and host-local tooling need it. Binding to 127.0.0.1 keeps the localhost workflow from .env.example without unnecessarily exposing Postgres.

Suggested change

-      - "${POSTGRES_HOST_PORT:-5544}:5432"
+      - "127.0.0.1:${POSTGRES_HOST_PORT:-5544}:5432"

🤖 Prompt for 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.

In `@docker-compose.yml` around lines 20 - 23, The Postgres service is currently
published on all host interfaces; update the ports mapping in the docker-compose
service so the host side binds to loopback only while keeping the same
configurable host port and container port. Preserve the existing localhost
workflow from the .env.example defaults, and keep the app-to-db communication
unchanged via the service name inside the Docker network.

src/db/__tests__/client.test.ts-48-74 (1)

48-74: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Don’t point the live suite at the app’s default DATABASE_URL.

This test block runs real migrations against whatever DATABASE_URL is set to, and package.json exposes it behind plain bun test. That makes routine test runs capable of mutating a developer’s configured app database. Prefer a dedicated TEST_DATABASE_URL or an explicit opt-in flag for the live suite.

🤖 Prompt for 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.

In `@src/db/__tests__/client.test.ts` around lines 48 - 74, The live Postgres
suite in client.test.ts is using the app’s default DATABASE_URL, which can
mutate a developer’s real database when running bun test. Update the liveDb
setup to require a dedicated TEST_DATABASE_URL or an explicit opt-in gate, and
make the Pool initialization in the live test block read from that test-only
source instead of DATABASE_URL.

src/db/idempotency.ts-31-32 (1)

31-32: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Anchor the expiry window to the stored result, not the claim timestamp.

expiresAt is fixed before action(tx) runs. If that transaction stays open past the 5-minute window, a retry that starts after expiry can unblock on the unique index, reclaim the row immediately, and execute the write again. That breaks the exactly-once contract for callers like src/domain/bulkadd/service.ts:114-117.

Suggested fix

     if (claimed.length > 0) {
       const result = await action(tx);
+      const replayUntil = new Date(Date.now() + IDEMPOTENCY_WINDOW_MS);
       await tx
         .update(idempotency)
-        .set({ result: result as unknown })
+        .set({ result: result as unknown, expiresAt: replayUntil })
         .where(
           and(
             eq(idempotency.userId, userId),

Also applies to: 46-50

🤖 Prompt for 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.

In `@src/db/idempotency.ts` around lines 31 - 32, The idempotency expiry is being
computed before `action(tx)` finishes, so a long-running transaction can let a
retry reclaim the row and re-execute the write. Update the flow in
`src/db/idempotency.ts` so `expiresAt` is anchored to when the result is
actually stored, not when the claim is created. Adjust both the initial
claim/insert path and the update/return path around `action(tx)` to derive the
expiry from the stored result time, keeping the unique-index claim valid until
the write is durably persisted.

src/db/migrations/0001_curious_devos.sql-13-25 (1)

13-25: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Add a grant(parent_type, parent_id) index before shipping the cleanup trigger.

src/db/migrations/0002_grant_cleanup_triggers.sql deletes grant rows with WHERE parent_type = ... AND parent_id = OLD.id, but this migration only creates grantee_id-prefixed indexes. Those do not support the trigger predicate, so every firearm/magazine delete will seq-scan grant, and owner cascades amplify that cost.

Suggested change

 CREATE INDEX "firearm_owner_id_idx" ON "firearm" USING btree ("owner_id");--> statement-breakpoint
 CREATE INDEX "grant_grantee_parent_type_idx" ON "grant" USING btree ("grantee_id","parent_type");--> statement-breakpoint
+CREATE INDEX "grant_parent_type_parent_id_idx" ON "grant" USING btree ("parent_type","parent_id");--> statement-breakpoint
 CREATE INDEX "idempotency_expires_at_idx" ON "idempotency" USING btree ("expires_at");--> statement-breakpoint
 CREATE INDEX "magazine_owner_id_idx" ON "magazine" USING btree ("owner_id");--> statement-breakpoint

Also applies to: 67-67

🤖 Prompt for 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.

In `@src/db/migrations/0001_curious_devos.sql` around lines 13 - 25, The `grant`
table is missing an index that matches the cleanup trigger predicate, so deletes
in `0002_grant_cleanup_triggers.sql` will scan the whole table. Add a composite
index on `grant(parent_type, parent_id)` in `0001_curious_devos.sql`, alongside
the existing `grant_grantee_parent_unique` and other constraints, so the
trigger’s `WHERE parent_type = ... AND parent_id = OLD.id` lookup is efficient.

🟡 Minor comments (15)

docs/plans/2026-06-28-001-feat-homelab-web-replatform-plan.md-308-311 (1)

308-311: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Align the proxy sequence diagram with KTD-6.

These lines show proxy.ts doing full session resolution, but KTD-6 says the proxy only performs an optimistic cookie/signature check and defers DB-backed session lookup to server components/actions. Keeping both versions in the same plan will send implementers down the wrong path.

🤖 Prompt for 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.

In `@docs/plans/2026-06-28-001-feat-homelab-web-replatform-plan.md` around lines
308 - 311, The proxy sequence diagram currently implies full session resolution
in proxy.ts, which conflicts with KTD-6. Update the diagram so the proxy only
performs the optimistic cookie/signature check and passes the request through,
while the DB-backed session lookup and any redirect/login handling are shown in
server components/actions instead; use the proxy sequence section in the plan to
locate and align the flow.

docs/plans/2026-06-28-001-feat-homelab-web-replatform-plan.md-401-401 (1)

401-401: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the U2 file path for proxy.ts.

Line 401 points implementers to app/proxy.ts, but the same plan later requires proxy.ts at the repo root. That contradiction is easy to cargo-cult into a broken auth gate.

Suggested doc fix

-| U2 | Better Auth + operator accounts + login rate limiting | `auth.ts`, `app/api/auth/[...all]/route.ts`, `app/proxy.ts` | U1 |
+| U2 | Better Auth + operator accounts + login rate limiting | `auth.ts`, `app/api/auth/[...all]/route.ts`, `proxy.ts` | U1 |

🤖 Prompt for 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.

In `@docs/plans/2026-06-28-001-feat-homelab-web-replatform-plan.md` at line 401,
The U2 task references the wrong proxy file path and should be corrected to
match the repo-root `proxy.ts` used elsewhere in the plan. Update the path list
in this table entry so implementers are directed to the same `proxy.ts` location
referenced by the auth gate work, keeping `auth.ts` and
`app/api/auth/[...all]/route.ts` as-is and replacing `app/proxy.ts` with the
correct root-level symbol.

README.md-62-67 (1)

62-67: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Use the proxy URL here, not the raw HTTP port.

This step contradicts the HTTPS/reverse-proxy guidance below and in docker-compose.yml; it points readers at cleartext login on :3000. Please make the URL match the deployment requirement.

♻️ Suggested fix

-Open `http://<your-server>:3000/login`, sign in, and add the rest of the
+Open `https://<your-proxy-host>/login`, sign in, and add the rest of the
 accounts (staff, members, family) from the **Accounts** screen.

🤖 Prompt for 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.

In `@README.md` around lines 62 - 67, The login URL in the README setup steps
points to the raw HTTP port and conflicts with the HTTPS/reverse-proxy
requirement. Update the account-setup instructions to use the proxy-facing
https:// URL instead of http://<your-server>:3000/login, and keep it aligned
with the deployment guidance referenced by BETTER_AUTH_URL and
docs/deployment.md.

docs/deployment.md-29-31 (1)

29-31: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Point the sign-in step at the reverse proxy, not the app port.

This contradicts the TLS guidance in the same file and can lead readers to bypass the proxy entirely. Use the https:// origin you actually expose.

♻️ Suggested fix

-   Sign in at `http://<host>:${APP_HOST_PORT}/login`. All other accounts are
+   Sign in at `https://<your-proxy-host>/login`. All other accounts are
    created by an operator from the **Accounts** screen — there is no public
    sign-up.

🤖 Prompt for 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.

In `@docs/deployment.md` around lines 29 - 31, The sign-in instruction in the
deployment docs points users directly at the app port instead of the reverse
proxy, which conflicts with the TLS guidance. Update the sign-in URL in the
deployment documentation to use the exposed https origin behind the proxy,
keeping the rest of the Accounts-screen guidance unchanged.

app/(app)/grants/share-control.tsx-44-52 (1)

44-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clear stale error/state in reload().

After a failed share/revoke, a later successful reload() updates state but leaves the old error banner visible. The inverse also happens on load failure: the previous grants list is kept even though the refresh failed.

Suggested fix

   const reload = useCallback(() => {
     startTransition(async () => {
       const result = await loadShareState(parentType, parentId);
-      if (result.ok && result.data) setState(result.data);
-      else
-        setError(
-          result.ok ? null : (result.error ?? "Could not load sharing."),
-        );
+      if (result.ok && result.data) {
+        setState(result.data);
+        setError(null);
+        return;
+      }
+      setState(null);
+      setError(result.error ?? "Could not load sharing.");
     });
   }, [parentType, parentId]);

🤖 Prompt for 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.

In `@app/`(app)/grants/share-control.tsx around lines 44 - 52, In reload(), update
both pieces of UI state together so stale data is cleared after each refresh
attempt. When loadShareState succeeds in ShareControl, set the new state and
explicitly clear error; when it fails, set the error and also clear the existing
state so the previous grants list is not left visible. Use the existing reload
callback and setState/setError logic in share-control.tsx to keep error and
state in sync.

app/(app)/magazines/magazine-form.tsx-198-219 (1)

198-219: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Don't expose an incomplete tabs pattern.

This control advertises tablist/tab semantics, but it doesn't provide the rest of the tabs contract (aria-controls/tabpanel mapping, roving focus, arrow-key navigation). Screen readers will interpret it as tabs while keyboard behavior stays button-like. Either implement full tabs behavior or drop the tab roles and use plain buttons with aria-pressed.

Proposed fix

         <div
           className="inline-flex w-fit rounded-[var(--radius)] border border-line-strong bg-paper-sunken p-0.5"
-          role="tablist"
-          aria-label="Add mode"
+          aria-label="Add mode"
         >
           {(["single", "bulk"] as const).map((m) => (
             <button
               key={m}
               type="button"
-              role="tab"
-              aria-selected={mode === m}
+              aria-pressed={mode === m}
               onClick={() => setMode(m)}
               className={cn(

🤖 Prompt for 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.

In `@app/`(app)/magazines/magazine-form.tsx around lines 198 - 219, The mode
switch in magazine-form.tsx is exposing incomplete tabs semantics: the
tablist/tab roles in the mode buttons imply full tabs behavior, but the
component does not implement the required tab contract. Either update the
control to a real tabs pattern with matching panel linkage and keyboard/focus
behavior, or remove the tablist/tab roles from the button group and make the
`mode` toggles plain buttons using `aria-pressed` for state. Use the existing
`mode`/`setMode` mapping to locate the control.

src/auth/__tests__/authorize.test.ts-2-4 (1)

2-4: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove optional chaining so cascade failures cannot pass silently.

db.query.grant?.findFirst?.(...) returns undefined if the relational query is not wired, making this assertion pass without checking cleanup. Query the grant table directly.

Proposed test fix

-import { eq } from "drizzle-orm";
+import { and, eq } from "drizzle-orm";
 import { db } from "`@/src/db/client`";
-import { firearm } from "`@/src/db/schema`";
+import { firearm, grant } from "`@/src/db/schema`";
@@
-    const stillVisible = await db.query.grant?.findFirst?.({
-      where: (g, { eq: e, and: a }) =>
-        a(e(g.parentId, fa.id), e(g.parentType, "firearm")),
-    });
-    expect(stillVisible).toBeFalsy();
+    const stillVisible = await db
+      .select({ id: grant.id })
+      .from(grant)
+      .where(and(eq(grant.parentId, fa.id), eq(grant.parentType, "firearm")));
+    expect(stillVisible).toHaveLength(0);

Also applies to: 97-101

🤖 Prompt for 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.

In `@src/auth/__tests__/authorize.test.ts` around lines 2 - 4, The test in
authorize.test.ts is using optional chaining on
db.query.grant?.findFirst?.(...), which can hide a missing relational query and
let cleanup assertions pass silently. Update the affected assertions to query
the grant table directly through the db instance, using the existing db and eq
helpers, so the checks in the authorize test and the related cleanup assertion
are enforced even if the relational query layer is not wired.

src/domain/csv/__tests__/serialize.test.ts-54-59 (1)

54-59: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

The CR branch is not actually tested.

The test name claims carriage-return coverage, but the loop only exercises =, +, -, @, and \t. Add "\r" (or a dedicated assertion) so regressions in the injection guard are caught.

🤖 Prompt for 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.

In `@src/domain/csv/__tests__/serialize.test.ts` around lines 54 - 59, The CSV
injection guard test is missing actual carriage-return coverage even though the
test name claims it, so update the `serializeMagazinesCsv`/`lines` test case to
include `"\r"` as a first character (or add a separate assertion for it)
alongside the existing `=`, `+`, `-`, `@`, and `\t` cases.

src/domain/magazines/__tests__/service.test.ts-110-112 (1)

110-112: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Clean up the one-off empty user.

Line 111 creates a persisted test user that never gets deleted, so repeated live runs keep accumulating rows in the shared DB. Delete it in the test itself or add it to the afterAll cleanup.

Suggested fix

   test("list orders by brand/model ascending, scoped to visibility; empty is [] (R27/R68)", async () => {
-    const empty = await listMagazines(await createUser("empty"));
-    expect(empty).toEqual([]);
+    const emptyUser = await createUser("empty");
+    try {
+      const empty = await listMagazines(emptyUser);
+      expect(empty).toEqual([]);
+    } finally {
+      await deleteUsers(emptyUser);
+    }
 
     await createMagazine(userA, {
       brandModel: "Zeta",

🤖 Prompt for 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.

In `@src/domain/magazines/__tests__/service.test.ts` around lines 110 - 112, The
test creates a persisted user with createUser("empty") in the listMagazines test
but never cleans it up, causing shared DB rows to accumulate. Update this test
in service.test.ts to either delete the created user after the assertion or
register it with the existing afterAll cleanup used in the magazine service
tests, so the test remains self-contained and does not leave behind data.

components/ui/feedback.tsx-64-80 (1)

64-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the live-region role conditional.

Callout always uses role="alert", but this component also supports "neutral", "blaze", and "ok". Those variants will be announced as urgent interruptions even when the content is informational. Default to no live role (or status) and reserve alert for the destructive/error case.

🤖 Prompt for 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.

In `@components/ui/feedback.tsx` around lines 64 - 80, Callout currently hardcodes
role="alert", which makes every tone announce as urgent even for non-error
variants. Update the Callout component so the live-region role is conditional
based on tone, using alert only for the danger/destructive case and no live role
or status for neutral, blaze, and ok. Use the Callout function and Tone/TONES
mapping to locate the role assignment and adjust it accordingly.

hooks/use-row-flash.ts-25-29 (1)

25-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Re-arm the flash when the same row is updated twice.

Line 26 becomes a no-op when flash(id) is called again before the first flash expires for that same id. Because TRow only derives data-flash from item.id === flashId in components/ui/table.tsx:50-71, the second edit never restarts the row animation.

💡 Proposed fix

 export function useRowFlash() {
   const [flashId, setFlashId] = useState<string | null>(null);
+  const frame = useRef<number | null>(null);
   const timer = useRef<ReturnType<typeof setTimeout> | null>(null);

   useEffect(
     () => () => {
+      if (frame.current) cancelAnimationFrame(frame.current);
       if (timer.current) clearTimeout(timer.current);
     },
     [],
   );

   function flash(id: string) {
-    setFlashId(id);
     if (timer.current) clearTimeout(timer.current);
-    timer.current = setTimeout(() => setFlashId(null), FLASH_MS);
+    if (frame.current) cancelAnimationFrame(frame.current);
+    setFlashId(null);
+    frame.current = requestAnimationFrame(() => {
+      setFlashId(id);
+      timer.current = setTimeout(() => setFlashId(null), FLASH_MS);
+    });
   }

🤖 Prompt for 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.

In `@hooks/use-row-flash.ts` around lines 25 - 29, The flash timer in flash(id) is
not re-arming when the same row id is passed again before the previous timeout
expires, so the row animation does not restart. Update useRowFlash so flash(id)
always resets the flash state even for the same id, and make sure the timer is
cleared and restarted in a way that re-triggers the data-flash change used by
TRow in components/ui/table.tsx.

src/db/inventory-schema.ts-96-102 (1)

96-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a non-negative CHECK for magazine_firearm.ordinal.

ordinal drives compatibility ordering, but the DB currently accepts negative values. A bad write or import would then sort ahead of every valid entry. The schema already uses CHECK constraints for other numeric invariants, so this needs the same backstop.

Suggested change

   (t) => [
     // Composite PK prevents duplicate (magazine, firearm) pairs (R34 backstop).
     primaryKey({ columns: [t.magazineId, t.firearmId] }),
+    check("magazine_firearm_ordinal_min", sql`${t.ordinal} >= 0`),
     index("magazine_firearm_firearm_id_idx").on(t.firearmId),
   ],

🤖 Prompt for 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.

In `@src/db/inventory-schema.ts` around lines 96 - 102, Add a CHECK constraint to
the magazine_firearm table so ordinal cannot be negative. Update the inventory
schema definition in the table builder that defines ordinal, using the same
CHECK-constraint pattern already used for other numeric invariants in
src/db/inventory-schema.ts, and ensure the constraint is enforced alongside the
existing primaryKey and index declarations.

proxy.ts-19-20 (1)

19-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the original query string in redirectTo.

Using only request.nextUrl.pathname drops filter/search state, so deep links won’t round-trip after login.

Suggested fix

-    loginUrl.searchParams.set("redirectTo", request.nextUrl.pathname);
+    loginUrl.searchParams.set(
+      "redirectTo",
+      `${request.nextUrl.pathname}${request.nextUrl.search}`,
+    );

🤖 Prompt for 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.

In `@proxy.ts` around lines 19 - 20, The redirectTo value in proxy handling
currently uses only request.nextUrl.pathname, which loses any existing query
string state. Update the login redirect logic around the loginUrl construction
to preserve the full original destination, including search params, so deep
links round-trip correctly after login. Use the request/nextUrl values in
proxy.ts to build redirectTo from the full path plus query string instead of
pathname alone.

scripts/seed-admin.ts-26-41 (1)

26-41: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Make the advertised idempotency race-safe.

This is still a check-then-create sequence: two concurrent runs can both miss findFirst() and then race into createUser(). Because user.email is unique, one invocation will fail instead of cleanly becoming a no-op, so the bootstrap is not actually idempotent under retry/concurrent execution. Catch duplicate-email creation and treat it as success, or move the guard to an atomic create path.

🤖 Prompt for 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.

In `@scripts/seed-admin.ts` around lines 26 - 41, The idempotency check in the
seed-admin flow is still a non-atomic check-then-create race, so concurrent runs
can both pass the existing-user lookup and then collide in auth.api.createUser.
Update the logic around the existing lookup/createUser sequence to handle
duplicate-email creation safely by catching the unique-email conflict and
treating it as a successful no-op, or replace the current guard with an atomic
create-if-missing path so the seeding stays race-safe.

scripts/seed-admin.ts-29-30 (1)

29-30: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Redact the bootstrap email in logs.

ADMIN_EMAIL is operator PII, and both branches write it verbatim to container logs. A generic message or a redacted address keeps the bootstrap observable without leaking the admin identifier.

Also applies to: 42-42

🤖 Prompt for 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.

In `@scripts/seed-admin.ts` around lines 29 - 30, The bootstrap logging in
seed-admin.ts currently prints ADMIN_EMAIL verbatim in both the existing-account
and creation paths, which leaks operator PII into container logs. Update the log
statements around the existing check and the admin creation flow to use a
generic message or a redacted identifier instead of the raw email, keeping the
script observable without exposing the admin address.

Source: Linters/SAST tools