Files
multica/server/internal/handler/agent_permission.go
14c2e4e831 MUL-6243 feat(issue-status): board fetches by category; archive retires without migration (#7084)
* feat(issue-status): board fetches by category; archive retires without migration (MUL-6243)

Product decisions from the issue thread:

1. Board fetches by CATEGORY, not per status. PAGINATED_STATUSES becomes
   PAGINATED_CATEGORIES and the cache buckets by category, so the board's
   fan-out stays fixed at 7 parallel requests however many custom statuses a
   workspace defines — instead of one more request per status added. Adds
   status_category / status_categories filters to the issue list endpoints,
   backed by the existing issue_effective_status() SQL function.

3. Archiving retires a status from FUTURE use only. It no longer refuses while
   issues still carry it: those issues keep their status and keep resolving to
   their category's behavior (Effective ignores archived_at), while Resolve
   still rejects the status so nothing new can be assigned to it. The exclusive
   catalog lock stays — it is what makes "no NEW assignment after archive"
   exact rather than approximate.

Also lands the frontend catalog foundation: packages/core/issue-statuses with
useIssueStatuses(wsId) (React Query, keyed by wsId per the state rules) and a
pure buildIssueStatusCatalog for non-React callers, plus IssueStatusCategory as
a distinct type from the status key and status_category on issue payloads.

NOT included, and deliberately: widening IssueStatus to string. That change
forces every Record<IssueStatus, …> presentation lookup in packages/views to
resolve a key to its category first, and it is the first task of the follow-up
PR. Everything here is the groundwork that makes it mechanical.

Co-authored-by: multica-agent <[email protected]>

* fix(issue-status): restore the status index, make category authoritative, keep archived resolvable

Review round 1 on #7084 (MUL-6243):

1. Category filtering wrapped issue.status in issue_effective_status(), which
   makes the (workspace_id, status) index unusable — and the board switched to
   it unconditionally, so every workspace paid for it immediately. Categories
   are now expanded to concrete status keys (issuestatus.ExpandCategories) and
   filtered with an indexed status = ANY(...). Measured on a 100k-row table
   with the production index: seq scan cost 27013 -> index-only scan cost 6.07.
   A workspace with no custom statuses expands each category to exactly its own
   key, reproducing the pre-feature query. Archived keys are included, since
   issues left on them still belong in their category's column.

2. status_category was only filled for built-ins, so a custom-status issue
   arriving over the websocket was silently dropped by addIssueToBuckets and a
   cross-category update was skipped by the cache coordinator, drifting
   off-window totals permanently. List, grouped, get, create, update and batch
   responses now carry an authoritative category via a per-request Resolver
   (archived included), and the client invalidates instead of no-oping when a
   category is unresolvable.

3. The catalog was fetched without archived entries, so after archiving a
   status the issues still on it lost their name and colour and had their
   category guessed as todo — contradicting the decision that archiving leaves
   existing issues untouched. Resolution now includes archived entries;
   activeStatuses / inCategory expose only assignable ones to pickers.

Co-authored-by: multica-agent <[email protected]>

* fix(issue-status): category on every payload exit, one resolver per request, no same-category duplication

Review round 2 on #7084 (MUL-6243):

1. Category fill was incomplete and N+1. The create WEBSOCKET payload was
   unfilled — only the creator's HTTP response was, which is too late for the
   other tabs the event exists to serve. GetIssue, the open_only list, search,
   the duplicate-conflict response, the table rows, the GitHub PR update and
   the onboarding shim were also unfilled. Grouped and batch called the
   single-response helper per row, so each custom-status row hit the catalog
   separately.

   Every exit now goes through newStatusCategoryFiller, a request-scoped
   closure over ONE Resolver: built-in statuses still cost no query, and a page
   of custom-status rows costs one catalog read rather than one per row.

2. A status KEY change within one category duplicated the card and inflated the
   total. patchIssueInBuckets compared patch.status (a key) against loc.status
   (a bucket CATEGORY), so in_review -> a custom human_review took the
   cross-bucket path, then deleted from and re-inserted into the same bucket
   from a pre-delete snapshot. It now resolves the destination category first
   and compares category to category; same-category changes replace in place
   and leave the total alone.

   The destination category is resolved from the patch alone — the merged issue
   inherits the previous status_category, which would have kept a genuinely
   moved card in its old column. patchNeedsInvalidation lets the loaded branch
   of the cache coordinator invalidate instead of silently no-oping on a status
   it cannot resolve.

Also drops the unrelated share_link.sql.go churn from an sqlc version
difference, per the review nit.

The four cache cases were verified to fail against the previous logic (card
duplicated, total +1) before the fix was restored.

Co-authored-by: multica-agent <[email protected]>

* fix(issue-status): normalize status_category on every cache write; one filler per create

Review round 3 on #7084 (MUL-6243):

Must-fix — a cached entity kept the STALE status_category after a status
change. The server now sends a category on every issue, so a cached entity is
{status: "todo", status_category: "todo"}; an optimistic patch carries only
{status: "done"}, and a bare {...issue, ...patch} moved the card to the done
bucket while the entity still claimed todo. A single update self-heals from the
server response, but the batch API returns only {updated: n} and does not
refetch bucketed lists, so the inconsistency was permanent — and the next
off-window count then decremented the wrong bucket, recreating total drift.

normalizeStatusPatch now rewrites status_category to match status before any
cache write, applied at applyIssueChange's entry (covering the flat, table and
detail spreads), in patchIssueInBuckets, and in the children mutation. An
unresolvable custom key drops the inherited category rather than keeping it;
patchNeedsInvalidation still routes it to a refetch.

