mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 09:26:50 +08:00
* 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]>
259 lines
11 KiB
Go
259 lines
11 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
)
|
|
|
|
// createPrivateAgentOwnedBy inserts a private agent owned by ownerID and
|
|
// returns its id. Mirrors privateAgentTestFixture's agent shape (visibility
|
|
// 'private', no explicit permission_mode so it keeps the private default the
|
|
// canInvokeAgent gate treats as owner-only), but lets a test stand up a SECOND
|
|
// private agent under the same human so a leader and a worker can both be
|
|
// private and owned by the same originator.
|
|
func createPrivateAgentOwnedBy(t *testing.T, name, ownerID string) string {
|
|
t.Helper()
|
|
var agentID string
|
|
if err := testPool.QueryRow(context.Background(), `
|
|
INSERT INTO agent (
|
|
workspace_id, name, description, runtime_mode, runtime_config,
|
|
runtime_id, visibility, max_concurrent_tasks, owner_id,
|
|
instructions, custom_env, custom_args
|
|
)
|
|
VALUES ($1, $2, '', 'cloud', '{}'::jsonb,
|
|
$3, 'private', 1, $4, '', '{}'::jsonb, '[]'::jsonb)
|
|
RETURNING id
|
|
`, testWorkspaceID, name, handlerTestRuntimeID(t), ownerID).Scan(&agentID); err != nil {
|
|
t.Fatalf("create private agent %q: %v", name, err)
|
|
}
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, agentID) })
|
|
return agentID
|
|
}
|
|
|
|
// TestAgentCreateOriginator_E2E_CreateAssignSquad_PrivateWorkerTriggered walks
|
|
// the exact line failure shape from MUL-4305 end to end, deliberately NOT
|
|
// re-testing the resolver in isolation but locking the real wiring between the
|
|
// handler create stamp, the create-time squad gate, the squad-leader task's
|
|
// stored originator, the comment source-task stamp, and the private-worker
|
|
// invocation gate:
|
|
//
|
|
// human H triggers agent A → A creates an issue via the ordinary
|
|
// `issue create` path AND assigns it to a squad whose leader is a private
|
|
// agent owned by H → the leader's assignment run @-mentions a *second*
|
|
// private agent J (also owned by H) → J must be triggered.
|
|
//
|
|
// Pre-fix, A's create left the issue unattributed, the leader task stored a
|
|
// NULL originator, and the leader's mention of J failed canInvokeAgent — J
|
|
// silently got 0 tasks. This asserts the leader task carries H and that J ends
|
|
// up with a queued task whose originator is the original human H.
|
|
func TestAgentCreateOriginator_E2E_CreateAssignSquad_PrivateWorkerTriggered(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
|
|
// Private worker J and the human originator H (J's owner).
|
|
workerJID, ownerH, _ := privateAgentTestFixture(t)
|
|
|
|
// Private squad leader L, owned by the same human H.
|
|
leaderID := createPrivateAgentOwnedBy(t, "mul4305-e2e-private-leader", ownerH)
|
|
|
|
var squadID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
|
|
VALUES ($1, 'MUL-4305 E2E Squad', '', $2, $3)
|
|
RETURNING id
|
|
`, testWorkspaceID, leaderID, testUserID).Scan(&squadID); err != nil {
|
|
t.Fatalf("create squad: %v", err)
|
|
}
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM squad WHERE id = $1`, squadID) })
|
|
|
|
// Creator agent A, running a task on behalf of the human H. resolveActor
|
|
// validates the (A, task) pair; the handler then trusts X-Task-ID as A's
|
|
// acting task and inherits H from it.
|
|
creatorAID := createHandlerTestAgent(t, "mul4305-e2e-creator-agent", nil)
|
|
var creatorTaskID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, status, priority, originator_user_id, accountable_user_id)
|
|
VALUES ($1, (SELECT runtime_id FROM agent WHERE id = $1), 'running', 0, $2, $2)
|
|
RETURNING id
|
|
`, creatorAID, ownerH).Scan(&creatorTaskID); err != nil {
|
|
t.Fatalf("create A's acting task: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, creatorTaskID)
|
|
})
|
|
|
|
// Step 1: agent A creates an issue through the ordinary create path and
|
|
// assigns it to the private-leader squad in the same call.
|
|
w := httptest.NewRecorder()
|
|
r := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
|
|
"title": "MUL-4305 E2E agent-created + squad-assigned",
|
|
"assignee_type": "squad",
|
|
"assignee_id": squadID,
|
|
})
|
|
r.Header.Set("X-Agent-ID", creatorAID)
|
|
r.Header.Set("X-Task-ID", creatorTaskID)
|
|
testHandler.CreateIssue(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
var created IssueResponse
|
|
if err := json.NewDecoder(w.Body).Decode(&created); err != nil {
|
|
t.Fatalf("decode issue: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id = $1`, created.ID)
|
|
testPool.Exec(context.Background(), `DELETE FROM comment WHERE issue_id = $1`, created.ID)
|
|
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, created.ID)
|
|
})
|
|
|
|
// The create must have stamped the issue back to A's acting task.
|
|
var originType, originID string
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT COALESCE(origin_type, ''), COALESCE(origin_id::text, '') FROM issue WHERE id = $1`, created.ID,
|
|
).Scan(&originType, &originID); err != nil {
|
|
t.Fatalf("load issue origin: %v", err)
|
|
}
|
|
if originType != "agent_create" || originID != creatorTaskID {
|
|
t.Fatalf("issue origin = (%q,%q), want (agent_create,%s)", originType, originID, creatorTaskID)
|
|
}
|
|
|
|
// The squad-leader assignment task must have been enqueued carrying H.
|
|
var leaderTaskID, leaderOriginator string
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT id, COALESCE(originator_user_id::text, '')
|
|
FROM agent_task_queue
|
|
WHERE issue_id = $1 AND agent_id = $2 AND is_leader_task
|
|
ORDER BY created_at DESC LIMIT 1
|
|
`, created.ID, leaderID).Scan(&leaderTaskID, &leaderOriginator); err != nil {
|
|
t.Fatalf("load squad-leader task (expected one enqueued on create): %v", err)
|
|
}
|
|
if leaderOriginator != ownerH {
|
|
t.Fatalf("squad-leader task originator = %q, want the original human H %q", leaderOriginator, ownerH)
|
|
}
|
|
|
|
// Step 2: the leader L, running its assignment task, posts a comment that
|
|
// @-mentions the private worker J. The comment stamps source_task_id = the
|
|
// leader task, so the originator chain resolves to H.
|
|
w = httptest.NewRecorder()
|
|
r = newRequest("POST", "/api/issues/"+created.ID+"/comments", map[string]any{
|
|
"content": "handing the private part to [@Worker](mention://agent/" + workerJID + ")",
|
|
})
|
|
r.Header.Set("X-Agent-ID", leaderID)
|
|
r.Header.Set("X-Task-ID", leaderTaskID)
|
|
r = withURLParam(r, "id", created.ID)
|
|
testHandler.CreateComment(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateComment (leader mentions worker): expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Step 3: the private worker J must now have a queued task whose originator
|
|
// is the original human H — the whole point of the fix.
|
|
var queuedForHuman int
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT count(*) FROM agent_task_queue
|
|
WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued' AND originator_user_id = $3
|
|
`, created.ID, workerJID, ownerH).Scan(&queuedForHuman); err != nil {
|
|
t.Fatalf("count worker tasks: %v", err)
|
|
}
|
|
if queuedForHuman == 0 {
|
|
t.Fatalf("private worker got 0 queued tasks attributed to H; the A2A mention was denied (MUL-4305 regression)")
|
|
}
|
|
}
|
|
|
|
// TestAgentCreateOriginator_E2E_UpdateAssignSquad_HandlerGateAdmitsPrivateLeader
|
|
// locks the specific squad-leader GATE change in this PR, which the create path
|
|
// above does not exercise (create routes through the ungated service enqueue).
|
|
//
|
|
// Agent A (running for human H) creates an UNASSIGNED issue via the ordinary
|
|
// path, then assigns it to a private-leader squad via UpdateIssue. That assign
|
|
// routes through the handler enqueueSquadLeaderTask gate. Pre-fix the gate saw
|
|
// an empty originator for an agent actor and denied the private leader, so no
|
|
// leader task was enqueued even though the HTTP assign returned 200. With the
|
|
// agent_create stamp feeding the shared OriginatorForIssueTask, the gate now
|
|
// resolves H and enqueues the leader task carrying H.
|
|
func TestAgentCreateOriginator_E2E_UpdateAssignSquad_HandlerGateAdmitsPrivateLeader(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
|
|
// Reuse the private-agent fixture purely for a private agent + its human
|
|
// owner H; the fixture agent here plays the squad leader.
|
|
leaderID, ownerH, _ := privateAgentTestFixture(t)
|
|
|
|
var squadID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
|
|
VALUES ($1, 'MUL-4305 E2E Update-Assign Squad', '', $2, $3)
|
|
RETURNING id
|
|
`, testWorkspaceID, leaderID, testUserID).Scan(&squadID); err != nil {
|
|
t.Fatalf("create squad: %v", err)
|
|
}
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM squad WHERE id = $1`, squadID) })
|
|
|
|
creatorAID := createHandlerTestAgent(t, "mul4305-e2e-update-creator", nil)
|
|
var creatorTaskID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, status, priority, originator_user_id, accountable_user_id)
|
|
VALUES ($1, (SELECT runtime_id FROM agent WHERE id = $1), 'running', 0, $2, $2)
|
|
RETURNING id
|
|
`, creatorAID, ownerH).Scan(&creatorTaskID); err != nil {
|
|
t.Fatalf("create A's acting task: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, creatorTaskID)
|
|
})
|
|
|
|
// Agent A creates an unassigned issue via the ordinary path.
|
|
w := httptest.NewRecorder()
|
|
r := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
|
|
"title": "MUL-4305 E2E unassigned then squad-assigned",
|
|
})
|
|
r.Header.Set("X-Agent-ID", creatorAID)
|
|
r.Header.Set("X-Task-ID", creatorTaskID)
|
|
testHandler.CreateIssue(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
var created IssueResponse
|
|
if err := json.NewDecoder(w.Body).Decode(&created); err != nil {
|
|
t.Fatalf("decode issue: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id = $1`, created.ID)
|
|
testPool.Exec(context.Background(), `DELETE FROM comment WHERE issue_id = $1`, created.ID)
|
|
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, created.ID)
|
|
})
|
|
|
|
// Agent A assigns the issue to the private-leader squad via UpdateIssue.
|
|
w = httptest.NewRecorder()
|
|
r = newRequest("PATCH", "/api/issues/"+created.ID, map[string]any{
|
|
"assignee_type": "squad",
|
|
"assignee_id": squadID,
|
|
})
|
|
r.Header.Set("X-Agent-ID", creatorAID)
|
|
r.Header.Set("X-Task-ID", creatorTaskID)
|
|
r = withURLParam(r, "id", created.ID)
|
|
testHandler.UpdateIssue(w, r)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("UpdateIssue (assign squad): expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// The gated handler path must have enqueued the private leader carrying H.
|
|
var leaderCount int
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT count(*) FROM agent_task_queue
|
|
WHERE issue_id = $1 AND agent_id = $2 AND is_leader_task AND originator_user_id = $3
|
|
`, created.ID, leaderID, ownerH).Scan(&leaderCount); err != nil {
|
|
t.Fatalf("count leader tasks: %v", err)
|
|
}
|
|
if leaderCount == 0 {
|
|
t.Fatalf("private squad leader got 0 tasks attributed to H after agent-triggered assign; the enqueue gate denied it (MUL-4305 gate regression)")
|
|
}
|
|
}
|