feat(notes): shared team notepad — server API and client panel#436
feat(notes): shared team notepad — server API and client panel#436mcharles-square wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1df00a91fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds an org-wide “shared team notepad” feature across the stack (DB + RBAC + Connect-RPC service + ProtoFleet UI) so team members can read/post notes, edit/delete their own notes, and moderators can delete any note, with activity logging.
Changes:
- Server: introduces
NoteService(List/Create/Update/Delete), note domain service + SQL store, activity category/event types, and new permission keys with seed/backfill migrations. - Authz/middleware: adds
HasAnywhere+*Anywherepermission gates to allow access based on any assignment scope (org or site) for org-shared resources. - Client: adds header toggle + slide-in notepad panel UI, feed hook with keyset pagination + head polling merge, mutations, and activity icons.
Reviewed changes
Copilot reviewed 49 out of 55 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/sqlc/queries/note.sql | sqlc queries for note CRUD + keyset pagination |
| server/migrations/000081_create_note_table.up.sql | Creates note table, index, and updated_at trigger |
| server/migrations/000081_create_note_table.down.sql | Drops note trigger/index/table |
| server/migrations/000082_seed_note_permissions.up.sql | Seeds note permissions and backfills built-in roles |
| server/migrations/000082_seed_note_permissions.down.sql | Rollback for seeded note permissions |
| server/internal/handlers/notes/handler.go | Connect-RPC NoteService handler + cursor encode/decode |
| server/internal/handlers/notes/translate.go | Domain ↔ proto translation for notes |
| server/internal/handlers/notes/handler_test.go | Handler tests for gates, pagination token behavior, and auth rules |
| server/internal/handlers/middleware/permission.go | Adds resolveCaller preamble + *Anywhere permission gates |
| server/internal/handlers/middleware/permission_test.go | Tests for RequirePermissionAnywhere, RequireAnyPermissionAnywhere, and probe semantics |
| server/internal/handlers/middleware/rpc_permissions.go | Registers NoteService procedures in the RBAC contract map |
| server/internal/handlers/middleware/rpc_permissions_test.go | Adds NoteService to RBAC contract reflection coverage |
| server/internal/handlers/handlerstest/permissions.go | Adds helpers for site-scoped assignments/session info in handler tests |
| server/internal/domain/notes/service.go | Notepad domain rules: normalize, author-only edit, moderator delete, activity events |
| server/internal/domain/notes/service_test.go | Domain tests for normalization, bounds, auth rules, activity metadata, nil activity svc |
| server/internal/domain/notes/models/models.go | Notepad domain shapes + page size/content limits |
| server/internal/domain/stores/interfaces/note.go | NoteStore interface definition |
| server/internal/domain/stores/interfaces/mocks/mock_note_store.go | Generated mock for NoteStore |
| server/internal/domain/stores/sqlstores/note.go | SQL-backed NoteStore implementation using sqlc |
| server/internal/domain/stores/sqlstores/note_integration_test.go | DB integration tests: pagination ties, cross-org isolation, soft delete |
| server/internal/domain/authz/catalog.go | Adds note permission keys and resource grouping to catalog |
| server/internal/domain/authz/catalog_test.go | Catalog completeness/resource order tests updated for note permissions |
| server/internal/domain/authz/builtin.go | Adds note permissions to FIELD_TECH seed + updates description |
| server/internal/domain/authz/reconcile_test.go | Ensures built-in roles seed note permissions on fresh install |
| server/internal/domain/authz/service_test.go | Adds validateReadPairing coverage for note permission pairing |
| server/internal/domain/authz/effective.go | Adds EffectivePermissions.HasAnywhere union semantics |
| server/internal/domain/authz/effective_test.go | Tests HasAnywhere behavior + FlatKeys parity |
| server/internal/domain/activity/models/models.go | Adds note activity category + validation support |
| server/internal/domain/activity/models/models_test.go | Tests CategoryNote validity |
| server/cmd/fleetd/main.go | Wires note store/service/handler into the server mux |
| proto/notes/v1/notes.proto | Defines NoteService RPCs and message schemas/validation |
| server/generated/sqlc/note.sql.go | Generated sqlc output for note queries |
| server/generated/sqlc/models.go | Generated sqlc model for note |
| server/generated/sqlc/db.go | Generated sqlc prepared statement wiring for note queries |
| server/generated/grpc/notes/v1/notes.pb.go | Generated Go protobuf types for NoteService |
| server/generated/grpc/notes/v1/notesv1connect/notes.connect.go | Generated Connect-RPC bindings for NoteService |
| client/src/protoFleet/api/generated/notes/v1/notes_pb.ts | Generated TS protobuf types/service for NoteService |
| client/src/protoFleet/api/clients.ts | Adds notesClient to the API client registry |
| client/src/protoFleet/api/notes.ts | Client mutation helpers for create/update/delete note |
| client/src/protoFleet/api/useNotesFeed.ts | Feed hook: pagination + head refresh merge logic |
| client/src/protoFleet/api/useNotesFeed.test.ts | Tests feed pagination/merge behavior and error routing |
| client/src/protoFleet/features/notes/noteFormat.ts | Formatting helpers (day grouping, timestamps, avatar color hashing) |
| client/src/protoFleet/features/notes/index.ts | Exports NotepadPanel feature entrypoint |
| client/src/protoFleet/features/notes/components/NotepadPanel.tsx | Slide-in notepad panel UI + polling integration |
| client/src/protoFleet/features/notes/components/NotepadPanel.test.tsx | Panel tests (gating, composer visibility, affordances, empty state, load more) |
| client/src/protoFleet/features/notes/components/NoteComposer.tsx | Note composer with trim/length guard + submit state |
| client/src/protoFleet/features/notes/components/NoteCard.tsx | Note card rendering + edit/delete flows + edited indicator |
| client/src/protoFleet/features/activity/utils/activityIcons.tsx | Adds activity icons for note events |
| client/src/protoFleet/features/activity/components/ActivityTable.tsx | Minor icon alignment tweak (mt-1) |
| client/src/protoFleet/store/slices/uiSlice.ts | Adds ephemeral isNotepadOpen UI state + setter |
| client/src/protoFleet/store/hooks/useUI.ts | Adds selectors for notepad open state + setter |
| client/src/protoFleet/store/index.ts | Re-exports notepad UI selectors |
| client/src/protoFleet/components/PageHeader/PageHeader.tsx | Adds header toggle button for opening/closing notepad |
| client/src/protoFleet/components/PageHeader/PageHeader.test.tsx | Tests notepad toggle visibility and click behavior |
| client/src/protoFleet/components/AppLayout/AppLayout.tsx | Mounts NotepadPanel in the ProtoFleet layout |
One org-wide, append-style feed every member can read and post to. Authors edit/delete their own notes; note:manage holders moderate (delete any note). Ships the full server stack: note table + permission seed migrations, note:read/create/manage catalog keys (FIELD_TECH seeded with read+create), NoteService with keyset pagination, and activity-log events for every mutation. Notepad gates use new any-scope middleware (HasAnywhere): the feed is an org-shared resource with no site dimension, so a role assignment at any scope -- including a site-only FIELD_TECH -- passes, matching the client's flat-key permission gating.
A persistent header toggle summons the shared team notepad as a right-side slide-in panel on any view. The panel is non-modal -- no backdrop or scroll lock -- so the page underneath stays interactive while notes are read or written. Composer, author-only inline edit, author-or-moderator delete with confirm, an edited indicator, and cursor-based Load more mirror the server contract. The feed polls only while the panel is open; each tick re-fetches just the first page and merges it in (new rows prepend, edits replace in place, in-window deletions drop) so pages loaded via Load more survive the poll. Surfaces gate on note:read / note:create / note:manage to match the server's any-scope enforcement.
Make note content the largest text in each card (author rides on weight, the colored avatar carries identity) and move timestamps to small right-justified JetBrains Mono, matching the app's treatment of API-key prefixes and event ids. Day-group labels and the composer counter share the same mono voice. The previous metadata-larger-than- content look traced to a nonexistent text-emphasis-100 token silently falling back to the 16px default. Map note.created/updated to the pencil icon and note.deleted to trash in the activity feed (they fell through to Info), and optically center activity row icons on the first text line — items-start was top- aligning every 16px glyph against the 24px line box. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Extract resolveCaller so all five permission gates share one copy of the session lookup, internal-actor allowlist, and fail-closed effective-permissions fetch — the preamble had grown to five hand-copied instances, where a missed edit silently diverges authorization semantics. Share one ClampPageSize between the notes handler and service: the has-more boundary previously compared row count against the unclamped requested page size, so a defaulted page_size would never emit a continuation token (masked today by proto validation). Teach mergeHeadPage to return the previous array reference when the fetched head changes nothing — id plus updated_at captures edits, inserts, and deletes, and the server trigger guarantees content never changes without an updated_at bump — so idle poll ticks no longer re-render the panel. Memoize day grouping and NoteCard to match. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Upstream main took 000081-000084 (notification stack, telemetry heartbeat, device identifier widening); golang-migrate requires unique version numbers, so the note table and permission seed move past them. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1df00a9 to
c230236
Compare
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM Findings[MEDIUM] Notepad Opens With A Head-Only Fetch That Drops Pagination State
NotesNo high-impact authentication bypass, SQL injection, command injection, network discovery, plugin boundary, pool hijack/cryptostealing, or protobuf wire-format issues were found in the reviewed diff. The new note SQL is sqlc-generated and parameterized, and server-side note authorization derives org/user identity from the authenticated session rather than client input. Generated by Codex Security Review | |
- Treat an empty nextPageToken on the poll tick as feed-complete: replace the feed with the head page (dropping rows deleted upstream below the window) and clear the held cursor so Load more is a no-op. - Surface refreshHead failures while nothing has loaded yet and clear the error on recovery, so a failed first load shows the error callout instead of an indefinite spinner while the poll keeps retrying and self-heals. - Make the seed migration's down-file header numberless so migration renumbers can't desync it. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The panel's first fetch runs through the poll tick, which never initialized pageToken/hasMore from the response — a feed deeper than one page showed no Load more control until something called refresh(). On the tick that performs the initial load, the head page is the entire accumulated list, so its continuation token is the correct cursor; later ticks still leave the cursor alone. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Addressed (both halves, in 641a226 and 6acb1a6):
|
What
Adds a shared team notepad: one org-wide, append-style feed of notes that every team member can read and post to, summonable from any view via a toggle in the protoFleet page header (right-side slide-in panel — the page underneath stays interactive). Authors can edit/delete their own notes (with an "edited" indicator); a moderation permission allows deleting any note.
Server —
NoteService(Connect-RPC):ListNotes(keyset pagination, opaque cursor mirroring the activity log),CreateNote,UpdateNote,DeleteNote. Newnotetable (org-scoped, soft-delete,updated_attrigger) and a domain service that trims/validates content (4096 runes), enforces author-only edit and author-or-moderator delete, and logsnote.created/updated/deletedactivity events under a newnotecategory.RBAC — three new catalog keys:
note:read,note:create(add + edit/delete own),note:manage(delete any; never grants editing others' words). All built-in roles get read + create; ADMIN and SUPER_ADMIN additionally get manage. Seed migration backfills existing orgs (boot reconciliation only seeds additive built-ins at role creation — same precedent as the pool/activity seeds, except FIELD_TECH is deliberately included here since the notepad is for every member).Client — header toggle + slide-in panel with composer, day-grouped feed (Today/Yesterday), per-author avatar colors from the extended palette, Load more pagination, and a 60s head-refresh poll while open that merges into the accumulated list without collapsing loaded pages (idle ticks return the previous array reference, so React skips the re-render). Note activity events get proper icons in the activity feed.
Why
With teams and roles in place, the org needs a lightweight shared space for operational notes ("genset maintenance at 3pm", "rack 12 PSU flaky") without leaving the app.
Decisions a reviewer should weigh
EffectivePermissions.Hasconsults only org-scope grants for org-scoped resources, which would lock out users whose only assignment is site-scoped (e.g. FIELD_TECH @ one site). Since the notepad is an org-shared surface with no site dimension to narrow on, new*Anywheremiddleware gates (backed byEffectivePermissions.HasAnywhere) pass when the caller holds the key under any assignment. This deliberately matches the client's flat-keyuseHasPermissiongating (FlatKeys()semantics), so client visibility and server authorization agree by construction. All five permission gates now share oneresolveCallerpreamble (session lookup → internal-actor allowlist → fail-closed permissions fetch).note:manageallows deleting any note but never editing one — a note's content always reflects its author's words. Ownership is enforced in the domain layer and repeated in the SQL UPDATE predicate so it can't race.updated_at > created_at); no extra column.Notes for reviewers
000081_create_note_tableand000082_seed_note_permissions(renumbered after upstream took 000080). If your local dev DB ran an earlier cut of this branch, recreate it — golang-migrate tracks by version number and will otherwise skip upstream's 000080.buf lintcurrently fails onmainfor pre-existingcollection.proto/device_set.protowarnings (protovalidateIGNORE_IF_ZERO_VALUEredundancy) — unrelated to this change;notes.protolints clean.site.*keys in the icon map) — left as is to keep this scoped; trivial follow-up if wanted.Testing
moderatedmetadata); DB-backed store tests for keyset pagination withcreated_atties, cross-org isolation on every verb, soft-delete exclusion; authz tests forHasAnywhere(narrowing interplay +FlatKeysparity) and reconcile seed assertions. RPC contract test classifies all four procedures.mergeHeadPagemerge/dedup/drop + same-reference bail), panel tests (permission gating, composer visibility, moderation affordances, edited indicator).