Commit Graph
10 Commits
Author SHA1 Message Date
Jiayuan Zhangandmultica-agent 231dedb552 MUL-8034: chore: clean historical comments across the codebase (#9274)
* chore: clean historical comments (MUL-8034)

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

* docs: preserve pending Composio expiry contract

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

---------

Co-authored-by: multica-agent <[email protected]>
2026-10-15 21:54:33 +08:00
e7742b052b MUL-7776: fix(api): stop returning raw error text in 500 responses (#8905)
* fix(api): stop returning raw error text in 500 responses

39 handlers answered a 500 with the message followed by err.Error(),
sending database and store errors (constraint names, SQL states, Redis
errors) to API clients, while the other 720 500 responses in the handler
package send a fixed message. These now send their fixed message too and
log the error with the request's attributes, as the handlers that already
logged it do.

* fix(api): drop raw error text from the task callbacks and skill import 500s

completeTask and failTask sent err.Error() as the whole 500 message, and
the structured skill import put it in the result's Reason on its three
500s. They now send fixed messages and log the error. The task callbacks
keep their 500, so the daemon still retries them: isTransientError reads
the status code, never the body.

* fix(api): close remaining raw error leaks

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

---------

Co-authored-by: Yushen <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-10-10 12:17:42 +08:00
eddb0dc2ad MUL-6884 feat(metrics): attribute agent_runtime lookups by product source (#7822)
* feat(metrics): attribute agent_runtime lookups by product source

MUL-6884 asked which product API is behind the hot GetAgentRuntime query.
It has no single answer: a dozen unrelated paths issue the same primary-key
read, so pg_stat_statements sums them into one row and can report that the
statement is busy without saying which behaviour is driving it. Deciding
between "raise the heartbeat interval" and "back off the 500ms browser polls"
needs that split, and today it is a guess.

Add multica_agent_runtime_lookup_total{source,result} and route every
production read through service.RuntimeLookup, which carries the source with
the connection and classifies pgx.ErrNoRows as not_found rather than as an
error — a deleted runtime is the daemon's self-heal signal, not an outage.

The labels are a closed enum (15 sources x 3 results, all prewarmed at zero)
because this fires on the hottest read in the system; an unclassified call
site degrades to "other" instead of minting a series. No query, no timing
histogram, and no extra SQL on /metrics — this PR only makes the existing
calls countable.

An AST lint test fails CI on any direct Queries.GetAgentRuntime call outside
the wrapper, since a bypassed call site would quietly undercount whichever
source it belonged to and send the follow-up work after the wrong caller.

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

* refactor(metrics): drop the standalone handler lookup file

The two helpers it held are not an endpoint — no route, no HTTP handler, no
new file under cmd/ — but a new agent_runtime_lookup.go under internal/handler
reads like one, and a file name that has to be opened before it can be ruled
out is a bad file name.

Move both into runtime.go next to requireRuntimeReadAccess, where the rest of
the runtime read helpers already live, and name the metrics test after what it
tests. No behaviour change.

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

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-31 17:46:56 +08:00
8442504201 fix(runtimes): hide private runtimes from non-owners (#7435)
* fix(runtimes): enforce private runtime access

* fix(runtimes): gate agent model discovery

* refactor(runtimes): reuse isRuntimeUsableForUser on runtime detail

The detail page re-derived the owner-or-public rule inline instead of
calling the shared helper every other surface in this PR uses. The two
disagreed on an ownerless runtime: the helper is fail-closed (matching
the server's read gate, which reuses canUseRuntimeForAgent), while the
inline copy rendered Usage for a public one and could only 404.

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

---------

Co-authored-by: m2keloop <[email protected]>
Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-26 01:48:44 +08:00
d38da27ed4 MUL-5619: fix(cli): surface the server's 409 message instead of the generic conflict template (#6267)
* fix(cli): show the server's conflict message instead of the generic 409 template

Every 409 this API returns is a deterministic refusal that names its own fix
("a skill with this name already exists", "set parent_id (--parent) to <id>").
The CLI replaced all of them with a template that says the opposite — that the
state changed underneath you and you should re-fetch and retry. Agents took the
retry hint literally: GH #6264 reports 15+ identical retries over 10 minutes
followed by hours spent chasing an optimistic-concurrency theory that never
existed, and GH #5948 is a second user misdiagnosing the same way. MUL-4417 had
already written the useful message server-side; it just never reached anyone.

Route 409 through the same server-message extraction 400/422 already uses, so
roughly forty hand-written conflict messages across skills, agents, runtimes,
labels, projects and comments become visible by default. A body we cannot
recognize still falls back to the template, so this never dumps a raw response.

extractServerMessage now prefers prose over a bare identifier, because a few
endpoints put a stable code in "error" and the sentence in "message".

MUL-5619

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

* fix(comments): stop telling a wrong --parent that it posted a top-level comment

The reply guard returns one message for two different mistakes. A resumed
session that carries a previous turn's --parent forward (GH #6264) did not ask
for a top-level comment, but is told it did — which sends it looking for a
new-thread opt-in (GH #5383) instead of correcting the parent it already passed.

Split the copy: name the rejected parent when one was supplied, and keep the
existing top-level wording for the parentless case. Both still point at the
trigger comment to use.

MUL-5619

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

* fix(runtime): return 500, not a 409 echo, when the update store fails

InitiateUpdate answered every UpdateStore.Create failure with a 409 carrying
err.Error(). The in-memory store only ever returns errUpdateInProgress, so this
looked safe — but the Redis store also wraps infrastructure failures as
"reserve active update: <dial error>" and "persist update request: <error>".

Surfacing 409 bodies in the CLI turns that into a user-visible leak of internal
addresses, and labels an outage as a conflict the caller could fix by retrying.
Classify instead: errUpdateInProgress keeps its 409 and its actionable message,
everything else is logged and answered with a 500 and fixed copy.

Also pins the prose-over-machine-code preference for validation bodies, which
the shared extractor applies to 400/422 as well as 409. Only the issue-table
endpoints are shaped that way and none is reachable from the CLI today, but the
change is intentional and should fail loudly if reverted.

MUL-5619

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

---------

Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-03 13:35:04 +08:00
2d9cf045d0 MUL-4861: align runtime CLI update permissions (#5514)
* fix(runtimes): align CLI update permissions

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

* fix(runtimes): address update review nits

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

---------

Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-16 13:54:01 +08:00
LinYushenandmultica-agent 44a0ced558 fix(runtime): persist CLI update requests in Redis (#2113)
Co-authored-by: multica-agent <[email protected]>
2026-05-06 13:00:11 +08:00
Bohan JiangandEve f628e48775 refactor(server): error-returning ParseUUID to prevent silent data loss
* refactor(server): make ParseUUID error-returning to prevent silent data loss (MUL-1410)

util.ParseUUID previously swallowed errors and returned a zero pgtype.UUID
on invalid input. When this zero UUID reached a write query (DELETE/UPDATE),
the SQL matched zero rows and the handler returned 2xx success — producing
silent data corruption. #1661 (DeleteIssue with identifier-style ID) was the
visible symptom; PR #1680 patched that one site, this commit closes the
class of bug.

Changes:

- util.ParseUUID now returns (pgtype.UUID, error). Add util.MustParseUUID
  for trusted round-trips that should panic on invalid input.
- handler/handler.go: parseUUID wrapper now calls MustParseUUID — any
  unguarded user-input string reaching it surfaces as a recovered panic
  (chi middleware.Recoverer → 500) instead of silently corrupting data.
  Add parseUUIDOrBadRequest(w, s, fieldName) for handler entry points.
- Convert every Queries.Delete*/Update* call site reachable from raw user
  input (autopilot, comment, project, skill, skill_file, label, pin,
  attachment, feedback, issue assignee, daemon runtime, workspace) to
  validate UUIDs explicitly with parseUUIDOrBadRequest, returning 400 on
  invalid input. Where a resolved entity.ID is already in scope, write
  queries now use it directly instead of re-parsing the URL string.
- Update getWorkspaceMember + loadIssueForUser to handle invalid UUIDs
  gracefully (404/400 instead of panic).
- Update util/middleware/cmd-level callers (subscriber_listeners,
  notification_listeners, activity_listeners, scope_authorizer,
  middleware/workspace) to use the error-returning API.
- Add server/internal/util/pgx_test.go covering valid/invalid input and
  the MustParseUUID panic contract.
- Add TestDeleteIssueByIdentifier + TestDeleteIssueRejectsInvalidUUID
  regression tests in handler_test.go (the original #1661 bug + the
  invalid-input case).
- Document the handler UUID parsing convention in CLAUDE.md so the rule
  is enforceable in future PR review.

* fix(server): address GPT-Boy review of #1748

P1 fixes from PR #1748 review:

1. Migrate remaining request-boundary UUIDs to parseUUIDOrBadRequest so
   malformed input returns 400 instead of panic/500. Was missing on:
   - issue.go: workspace_id in CreateIssue/ChildIssueProgress/ListIssues/
     SearchIssues/BatchUpdateIssues/BatchDeleteIssues; project_id /
     parent_issue_id / lead_id / assignee_id / assignee_ids / creator_id
     filters; batch issue_ids and assignee/parent/project fields in
     BatchUpdateIssues (skip on bad input via util.ParseUUID, matching
     the existing per-row continue semantics).
   - project.go: project id + workspace_id in GetProject/UpdateProject/
     DeleteProject; lead_id in CreateProject/UpdateProject;
     workspace_id in ListProjects + SearchProjects.
   - handler.go: resolveActor now uses util.ParseUUID for X-Agent-ID /
     X-Task-ID headers; invalid UUID falls back to "member" (matches
     pre-existing semantics) instead of panicking.
   - issue.go: validateAssigneePair returns 400 on invalid workspace_id
     instead of panicking.

2. Fix issue:deleted WS event payloads to emit uuidToString(issue.ID)
   instead of the raw URL string. After an identifier-path delete
   ("MUL-7"), the previous payload would have leaked the identifier to
   subscribers, leaving stale entries in frontend caches that key by
   UUID. Updated DeleteIssue (issue.go:1341) and BatchDeleteIssues
   (issue.go:1641). The slog "issue deleted" log line also now records
   the resolved UUID so logs match the WS payload.

3. Extend TestDeleteIssueByIdentifier to subscribe to the bus and
   assert issue:deleted.payload.issue_id is the resolved UUID, not
   the identifier.

* fix(server): validate remaining reviewed UUID inputs

* fix(server): validate remaining handler UUID inputs

* fix(server): finish request boundary UUID audit

* fix(server): validate remaining request body UUIDs

* fix(server): validate runtime path UUIDs

* fix(server): validate remaining audit UUID inputs

---------

Co-authored-by: Eve <[email protected]>
2026-04-28 14:50:28 +08:00
Jiayuan ZhangandClaude Opus 4.6 82bbce98fd fix(security): add workspace ownership checks to daemon API routes (#684)
* fix(security): add workspace ownership checks to all daemon API routes

Switch daemon routes from middleware.Auth to middleware.DaemonAuth and
add per-handler workspace ownership verification. This prevents
cross-workspace access to runtimes, tasks, usage, and daemon lifecycle
endpoints (HIGH-1/2/3 + CHAIN-1/2/3).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(security): support mdt_ daemon tokens in DaemonRegister + add regression tests

DaemonRegister now handles both auth paths:
- mdt_ daemon tokens: verify workspace match, skip member check, zero OwnerID
  (SQL COALESCE preserves existing owner on upsert)
- PAT/JWT: existing member check + OwnerID from member

Also adds WithDaemonContext helper and regression tests covering:
- Successful register with daemon token
- Workspace mismatch rejection
- Cross-workspace heartbeat rejection
- Cross-workspace task status rejection

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-11 12:49:23 +08:00
LinYushenandClaude Opus 4.6 fdba410f11 feat(runtime): support CLI update from web runtime page (#331)
* feat(runtime): support CLI update from web runtime page

Add the ability to update the CLI daemon from the web Runtime detail page.
When a newer version is available on GitHub Releases, an update button
appears. Clicking it sends an update command through the server to the
daemon via the heartbeat mechanism (same pattern as ping). The daemon
executes `brew upgrade`, reports the result, and restarts itself with the
new binary.

Changes across all three layers:
- Frontend: version display, GitHub latest check, UpdateSection component
- Server: UpdateStore (in-memory), heartbeat extension, 3 new endpoints
- CLI: shared update logic, daemon handleUpdate + graceful restart

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(runtime): handle 'running' status in ReportUpdateResult

The daemon sends {"status":"running"} when it starts executing the
update, but ReportUpdateResult treated any non-"completed" status as
failure — immediately marking the update as failed before brew upgrade
even ran.

Fix: use a switch statement to handle "running" as a no-op (status is
already "running" from PopPending), and also timeout running updates
after 120 seconds in case brew upgrade hangs.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-02 14:12:49 +08:00