fix(issues): attach labels in the create transaction (MUL-4832) (#5510)

* fix(issues): attach labels in the create transaction (MUL-4832)

Labels chosen at issue creation were attached in a second, non-atomic
round-trip after the issue was already committed (web modal looped
attachLabelToIssue per label; the create endpoint took no labels). A
partial failure of that follow-up left the issue committed but
mis-categorized, surfaced only as a toast.

Carry label_ids through the create request instead: the service
validates them (workspace + resource_type='issue') and attaches them
inside the same transaction as the issue insert, so the issue and its
labels commit together or not at all. An unknown or wrong-scope label
id now fails the whole create with 400 rather than being silently
dropped. Duplicate ids are idempotent (dedupe + ON CONFLICT DO NOTHING).

- server: IssueCreateParams.LabelIDs + ErrIssueLabelNotFound; validate
  and attach in IssueService.Create; handler parses label_ids and maps
  the error to 400.
- web: create-issue modal forwards label_ids and drops the post-create
  attach loop and its dead toast key.
- tests: handler coverage for atomic attach, stale-id 400 (no issue
  left behind), duplicate-id idempotency, wrong-scope rejection; web
  test asserts label_ids is forwarded.

Scope is deliberately labels-only. The contributor PR #5475 also
auto-parsed an acceptance_criteria field from description text; that
introduces a new user-facing data contract with no defined edit/display
rules and is left out for a separate product decision.

Co-authored-by: multica-agent <github@multica.ai>

* fix(issues): echo created labels + guard old-backend compat (MUL-4832)

Addresses review of #5510.

1. Create response + issue:created event now carry the labels attached in
   the create transaction. IssueService.Create returns the authoritative
   label snapshot; the handler sets it on both the HTTP response and the
   issue:created broadcast payload. Without this, online members other than
   the creator saw the new issue unlabeled indefinitely (staleTime: Infinity,
   no invalidation) because this PR removed the old post-create
   issue_labels:changed broadcast.

2. New web + old backend compatibility. During the rolling deploy window the
   web app can run ahead of the backend (web auto-deploys on merge, backend
   deploys manually). An older backend ignores label_ids and returns an issue
   with no labels field. The create modal now falls back to the legacy
   per-label attach only when the response omits labels, so labels aren't
   silently dropped; when labels is present the atomic path already ran and
   no fallback fires. The backend always returns an explicit labels array
   (empty when none) as the detection signal.

Tests: handler issue:created-carries-labels + response-carries-labels +
response-always-includes-labels; web modal no-fallback (new backend) and
fallback (old backend); ws-updaters keeps the label snapshot in list cache.

Co-authored-by: multica-agent <github@multica.ai>

* fix(issues): validate create response through a schema (MUL-4832)

Addresses review of #5510 (must-fix 3).

The create modal keys its label-attach compatibility fallback off
`issue.labels` being absent (older backend) vs a validated Label[]
(current backend). But api.createIssue cast the raw JSON straight to
Issue, so a malformed labels value (null, an object, a garbage array)
would be !== undefined and wrongly suppress the fallback, caching a bad
shape — violating the repo's API Compatibility rule.

Parse the create response through CreateIssueResponseSchema:
- labels absent  -> undefined (older-backend signal; fallback runs)
- labels valid   -> Label[] (fully validated elements, not z.unknown())
- labels malformed -> undefined via .catch (safe: never masquerades as
  handled; worst case a redundant re-attach, never a silent drop)
A whole-body parse failure degrades to EMPTY_ISSUE (never throws into
React), matching the existing parseWithFallback contract.

Tests: schema.test.ts covers absent / valid / null / wrong-element-shape
labels and the empty-issue degrade.

Co-authored-by: multica-agent <github@multica.ai>

* fix(issues): reject a malformed create response instead of faking success (MUL-4832)

Follow-up to review of #5510. The prior fix degraded a schema-failed
create response to EMPTY_ISSUE, which React Query treats as a successful
mutation: the empty issue is written into the list cache, a blank
"created" toast shows, the "View issue" link points at an empty id, and
with labels the fallback attach runs against an empty issue id.

A create that returns an unusable body is a failed mutation, not a
safe-empty read. Fall back to null and reject: mutateAsync is already
inside the create modal's try/catch, so a controlled rejection preserves
the draft and shows the failure toast, and onSettled still refreshes the
list so a genuinely-created issue can still surface.

- CreateIssueResponseSchema tightens id to non-empty; an id-less body
  routes to the same reject path.
- createIssue throws on parse failure (empty-message Error so the modal
  renders its localized "failed to create" toast; parseWithFallback
  already logged the schema issues + raw body).
- Dropped the EMPTY_ISSUE fallback constant.
- Tests: whole-body malformed and empty-id now assert createIssue
  rejects; only-labels-malformed still returns the real issue with
  labels undefined.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-07-16 13:31:06 +08:00
committed by GitHub
parent b13b6d06bc
commit 4fcb27a729
10 changed files with 641 additions and 32 deletions

View File

@@ -202,6 +202,7 @@ import {
AutopilotRunSchema,
FALLBACK_AUTOPILOT_RUN,
ListIssuesResponseSchema,
CreateIssueResponseSchema,
ListWebhookDeliveriesResponseSchema,
RuntimeHourlyActivityListSchema,
RuntimeUsageByAgentListSchema,
@@ -658,10 +659,26 @@ export class ApiClient {
}
async createIssue(data: CreateIssueRequest): Promise<Issue> {
return this.fetch("/api/issues", {
// Parse through a schema (not a raw cast): the create modal keys its
// label-attach compatibility fallback off `labels` being absent vs a
// validated Label[], so an unvalidated wrong shape must not slip through.
// Unlike list endpoints, a create that returns an unusable body is a
// FAILED mutation, not a safe-empty read: fall back to null and reject so
// the modal keeps the draft and shows a failure toast instead of a blank
// "created" card pointing at an empty issue id. parseWithFallback already
// logged the schema issues + raw payload; the empty message lets the modal
// render its localized "failed to create" toast.
const raw = await this.fetch<unknown>("/api/issues", {
method: "POST",
body: JSON.stringify(data),
});
const issue = parseWithFallback<Issue | null>(raw, CreateIssueResponseSchema, null, {
endpoint: "POST /api/issues",
});
if (!issue) {
throw new Error();
}
return issue;
}
async quickCreateIssue(data: {

View File

@@ -91,6 +91,91 @@ describe("ApiClient schema fallback", () => {
});
});
describe("createIssue", () => {
// The create modal decides whether to run its label-attach fallback by
// reading `labels` off the parsed response, and treats a rejection as a
// failed create (keep the draft, failure toast). So: a valid issue with
// any labels shape resolves (labels absent → undefined, valid → Label[],
// malformed → undefined), but a body that isn't a usable issue rejects
// rather than fabricating a blank "success".
const validIssue = {
id: "issue-1",
workspace_id: "ws-1",
number: 1,
identifier: "MUL-1",
title: "Created",
description: null,
status: "todo",
priority: "none",
assignee_type: null,
assignee_id: null,
creator_type: "member",
creator_id: "user-1",
parent_issue_id: null,
project_id: null,
position: 0,
stage: null,
start_date: null,
due_date: null,
metadata: {},
properties: {},
created_at: "2025-01-01T00:00:00Z",
updated_at: "2025-01-01T00:00:00Z",
};
const label = {
id: "label-1",
workspace_id: "ws-1",
name: "bug",
color: "#ef4444",
created_at: "2025-01-01T00:00:00Z",
updated_at: "2025-01-01T00:00:00Z",
};
it("keeps labels undefined when the backend omits the field (older backend)", async () => {
stubFetchJson(validIssue, 201);
const client = new ApiClient("https://api.example.test");
const issue = await client.createIssue({ title: "Created" });
expect(issue.id).toBe("issue-1");
expect(issue.labels).toBeUndefined();
});
it("validates a well-formed labels array", async () => {
stubFetchJson({ ...validIssue, labels: [label] }, 201);
const client = new ApiClient("https://api.example.test");
const issue = await client.createIssue({ title: "Created" });
expect(issue.labels?.map((l) => l.id)).toEqual(["label-1"]);
});
it("degrades a null labels field to undefined so the client falls back", async () => {
stubFetchJson({ ...validIssue, labels: null }, 201);
const client = new ApiClient("https://api.example.test");
const issue = await client.createIssue({ title: "Created" });
// The issue itself still parses; only the malformed labels degrade.
expect(issue.id).toBe("issue-1");
expect(issue.labels).toBeUndefined();
});
it("degrades a labels array of the wrong element shape to undefined", async () => {
stubFetchJson({ ...validIssue, labels: [{ nope: true }] }, 201);
const client = new ApiClient("https://api.example.test");
const issue = await client.createIssue({ title: "Created" });
expect(issue.id).toBe("issue-1");
expect(issue.labels).toBeUndefined();
});
it("rejects when the whole response body is not a usable issue (no fake success)", async () => {
stubFetchJson({ not: "an issue" }, 201);
const client = new ApiClient("https://api.example.test");
await expect(client.createIssue({ title: "Created" })).rejects.toThrow();
});
it("rejects when the created issue has an empty id", async () => {
stubFetchJson({ ...validIssue, id: "" }, 201);
const client = new ApiClient("https://api.example.test");
await expect(client.createIssue({ title: "Created" })).rejects.toThrow();
});
});
describe("searchIssues", () => {
it("falls back to an empty result when the response is malformed", async () => {
stubFetchJson({ issues: "not-an-array", total: 0 });

View File

@@ -469,6 +469,25 @@ export const ListIssuesResponseSchema = z.object({
total: z.number().default(0),
}).loose();
// Response schema for POST /api/issues. Two tightenings over IssueSchema:
//
// - `id` must be non-empty. A created issue always carries a real id, so an
// empty/absent id means the create effectively failed. createIssue turns a
// schema failure into a rejection (not a fabricated success), so tightening
// id here routes an id-less body to that same failure path.
// - `labels` is the backend-compatibility signal the create modal reads to
// decide whether the backend attached labels in the create transaction
// (present) or predates that (absent → fall back to per-label attach).
// Validate it strictly as Label[] and degrade a malformed value to
// `undefined` — the same as an absent field — so a wrong shape (null,
// object, a garbage array) can never masquerade as "handled" and suppress
// the fallback. Unlike the loose IssueSchema.labels (z.array(z.unknown())),
// the elements are fully validated. See packages/views/modals/create-issue.tsx.
export const CreateIssueResponseSchema = IssueSchema.extend({
id: z.string().min(1),
labels: z.array(LabelSchema).optional().catch(undefined),
}).loose();
export const EMPTY_LIST_ISSUES_RESPONSE: ListIssuesResponse = {
issues: [],
total: 0,

View File

@@ -264,6 +264,23 @@ describe("project progress invalidation", () => {
});
});
describe("onIssueCreated — carries the label snapshot into list cache", () => {
it("keeps the created issue's labels so members other than the creator render it already labeled", () => {
// The backend now attaches labels in the create transaction and echoes
// them on the issue:created event. Guard that the cache insert doesn't
// strip them — otherwise online members would see the new issue blank
// until a refetch (staleTime: Infinity means no self-heal).
const qc = new QueryClient();
qc.setQueryData<ListIssuesCache>(issueKeys.list(WS_ID), makeListCache());
onIssueCreated(qc, WS_ID, { ...baseIssue, labels: [labelA, labelB] });
const cache = qc.getQueryData<ListIssuesCache>(issueKeys.list(WS_ID));
const cached = cache?.byStatus.todo?.issues.find((i) => i.id === ISSUE_ID);
expect(cached?.labels).toEqual([labelA, labelB]);
});
});
describe("onIssueUpdated — position move is surgical, not a list refetch", () => {
let qc: QueryClient;

View File

@@ -17,6 +17,9 @@ export interface CreateIssueRequest {
start_date?: string;
due_date?: string;
attachment_ids?: string[];
/** Issue-scoped label IDs to attach in the same transaction as the create.
* Unknown or non-issue ids are rejected by the server with 400. */
label_ids?: string[];
}
export interface UpdateIssueRequest {

View File

@@ -23,6 +23,7 @@ function I18nWrapper({ children }: { children: ReactNode }) {
const mockPush = vi.hoisted(() => vi.fn());
const mockCreateIssue = vi.hoisted(() => vi.fn());
const mockAttachLabel = vi.hoisted(() => vi.fn());
const mockSetDraft = vi.hoisted(() => vi.fn());
const mockClearDraft = vi.hoisted(() => vi.fn());
const mockSetLastAssignee = vi.hoisted(() => vi.fn());
@@ -140,6 +141,10 @@ vi.mock("@multica/core/issues/mutations", () => ({
useUpdateIssue: () => ({ mutate: vi.fn() }),
}));
vi.mock("@multica/core/labels", () => ({
useAttachLabelToIssue: () => ({ mutateAsync: mockAttachLabel }),
}));
vi.mock("@multica/core/hooks/use-file-upload", () => ({
useFileUpload: () => ({ uploadWithToast: mockUploadWithToast }),
}));
@@ -455,7 +460,12 @@ describe("CreateIssueModal", () => {
identifier: "TES-123",
title: "Ship create issue regression coverage",
status: "todo",
// Current backend echoes the attached labels, so the create flow skips
// the legacy per-label attach fallback. Empty is enough — what matters
// is that the field is present (not undefined).
labels: [],
});
mockAttachLabel.mockResolvedValue({ labels: [] });
});
it("shows success feedback with a direct path to the new issue", async () => {
@@ -505,6 +515,71 @@ describe("CreateIssueModal", () => {
expect(mockToastDismiss).toHaveBeenCalledWith("toast-1");
});
it("forwards selected labels in the create payload so they attach in the same transaction", async () => {
const user = userEvent.setup();
mockDraftStore.draft.labelIds = [
"aaaaaaaa-1111-2222-3333-444444444444",
"bbbbbbbb-1111-2222-3333-444444444444",
];
renderModal(<CreateIssueModal onClose={vi.fn()} />);
fireEvent.change(screen.getByPlaceholderText("Issue title"), {
target: { value: "Labeled issue" },
});
await user.click(screen.getByRole("button", { name: "Create Issue" }));
await waitFor(() => {
expect(mockCreateIssue).toHaveBeenCalledWith(
expect.objectContaining({
title: "Labeled issue",
label_ids: [
"aaaaaaaa-1111-2222-3333-444444444444",
"bbbbbbbb-1111-2222-3333-444444444444",
],
}),
);
});
// Backend echoed `labels`, so the atomic path handled it — no legacy
// per-label attach fallback should run.
expect(mockAttachLabel).not.toHaveBeenCalled();
});
it("falls back to per-label attach when an older backend omits labels from the create response", async () => {
const user = userEvent.setup();
// Older backend: ignores label_ids and returns an issue with no `labels`
// field (the rolling-deploy window where web is ahead of the backend).
mockCreateIssue.mockResolvedValueOnce({
id: "issue-123",
identifier: "TES-123",
title: "Labeled issue",
status: "todo",
});
mockDraftStore.draft.labelIds = [
"aaaaaaaa-1111-2222-3333-444444444444",
"bbbbbbbb-1111-2222-3333-444444444444",
];
renderModal(<CreateIssueModal onClose={vi.fn()} />);
fireEvent.change(screen.getByPlaceholderText("Issue title"), {
target: { value: "Labeled issue" },
});
await user.click(screen.getByRole("button", { name: "Create Issue" }));
await waitFor(() => {
expect(mockAttachLabel).toHaveBeenCalledTimes(2);
});
expect(mockAttachLabel).toHaveBeenCalledWith({
issueId: "issue-123",
labelId: "aaaaaaaa-1111-2222-3333-444444444444",
});
expect(mockAttachLabel).toHaveBeenCalledWith({
issueId: "issue-123",
labelId: "bbbbbbbb-1111-2222-3333-444444444444",
});
});
it("keeps manual mode open and clears content when create another is enabled", async () => {
const user = userEvent.setup();
const onClose = vi.fn();

View File

@@ -362,6 +362,12 @@ export function ManualCreatePanel({
start_date: startDate || undefined,
due_date: dueDate || undefined,
attachment_ids: activeAttachmentIds.length > 0 ? activeAttachmentIds : undefined,
// The server attaches these in the same transaction as the create and
// echoes them back as `issue.labels`, so a stale selection fails the
// create instead of leaving a committed-but-unlabeled issue. A legacy
// backend that predates this ignores the field — handled by the
// compatibility fallback below.
label_ids: labelIds.length > 0 ? labelIds : undefined,
parent_issue_id: parentIssueId,
// Stage is only meaningful for a sub-issue (relative to its siblings).
stage: parentIssueId && stage != null ? stage : undefined,
@@ -402,11 +408,15 @@ export function ManualCreatePanel({
}
}
// Attach the labels chosen in the dialog. Like the sub-issue links
// above, this is deferred to after create because the new issue's ID
// doesn't exist yet, and the create endpoint takes no labels. Partial
// failures don't roll back the committed issue.
if (labelIds.length > 0) {
// Backend-compatibility fallback for the rolling deploy window: the web
// app auto-deploys on merge but the backend deploys manually, so a newer
// web build can briefly talk to a backend that predates atomic label
// creation. That backend silently ignores `label_ids` and returns an
// issue with no `labels` field. Only then do we fall back to the legacy
// per-label attach so the user's labels aren't silently dropped. When
// `labels` is present (current backend) the atomic path already ran, so
// we skip this — no double-write, no per-label fan-out.
if (labelIds.length > 0 && issue.labels === undefined) {
const results = await Promise.allSettled(
labelIds.map((labelId) =>
attachLabelMutation.mutateAsync({ issueId: issue.id, labelId }),
@@ -416,7 +426,7 @@ export function ManualCreatePanel({
for (const result of results) {
if (result.status === "rejected") {
labelsFailed += 1;
console.error("[create-issue] label attach failed", result.reason);
console.error("[create-issue] label attach fallback failed", result.reason);
}
}
if (labelsFailed > 0) {

View File

@@ -2185,6 +2185,10 @@ type CreateIssueRequest struct {
StartDate *string `json:"start_date"`
DueDate *string `json:"due_date"`
AttachmentIDs []string `json:"attachment_ids,omitempty"`
// LabelIDs are issue-scoped labels to attach to the new issue in the same
// transaction as the create. Unknown or non-issue ids are rejected with
// 400 (service.ErrIssueLabelNotFound) rather than silently dropped.
LabelIDs []string `json:"label_ids,omitempty"`
// OriginType / OriginID stamp the new issue with its provenance so
// platform-internal flows can deterministically locate it later. Only
// trusted callers should set these — currently the daemon CLI passes
@@ -2287,6 +2291,11 @@ func (h *Handler) CreateIssue(w http.ResponseWriter, r *http.Request) {
return
}
labelIDs, ok := parseUUIDSliceOrBadRequest(w, req.LabelIDs, "label_ids")
if !ok {
return
}
var startDate pgtype.Date
if req.StartDate != nil && *req.StartDate != "" {
d, err := util.ParseCalendarDate(*req.StartDate)
@@ -2405,14 +2414,21 @@ func (h *Handler) CreateIssue(w http.ResponseWriter, r *http.Request) {
OriginID: originID,
Stage: ptrToInt4(req.Stage),
AttachmentIDs: attachmentIDs,
LabelIDs: labelIDs,
AllowDuplicate: req.AllowDuplicate,
}, service.IssueCreateOpts{
ActorID: actualCreatorID,
AnalyticsAgentID: analyticsAgentID,
Platform: func() string { p, _, _ := middleware.ClientMetadataFromContext(r.Context()); return p }(),
BroadcastPayload: func(issue db.Issue, atts []db.Attachment) map[string]any {
BroadcastPayload: func(issue db.Issue, atts []db.Attachment, labels []db.IssueLabel) map[string]any {
payload := issueToResponse(issue, prefix)
payload.Attachments = buildAttachmentResponses(atts)
// Carry the authoritative label snapshot so every online client
// renders the new issue already labeled. Non-nil (even empty)
// pointer = authoritative list; the old flow's separate
// issue_labels:changed broadcast is gone.
labelResponses := labelsToResponse(labels)
payload.Labels = &labelResponses
return map[string]any{"issue": payload}
},
})
@@ -2435,6 +2451,10 @@ func (h *Handler) CreateIssue(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusBadRequest, "project not found in this workspace")
return
}
if errors.Is(err, service.ErrIssueLabelNotFound) {
writeError(w, http.StatusBadRequest, "one or more labels not found in this workspace")
return
}
if err != nil {
slog.Warn("create issue failed", append(logger.RequestAttrs(r), "error", err, "workspace_id", workspaceID)...)
writeError(w, http.StatusInternalServerError, "failed to create issue: "+err.Error())
@@ -2446,6 +2466,11 @@ func (h *Handler) CreateIssue(w http.ResponseWriter, r *http.Request) {
resp := issueToResponse(issue, prefix)
resp.Attachments = buildAttachmentResponses(res.Attachments)
// Echo the authoritative labels attached in the create transaction. Always
// non-nil (empty slice when none) so a newer client can tell the backend
// understood label_ids and skip its legacy post-create attach fallback.
labelResponses := labelsToResponse(res.Labels)
resp.Labels = &labelResponses
writeJSON(w, http.StatusCreated, resp)
}

View File

@@ -0,0 +1,268 @@
package handler
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/google/uuid"
"github.com/multica-ai/multica/server/internal/events"
"github.com/multica-ai/multica/server/pkg/protocol"
)
// createTestIssueLabel creates an issue-scoped label in the test workspace via
// the public handler and registers cleanup. It returns the new label id.
func createTestIssueLabel(t *testing.T, name string) string {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("POST", "/api/labels", map[string]any{
"name": name,
"color": "#ef4444",
})
testHandler.CreateLabel(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateLabel %q: expected 201, got %d: %s", name, w.Code, w.Body.String())
}
var created LabelResponse
if err := json.NewDecoder(w.Body).Decode(&created); err != nil {
t.Fatalf("decode label: %v", err)
}
t.Cleanup(func() {
testPool.Exec(context.Background(), `DELETE FROM issue_label WHERE id = $1`, created.ID)
})
return created.ID
}
func countIssueLabel(t *testing.T, issueID, labelID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(),
`SELECT COUNT(*) FROM issue_to_label WHERE issue_id = $1 AND label_id = $2`,
issueID, labelID,
).Scan(&n); err != nil {
t.Fatalf("count issue_to_label: %v", err)
}
return n
}
func countIssuesWithTitle(t *testing.T, title string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(),
`SELECT COUNT(*) FROM issue WHERE workspace_id = $1 AND title = $2`,
testWorkspaceID, title,
).Scan(&n); err != nil {
t.Fatalf("count issues: %v", err)
}
return n
}
// TestCreateIssueAttachesLabelsAtomically verifies that labels passed in the
// create request are written in the same transaction as the issue, so a
// created issue already carries its labels without a second round-trip.
func TestCreateIssueAttachesLabelsAtomically(t *testing.T) {
labelA := createTestIssueLabel(t, "cl-a-"+uuid.NewString()[:8])
labelB := createTestIssueLabel(t, "cl-b-"+uuid.NewString()[:8])
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "create-labels attaches",
"status": "todo",
"priority": "low",
"label_ids": []string{labelA, labelB},
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
t.Cleanup(func() { deleteTestIssue(t, issue.ID) })
if got := countIssueLabel(t, issue.ID, labelA); got != 1 {
t.Errorf("label A: expected 1 attachment, got %d", got)
}
if got := countIssueLabel(t, issue.ID, labelB); got != 1 {
t.Errorf("label B: expected 1 attachment, got %d", got)
}
// The create response must echo the authoritative labels so every online
// client (not just the creator) renders the new issue already labeled and
// a newer client can tell the backend understood label_ids.
if issue.Labels == nil {
t.Fatalf("create response must include a labels field")
}
got := map[string]bool{}
for _, l := range *issue.Labels {
got[l.ID] = true
}
if !got[labelA] || !got[labelB] {
t.Errorf("response labels %v missing labelA=%s or labelB=%s", got, labelA, labelB)
}
}
// TestCreateIssueResponseAlwaysIncludesLabelsField verifies that even when no
// labels are requested the create response carries an explicit (empty) labels
// list — the signal a newer client uses to know the backend handled labels and
// skip its legacy per-label attach fallback.
func TestCreateIssueResponseAlwaysIncludesLabelsField(t *testing.T) {
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "create-labels no-label response",
"status": "todo",
"priority": "low",
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
t.Cleanup(func() { deleteTestIssue(t, issue.ID) })
if issue.Labels == nil {
t.Fatalf("create response must include a labels field even when empty")
}
if len(*issue.Labels) != 0 {
t.Errorf("expected empty labels, got %d", len(*issue.Labels))
}
}
// TestCreateIssueEventCarriesLabels verifies that the issue:created websocket
// event carries the attached labels, so online members other than the creator
// render the new issue already labeled instead of blank until a refetch (the
// old flow relied on a separate issue_labels:changed broadcast this PR removed).
func TestCreateIssueEventCarriesLabels(t *testing.T) {
label := createTestIssueLabel(t, "cl-evt-"+uuid.NewString()[:8])
gotEvent := make(chan events.Event, 1)
testHandler.Bus.Subscribe(protocol.EventIssueCreated, func(e events.Event) {
select {
case gotEvent <- e:
default:
}
})
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "create-labels event carries",
"status": "todo",
"priority": "low",
"label_ids": []string{label},
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
t.Cleanup(func() { deleteTestIssue(t, issue.ID) })
select {
case ev := <-gotEvent:
pm, ok := ev.Payload.(map[string]any)
if !ok {
t.Fatalf("issue:created payload has unexpected type %T", ev.Payload)
}
resp, ok := pm["issue"].(IssueResponse)
if !ok {
t.Fatalf("issue:created payload issue has unexpected type %T", pm["issue"])
}
if resp.Labels == nil {
t.Fatalf("issue:created payload must include a labels field")
}
found := false
for _, l := range *resp.Labels {
if l.ID == label {
found = true
}
}
if !found {
t.Errorf("issue:created labels %+v missing %s", *resp.Labels, label)
}
case <-time.After(2 * time.Second):
t.Fatal("did not receive issue:created event")
}
}
// TestCreateIssueDedupesDuplicateLabelIDs verifies that a repeated label id in
// the request attaches exactly once (ON CONFLICT DO NOTHING) and still 201s.
func TestCreateIssueDedupesDuplicateLabelIDs(t *testing.T) {
label := createTestIssueLabel(t, "cl-dup-"+uuid.NewString()[:8])
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": "create-labels dedupe",
"status": "todo",
"priority": "low",
"label_ids": []string{label, label},
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
t.Cleanup(func() { deleteTestIssue(t, issue.ID) })
if got := countIssueLabel(t, issue.ID, label); got != 1 {
t.Errorf("expected label attached exactly once, got %d", got)
}
}
// TestCreateIssueRejectsUnknownLabelWithoutCreating verifies that an unknown
// label id fails the whole create with 400 and leaves no issue behind — the
// atomicity guarantee the old post-create attach flow could not offer.
func TestCreateIssueRejectsUnknownLabelWithoutCreating(t *testing.T) {
const title = "create-labels unknown rejected"
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": title,
"status": "todo",
"priority": "low",
"label_ids": []string{uuid.NewString()},
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("CreateIssue: expected 400, got %d: %s", w.Code, w.Body.String())
}
if got := countIssuesWithTitle(t, title); got != 0 {
t.Errorf("expected no issue created on invalid label, found %d", got)
}
}
// TestCreateIssueRejectsNonIssueScopedLabel verifies that a label scoped to a
// non-issue resource (e.g. agent) cannot be attached to an issue at create,
// mirroring the resource_type guard baked into AttachLabelToIssue.
func TestCreateIssueRejectsNonIssueScopedLabel(t *testing.T) {
var agentLabelID string
if err := testPool.QueryRow(context.Background(),
`INSERT INTO issue_label (workspace_id, resource_type, name, color)
VALUES ($1, 'agent', $2, '#000000') RETURNING id`,
testWorkspaceID, "create-labels agent "+uuid.NewString(),
).Scan(&agentLabelID); err != nil {
t.Fatalf("insert agent label: %v", err)
}
t.Cleanup(func() {
testPool.Exec(context.Background(), `DELETE FROM issue_label WHERE id = $1`, agentLabelID)
})
const title = "create-labels wrong scope rejected"
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": title,
"status": "todo",
"priority": "low",
"label_ids": []string{agentLabelID},
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("CreateIssue: expected 400, got %d: %s", w.Code, w.Body.String())
}
if got := countIssuesWithTitle(t, title); got != 0 {
t.Errorf("expected no issue created on wrong-scope label, found %d", got)
}
}

View File

@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/analytics"
"github.com/multica-ai/multica/server/internal/events"
@@ -50,22 +51,28 @@ func NewIssueService(q *db.Queries, tx TxStarter, bus *events.Bus, ac analytics.
// to IssueService.Create. The handler owns the parsing step that turns its
// request payload into this struct; the service stays transport-agnostic.
type IssueCreateParams struct {
WorkspaceID pgtype.UUID
Title string
Description pgtype.Text
Status string
Priority string
AssigneeType pgtype.Text
AssigneeID pgtype.UUID
CreatorType string // "agent" or "member"
CreatorID pgtype.UUID
ParentIssueID pgtype.UUID
ProjectID pgtype.UUID
StartDate pgtype.Date
DueDate pgtype.Date
OriginType pgtype.Text
OriginID pgtype.UUID
AttachmentIDs []pgtype.UUID
WorkspaceID pgtype.UUID
Title string
Description pgtype.Text
Status string
Priority string
AssigneeType pgtype.Text
AssigneeID pgtype.UUID
CreatorType string // "agent" or "member"
CreatorID pgtype.UUID
ParentIssueID pgtype.UUID
ProjectID pgtype.UUID
StartDate pgtype.Date
DueDate pgtype.Date
OriginType pgtype.Text
OriginID pgtype.UUID
AttachmentIDs []pgtype.UUID
// LabelIDs are the issue-scoped labels to attach to the new issue. They
// are validated and written inside the create transaction (see Create),
// so the issue is never committed with a partial or wrong label set. An
// unknown or non-issue label id fails the whole create with
// ErrIssueLabelNotFound rather than being silently dropped.
LabelIDs []pgtype.UUID
AllowDuplicate bool
// Stage groups this issue into an ordered barrier group under its parent
// (NULL = unstaged). See issue_child_done.go for the staged-barrier wake.
@@ -82,8 +89,11 @@ type IssueCreateOpts struct {
// package to depend on handler-layer types. If nil, the service
// emits a minimal `{"issue_id": <uuid>}` payload — enough for cache
// invalidation, but front-ends that expect the full response shape
// must provide BroadcastPayload.
BroadcastPayload func(issue db.Issue, attachments []db.Attachment) map[string]any
// must provide BroadcastPayload. The labels argument is the authoritative
// snapshot attached in the create transaction, so the emitted payload can
// carry the new issue's labels and every online client renders it already
// labeled instead of blank until a refetch.
BroadcastPayload func(issue db.Issue, attachments []db.Attachment, labels []db.IssueLabel) map[string]any
// ActorID overrides the actor ID used for broadcast + analytics
// when it differs from the creator on the row. Agent-created issues
@@ -124,6 +134,12 @@ var ErrParentIssueNotFound = errors.New("parent issue not found in this workspac
// having to remember it. Callers translate this into 400.
var ErrProjectNotFound = errors.New("project not found in this workspace")
// ErrIssueLabelNotFound signals that one of the supplied LabelIDs does not
// exist in the issue's workspace or is not an issue-scoped label. The whole
// create is rejected so a new issue is never born with a partial or wrong
// label set. Callers translate this into their transport's 400.
var ErrIssueLabelNotFound = errors.New("issue label not found in this workspace")
// IssueCreateResult is the typed return from IssueService.Create.
//
// - On the happy path: Issue is the new row, Attachments lists the
@@ -131,8 +147,14 @@ var ErrProjectNotFound = errors.New("project not found in this workspace")
// - On ErrActiveDuplicate: DuplicateIssue is the row that blocked the
// create; Issue and Attachments are zero.
type IssueCreateResult struct {
Issue db.Issue
Attachments []db.Attachment
Issue db.Issue
Attachments []db.Attachment
// Labels is the authoritative set of labels attached to the new issue in
// the create transaction (empty when none were requested). Callers echo it
// on the create response + issue:created event so every client renders the
// new issue already labeled and a new client can detect that the backend
// understood label_ids (see the create handler's compatibility contract).
Labels []db.IssueLabel
DuplicateIssue *db.Issue
}
@@ -194,6 +216,15 @@ func (s *IssueService) Create(ctx context.Context, p IssueCreateParams, opts Iss
}
}
// Validate labels before we increment the issue counter so a stale or
// wrong-scope selection fails the create cheaply. The de-duplicated rows
// are attached to the issue below, inside this same transaction, and
// echoed back as the authoritative snapshot in the result.
labels, err := validateIssueLabels(ctx, qtx, p.WorkspaceID, p.LabelIDs)
if err != nil {
return IssueCreateResult{}, err
}
duplicate, found, err := issueguard.LockAndFindActiveDuplicate(ctx, qtx, p.WorkspaceID, projectID, p.ParentIssueID, p.Title, p.AllowDuplicate)
if err != nil {
return IssueCreateResult{}, fmt.Errorf("duplicate guard: %w", err)
@@ -268,6 +299,22 @@ func (s *IssueService) Create(ctx context.Context, p IssueCreateParams, opts Iss
return IssueCreateResult{}, fmt.Errorf("create issue: %w", err)
}
// Attach labels inside the create transaction so the issue and its
// labels commit together — the old flow created the issue first and
// attached labels in a second, non-atomic round-trip whose partial
// failure left the issue mis-categorized. AttachLabelToIssue is
// workspace/resource_type-guarded and ON CONFLICT DO NOTHING, and the
// ids were already validated above.
for _, label := range labels {
if err := qtx.AttachLabelToIssue(ctx, db.AttachLabelToIssueParams{
IssueID: issue.ID,
LabelID: label.ID,
WorkspaceID: p.WorkspaceID,
}); err != nil {
return IssueCreateResult{}, fmt.Errorf("attach issue label: %w", err)
}
}
if err := tx.Commit(ctx); err != nil {
return IssueCreateResult{}, fmt.Errorf("commit: %w", err)
}
@@ -279,11 +326,54 @@ func (s *IssueService) Create(ctx context.Context, p IssueCreateParams, opts Iss
actorID = util.UUIDToString(issue.CreatorID)
}
s.publishIssueCreated(issue, attachments, p.CreatorType, actorID, opts)
s.publishIssueCreated(issue, attachments, labels, p.CreatorType, actorID, opts)
s.captureCreatedAnalytics(issue, p.CreatorType, actorID, opts)
s.maybeEnqueueOnAssign(ctx, issue, p.CreatorType, actorID)
return IssueCreateResult{Issue: issue, Attachments: attachments}, nil
return IssueCreateResult{Issue: issue, Attachments: attachments, Labels: labels}, nil
}
// validateIssueLabels checks that every requested label exists in the
// workspace and is issue-scoped, returning the de-duplicated label rows to
// attach. Returning the full rows (not just ids) lets Create echo an
// authoritative label snapshot on the create response + issue:created event
// without a second query. It mirrors the workspace + resource_type='issue'
// guard already enforced by AttachLabelToIssue so an unknown or wrong-scope id
// surfaces as ErrIssueLabelNotFound instead of a silent no-op insert. Invalid
// (zero) UUIDs are skipped. The label count per issue is small, so a GetLabel
// per distinct id is fine and avoids introducing a new batch query.
func validateIssueLabels(ctx context.Context, qtx *db.Queries, workspaceID pgtype.UUID, labelIDs []pgtype.UUID) ([]db.IssueLabel, error) {
if len(labelIDs) == 0 {
return nil, nil
}
seen := make(map[string]struct{}, len(labelIDs))
deduped := make([]db.IssueLabel, 0, len(labelIDs))
for _, labelID := range labelIDs {
if !labelID.Valid {
continue
}
key := util.UUIDToString(labelID)
if _, ok := seen[key]; ok {
continue
}
seen[key] = struct{}{}
label, err := qtx.GetLabel(ctx, db.GetLabelParams{
ID: labelID,
WorkspaceID: workspaceID,
})
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return nil, ErrIssueLabelNotFound
}
return nil, fmt.Errorf("get issue label: %w", err)
}
if label.ResourceType != "issue" {
return nil, ErrIssueLabelNotFound
}
deduped = append(deduped, label)
}
return deduped, nil
}
// linkAttachments links the given attachment IDs to the newly created
@@ -318,13 +408,13 @@ func (s *IssueService) linkAttachments(ctx context.Context, issue db.Issue, ids
return list
}
func (s *IssueService) publishIssueCreated(issue db.Issue, attachments []db.Attachment, creatorType, actorID string, opts IssueCreateOpts) {
func (s *IssueService) publishIssueCreated(issue db.Issue, attachments []db.Attachment, labels []db.IssueLabel, creatorType, actorID string, opts IssueCreateOpts) {
if s.Bus == nil {
return
}
var payload map[string]any
if opts.BroadcastPayload != nil {
payload = opts.BroadcastPayload(issue, attachments)
payload = opts.BroadcastPayload(issue, attachments, labels)
} else {
// Minimal fallback so cache invalidations still fire even if the
// caller forgot to supply a builder. Front-ends that expect the