feat(issue-status): Phase 1 schema + built-in status seeding (MUL-4809)#5505
Open
Bohan-J wants to merge 16 commits into
Open
feat(issue-status): Phase 1 schema + built-in status seeding (MUL-4809)#5505Bohan-J wants to merge 16 commits into
Bohan-J wants to merge 16 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
First stageable slice of the Custom Issue Status plan: rollback-safe schema and idempotent seeding, with no behavior change to existing status handling. - Add issue_status catalog table (per-workspace, 5 immutable Categories as the only machine semantics; name/icon/color/description human-facing). Built-ins carry a stable system_key; custom statuses are NULL. No FKs (CLAUDE.md). - Four indexes, each CONCURRENTLY in its own migration: three partial catalog uniqueness indexes (name-active, system_key, at-most-one category default) plus a non-partial workspace_id index covering the delete/cleanup path (archived rows included). - Add nullable issue.status_id + its index (unused this phase; legacy issue.status TEXT stays authoritative until Phase 2 double-write). - internal/issuestatus.Ensure idempotently seeds the 7 built-ins with an explicit ON CONFLICT (workspace_id, system_key) arbiter and a FOR KEY SHARE existence gate, so a re-run is a no-op and a since-deleted workspace is never re-seeded. Wired into workspace creation inside its tx. - internal/issuestatus.Backfill reconciles pre-existing workspaces on boot, advisory-locked so one replica walks during a rolling deploy. - DeleteWorkspace sweeps issue_status in the same CTE (no-FK integrity), so a deleted workspace leaves no orphan status rows. Migrations renumbered to 200-206 after rebasing onto main (197-199 taken). Verified on pg17: full migrate up + down/up roundtrip; DB tests cover seed correctness, one-default-per-category, idempotency, backfill, delete cleanup, and the orphan-race guard; workspace handler tests still pass. Co-authored-by: multica-agent <github@multica.ai>
Bohan-J
force-pushed
the
agent/j/acf19200
branch
from
July 16, 2026 03:43
5300cbe to
0c8479a
Compare
…UL-4809)
Phase 2 foundation. Resolve() maps a status string to a workspace's
issue_status row with a fixed priority order (plan §3.1):
1. Category alias (backlog|todo|in_progress|done|cancelled) -> that
Category's current default status (survives renames of the default).
2. Legacy alias (in_review|blocked) -> the built-in with that system_key.
3. Exact active display name (case-insensitive).
No fuzzy matching; anything else returns *InvalidStatusError enumerating the
legal Category aliases, legacy aliases, and active names so the API/CLI can
echo them back instead of leaving an agent to guess after a rename. Category
aliases use underscores and never collide with space-rendered display names.
Also exposes IsReservedStatusToken (the 7 tokens no custom status may use) and
the Categories list, both consumed by the upcoming status-management API and
double-write paths.
DB-backed tests cover every branch, including plan example A (renamed Todo
default still reached by `todo`, custom Todo reached only by exact name).
Co-authored-by: multica-agent <github@multica.ai>
…4809) Phase 2 double read/write, write side. Every issue write query that sets the legacy status token now also populates the authoritative issue.status_id from the built-in status with the matching system_key, scoped to the issue's workspace: - CreateIssue / CreateIssueWithOrigin: derive status_id in the INSERT. - UpdateIssueStatus: re-derive alongside the status write. - UpdateIssue: re-derive only when status is actually being changed. status stays the source of truth this phase; status_id is a mirror. The derivation is a scalar subquery on the (workspace_id, system_key) unique index, so it is cheap and returns NULL when the workspace catalog is not yet seeded (rolling deploy) — status_id simply stays NULL, no write fails. Subquery/target columns are table-qualified to avoid the issue/issue_status ambiguity. No new query params, so no call-site changes. DB tests assert status_id is written on create, re-derived on status update (both paths), left untouched on a non-status update, and NULL for an unseeded workspace. Full handler and service suites pass unchanged. Co-authored-by: multica-agent <github@multica.ai>
Adds GET/POST/PATCH/DELETE /api/issue-statuses (plan §5): - GET returns the catalog plus the alias resolution table (5 Category aliases + in_review/blocked) and per-Category defaults; readable by any member/agent since it drives `issue status`. - POST creates a custom status (system_key NULL), validated against reserved alias tokens, the 5 Categories, an icon/color allowlist, a length cap, and a per-workspace active-custom cap. - PATCH edits name/description/icon/color/position and swaps the Category default; category/system_key/workspace_id are immutable (400). - DELETE archives (soft delete): system statuses and the current default are refused, and an in-use status requires a same-Category migrate_to_status_id, moving issues in the same tx. All writes run under a workspace advisory lock so the cap, name uniqueness, and the clear-then-set default swap stay atomic; the DB keeps at most one default per Category, this layer keeps at least one. No FK: workspace scoping is the WHERE guard. Adds issue_status:created/updated events and full handler test coverage. Co-authored-by: multica-agent <github@multica.ai>
…, gates (MUL-4809)
Address the status-management API review (comment e5db7038).
P0:
- CreateCustomIssueStatus now takes a FOR KEY SHARE workspace existence gate
(mirrors Ensure), so a create that races a workspace delete inserts zero
rows instead of leaving an orphan status. Zero rows -> pgx.ErrNoRows -> 404.
- Extract issuestatus.LockWorkspaceForStatusWrite / WorkspaceLockKey as the
single canonical lock protocol, keyed on the canonical workspace UUID (not
the raw request string), so differently-cased UUIDs can no longer take
distinct advisory locks and bypass mutual exclusion. All catalog writes
route through it; the archive census/reassign/archive stays under it.
P1:
- Reserved-alias rule applies to custom statuses only; a built-in may be
renamed back to its reserved default name ("Todo", ...).
- Immutable fields (category/system_key/workspace_id) are rejected on field
presence in the raw JSON, so an explicit null is a 400, not a silent 200.
- include_archived=true is gated to owner/admin (rejects agents); the active
catalog stays readable by any member/agent.
Tests: create-after-delete orphan gate, canonical-key equality, controlled-
concurrency lock serialization + archive/assignment closure (no issue
stranded on an archived status), immutable-null 400, built-in reserved
rename, admin-only archived view, cross-workspace 404. Scope unchanged:
status_id still not read on machine paths, no issue-write path is locked.
Co-authored-by: multica-agent <github@multica.ai>
main advanced past the earlier rebase and took 200/201 for the inbox archived-listing indexes, so the PR's 200-206 collide once GitHub tests the branch merged with main (TestMigrationNumericPrefixesStayUniqueAfterLegacySet fails on the merge). Shift this slice's seven migrations to the next free block, 202-208, and update the three in-file cross-references. SQL bodies are unchanged; each concurrent index stays in its own single-statement file. Co-authored-by: multica-agent <github@multica.ai>
…/delete deadlock (MUL-4809) Re-review found a lock-order inversion: a create with is_default=true took the status advisory lock and ClearCategoryDefault (locking a status row) before reaching for the workspace row FOR KEY SHARE inside the INSERT, while a concurrent DeleteWorkspace holds the workspace row FOR UPDATE then deletes the status rows — a cycle that deadlocks (40P01). Hoist the workspace-row FOR KEY SHARE gate into LockWorkspaceForStatusWrite, so every status write locks in one order: workspace row -> advisory -> status rows, matching DeleteWorkspace's workspace-row-first order. A missing workspace short-circuits to ErrWorkspaceGone (mapped to 404) before any default swap or write. The existence CTE in CreateCustomIssueStatus stays as defense-in-depth. Adds a controlled-concurrency regression test reproducing the exact interleave (delete grabs FOR UPDATE while the create holds a status row, then the create inserts): it deadlocks on the old order and passes only with the FKS-first fix (verified by reverting the fix — test fails with 40P01 — and restoring it). Co-authored-by: multica-agent <github@multica.ai>
…lters (MUL-4809) Phase 6.2 read side, backend. Issue responses now expose the resolved custom status catalog, and the list endpoints filter by it. - IssueResponse gains status_id + status_detail (id/name/category/icon/color), bulk-attached by the list/detail endpoints exactly like labels: a single StatusDetailsByIssues query joins issue.status_id -> issue_status (workspace- scoped, no FK; archived rows included). A load failure degrades to nil, and issues with a NULL status_id (unseeded workspace) resolve to nil — the client falls back to the legacy `status` token, which stays authoritative this phase. Wired at ListIssues (both branches), ListGroupedIssues, and GetIssue, mirroring where labels attach; create/update/WS omit them (client cache fallback), like the Labels field already documents. - ListIssues and ListGroupedIssues accept status_id and status_category filters: status_id = i.status_id equality; status_category = EXISTS against the catalog. Both validated (400 on a bad UUID / unknown Category) via a shared helper, and applied through the same dynamic WHERE so CountIssues stays consistent. status stays the source of truth; these fields are additive. No machine logic (Autopilot/notifications/Inbox) changes. DB tests cover status_detail on get/list, the two filters, their validation, and the NULL-status_id fallback. Co-authored-by: multica-agent <github@multica.ai>
…UL-4809) Read-side client parsing for the new issue fields. Both are optional + nullable so older backends that omit them (and endpoints that don't hydrate them) still parse — the client falls back to the legacy `status` token. - IssueSchema gains status_id (nullish) and status_detail (StatusDetailSchema, nullish). StatusDetailSchema keeps `category` a plain string and stays `.loose()`, so an unknown future Category passes through instead of failing the whole issue. List/search/grouped/child schemas inherit the fields; mobile reuses IssueSchema so it inherits them too (fallback unchanged — fields optional). - Issue type gains status_id?/status_detail?, plus StatusCategory (the 5 categories) and a StatusDetail interface. - Contract tests: an old-server issue without the fields parses (no fallback); status_id/status_detail pass through and tolerate an unknown category; an explicit null is preserved. Co-authored-by: multica-agent <github@multica.ai>
# Conflicts: # packages/core/api/schema.test.ts
…n detail (MUL-4809) Address review P1s #1 and #4 (backend half) on the read side. - P1-1: the open_only=true fast path returned before the status_id/status_category filters were parsed, so they were silently ignored and invalid values wrongly returned 200. Hoist parseStatusCatalogFilters above the branch (validation now covers both paths) and apply both predicates in ListOpenIssues (status_id equality; status_category via an EXISTS against the catalog, matching the dynamic paths). - P1-4: SearchIssues, ListChildIssues, and ListChildrenByParents returned IssueResponse without status_detail, so those entries fell back to Category and lost the precise name. All three now hydrate it (a shared hydrateStatusDetails helper for the child lists; an inline batch for search, whose scan omits status_id). Tests: open_only status_id/status_category filtering + invalid-value 400s, and status_detail on the child-list endpoint. Co-authored-by: multica-agent <github@multica.ai>
…ory + client filters (MUL-4809) Address review P1s #2, #3, and #4 (client half) on the read side. - P1-2 (§7.3 safe degrade): a malformed status_detail subfield used to fail the outer IssueSchema and drop the whole list to [] via parseWithFallback. `.catch(null)` now isolates the failure to status_id/status_detail — a bad value degrades to null and the issue (and list) still parses. - P1-3 (honest type): the parser allowed any category string while the public StatusDetail.category is a closed 5-value union. category is a strict enum at runtime now (a 6th Category is never added by product design), so an unknown value degrades the single detail field to null and the type stays truthful for exhaustive switches. - P1-4 (client capability): ListIssuesParams / ListGroupedIssuesParams and the ApiClient now expose and serialize status_id / status_category so Web/Mobile can call the backend filters through the shared client. Tests updated: unknown category -> status_detail null (issue kept), malformed detail on one row doesn't blank the list, explicit-null asserted as exact null. Co-authored-by: multica-agent <github@multica.ai>
…status (MUL-4809 §4.1) Decouple Autopilot runs from Issue status: a run now finalizes purely on the terminal state of the task it dispatched, never on the issue's status. This removes Issue status's hidden control over Autopilot (plan §1.1 #2 / §4.1) and, with it, `in_review`/`blocked` as machine states in this path. - dispatchCreateIssue now captures its dispatched task and binds it onto the run (UpdateAutopilotRunRunning: issue_created -> running, task_id set). The task keeps NO autopilot_run_id, so it stays an ordinary issue task for context / classification — the run owns the pointer instead. - New SyncRunFromCreateIssueTask finalizes the run from that task's terminal state: found by issue_id, matched to the run's dispatched-task lineage via run.task_id + the retry_of_task_id chain. That precise match is what stops a LATER comment-triggered task on the same issue from finalizing the run. A still-queued system retry (HasPendingRetryForTask) keeps the run open until the final attempt; when run.task_id was never bound (crash before bind, or a run dispatched by a pre-§4.1 pod mid-deploy) it falls back to the issue-scoped "no active task" check so the run can't hang. - Delete SyncRunFromIssue + its EventIssueUpdated listener, and SyncRunFromLinkedIssueTask (folded in). Issue status no longer ends/fails a run. - Split GetAutopilotRunByIssue: attribution now uses GetLatestAutopilotRunByIssue (latest run in ANY status) so the firing trigger's owner survives the run completing early on task outcome; the sync keeps the active-only query, which also makes finalization idempotent under a rolling deploy. run_only is unchanged (already task-driven). The failure-rate auto-pause monitor still reads completed/failed runs, now produced by the task path. Tests: create_issue completion via task, a non-lineage comment task NOT finalizing the run, plus the existing failure / retry-pending / dispatch suites updated to the bound running state; webhook crash-window repair remodeled to the pre-bind issue_created state it now represents. Co-authored-by: multica-agent <github@multica.ai>
…P0-1)
Elon review P0-1: the run bind and terminal transitions had no expected-state
guard (`WHERE id = $1` only), so a run could be resurrected after completion or
have two finalizers overwrite each other (last-writer-wins), and a rolling
deploy's old issue-status listener could race the new task listener.
- UpdateAutopilotRunRunning now only advances a run that is still pending/
issue_created/running; UpdateAutopilotRun{Completed,Failed} only finalize an
in-flight (issue_created/running) run. A CAS-lost write matches zero rows
(pgx.ErrNoRows) instead of clobbering a terminal state.
- Callers treat ErrNoRows as first-writer-wins: the two Sync finalizers and
failRun no-op silently (no capture / no duplicate run_done publish); the
create_issue and webhook binds reload the already-terminal run instead of
resurrecting it to running or erroring the dispatch.
Adds a deterministic CAS test: a completed run rejects a late fail and a late
bind (both ErrNoRows) and stays completed with no task_id. Existing autopilot
service + cmd/server suites unchanged.
Co-authored-by: multica-agent <github@multica.ai>
…e reload (MUL-4809 §4.1) Address the CAS-slice review of 845efb9 (4 items). P0-1 — bind is now a true CAS. UpdateAutopilotRunRunning gains `AND (task_id IS NULL OR task_id = $2)`: NULL first-binds, the same task replays idempotently, a DIFFERENT task is rejected (zero rows). The first dispatched task can no longer be silently rebound to another. P0-2 — single bindAutopilotRunTask helper used by all three dispatch entry points (create_issue, run_only, webhook repair). On a CAS miss it reloads to establish the authoritative state and only reports success when the run is already terminal (a racing task finalized it) or already bound to this task; an active-but-different bind, a missing row, or a failed reload is an error — so schedule/webhook callers never record a phantom dispatch. run_only and webhook binds now return that error instead of logging-and-continuing. P1-3 — comments/tests no longer claim this CAS solves the rolling-deploy race: an old-version pod still runs the unguarded write, so the guarantee is only first-writer-wins among same-version finalizers; the deploy-enable gate is later work. P1-4 — failRun returns whether it won the transition; the three dispatch-failure call sites capture a failure only when the write actually landed, ending the double-count when another finalizer already terminated the run. Tests: bindAutopilotRunTask idempotent same-task / rejects different-task / returns the authoritative terminal run / errors when the run row is gone. Existing service, cmd/server autopilot, and handler webhook suites unchanged. Co-authored-by: multica-agent <github@multica.ai>
…MUL-4809 §4.1) Re-review P0: bindAutopilotRunTask's terminal-reload branch returned success for ANY terminal run, so `A bind -> A finalizes run -> B bind` reported B's dispatch as landed even though the run belongs to A — a competing dispatch the CAS is meant to reject (run_only would then wake B; create_issue's B is already enqueued). A terminal run is now an idempotent success ONLY when it is unbound (the pre-bind crash-window compatibility, handled explicitly) or already owned by THIS task; a terminal run owned by a DIFFERENT task returns an error, so the caller treats the bind as a dispatch failure rather than a landed one. "terminal" alone is no longer treated as proof that this task landed. Regression extended: `A bind -> terminal -> A bind` succeeds; `A bind -> terminal -> B bind` is refused and the run stays owned by A. Full service + cmd/server autopilot suites unchanged. Co-authored-by: multica-agent <github@multica.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First stageable slice of the Custom Issue Status plan (MUL-4809): Phase 1 — rollback-safe schema + idempotent built-in seeding. It adds the new catalog and column but changes no existing status behavior —
issue.statusremains authoritative, nothing readsstatus_idyet, and no machine logic is touched. This keeps the change independently verifiable and safe to ship/roll back on its own, ahead of the later double-write / alias-resolver / autopilot-decoupling phases.What's included
issue_statuscatalog table (per workspace).categoryis the only machine-readable semantics — one of 5 immutable Categories (backlog | todo | in_progress | done | cancelled);name/icon/color/descriptionare human-facing. Built-in statuses carry a stablesystem_key; custom statuses havesystem_key = NULL.categoryandsystem_keyare immutable after creation. No foreign keys (workspace relationship resolved in app code).CONCURRENTLYin its own single-statement migration:(workspace_id, lower(name)) WHERE archived_at IS NULL— case-insensitive active-name uniqueness.(workspace_id, system_key) WHERE system_key IS NOT NULL— one row per built-in; the explicitON CONFLICTarbiter for seeding.(workspace_id, category) WHERE is_default AND archived_at IS NULL— at most one active default per Category. The index enforces only this upper bound; ensuring at least one default per Category is the service layer's responsibility (the seed establishes exactly one).(workspace_id)non-partial — covers the workspace-scoped delete/cleanup path, including archived rows the three partial indexes above skip.issue.status_idnullable column + index. Unused this phase; wired for the Phase 2 double-write.issuestatus.Ensure— idempotently seeds the 7 built-in statuses with an explicitON CONFLICT (workspace_id, system_key) DO NOTHINGarbiter (so any other unique violation surfaces loudly instead of silently dropping a built-in) and aFOR KEY SHAREexistence gate on the workspace row. A re-run is a no-op, and a since-deleted workspace inserts nothing — soBackfillcan never re-create orphans for a workspace deleted mid-walk. Called inside the workspace-create transaction so the catalog commits atomically with the workspace.issuestatus.Backfill— one-shot boot reconcile for workspaces created before this shipped, guarded by a Postgres advisory lock so a single replica walks during a rolling deploy.DeleteWorkspacesweepsissue_status(archived rows included) in the same CTE as the workspace row, so deleting a workspace leaves no orphan status definitions.Seeded
icon/colorvalues mirror the current hardcoded status visuals (icon= the enum key the frontend renders a bespoke SVG for;color= the existing semantic token), so no visual drift when later phases read the catalog.Migrations
Numbered 202–208 (rebased onto
main, which took 197–199 foragent_task_attributionand 200–201 for the inbox archived-listing indexes):202issue_statustable203name-active unique index ·204system_key unique index ·205category-default unique index ·206workspace_idindex207issue.status_idcolumn ·208its indexEach concurrent index is isolated in its own single-statement file per the repo's migration rules; the table create carries no FKs.
How it was tested
make sqlcregenerates cleanly;go build ./...,gofmt -l,go vetall clean; migration lint test passes (unique prefixes 202–208).pgvector/pgvector:pg17container (matching CI): fullmigrate upapplies 202–208; a fulldownthenuproundtrip confirms rollback safety (issue_statusandissue.status_iddrop and re-create cleanly).internal/issuestatus:Backfillseeds a pre-existing workspace;DeleteWorkspaceremoves the whole catalog (no orphans);Ensureon a deleted workspace is a no-op (the backfill-after-delete orphan-race guard).Also in this PR now (later commits, per the single-PR request)
The requirement is being built up in one PR. On top of the Phase 1 slice above, later commits add, each with tests:
issue_statuswith the Category → default / legacysystem_key/ exact-name priority.statusinto the authoritativestatus_idviasystem_key(NULL until the workspace is seeded;statusstays the source of truth).GET/POST/PATCH/DELETE /api/issue-statuses(catalog + alias table, create/rename/recolor/reorder, default swap, archive-with-migration), owner/admin-gated, agents rejected.include_archived. Adds controlled-concurrency (lock serialization + archive/assignment closure), role-permission, cross-workspace, and field-presence coverage.Not in this PR yet (remaining phases)
status_id/status_categoryfilters +status_detailpayload + client contract tests), Autopilot task-outcome decoupling, batched backfill/audit + pre-enable gating, agent-context catalog injection + CLI/daemon dynamic catalogs, Web/Desktop/Mobile UI, and the Phase 5 constraint tightening.