Skip to content

feat(customers): follow-ups — a timeline entry can say what happens next, title only on the contract, and the code-scanning alerts - #118

Merged
andersro93 merged 16 commits into
mainfrom
feat/customers-followups
Sep 23, 2026
Merged

andersro93 merged 16 commits into
mainfrom
feat/customers-followups

Conversation

@andersro93

Copy link
Copy Markdown
Contributor

What

Phase 4 of the Customers roadmap ("Light CRM"), delivery C: what happens next. Plus the two things you asked for after #117: the contact association's role alias is gone (title only), and the open code-scanning alerts are fixed. Design: docs/superpowers/specs/2026-09-23-customers-follow-ups-design.md; reference: docs/customers.md → Follow-ups.

  • A follow-up is part of a manual timeline entry — a due date and, optionally, an assignee (a user of the installation), on customers_timeline_entries and its revisions table (migration 00026). POST/PUT …/timeline gain followUp?: {dueOn, assigneeUserId?} | null (a full-replace PUT: omitted or null clears, and clearing clears done); dueOn may be in the future; a new assignee must exist and be active, an unchanged one is kept without being asked again. POST/DELETE …/timeline/{entryId}/follow-up/done tick and reopen: idempotent, no expected revision (a tick from a list must not conflict with a note edit), but a real change bumps current_revision and records a revision row with the actor — so who ticked it and when is the revision's own actor and time in the revisions view.
  • Attention: followUpOverdue and followUpDue on /stats/attention for open follow-ups assigned to the caller or unassigned (unassigned ones are everyone's until taken), occurredAt = the due date, shaped by customers:timeline-view — a caller with customers:view alone sees none of them (never a 403), because they carry timeline-entry ids. The dashboard has the two sentences in en + nb.
  • A Follow-ups page (/customers/follow-ups, nav entry under Customers with customers:timeline-view): GET /customers/follow-ups (timeline-view + view) with assignee=me|none, state=open|overdue|done|all, paging, ordered by due date; rows link to the customer; a Done tick per row with canManageTimeline.
  • The timeline card gains a Follow-up section in the entry form (due date + assignee picker — the owner picker generalised into a UserPicker), a follow-up line per entry (red and "overdue" when past, struck through when done) with Done/Reopen, and the revisions panel shows each revision's follow-up state. A new canManageTimeline capability prop from the host also hides Add/Edit/Delete for readers, who used to see buttons that 403'd.
  • title only (approved break, nothing live): role removed from the four contact-association schemas and every reader; the rule is title-or-roles; the contact event payload version bumps to 2; the frozen corpus is untouched and still validates (no schema sets additionalProperties: false). GET /customers/assignable-users relaxed from customers:update to customers:view so a timeline writer can pick an assignee.
  • Code scanning (commit 0e0d544, verified by running CodeQL locally): the three Argon2 parameter conversions and the Seal allocation size now carry bounds the scanner can see (Seal refuses plaintext over 64 MiB with ErrTooLarge). The cookie Secure alert is a false positive — CodeQL loses the field through the cookies slice and fires its "insecure default" branch whatever the value; hard-coding Secure: true leaves it firing and breaks ~40 identity tests and the plain-http deployments docs/transport-security.md supports. Documented in code and docs; please dismiss alert Bump the "all-dependencies" group with 2 updates across multiple ecosystems #5 as false positive (I don't change the repo's security state).

Decisions made without asking (please overrule any of these)

  1. A follow-up lives on a manual timeline entry, never as a separate task object and never on a generated event.
  2. Done/reopen take no expected revision but are revisions of the entry; a no-change tick writes nothing and resolves no actor.
  3. Attention is caller-dependent — yours or unassigned — and shaped by customers:timeline-view; for an installation where most people hold only customers:view, the dashboard's follow-up half is silent for them (the docs say so).
  4. An omitted followUp on PUT clears it — the timeline PUT has always been a full replace; the form re-sends what it holds.
  5. Only a new assignee is validated; an unchanged one is kept even if since disabled or deleted from the directory (shown inactive / "Unknown user").
  6. GET /customers/assignable-users on customers:view — it answers display names of active users, which every customer reader already sees as the owner.
  7. Dates are UTC — "today" is the server's UTC calendar day, as everywhere else in this module; a user far from UTC sees "due today" flip a few hours early or late.
  8. Two filters on the page (Mine/Unassigned; Open/Overdue/Done/All) — the API's assignee=<uuid> form is reachable but the page does not offer it.
  9. canManageTimeline also gates the existing Add/Edit/Delete — a consistency fix the design called out; the server always enforced it.
  10. The cookie alert is not "fixed" — see above; a contortion to silence the scanner would have made the code worse.

How it was built

Spec → plan (pre-flight on the strongest model: fourteen rulings, the doneBy question decided as "the revision's own actor") → seven tasks with task reviews; fix rounds: the attention items reached customers:view-only callers (now shaped); a full-replace PUT re-validated an unchanged assignee (now exempt); the page's failed tick left a deleted entry's row on screen (now refreshes); three docs sentences the break falsified → two-seat whole-branch review (backend + contract + docs / frontend) → one fix wave per seat, scoped re-review → verification.

What the whole-branch review changed: the attention half is capped at the twenty most overdue (an installation with hundreds of open follow-ups would have flooded the card); state=all now contains state=done (both admit archived customers — all was not a superset of its own values); the docs say what happens to a departed assignee's follow-ups (kept, on nobody's list, reachable only via assignee=<uuid>; reassignment parked in the roadmap) and that "today" is the server's UTC day; the 64 MiB Seal boundary test lowers the cap through a package seam instead of allocating 250 MB; the edit form's assignee round trip is pinned by a test (dropping the seed would have silently unassigned on a note edit); clearing the date clears the assignee (a follow-up is the date) and the picker is disabled until a date is set; the Follow-ups page shows an Alert with Retry on error (a timeline-view-only caller is admitted by the nav and refused by the endpoint — now they see why).