Test fixtures now carry status_category for built-ins, matching what the server
actually sends — without that the regression was invisible.

Non-blocking items also addressed:
- CreateIssue builds one filler and shares it between the broadcast payload and
  the HTTP response, so a custom-status create reads the catalog once.
- Background events via IssueToMap now resolve an authoritative category
  (IssueToMapWithCategory) for the task, issue and autopilot publishers. The
  channel router keeps plain IssueToMap and says why: it always creates with the
  built-in "todo", where key IS category.

The "one resolver serves a whole page" assertion was REMOVED rather than kept:
pg_stat counters lag, so a delta around one request passes even with a per-row
filler. Verified that directly before dropping it. The comment now states what
is and is not pinned instead of implying coverage that does not exist.

Co-authored-by: multica-agent <[email protected]>

* fix(issue-status): address review nits

- Normalize the single-update children optimistic patch too, matching the batch
  path. It self-healed on onSettled, but it broke the entity/bucket invariant
  for the duration of the request.
- Rename TestListEndpointsCarryStatusCategory: the old name claimed it covered
  "without per row queries", which the test explicitly does not assert.
- Drop the unrelated gofmt churn in empty_claim_cache.go.
- Add a background-event assertion: IssueToMap leaves a custom status
  uncategorized, IssueToMapWithCategory resolves it — the reason the publishers
  use the latter.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-18 03:00:21 +08:00

305 lines
12 KiB
Go

