fix(web): reuse project combobox in timeline popovers

This commit is contained in:
2026-03-30 13:34:59 +02:00
parent 9268a38df4
commit f0bea6235d
13 changed files with 525 additions and 203 deletions
+2 -19
View File
@@ -19,6 +19,7 @@
- `project.isImageGenConfigured`, `project.isDalleConfigured`: covered as authenticated low-risk configuration checks
- `notification` self-service and manager boundaries: auth-covered across list, unread counts, reminders, deletes, broadcasts, task creation, and assignment boundaries
- `assistant-tools` parity metadata: descriptions and parity assertions now match narrowed router audiences for resource overview, controller-only, self-service, and manager broadcast/task tools
- `comment` architecture phase 1: generic free-form entity comments replaced by an explicit supported-entity registry, currently limited to `estimate` with controller/manager/admin access plus entity-aware checks on list/count/create/resolve/delete
### Dirty Files To Avoid Mixing Into This Batch
@@ -45,27 +46,9 @@ These files already have unrelated local edits. Audience parity work that would
- the previously identified small hardening and tests/docs candidates have been completed, including the notification auth follow-up and assistant tool parity metadata cleanup
- the remaining audience work is now architectural (`comment.ts`) or depends on broader policy decisions rather than another ready-made auth slice
### Needs Architecture Or Policy Design
These routes should not be batch-edited as “small safe slices” until a visibility model exists.
#### `packages/api/src/router/comment.ts`
- Current state: all core routes are `protectedProcedure`
- Why this is not a small slice:
- reads and writes are keyed by generic `entityType` and `entityId`
- visibility depends on the backing entity, not on the comment record alone
- the current author/admin checks for resolve/delete are not enough to decide who may list or create comments on each entity class
- Design work needed:
- define which entity types can host comments
- map each entity type to its audience source of truth
- centralize entity visibility checks before any comment read/write
- Recommended path:
- treat comments as a separate architecture ticket, not part of the quick hardening batch
## Recommended Next Order
1. `comment` architecture design ticket
1. extend the comment entity registry only when a second real consumer exists and its backing audience is explicitly documented
## Slice Definition
+74
View File
@@ -0,0 +1,74 @@
# Comment Visibility Architecture
**Date:** 2026-03-30
**Status:** Phase 1 implemented
## Problem
The original comment router accepted arbitrary `entityType` and `entityId` pairs behind `protectedProcedure`.
That was too broad:
- comment visibility depends on the backing entity, not on the comment record alone
- generic strings allowed clients and assistant tools to imply support for entity types that had no explicit policy
- author/admin checks on `resolve` and `delete` were not enough, because list/create access was still effectively "any authenticated user"
## Current Product Reality
There is only one real first-party consumer today:
- web UI estimate workspace comments via `entityType: "estimate"`
The older examples for `scope_item`, `estimate_version`, and `demand_line` were aspirational, not backed by an explicit visibility model or active UI.
## Architecture Decision
Comments now use an explicit entity registry.
- supported entity types are allowlisted, not free-form
- each entity type owns:
- its audience rule
- its existence check
- its deep link builder for notifications
- every comment route calls the entity access layer before touching comment data
## Phase 1 Policy
Supported entity types:
- `estimate`
Audience:
- same audience as the estimate workspace
- controller, manager, or admin only
Route effects:
- `list`, `count`, `create`, `resolve`, and `delete` all require estimate visibility first
- `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
## Why This Shape
- It closes the real security gap now without pretending a generic multi-entity policy already exists.
- 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.
## Extension Rules For Future Entity Types
To add another commentable entity:
1. Add the entity type to the registry, not just to input examples.
2. Define the backing audience source of truth.
3. Add an existence check for that entity.
4. Add a notification link builder for that entity.
5. Update assistant tool metadata and assistant visibility gates in the same change.
6. Add router auth tests for unauthenticated, plain authenticated, and elevated callers.
7. Update `docs/route-access-matrix.md`.
## Non-Goals In Phase 1
- generic comment support for arbitrary entities
- row-level polymorphic authorization based only on `entityType` strings
- automatic inheritance for future entities without explicit onboarding
+11
View File
@@ -73,6 +73,17 @@ Reasoning:
- `list`: `controller-finance`
- drafting, versioning, export generation, and approval writes: `manager-write`
### `packages/api/src/router/comment.ts`
- `list`, `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
- 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`
- all reads and writes: `admin-only`