mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 18:13:27 +02:00
728b90c7c4
All three are production-boundary issues rather than logic errors in the
delegated rule itself. Each fix is pinned by a test that fails without it.
1. Subtree unsubscribe silently degraded to "this issue only" against an
older backend. It travelled as a `subtree: true` BODY FIELD on
POST /unsubscribe, and Go's json.Decode drops unknown fields — so a
server that predates the feature unsubscribed the root, answered 200,
and the user was told the whole tree was muted while every existing and
future child kept notifying them. Frontend staging deploys on merge
while the backend is deployed by hand, so this skew is routine, not
theoretical.
A capability carried by a field cannot fail loudly; one carried by a
route can. Subtree now has its own endpoint
(POST /api/issues/{id}/unsubscribe/subtree), which 404s on an old
backend and surfaces as a rejected mutation. The shared endpoint keeps
no subtree behavior at all, so old and new servers answer a plain
/unsubscribe identically. Tests pin both halves: the flag must stay
inert on /unsubscribe, and the dedicated route must reach descendants.
2. Delegated auto-subscribe did not re-check that the originator is still
a workspace member. originator_user_id is stamped when the task is
QUEUED and never revisited, and GetAgentTaskInWorkspace only proves the
task's AGENT still belongs to the workspace — a different claim from
"the human does". A revoked member's long-lived tasks keep running on a
shared runtime, and every child they file wrote a subscriber row for a
non-member: a ghost watcher accruing inbox rows outside the membership
boundary, and restored history on re-join. Membership is now re-checked
at write time, which is the only moment it is actually true.
3. Migration 241 revalidated a hot table inside a strong lock. The CHECK is
only WIDENED, so every existing row already satisfied it, yet a plain
ADD CONSTRAINT still holds ACCESS EXCLUSIVE on issue_subscriber for a
full scan and blocks subscriber reads and writes for the duration.
Switched to the NOT VALID + VALIDATE CONSTRAINT pattern already used by
migration 060: VALIDATE scans under SHARE UPDATE EXCLUSIVE, which
blocks neither. The constraint still ends up convalidated=t, so nothing
un-trusted is left behind. The down migration gets the same treatment —
a rollback runs under load by definition.
Also addresses the review's non-blocking nit. The mobile test claimed to
be a server wire contract but is a hand-written fixture against a Zod
schema, so it could never fail on a Go-side regression. Renamed to
inbox-schema.test.ts and reworded to state what it actually pins, and the
server half is now structural instead of a convention: the one details map
in notification_listeners.go that was map[string]any is map[string]string,
making a non-string value a compile error at the source.
Verified locally:
- go test ./cmd/server ./internal/handler ./internal/attribution ./internal/service
- pnpm test (5/5 tasks, incl. 3 new client contract tests)
- pnpm typecheck (6/6)
- pnpm exec turbo typecheck lint test --filter=@multica/mobile --force (4/4)
- full migrate down + up roundtrip on a scratch DB; confirmed convalidated=t
- both new regression tests confirmed failing with their fix reverted
Co-authored-by: multica-agent <github@multica.ai>