package handler
import (
"context"
"fmt"
"net/http"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/util"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
// AgentInvocationTargetDTO is the wire shape of one invocation allow-list
// entry (MUL-3963). target_id is null for team placeholders that a client
// omitted, but is always present for workspace (the workspace id) and member
// (the user id) rows persisted by the backend.
type AgentInvocationTargetDTO struct {
TargetType string `json:"target_type"`
TargetID *string `json:"target_id"`
}
const (
permissionModePrivate = "private"
permissionModePublicTo = "public_to"
invocationTargetWorkspace = "workspace"
invocationTargetMember = "member"
invocationTargetTeam = "team"
)
// deriveLegacyVisibility maps the permission model back onto the legacy
// two-value visibility field so old clients never observe a widening:
// - public_to WITH a workspace target -> "workspace" (everyone can invoke)
// - everything else (private, or public_to limited to member/team) -> "private"
func deriveLegacyVisibility(permissionMode string, targets []db.AgentInvocationTarget) string {
if permissionMode == permissionModePublicTo {
for _, t := range targets {
if t.TargetType == invocationTargetWorkspace {
return "workspace"
}
}
}
return "private"
}
// applyInvocationTargetsToResponse fills InvocationTargets and recomputes the
// derived legacy Visibility from the loaded targets, keeping both views of the
// permission consistent in a single place.
func applyInvocationTargetsToResponse(resp *AgentResponse, targets []db.AgentInvocationTarget) {
dto := make([]AgentInvocationTargetDTO, 0, len(targets))
for _, t := range targets {
var idPtr *string
if t.TargetID.Valid {
s := uuidToString(t.TargetID)
idPtr = &s
}
dto = append(dto, AgentInvocationTargetDTO{TargetType: t.TargetType, TargetID: idPtr})
}
resp.InvocationTargets = dto
resp.Visibility = deriveLegacyVisibility(resp.PermissionMode, targets)
}
// resolvedPermission is the normalised outcome of parsing the permission
// fields (or legacy visibility) off a create/update request.
type resolvedPermission struct {
mode string
targets []targetSpec
}
type targetSpec struct {
targetType string
targetID pgtype.UUID // invalid for team placeholders
}
// legacyVisibility is what this permission maps to for the visibility column
// we keep in sync for backwards compatibility.
func (p resolvedPermission) legacyVisibility() string {
if p.mode == permissionModePublicTo {
for _, t := range p.targets {
if t.targetType == invocationTargetWorkspace {
return "workspace"
}
}
}
return "private"
}
// parsePermissionInput normalises a permission_mode + invocation_targets pair,
// falling back to a legacy visibility value when the new fields are absent.
//
// - permissionMode == nil && visibility == nil -> caller default (returns ok=false, nil)
// - permissionMode provided -> authoritative
// - only legacy visibility provided -> mapped:
// "private" -> private
// "workspace" -> public_to + workspace target
//
// workspaceID seeds workspace targets (stored as the workspace id). The
// returned error is a client-facing 400 message.
func parsePermissionInput(workspaceID pgtype.UUID, permissionMode *string, targets []AgentInvocationTargetDTO, hasPermissionMode, hasTargets bool, legacyVisibility *string) (resolvedPermission, bool, error) {
if !hasPermissionMode && legacyVisibility == nil {
return resolvedPermission{}, false, nil
}
// Legacy-only path: map visibility onto the new model.
if !hasPermissionMode {
switch *legacyVisibility {
case "workspace":
return resolvedPermission{
mode: permissionModePublicTo,
targets: []targetSpec{{targetType: invocationTargetWorkspace, targetID: workspaceID}},
}, true, nil
case "private", "":
return resolvedPermission{mode: permissionModePrivate}, true, nil
default:
return resolvedPermission{}, false, fmt.Errorf("visibility must be 'private' or 'workspace'")
}
}
mode := permissionModePrivate
if permissionMode != nil && *permissionMode != "" {
mode = *permissionMode
}
if mode != permissionModePrivate && mode != permissionModePublicTo {
return resolvedPermission{}, false, fmt.Errorf("permission_mode must be 'private' or 'public_to'")
}
res := resolvedPermission{mode: mode}
if mode == permissionModePrivate {
// Private ignores any submitted targets: deny-by-default.
return res, true, nil
}
// public_to: normalise the target list, de-duping and validating.
if hasTargets {
seen := map[string]struct{}{}
for _, t := range targets {
switch t.TargetType {
case invocationTargetWorkspace:
key := "workspace"
if _, dup := seen[key]; dup {
continue
}
seen[key] = struct{}{}
res.targets = append(res.targets, targetSpec{targetType: invocationTargetWorkspace, targetID: workspaceID})
case invocationTargetMember:
if t.TargetID == nil || *t.TargetID == "" {
return resolvedPermission{}, false, fmt.Errorf("member invocation target requires target_id")
}
uid, err := util.ParseUUID(*t.TargetID)
if err != nil {
return resolvedPermission{}, false, fmt.Errorf("member invocation target_id is not a valid uuid")
}
key := "member:" + *t.TargetID
if _, dup := seen[key]; dup {
continue
}
seen[key] = struct{}{}
res.targets = append(res.targets, targetSpec{targetType: invocationTargetMember, targetID: uid})
case invocationTargetTeam:
if t.TargetID == nil || *t.TargetID == "" {
return resolvedPermission{}, false, fmt.Errorf("team invocation target requires target_id")
}
tid, err := util.ParseUUID(*t.TargetID)
if err != nil {
return resolvedPermission{}, false, fmt.Errorf("team invocation target_id is not a valid uuid")
}
key := "team:" + *t.TargetID
if _, dup := seen[key]; dup {
continue
}
seen[key] = struct{}{}
res.targets = append(res.targets, targetSpec{targetType: invocationTargetTeam, targetID: tid})
default:
return resolvedPermission{}, false, fmt.Errorf("invocation target_type must be 'workspace', 'member', or 'team'")
}
}
}
// An empty public_to is a phantom: "shared with nobody" that the front-end
// would render as workspace-shared while the backend admits no one. Per the
// MUL-3963 ruling, a public_to with no resolvable targets is normalised to
// a single workspace target — this also makes `--permission-mode public_to`
// with no --public-to-* flags mean "public to workspace".
if len(res.targets) == 0 {
res.targets = append(res.targets, targetSpec{targetType: invocationTargetWorkspace, targetID: workspaceID})
}
return res, true, nil
}
// replaceInvocationTargets rewrites an agent's invocation allow-list wholesale:
// clear then re-insert. Called inside create/update after the agent row exists.
func (h *Handler) replaceInvocationTargets(ctx context.Context, agentID pgtype.UUID, createdBy pgtype.UUID, targets []targetSpec) error {
return replaceInvocationTargetsWithQueries(ctx, h.Queries, agentID, createdBy, targets)
}
// replaceInvocationTargetsWithQueries is the tx-friendly variant: callers that
// hold a `qtx := h.Queries.WithTx(tx)` can pass it here so the invocation
// target rows are written inside the same transaction as the agent row. A
// fresh agent row must not observe a state where the row exists but its targets
// are missing.
func replaceInvocationTargetsWithQueries(ctx context.Context, q *db.Queries, agentID pgtype.UUID, createdBy pgtype.UUID, targets []targetSpec) error {
if err := q.DeleteAgentInvocationTargets(ctx, agentID); err != nil {
return err
}
for _, t := range targets {
if err := q.CreateAgentInvocationTarget(ctx, db.CreateAgentInvocationTargetParams{
AgentID: agentID,
TargetType: t.targetType,
TargetID: t.targetID,
CreatedBy: createdBy,
}); err != nil {
return err
}
}
return nil
}
// permissionInputChangesAgent reports whether the permission fields on an
// update request would actually CHANGE the agent's current invocation
// permission (mode or the set of targets). Used to let a non-owner's no-op
// resubmit through (PATCH-as-PUT that echoes unchanged permission) while
// rejecting a real change with 403. Invalid/absent permission input counts as
// "no change" — a non-owner sending garbage should not get a 403 either, it is
// simply ignored. Fails safe: on a DB error it returns changed=true so a
// non-owner attempt is rejected rather than silently applied.
func (h *Handler) permissionInputChangesAgent(ctx context.Context, existing db.Agent, req UpdateAgentRequest, hasPermissionMode, hasTargets bool) (bool, error) {
// Legacy-only request: the caller sent ONLY `visibility`, no permission_mode
// and no invocation_targets (an old client / PATCH-as-PUT echoing the field
// back while editing something else). Compare on the DERIVED legacy
// visibility, NOT by expanding "private" into a real private permission.
// A member-only public_to agent derives to legacy "private", so an admin
// resubmitting visibility:"private" is a NO-OP, not a public_to→private
// downgrade. Only a real legacy change (e.g. "workspace") counts. (MUL-3963
// review — this is the compatibility fix for PR #4853.)
if !hasPermissionMode && !hasTargets {
if req.Visibility == nil {
return false, nil
}
current, err := h.Queries.ListAgentInvocationTargets(ctx, existing.ID)
if err != nil {
return true, err
}
submitted := "private"
if *req.Visibility == "workspace" {
submitted = "workspace"
}
return submitted != deriveLegacyVisibility(existing.PermissionMode, current), nil
}
var targetsDTO []AgentInvocationTargetDTO
if req.InvocationTargets != nil {
targetsDTO = *req.InvocationTargets
}
perm, ok, err := parsePermissionInput(existing.WorkspaceID, req.PermissionMode, targetsDTO, hasPermissionMode, hasTargets, req.Visibility)
if err != nil || !ok {
// Unparseable or effectively no permission fields → treat as no change.
return false, nil
}
if perm.mode != existing.PermissionMode {
return true, nil
}
current, err := h.Queries.ListAgentInvocationTargets(ctx, existing.ID)
if err != nil {
return true, err
}
want := make(map[string]struct{}, len(perm.targets))
for _, tgt := range perm.targets {
want[tgt.targetType+":"+uuidToString(tgt.targetID)] = struct{}{}
}
have := make(map[string]struct{}, len(current))
for _, row := range current {
have[row.TargetType+":"+uuidToString(row.TargetID)] = struct{}{}
}
if len(want) != len(have) {
return true, nil
}
for k := range want {
if _, ok := have[k]; !ok {
return true, nil
}
}
return false, nil
}
// enrichAgentResponseWithTargets loads an agent's invocation targets and
// applies them to the response (InvocationTargets + derived Visibility). Used
// by the single-agent detail / create / update responses.
func (h *Handler) enrichAgentResponseWithTargets(ctx context.Context, resp *AgentResponse, agentID pgtype.UUID) error {
targets, err := h.Queries.ListAgentInvocationTargets(ctx, agentID)
if err != nil {
return err
}
applyInvocationTargetsToResponse(resp, targets)
return nil
}
// enrichAgentResponseWithTargetsHTTP is the HTTP-boundary wrapper that writes a
// 500 and returns false on failure.
func (h *Handler) enrichAgentResponseWithTargetsHTTP(w http.ResponseWriter, r *http.Request, resp *AgentResponse, agentID pgtype.UUID) bool {
if err := h.enrichAgentResponseWithTargets(r.Context(), resp, agentID); err != nil {
writeError(w, http.StatusInternalServerError, "failed to load agent invocation targets")
return false
}
return true
}