The scoped re-review of those waves found the list operation's contract description still saying "unless state=done" — fixed in the last commit, with two tests that could not fail (a utcToday test that recomputed its own body; the twenty-cap test counting the whole attention list) made able to.

Known leftovers, parked on purpose: clearing and retyping a date in the edit form drops the assignee (the literal shape of "no date, no follow-up"; a restore-on-retype is a nicety); the dashboard card orders newest-first and shows eight, so of the twenty most overdue the least overdue reach the card — the page is where the backlog is read (documented); rows on state=all/done carry no "archived customer" marker; a departed assignee's follow-ups need the API's assignee=<uuid> form (no bucket or reassign path yet); the twenty-cap is invisible on the wire; the page's pageSize is not a route param; UserPicker re-fetches the current assignee's label per mount; the "Unknown user" sentinel is English on both locales (the owner's precedent).

Verification (local)

  • On 55ac906: go generate ./..., bun run gen:client → no drift · go vet, golangci-lint run clean · go test -count=1 ./... 42 packages ok · go test -race on 4 CPUs for internal/customers/... ok · mise run frontend:check (all ten workspaces) ok. The last commit on top changes a contract description (regenerated; contract + openapi tests re-run green), two tests and a docs clause — the pre-commit hook (i18n, biome, gofmt) passed; CI runs the rest.
  • corpus test green; openapi/testdata/exchanges/customers.jsonl untouched; CodeQL 2.27.1 run locally: one result left (the documented false positive).

🤖 Generated with Claude Code

andersro93 and others added 16 commits September 23, 2026 13:18
Phase 4 delivery C: a manual timeline entry carries a due date and an
assignee, can be ticked done, feeds the attention list for the caller
and a Follow-ups page. The same PR removes the contact association's
deprecated role alias (nothing is live) and fixes the five open
code-scanning alerts.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…arry bounds a scanner can see

Four of the five open code-scanning alerts were the analyser being unable to
see a bound the code does enforce. Each is now stated where the value is used,
which is also where a reader would look for it.

go/incorrect-integer-conversion x3 (passwords.go): verifyPassword bounded the
Argon2 parameters inside phcParam, whose [lo, hi] arrived as arguments — so
nothing at the conversion said the value fits a uint32 or a uint8. phcParam now
only parses (as a uint64, via ParseUint: every PHC parameter is unsigned, and
too long a run of digits stays a parse failure rather than a wrapped number),
and verifyPassword checks the documented bounds against the constants
themselves immediately before it converts. Behaviour is unchanged; the tests
gain a case per type for a value that fits the field but not the parameter
(m=4294967296, t=4294967296, p=256), each of which panics Argon2 without the
check.

go/allocation-size-overflow (secrets.go): Seal computed its output size from
len(plaintext) with no stated maximum. It now refuses a plaintext above
maxPlaintextBytes (64 MiB) with ErrTooLarge, before a key is derived or a
buffer allocated. Every purpose the box serves is a cookie, a TOTP secret or a
small JSON blob, so the bound refuses nothing legitimate, and the size
arithmetic is provably in range whatever produced the plaintext.

go/cookie-secure-not-set (cookies.go:26) is a false positive and is left as
code, documented instead. Measured against CodeQL 2.27, CI's version: the query
flags a cookie write no boolean reaches at all (isInsecureDefault), because its
taint tracking loses the Secure field between the literal in Access.cookie and
the loop in cookies.set. Hard-coding Secure: true does not clear the alert —
and it would lock every plain-http installation out of signing in, a deployment
transport-security.md documents and identity's own harness uses. Setting false
through a separate documented path instead makes the query report the cookie as
explicitly insecure. The comment above cookie and the transport document now
say so; the alert wants a dismissal with a reason.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Who ticked a follow-up, and when, is the revision's own actor and time
in the revisions view; no doneBy on the wire. Fourteen smaller rulings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…else

The deprecated role alias is removed from all four association schemas while nothing is live: title is the field, the title-or-roles rule keeps its wording, and the contact events' payload version is 2 without it. The frozen corpus is untouched and still validates — no schema forbids an unknown key — so a body that still sends role now names no title and is refused by the rule that already existed. GET /customers/assignable-users drops to customers:view: it answers display names every timeline reader already sees, and a follow-up's assignee has to be pickable without customers:update.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Migration 00026 adds follow_up_on, follow_up_assignee_user_id and follow_up_done_at to the timeline entries table AND to its revisions table, so history stays point-in-time, with two partial indexes for the open list and the assignee filter and deliberately no foreign key to identity.users. Every timeline statement carries the three columns; a PUT replaces the follow-up and clearing it clears the done stamp. Nothing on the wire moves yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the raw contact type says what the wire omits

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…is to do it

A manual, active entry carries a follow-up — a due date that may be in the future and an optional assignee validated against the directory before any transaction — set, replaced and cleared by the entry's own PUT under the revision it already takes. Two new paths tick it done and reopen it: idempotent, no expectedRevision (a tick from a list must not lose a race with an edit of the note), each real change bumping current_revision with a revision row naming who ticked it. /stats/attention gains followUpOverdue and followUpDue for the caller's and unassigned open follow-ups — the first item here that depends on who asks — and GET /customers/follow-ups answers the list, defaulting to my open ones.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n unchanged assignee is not asked to be active again

The attention list handed follow-up items — timeline entry ids — to a
caller holding customers:view alone, while the timeline itself is
behind the sensitive timeline-view; the items are now shaped by that
permission the way legal-identity figures are, never a 403. A
full-replace PUT re-validated the follow-up's assignee, so a note edit
failed once that account was disabled; only a new assignee must exist
and be active. The attention sort comment says what the order is, and
the list's filter table pins state=open.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… stops seeing buttons that 403

The entry form gains a Follow-up section — a date that may be in the future and an assignee picked from the same directory search the owner uses — and each entry shows its follow-up, red while overdue and struck through once done, with a Done/Reopen control. The owner picker is generalised into a UserPicker with the owner card calling it through a thin wrapper, so nothing about the owner moved. canManageTimeline arrives as a capability prop and hides Add, Edit, Delete and the follow-up controls from a reader.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… say a date has passed

GET /customers/follow-ups gets its page: both filters in the URL so a filtered list is a link, rows linking to the customer whose timeline holds the entry, and a Done tick behind customers:timeline-manage. The host adds the sidebar entry on customers:timeline-view, the route with its search validator, and the capability prop — which the customer page's Overview tab now derives too. The dashboard names followUpOverdue and followUpDue in both languages; the link needed nothing, because the item carries the customer id.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A Follow-ups section covering where a follow-up lives and why, what setting and clearing one does to the done state, why the two done paths take no expected revision, and the list with its filters. The attention section gains the two caller-dependent types, the permissions section explains why no key was added and why assignable-users relaxed, and the API table gains three operations. ROADMAP marks phase 4 delivery C done.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ual entry, and the review minors

On the Follow-ups page a tick that failed left the row — and its live
button — on screen for an entry that may be gone; the list refreshes
first, as the timeline card already did. Done and Reopen show only on
a manual entry. The done mutation's comment no longer claims a 409
cannot happen; the two write tests prove one write again; a fetch
whose body omits the follow-up is proven to normalise to null; the
unused revision response type is gone; the clear-on-edit path and the
overdue tone are tested; date-only formatting is one helper. The host's
nav comment and the dashboard's attention comment say what the code
does, and the two new dashboard keys resolve through both catalogs in
the existing loop. The docs stop contradicting themselves about the
Follow-ups list's two permissions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…llow-ups, state=all contains done, and the docs say what leaves every view

The attention query had no bound: an installation's whole queue of
open follow-ups was scanned and serialised on every dashboard load for
the eight items the card shows. It now reports the twenty most overdue
the caller may see; the contract, the design and the docs say so, and
say that the dashboard orders its card newest-first across modules
rather than by how overdue. state=all now includes what state=done
does. The docs say that "today" is the server's UTC day, and that a
disabled or removed assignee's follow-ups stay theirs — on nobody's
list, reachable only by an explicit assignee filter — with
reassignment parked. The secrets test lowers the box's plaintext cap
through a seam instead of allocating a quarter of a gigabyte.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…aring the date clears the assignee, and the page explains its errors

Editing only the note of an assigned follow-up re-sends the assignee
the entry holds — now proven by a test. Emptying the due date clears
the assignee with it (a follow-up is the date), and the assignee picker
is disabled until a date is set, so an assignee without a date cannot
be asked for. Overdue is decided by one helper on both surfaces; the
Follow-ups page shows the error it got with a retry, scrolls its table
on a phone, hides the actions column from a reader, and its fixtures
are the wire's shapes. The docs' frontend bullets describe the
follow-up controls and the reader-facing change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… too, and two tests that could not fail now can

The list operation's description still carried the pre-fix rule
("excluded unless state=done") while the query, the docs and the design
all say `all` is the superset — the normative text contradicted the
code one layer up. Both spec copies say it now, and the `state`
parameter says why.

The `utcToday` test recomputed the helper's own body; it now pins a
clock at 23:30 UTC and asserts the literal day a local-date reading
would get wrong. The cap-at-twenty test counted the whole attention
list; it now counts the follow-up half, so a registry record added to
that fixture later cannot fail it for an unrelated reason. The docs'
Timeline bullet mentions the picker that waits for a date.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant