Skip to content

feat(accessories): per-firearm accessories tracker (#8) - #59

Merged
unclesp1d3r merged 18 commits into
mainfrom
8-accessories-tracker-per-firearm-log-aftermarket-parts
Jul 9, 2026
Merged

unclesp1d3r merged 18 commits into
mainfrom
8-accessories-tracker-per-firearm-log-aftermarket-parts

Conversation

@unclesp1d3r

@unclesp1d3r unclesp1d3r commented Jul 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

Adds a per-firearm accessories tracker (closes #8): aftermarket parts (optics, suppressors, triggers, barrels, grips, lights, slings…) as a fourth owner-scoped inventory item that mounts to one firearm at a time, moves between firearms while keeping its identity/cost/serial, carries an NFA flag, and rolls up a per-firearm value total. Range sessions snapshot which accessories were mounted, seeding future range-performance logging.

Shaped through /ce-brainstorm → /ce-doc-review → /ce-plan; the implementation-ready plan is at docs/plans/2026-07-07-001-feat-accessories-tracker-plan.md (R1–R19, KTD1–KTD7).

Impact 54 files · 17 commits (8 feat, 3 test, 3 docs, 2 fix, 1 refactor)
Functional surface ~20 source files; the rest are Drizzle migration snapshots, the plan doc, and binary demo images
Risk 🟡 Medium — new owned entity + a bespoke auth seam — but additive, no breaking changes, and heavily tested
Migrations 0011 (accessory + firearm.is_nfa), 0012 (range-session join), 0013 (installed-date CHECK)
Tests 413 unit/integration + 30 Playwright e2e, just ci-check green
Review time ~25–35 min — start with the two files under "Where to focus"

Where to focus (review guide)

The feature is 90% a straightforward mirror of the existing Ammo entity. The one novel, security-sensitive seam is the visibility model — start there:

  1. src/auth/accessory-visibility.ts — accessories are not a grant ParentType. A mounted accessory inherits its firearm's visibility (view/edit); an unmounted one is owner-only. authorizeMount requires firearm-edit and a same-owner target (closes a cross-tenant mount/leak). This is the file to scrutinize.
  2. src/domain/accessories/service.ts — CRUD + mount/reassign/unmount + the bespoke delete (accessories have no grants).
  3. src/domain/range-sessions/service.ts — the range_session_accessory snapshot, accessoryRoundsFired (scoped to visible firearms), and listSessionAccessories (per-viewer field gating — no leak of a since-unmounted accessory).
  4. src/db/inventory-schema.ts + migrations — the new tables and CHECKs.

Everything else (app/(app)/accessories/*, the firearm-detail integration, validation/constants) follows the Ammo/firearm patterns.


What changed

🔧 Data & domain

  • accessory table (owner-scoped, nullable current_firearm_id FK set null, cost_cents, is_nfa, sensitive serial_number), firearm.is_nfa, and a range_session_accessory join (surrogate PK, set null).
  • Accessory validation (free-text category + suggestions), CRUD service, and the inherit-from-mount visibility/authorization layer.
  • Range-session ↔ accessory linkage + per-accessory rounds-fired derivation.

🎨 UI

  • Top-level Accessories surface (/accessories list + detail + form) with nav entry, mirroring Ammo.
  • Firearm detail gains a mounted-accessories section + derived value total; firearm NFA flag; "Add accessory" pre-fills the mount target.

✅ Tests

  • Integration tests for the visibility/authz seam (incl. the adversarial cross-tenant-mount rejection and R7 range-session gating), service CRUD, validation boundaries, schema constraints.
  • New e2e/accessories.spec.ts covering AE1 (sharing inheritance), AE2 (valuation), AE3 (move), AE4 (CSV exclusion), AE5 (NFA display).

📝 Docs & tooling

  • README accessories showcase + refreshed demo GIF and light/dark screenshots (current shadcn-token UI).
  • Reusable demo-image generator: just demo-images regenerates every README asset from the live UI (specs in e2e/demo-*.spec.ts sharing e2e/fixtures/demo-seed.ts, gated behind DEMO=1).

Key design decisions

  • Inherit-from-mount sharing, not a fourth grant target. No new grant plumbing; sharing a firearm shares its mounted accessories. Simpler and airtight (KTD1).
  • Cost as integer minor units (cost_cents) — matches the repo's integer-column convention; no floating-point money (KTD2).
  • Category free-text with suggestions (Ammo pattern), not an enforced taxonomy (KTD3).
  • installed_date is coupled to mount — a DB CHECK + service guards keep it null while unmounted (R6).
  • Cross-tenant mount guard — an accessory can only mount on a firearm owned by its owner (KTD5).

Testing

  • just ci-check green: biome lint + format + typecheck + pre-commit + 413 unit/integration + 30 Playwright e2e (Testcontainers Postgres + Docker).
  • Two independent review passes (an inline review + the multi-agent /review-pr toolkit) plus Copilot + CodeRabbit — all findings addressed (security scoping on accessoryRoundsFired, N+1 batching, category trim, empty-string firearmId normalization, a shared mount-context helper, and copy/comment fixes).

Breaking changes

None. Purely additive — new table/columns, new routes, new nav entry. The grant parent_type set is unchanged. Accessories are absent from CSV export, so the "serials never exported" rule (R13) holds by construction.

Out of scope (deferred)

Owner-wide valuation rollup, a general mount-history timeline, photos/documents/service-intervals/shot-count, independent per-accessory sharing, and surfacing the range-session accessory list in the UI (the data is captured for future range-performance logging).

Review checklist

  • Visibility seam (accessory-visibility.ts): mounted inherits firearm, unmounted is owner-only, cross-tenant mount rejected
  • Delete permission split (R9): owner-or-firearm-edit for mounted, owner-only for unmounted
  • Migrations apply cleanly and the CHECKs (cost_cents >= 0, installed-date-requires-mount) are correct
  • No data-testid (repo rule) — UI targeted via ARIA/roles ✓
  • Serials stay out of any export surface ✓

…d their association with firearms

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Enrich the accessories requirements plan with Planning Contract, 8 implementation units, verification contract, and DoD; resolve outstanding questions and apply review fixes.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U1: owner-scoped accessory table (nullable current_firearm_id set-null, cost_cents, is_nfa, empty-not-null text) + firearm.is_nfa; 0011 migration; schema tests.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U2: validateAccessory (category required, cost_cents int4-bounded, installed_date) + free-text category suggestions incl suppressor.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U3: listVisibleAccessoryIds/resolveAccessoryPermission inherit from the mounted firearm; authorizeMount requires firearm-edit + same-owner target (closes cross-tenant mount). Accessory is not a grant ParentType.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U4: create/update/get/list scoped via inherited visibility; mountAccessory resets installed_date on reassign; delete of a mounted accessory follows firearm-edit permission.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
#8)

U6: firearm detail shows mounted accessories + derived value total; firearm gains an NFA flag; listMountedForFirearm/firearmAccessoryValueCents service helpers.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U5: accessories list/detail/form/actions mirroring ammo; NFA + cost + mount selector; firearm-detail 'Add accessory' pre-fills the mount target (F1); Accessories nav entry.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U7: range_session_accessory join snapshots mounted accessories on session create (surrogate PK, accessory set-null); accessoryRoundsFired + visibility-gated listSessionAccessories (R7/R19).
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U8: accessories e2e (create/move, sharing inheritance AE1, valuation AE2, NFA display AE5, CSV exclusion AE4) via ARIA locators; makeAccessory factory.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Code review follow-up: compute the mounted-accessory value total from the already-fetched rows on the firearm detail page instead of re-querying; remove the now-unused firearmAccessoryValueCents helper.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Copilot AI review requested due to automatic review settings July 8, 2026 04:54
@unclesp1d3r unclesp1d3r linked an issue Jul 8, 2026 that may be closed by this pull request
2 tasks
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 40b1747f-345a-4c5b-9c76-ea49bce44480

📥 Commits

Reviewing files that changed from the base of the PR and between 1d0ac2b and 34737ba.

📒 Files selected for processing (8)
  • app/(app)/accessories/[id]/page.tsx
  • app/(app)/accessories/page.tsx
  • app/(app)/firearms/firearm-form.tsx
  • src/auth/__tests__/accessory-visibility.test.ts
  • src/domain/accessories/service.ts
  • src/domain/firearms/mount-options.ts
  • src/domain/range-sessions/service.ts
  • src/test-support/factories.ts

Walkthrough

Adds accessory storage, visibility, CRUD, mount and unmount flows, firearm detail integration, range-session accessory linkage, demo assets, and end-to-end coverage. Firearms now carry an isNfa flag, and accessory serials remain excluded from CSV output.

Changes

Accessories Tracker

Layer / File(s) Summary
Schema, migrations, and validation
src/db/inventory-schema.ts, src/db/migrations/0011_daffy_microbe.sql, src/db/migrations/0012_dapper_reavers.sql, src/db/migrations/0013_mature_christian_walker.sql, src/db/migrations/meta/*, src/db/__tests__/schema.test.ts, src/domain/accessories/validate.ts, src/domain/accessories/constants.ts, src/domain/accessories/__tests__/validate.test.ts, src/domain/validation-messages.ts, src/domain/firearms/service.ts
Adds accessory and range-session accessory tables, firearm isNfa, migration snapshots and journal entries, accessory validation and messages, category suggestions, and schema and validation tests.
Accessory auth and domain service
src/auth/accessory-visibility.ts, src/auth/__tests__/accessory-visibility.test.ts, src/domain/accessories/service.ts, src/domain/accessories/display.ts, src/test-support/factories.ts, src/domain/accessories/__tests__/service.test.ts
Adds accessory visibility and mount authorization, CRUD and mount and unmount service methods, display helpers, a test factory, and service and auth coverage.
Accessories pages and actions
app/(app)/accessories/actions.ts, app/(app)/accessories/page.tsx, app/(app)/accessories/accessories-view.tsx, app/(app)/accessories/[id]/page.tsx, app/(app)/accessories/accessory-detail-view.tsx, app/(app)/accessories/accessory-form.tsx, app/(app)/app-shell.tsx
Adds accessory server actions, list and detail pages, create and edit form, mount controls, delete flows, and the accessories nav entry.
Firearm detail integration
app/(app)/firearms/[id]/page.tsx, app/(app)/firearms/firearm-detail-view.tsx, app/(app)/firearms/mounted-accessories.tsx, app/(app)/firearms/firearm-form.tsx, app/(app)/firearms/page.tsx
Adds mounted accessory summaries, accessory valuation totals, NFA display, and firearm form and list-item NFA support.
Range-session accessory linkage
src/domain/range-sessions/service.ts, src/domain/range-sessions/__tests__/service.test.ts
Snapshots mounted accessories at session creation and adds accessory round totals plus session-accessory listing APIs with tests.
End-to-end accessories workflow
e2e/accessories.spec.ts, e2e/fixtures/user-pool.ts
Adds the accessories Playwright suite and seeded users for create, mount, move, sharing, visibility, not-found, and CSV coverage.
Demo assets and docs
docs/plans/2026-07-07-001-feat-accessories-tracker-plan.md, README.md, e2e/demo-accessories.spec.ts, e2e/demo-magazines.spec.ts, e2e/demo-summary.spec.ts, e2e/demo-walkthrough.spec.ts, e2e/fixtures/demo-seed.ts, justfile
Adds the planning document, README updates, demo screenshot and walkthrough tests, shared demo fixtures, and the demo-images recipe.

Possibly related PRs

Suggested labels: enhancement, backend, frontend, shared, testing, documentation

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Adds range-session accessory snapshot/linkage APIs, which go beyond #8’s accessory CRUD and sharing requirements. Move the range-session/history work to a separate PR unless it is explicitly part of #8’s acceptance criteria.
Docstring Coverage ⚠️ Warning Docstring coverage is 74.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the accessories tracker change.
Description check ✅ Passed Mostly follows the template with summary, changes, and test plan; missing the explicit Related issue, AI disclosure, and checklist sections.
Linked Issues check ✅ Passed Implements the accessory CRUD, mount permissions, firearm-sharing inheritance, firearm detail UI, and CSV exclusion required by #8.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch 8-accessories-tracker-per-firearm-log-aftermarket-parts

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Notion


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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new owner-scoped Accessories inventory entity that can be mounted to a firearm (with inherited visibility), tracked for cost/NFA status, and snapshotted onto range sessions to seed future performance/round-count derivations.

Changes:

  • Add accessory and range_session_accessory tables + firearm.is_nfa, with migrations and schema tests.
  • Implement accessory domain layer: validation/messages, visibility/authz seam, CRUD + mount/unmount, range-session snapshot + derived helpers.
  • Add UI surfaces: top-level /accessories list/detail/form, firearm detail “Mounted accessories” section + valuation total, plus Playwright e2e coverage.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/test-support/factories.ts Adds shared makeAccessory factory for tests.
src/domain/validation-messages.ts Adds accessory-related validation messages.
src/domain/range-sessions/service.ts Snapshots mounted accessories on session create; adds per-accessory/session accessory APIs.
src/domain/range-sessions/tests/service.test.ts Adds integration tests for range session ↔ accessory linkage behaviors.
src/domain/firearms/service.ts Persists firearm.isNfa on create/update.
src/domain/accessories/validate.ts Adds pure accessory validation + bounds.
src/domain/accessories/service.ts Implements accessory CRUD + mount/unmount + listing.
src/domain/accessories/display.ts Adds accessory display name + money parsing/format helpers.
src/domain/accessories/constants.ts Adds suggested category seed list.
src/domain/accessories/tests/validate.test.ts Unit tests for accessory validation and category suggestions.
src/domain/accessories/tests/service.test.ts Integration tests for accessory service behavior and permissions.
src/db/migrations/meta/0012_snapshot.json Drizzle snapshot update for new join table.
src/db/migrations/meta/0011_snapshot.json Drizzle snapshot update for accessory + firearm NFA.
src/db/migrations/meta/_journal.json Records new migrations in Drizzle journal.
src/db/migrations/0012_dapper_reavers.sql Migration creating range_session_accessory.
src/db/migrations/0011_daffy_microbe.sql Migration creating accessory + adding firearm.is_nfa.
src/db/inventory-schema.ts Defines accessory + range_session_accessory + firearm.is_nfa.
src/db/tests/schema.test.ts Schema-level tests for accessory defaults, constraints, and FK behaviors.
src/auth/accessory-visibility.ts Implements inherit-from-mount visibility + mount authorization (same-owner constraint).
src/auth/tests/accessory-visibility.test.ts Integration tests for accessory visibility and mount authorization.
e2e/fixtures/user-pool.ts Adds e2e users for accessory scenarios.
e2e/accessories.spec.ts End-to-end coverage for accessory CRUD/mounting/sharing/CSV serial exclusion.
docs/plans/2026-07-07-001-feat-accessories-tracker-plan.md Adds implementation-ready plan/contract for the feature.
app/(app)/firearms/page.tsx Threads isNfa through firearm list shaping.
app/(app)/firearms/mounted-accessories.tsx New client component for mounted accessories list + valuation total.
app/(app)/firearms/firearm-form.tsx Adds firearm NFA checkbox input.
app/(app)/firearms/firearm-detail-view.tsx Renders firearm NFA badge + mounted accessories section.
app/(app)/firearms/[id]/page.tsx Fetches mounted accessories and derives accessory value total.
app/(app)/app-shell.tsx Adds “Accessories” to nav.
app/(app)/accessories/page.tsx New accessories index page; wires list + editable firearm options + mount prefill.
app/(app)/accessories/actions.ts Server actions for accessory create/update/delete/mount.
app/(app)/accessories/accessory-form.tsx Client form for creating/editing accessories (incl. cost parsing, NFA, mount-on-create).
app/(app)/accessories/accessory-detail-view.tsx Client detail view with mount control + edit-in-place + delete flow.
app/(app)/accessories/accessories-view.tsx Client list/table view with create flow and owner-only row delete.
app/(app)/accessories/[id]/page.tsx Accessory detail page with UUID boundary guard + permission-aware rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/(app)/accessories/accessory-detail-view.tsx Outdated
Comment thread app/(app)/firearms/firearm-form.tsx
Comment thread app/(app)/firearms/mounted-accessories.tsx Outdated
Comment thread app/(app)/firearms/firearm-detail-view.tsx
Comment thread src/domain/range-sessions/service.ts Outdated
Comment thread src/test-support/factories.ts
Comment thread src/auth/__tests__/accessory-visibility.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (3)
app/(app)/accessories/accessories-view.tsx (1)

58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

orDash duplicated across accessories-view.tsx and accessory-detail-view.tsx.

Same implementation appears verbatim in accessory-detail-view.tsx (lines 53-59). Worth hoisting into a shared UI helper (this pattern is already reused per the "mirrors ammo view" comments elsewhere), but it's a small, low-risk duplication.

🤖 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)/accessories/accessories-view.tsx around lines 58 - 64, The orDash
helper is duplicated in accessories-view.tsx and accessory-detail-view.tsx, so
hoist that shared formatting logic into a reusable UI helper and import it in
both views. Update the existing orDash usage in the accessories view and the
matching implementation in accessory-detail-view.tsx to reference the shared
helper so the identical trimmed-string-to-dash behavior lives in one place.
src/db/inventory-schema.ts (1)

172-215: 🗄️ Data Integrity & Integration | 🔵 Trivial

Solid schema; cross-owner mount invariant only enforced at app layer.

Table design matches the documented rationale (nullable current_firearm_id, nullable cost_cents with bounded CHECK). Note that nothing at the DB layer prevents current_firearm_id from pointing at a firearm owned by a different user — that invariant is enforced entirely in authorizeMount/authorizeCreateMount (confirmed in src/auth/accessory-visibility.ts and src/domain/accessories/service.ts). That's a reasonable trade-off given the existing authorization layer and test coverage, just flagging it as a single point of enforcement to keep in mind for any future direct-DB tooling (seed scripts, admin imports, etc.) that might bypass the domain layer.

🤖 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 172 - 215, The cross-owner mount
invariant for accessory.currentFirearmId is only enforced in authorizeMount and
authorizeCreateMount, so direct DB writes can still attach an accessory to
another user’s firearm. Add a database-level guard in the accessory schema or
the mount write path to ensure the firearm owner matches accessory.ownerId, and
keep the existing app-layer checks in src/auth/accessory-visibility.ts and
src/domain/accessories/service.ts as defense in depth.
app/(app)/firearms/mounted-accessories.tsx (1)

30-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared accessory display helpers

src/domain/accessories/display.ts already has the brand/model→category fallback and cost formatting logic. Pull this view onto those helpers (or a thin wrapper) so the firearm-mounted list stays aligned with the rest of the accessories UI and doesn’t drift.

🤖 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/mounted-accessories.tsx around lines 30 - 44, The mounted
accessories view is duplicating accessory formatting logic instead of reusing
the shared display helpers. Update mounted-accessories.tsx to use the existing
helpers from src/domain/accessories/display.ts (or a thin wrapper around them)
for both the brand/model→category label fallback and the cost formatting,
keeping the mounted list aligned with the rest of the accessories UI. Use the
existing accessoryLabel and formatCostCents call sites in MountedAccessories to
swap in the shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/`(app)/accessories/accessory-form.tsx:
- Around line 119-147: The submit flow in accessory-form.tsx is persisting the
raw category string from persistableFields(), so trailing or leading whitespace
can survive validation and affect exact-match grouping. Normalize category
before building fields/input in the accessory form flow, using the existing
submit/validation path around validateAccessory, persistableFields, and the
accessory submit handler so stored categories are trimmed consistently.

In `@app/`(app)/accessories/page.tsx:
- Around line 31-41: The firearm mount option assembly is duplicated in this
page and the accessory detail page, so extract it into a shared helper and use
that from both places. Move the `firearmNames` population plus the
`editableFirearms` owner/edit filtering logic from the current page into a
reusable function such as `buildFirearmMountContext`, and have both
`app/(app)/accessories/page.tsx` and `app/(app)/accessories/[id]/page.tsx` call
it with `firearms` and `permissions` so the eligibility rule stays in one place.

In `@src/domain/accessories/service.ts`:
- Around line 121-133: The create flow in service.ts is inconsistent because the
`if (input.firearmId)` guard skips authorization for empty strings while
`currentFirearmId: input.firearmId ?? null` still persists `""`. Update the
`authorizeCreateMount` check and the insert in the same create-accessory path so
`firearmId` is normalized consistently (treat empty string the same as absent,
or validate before use) and only a real firearm id reaches `currentFirearmId`.

In `@src/domain/range-sessions/service.ts`:
- Around line 170-196: The accessoryRoundsFired query currently sums all linked
sessions without filtering by firearm visibility, so update it to match the
access control used in lifetimeRoundTotals. In accessoryRoundsFired, add the
same getVisibleIds(db, actorId, "firearm") restriction to the rangeSession
join/query so only sessions on currently visible firearms contribute to the
total, while keeping the existing resolveAccessoryPermission and NotFoundError
behavior intact.

---

Nitpick comments:
In `@app/`(app)/accessories/accessories-view.tsx:
- Around line 58-64: The orDash helper is duplicated in accessories-view.tsx and
accessory-detail-view.tsx, so hoist that shared formatting logic into a reusable
UI helper and import it in both views. Update the existing orDash usage in the
accessories view and the matching implementation in accessory-detail-view.tsx to
reference the shared helper so the identical trimmed-string-to-dash behavior
lives in one place.

In `@app/`(app)/firearms/mounted-accessories.tsx:
- Around line 30-44: The mounted accessories view is duplicating accessory
formatting logic instead of reusing the shared display helpers. Update
mounted-accessories.tsx to use the existing helpers from
src/domain/accessories/display.ts (or a thin wrapper around them) for both the
brand/model→category label fallback and the cost formatting, keeping the mounted
list aligned with the rest of the accessories UI. Use the existing
accessoryLabel and formatCostCents call sites in MountedAccessories to swap in
the shared implementation.

In `@src/db/inventory-schema.ts`:
- Around line 172-215: The cross-owner mount invariant for
accessory.currentFirearmId is only enforced in authorizeMount and
authorizeCreateMount, so direct DB writes can still attach an accessory to
another user’s firearm. Add a database-level guard in the accessory schema or
the mount write path to ensure the firearm owner matches accessory.ownerId, and
keep the existing app-layer checks in src/auth/accessory-visibility.ts and
src/domain/accessories/service.ts as defense in depth.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: be8b066e-3dd0-4b60-90b6-a0ca5cca9218

📥 Commits

Reviewing files that changed from the base of the PR and between d209556 and 3707250.

📒 Files selected for processing (35)
  • app/(app)/accessories/[id]/page.tsx
  • app/(app)/accessories/accessories-view.tsx
  • app/(app)/accessories/accessory-detail-view.tsx
  • app/(app)/accessories/accessory-form.tsx
  • app/(app)/accessories/actions.ts
  • app/(app)/accessories/page.tsx
  • app/(app)/app-shell.tsx
  • app/(app)/firearms/[id]/page.tsx
  • app/(app)/firearms/firearm-detail-view.tsx
  • app/(app)/firearms/firearm-form.tsx
  • app/(app)/firearms/mounted-accessories.tsx
  • app/(app)/firearms/page.tsx
  • docs/plans/2026-07-07-001-feat-accessories-tracker-plan.md
  • e2e/accessories.spec.ts
  • e2e/fixtures/user-pool.ts
  • src/auth/__tests__/accessory-visibility.test.ts
  • src/auth/accessory-visibility.ts
  • src/db/__tests__/schema.test.ts
  • src/db/inventory-schema.ts
  • src/db/migrations/0011_daffy_microbe.sql
  • src/db/migrations/0012_dapper_reavers.sql
  • src/db/migrations/meta/0011_snapshot.json
  • src/db/migrations/meta/0012_snapshot.json
  • src/db/migrations/meta/_journal.json
  • src/domain/accessories/__tests__/service.test.ts
  • src/domain/accessories/__tests__/validate.test.ts
  • src/domain/accessories/constants.ts
  • src/domain/accessories/display.ts
  • src/domain/accessories/service.ts
  • src/domain/accessories/validate.ts
  • src/domain/firearms/service.ts
  • src/domain/range-sessions/__tests__/service.test.ts
  • src/domain/range-sessions/service.ts
  • src/domain/validation-messages.ts
  • src/test-support/factories.ts

Comment thread app/(app)/accessories/accessory-form.tsx
Comment thread app/(app)/accessories/page.tsx Outdated
Comment thread src/domain/accessories/service.ts Outdated
Comment thread src/domain/range-sessions/service.ts
…to mount (#8)

PR review: the mount picker only offers firearms owned by the accessory's owner (KTD5). installed_date now requires a mount (DB CHECK + service guards, R6). Edit-grantee delete exposed in the detail view (R9). Migration 0013.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…reate-mount (#8)

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…mments (#8)

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@unclesp1d3r unclesp1d3r self-assigned this Jul 9, 2026
@coderabbitai coderabbitai Bot removed the documentation Improvements or additions to documentation label Jul 9, 2026
@coderabbitai coderabbitai Bot removed the security label Jul 9, 2026
Per-surface demo specs (e2e/demo-*.spec.ts) sharing one sample dataset (e2e/fixtures/demo-seed.ts), gated behind DEMO=1 so they stay out of CI. 'just demo-images' regenerates every screenshot + the walkthrough gif from the current UI (Docker + ffmpeg).

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Add an accessories showcase, refresh the demo gif + magazines/summary shots to the current shadcn-token UI, and add accessories-*/firearm-accessories/accessory-detail images. Note 'just demo-images' for release-time regeneration.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@coderabbitai coderabbitai Bot added the documentation Improvements or additions to documentation label Jul 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
e2e/demo-walkthrough.spec.ts (1)

92-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant injectOverlays call.

sbrFirearmId navigates internally without injecting overlays, and the immediately following go() navigates again and re-injects — the standalone injectOverlays(page) call on line 93 has no visible effect before the next navigation wipes the DOM.

♻️ Proposed cleanup
     const sbr = await sbrFirearmId(page);
-    await injectOverlays(page);
     await go(page, `/firearms/${sbr}`);
🤖 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 `@e2e/demo-walkthrough.spec.ts` around lines 92 - 94, The standalone
injectOverlays(page) call in the walkthrough is redundant because the subsequent
go() navigation replaces the DOM before it can have any effect. Remove that
extra injection and keep the flow in sbrFirearmId/page navigation paths so
overlays are only injected on the page that is actually being asserted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@e2e/demo-walkthrough.spec.ts`:
- Around line 92-94: The standalone injectOverlays(page) call in the walkthrough
is redundant because the subsequent go() navigation replaces the DOM before it
can have any effect. Remove that extra injection and keep the flow in
sbrFirearmId/page navigation paths so overlays are only injected on the page
that is actually being asserted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: afcbaf54-3b7d-4f9e-8555-54776822b155

📥 Commits

Reviewing files that changed from the base of the PR and between e5b75ba and 1d0ac2b.

⛔ Files ignored due to path filters (9)
  • docs/images/accessories-dark.png is excluded by !**/*.png
  • docs/images/accessories-light.png is excluded by !**/*.png
  • docs/images/accessory-detail.png is excluded by !**/*.png
  • docs/images/demo.gif is excluded by !**/*.gif
  • docs/images/firearm-accessories.png is excluded by !**/*.png
  • docs/images/magazines-dark.png is excluded by !**/*.png
  • docs/images/magazines-light.png is excluded by !**/*.png
  • docs/images/summary-dark.png is excluded by !**/*.png
  • docs/images/summary-light.png is excluded by !**/*.png
📒 Files selected for processing (8)
  • README.md
  • e2e/demo-accessories.spec.ts
  • e2e/demo-magazines.spec.ts
  • e2e/demo-summary.spec.ts
  • e2e/demo-walkthrough.spec.ts
  • e2e/fixtures/demo-seed.ts
  • e2e/fixtures/user-pool.ts
  • justfile
✅ Files skipped from review due to trivial changes (2)
  • e2e/demo-accessories.spec.ts
  • README.md

- Scope accessoryRoundsFired to visible firearms (no private-firearm round leak via a remounted accessory)
- Batch listSessionAccessories (drop the per-accessory N+1, same visible:false placeholder)
- Trim category before persist; normalize empty-string firearmId to null on create
- Extract buildFirearmMountContext (dedupe the two accessories pages)
- Firearm NFA label examples -> SBR/SBS/AOW (suppressors are accessories); fix two stale test/factory comments

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend documentation Improvements or additions to documentation enhancement New feature or request frontend shared testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accessories tracker (per firearm): log aftermarket parts

2 participants