Commit Graph

1 Commits

Author SHA1 Message Date
Bohan Jiang
4fcb27a729 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>
2026-07-16 13:31:06 +08:00