fix(comment): align mention audience with entity visibility
This commit is contained in:
@@ -0,0 +1,35 @@
|
||||
# Architecture Hardening Backlog
|
||||
|
||||
**Date:** 2026-03-30
|
||||
**Purpose:** Keep the remaining cleanup work for the current quality/security scope in a single prioritized list, so small hardening slices can be completed before larger redesign work.
|
||||
|
||||
## Recently Completed
|
||||
|
||||
- SSE audience model narrowed to canonical `user:*`, `permission:*`, and `resource:*` scopes only
|
||||
- CI architecture guardrail added for SSE audience scoping
|
||||
- import boundaries hardened for server dispo workbooks and browser spreadsheet uploads
|
||||
- AI and SMTP runtime diagnostics sanitized before they reach logs or admin-facing error messages
|
||||
- transitive audit hotspots for `flatted` and `picomatch` pinned through root `pnpm.overrides`
|
||||
- `apps/web` export paths migrated from direct `xlsx` usage to a shared `exceljs` workbook export helper
|
||||
- `packages/application` workbook reading and `packages/engine` XLSX export serialization migrated from `xlsx` to `exceljs`
|
||||
- `pnpm audit --audit-level=high` no longer reports high-severity dependency findings
|
||||
- `apps/web` now has focused Vitest coverage for browser spreadsheet parsing and skill-matrix workbook parsing
|
||||
- cron routes, Redis helpers, reminder scheduling, webhook dispatching, and SSE fallback paths now use structured logger calls instead of raw `console.*`
|
||||
- `packages/api` now has focused Vitest coverage for reminder scheduler and webhook dispatcher logging failures
|
||||
- `apps/web` typecheck is now decoupled from generated `.next-e2e` artifacts via a dedicated `tsconfig.typecheck.json`
|
||||
- comment entity support is now centralized across shared constants, API registry policy, assistant tool metadata, and the web comment target API without pretending a second consumer exists
|
||||
- `resource` is now onboarded as the second real comment entity, reusing the same ownership and staff-visibility rules as the resource detail route
|
||||
- comment mention autocomplete now uses a dedicated entity-scoped API route instead of inheriting the narrower `user.listAssignable` audience
|
||||
|
||||
## Next Up
|
||||
|
||||
No queued hardening slice is currently pinned in this document.
|
||||
Reassess after the current batch so the next item reflects the then-real highest-risk gap instead of stale cleanup residue.
|
||||
|
||||
## Working Rule
|
||||
|
||||
For the next batches, prefer work in this order:
|
||||
|
||||
1. remove or isolate known-risk runtime dependencies
|
||||
2. add guardrails and tests around already-hardened code
|
||||
3. only then expand architecture surface area
|
||||
@@ -1,7 +1,7 @@
|
||||
# Comment Visibility Architecture
|
||||
|
||||
**Date:** 2026-03-30
|
||||
**Status:** Phase 1 implemented
|
||||
**Status:** Phase 4 mention audience aligned with entity visibility
|
||||
|
||||
## Problem
|
||||
|
||||
@@ -14,11 +14,12 @@ That was too broad:
|
||||
|
||||
## Current Product Reality
|
||||
|
||||
There is only one real first-party consumer today:
|
||||
There are now two real first-party consumers:
|
||||
|
||||
- web UI estimate workspace comments via `entityType: "estimate"`
|
||||
- web UI resource detail comments via `entityType: "resource"`
|
||||
|
||||
The older examples for `scope_item`, `estimate_version`, and `demand_line` were aspirational, not backed by an explicit visibility model or active UI.
|
||||
The older examples for `scope_item`, `estimate_version`, and `demand_line` remain aspirational, not backed by an explicit visibility model or active UI.
|
||||
|
||||
## Architecture Decision
|
||||
|
||||
@@ -31,20 +32,26 @@ Comments now use an explicit entity registry.
|
||||
- its deep link builder for notifications
|
||||
- every comment route calls the entity access layer before touching comment data
|
||||
|
||||
## Phase 1 Policy
|
||||
## Current Policy
|
||||
|
||||
Supported entity types:
|
||||
|
||||
- `estimate`
|
||||
- `resource`
|
||||
|
||||
Audience:
|
||||
|
||||
- same audience as the estimate workspace
|
||||
- controller, manager, or admin only
|
||||
- `resource` comments inherit the exact resource detail audience:
|
||||
- self-service for the caller's own linked resource
|
||||
- broad visibility for users who already have resource overview access
|
||||
|
||||
Route effects:
|
||||
|
||||
- `list`, `count`, `create`, `resolve`, and `delete` all require estimate visibility first
|
||||
- `listMentionCandidates` uses the same entity visibility gate before returning mention candidates
|
||||
- the same routes require resource visibility first for `entityType: "resource"`
|
||||
- `resolve` and `delete` still require comment author or admin after entity visibility is granted
|
||||
- replies are only allowed when the parent comment belongs to the same entity tuple
|
||||
- mention notifications use the entity policy link builder instead of hardcoded route assumptions scattered through the router
|
||||
@@ -55,6 +62,53 @@ Route effects:
|
||||
- It keeps future comment expansion additive: a new entity type must be onboarded deliberately.
|
||||
- It gives the assistant and UI one source of truth for what is actually supported today.
|
||||
|
||||
## Phase 2 Foundation
|
||||
|
||||
Phase 2 does not add a fake second entity type.
|
||||
Instead, it removes duplication around the real registry:
|
||||
|
||||
- supported comment entity types now live in shared constants instead of being re-declared in multiple layers
|
||||
- the API owns a dedicated comment entity registry module for:
|
||||
- access checks
|
||||
- notification link building
|
||||
- assistant-facing supported-entity metadata
|
||||
- assistant tool schemas and descriptions derive their comment entity metadata from that registry layer
|
||||
- the web comment thread now receives an explicit `{ entityType, entityId }` target instead of hardcoding `estimate` internally
|
||||
|
||||
This keeps the product honest today while making a future second consumer additive rather than copy-paste driven.
|
||||
|
||||
## Phase 3 Resource Onboarding
|
||||
|
||||
`resource` is now the second deliberate commentable entity because it already had:
|
||||
|
||||
- a real product detail surface
|
||||
- an explicit access model in the API
|
||||
- a coherent notification deep link target
|
||||
|
||||
Implementation shape:
|
||||
|
||||
- shared constants now expose both supported entity types
|
||||
- the API registry now maps `resource` to:
|
||||
- existence checks via the backing resource record
|
||||
- audience checks via the same resource-read ownership rules as `resource.getById`
|
||||
- notification links via `/resources/:id#comments`
|
||||
- the resource detail page now renders a first-party comment thread instead of leaving the second consumer theoretical
|
||||
- assistant comment tools now remain entity-scoped instead of pretending every comment operation is controller-only
|
||||
|
||||
## Phase 4 Mention Audience Alignment
|
||||
|
||||
Mention autocomplete no longer depends on `user.listAssignable`.
|
||||
Instead, comments own a dedicated mention-candidate route scoped by the same entity tuple as comment read/write access.
|
||||
|
||||
Current shape:
|
||||
|
||||
- `comment.listMentionCandidates` first enforces the entity access policy
|
||||
- estimate mention candidates are limited to `ADMIN`, `MANAGER`, and `CONTROLLER` users, matching estimate comment visibility
|
||||
- resource mention candidates include:
|
||||
- the linked owner of that resource
|
||||
- users who already have broad resource visibility through effective permissions
|
||||
- `user.listAssignable` remains a separate operational lookup for assignment flows and is not widened as a side effect of comment support
|
||||
|
||||
## Extension Rules For Future Entity Types
|
||||
|
||||
To add another commentable entity:
|
||||
@@ -67,7 +121,7 @@ To add another commentable entity:
|
||||
6. Add router auth tests for unauthenticated, plain authenticated, and elevated callers.
|
||||
7. Update `docs/route-access-matrix.md`.
|
||||
|
||||
## Non-Goals In Phase 1
|
||||
## Non-Goals
|
||||
|
||||
- generic comment support for arbitrary entities
|
||||
- row-level polymorphic authorization based only on `entityType` strings
|
||||
|
||||
@@ -75,13 +75,16 @@ Reasoning:
|
||||
|
||||
### `packages/api/src/router/comment.ts`
|
||||
|
||||
- `list`, `count`, `create`, `resolve`, `delete`: `entity-scoped`
|
||||
- `list`, `listMentionCandidates`, `count`, `create`, `resolve`, `delete`: `entity-scoped`
|
||||
|
||||
Reasoning:
|
||||
|
||||
- comments must inherit the audience of the backing entity, not the comment row itself
|
||||
- Phase 1 intentionally supports only `estimate`, because that is the only real product consumer today
|
||||
- estimate comments therefore inherit the estimate workspace audience: controller, manager, or admin
|
||||
- supported entity types are currently `estimate` and `resource`
|
||||
- estimate comments inherit the estimate workspace audience: controller, manager, or admin
|
||||
- resource comments inherit the resource detail audience: self-service for the caller's own linked resource, plus broad access for users who already have resource overview visibility
|
||||
- mention autocomplete uses the same entity-scoped access check instead of reusing assignment-oriented user directory routes
|
||||
- the registry keeps router policy, assistant metadata, and web comment targets on the same supported-entity definition
|
||||
- future entity types must be added through an explicit registry with per-entity access checks, assistant parity, and router tests in the same slice
|
||||
|
||||
### `packages/api/src/router/system-role-config.ts`
|
||||
|
||||
Reference in New Issue
Block a user