Commit Graph

10 Commits

Author SHA1 Message Date
Bohan Jiang
683ff132ca fix(server/heartbeat): probe/claim split + slow-log + model-list running timeout (#1644)
Mitigates #1637 and the related model-discovery failure in MUL-1397 by bounding the /api/daemon/heartbeat hot path with an ack-safe probe/claim split, adding structured slow-log attribution, and closing the ModelListStore running-state gap. See PR description for details.
2026-04-25 02:06:00 +08:00
Bohan Jiang
7ada72faa6 fix(server/task): synthesize result comment for comment-triggered tasks too (#1440)
Agents can end a comment-triggered run without calling `multica issue comment
add` — the final reply stays in terminal / run-log text and never reaches
the user, even though the run panel shows "Completed". PR #1372 addressed
this via prompt wording, but compliance is inherently best-effort.

The server already had an exact fix for the assignment-triggered branch:
`HasAgentCommentedSince` + fallback synthesis from `payload.Output`. The
comment-triggered branch was explicitly exempted on the theory that the
agent "replies via CLI with --parent, so posting here would create a
duplicate" — but that is precisely the path that's failing.

Remove the `!task.TriggerCommentID.Valid` guard so the invariant "every
completed issue task has at least one agent comment on the issue" holds for
both branches. The existing `HasAgentCommentedSince` check still prevents
duplicates for compliant agents, and `createAgentComment` already threads
the synthesized comment under `task.TriggerCommentID` when present.

Regression tests cover both:
  - comment-triggered + silent agent → synthesized comment threaded under trigger
  - comment-triggered + agent already posted → no duplicate
2026-04-21 16:09:59 +08:00
Bohan Jiang
8eb81aa396 fix(daemon): enforce workspace isolation for agent execution (#1235) (#1260)
Phase 0 hotfix for the cross-workspace contamination reported in MUL-1027
/ #1235: an agent running for workspace A ended up commenting on (and
renaming) a two-day-old issue in workspace B.

#1249/#1259 fixed resolution for autopilot tasks and consolidated the
task-workspace resolver, and #1294 populated workspace_id in the claim
response for run_only autopilot tasks. Those closed the known fallthroughs
but the failure mode is still broader: whenever the daemon or server fails
to supply a workspace, the CLI silently falls back to
`~/.multica/config.json`, which is user-global, not workspace-scoped. On a
host running daemons for multiple workspaces, a single gap in workspace
propagation is enough to leak writes across workspaces.

This PR adds three coordinated guards so no single layer's bug can cause a
cross-workspace write:

1. `server/cmd/multica/cmd_agent.go` — `resolveWorkspaceID` detects the
   agent execution context (`MULTICA_AGENT_ID` / `MULTICA_TASK_ID` env,
   both daemon-only markers) and in that context refuses to fall back to
   the user-global CLI config. Human / script usage (no agent env) is
   unchanged: flag → env → config fallback chain still applies.

2. `server/internal/handler/daemon.go` — `ClaimTaskByRuntime` now
   captures the runtime's workspace from `requireDaemonRuntimeAccess` and
   enforces `resolved_task_workspace == runtime_workspace` after the
   existing issue/chat/autopilot branches. On mismatch or empty, the
   handler explicitly cancels the just-dispatched task (via
   `TaskService.CancelTask`, which also reconciles agent status) and
   returns 500. Without the explicit cancel, `ClaimTaskForRuntime` had
   already transitioned the task to 'dispatched' and the agent status to
   'working', so a plain 500 would leave both stuck for the ~5 min
   stale-task sweep window.

3. `server/internal/daemon/daemon.go` — `runTask` refuses to spawn the
   agent when `task.WorkspaceID` is empty (defense-in-depth against
   server bugs and reused workdirs).

Tests:
- `cmd/multica/cmd_agent_test.go`:
  `TestResolveWorkspaceID_AgentContextSkipsConfig` — five subtests
  covering the full fallback matrix (outside agent context still reads
  config; agent context uses env; agent context with empty env returns
  empty; task-id-only marker also counts; requireWorkspaceID surfaces the
  agent-context error message).
- `internal/handler/daemon_test.go`:
  `TestClaimTaskByRuntime_TaskWorkspaceMismatch_CancelsAndRejects` —
  constructs a data-inconsistent task (runtime_id in workspace A,
  issue_id in workspace B) and asserts the handler returns 500 AND
  leaves the task in 'cancelled' state (not 'dispatched').

Phase 1/2 follow-ups (prompt injection of workspace slug, session lookup
workspace filter, cross-workspace audit of agent-facing endpoints,
observability) are out of scope for this PR and tracked separately.
2026-04-21 12:55:12 +08:00
Kagura
aa9305f7e4 fix(daemon): populate workspace_id in ClaimTaskByRuntime for autopilot run_only tasks (#1294)
* fix(daemon): populate workspace_id in ClaimTaskByRuntime for autopilot run_only tasks (#1276)

* test: add regression test for #1276 — ClaimTaskByRuntime autopilot workspace_id
2026-04-18 12:47:29 +08:00
Bohan Jiang
a73336dcf8 feat(daemon): persistent UUID identity + legacy-id merge at register-time (#1220)
* feat(daemon): persistent UUID identity + legacy-id merge at register-time

daemon_id is now a stable UUID persisted to `<profile-dir>/daemon.id` on
first start, replacing the hostname-derived id that drifted whenever
`.local` appeared/disappeared, a system was renamed, or a profile
switched — each of which used to mint a fresh `agent_runtime` row and
strand agents on the old one.

To migrate existing installs without operator intervention, the daemon
reports every legacy id it may have registered under previously
(`host`, `host` with `.local` stripped, and `host[-profile]` variants
for both). At register-time the server looks up each candidate row
scoped to (workspace, provider), re-points its agents and tasks onto
the new UUID-keyed row, records which legacy id was subsumed in the
new `legacy_daemon_id` column for audit, and deletes the stale row.
Result: users running `xxx.local`-keyed runtimes today transparently
land on the new UUID row on next daemon restart.

The hostname-prefix `MigrateAgentsToRuntime` / `daemon_id LIKE '...-%'`
compatibility shim is no longer needed and has been removed along with
the handler call that invoked it.

* fix(daemon): handle bidirectional .local drift and case drift in legacy merge

Review on #1220 flagged two gaps in the legacy-id migration candidate set:

1. Reverse .local: LegacyDaemonIDs only added the stripped variant when the
   current hostname ended in `.local`. The opposite direction — DB has
   `foo.local`, current host is `foo` — was missed, so runtimes registered
   under the `.local` variant stayed orphaned after upgrade. Now both
   variants (`foo` and `foo.local`) are always emitted, regardless of what
   `os.Hostname()` currently returns, plus their `-<profile>` suffix forms.

2. Case drift: os.Hostname() has been observed returning different casings
   on the same machine across mDNS/reboot state. A case-sensitive `=`
   comparison stranded rows like `Jiayuans-MacBook-Pro.local` when the
   daemon later reported `jiayuans-macbook-pro.local`. FindLegacyRuntimeByDaemonID
   now uses `LOWER(daemon_id) = LOWER(@daemon_id)` on both sides, so casing
   differences merge rather than orphan. The (workspace_id, provider) prefix
   still bounds the scan to a tiny set of rows so the non-indexed LOWER()
   comparison has negligible cost.

Tests: TestLegacyDaemonIDs gets the mixed-case + reverse-direction cases;
daemon_test.go adds TestDaemonRegister_MergesLegacyDaemonIDRuntime_ReverseDotLocal
and TestDaemonRegister_MergesLegacyDaemonIDRuntime_CaseDrift.

* fix(daemon): consolidate every case-duplicate legacy runtime, not just the first

Follow-up review on #1220: after switching to `LOWER(daemon_id) =
LOWER(@daemon_id)`, the single-row lookup still only merged one legacy
row per candidate. If a machine already had two rows in the DB that
differed only in casing (e.g. `Jiayuans-MacBook-Pro.local` AND
`jiayuans-macbook-pro.local` coexisting because earlier hostname drift
already minted a duplicate), only one of them got consolidated and the
other stayed orphaned — violating the "no duplicate runtime per machine
after backfill" acceptance.

- FindLegacyRuntimeByDaemonID → FindLegacyRuntimesByDaemonID (:many)
- mergeLegacyRuntimes iterates every returned row and dedupes across
  overlapping legacy candidates so `foo` and `foo.local` both resolving
  to the same stored row don't double-process

Test: TestDaemonRegister_MergesAllCaseDuplicateLegacyRuntimes seeds two
case-duplicate rows with one agent each and confirms both rows are
deleted and both agents end up on the new UUID-keyed row.
2026-04-17 15:10:38 +08:00
Bohan Jiang
5a6a44a69e refactor(daemon): consolidate task workspace resolver + regression test (#1259)
Follow-up to #1249. Two small follow-ups requested in review:

1. `resolveTaskWorkspaceID` was duplicated between `handler/daemon.go` and
   `service/task.go`. #1249 fixed the handler copy but left both in place,
   meaning any future branch (e.g. a fourth task link type) still needs
   to be added in two files. Promote the service method to the exported
   `TaskService.ResolveTaskWorkspaceID` and delete the handler copy.
   Handler's `requireDaemonTaskAccess` and `ListTaskMessagesByUser` now
   call through `h.TaskService`.

2. Add a regression test `TestStartTask_AutopilotRunOnlyTask_ResolvesWorkspace`
   covering the exact scenario from #1224: a task linked only via
   `AutopilotRunID` must resolve to the autopilot's workspace. The test
   asserts 404 for a cross-workspace daemon token and 200 (with status
   transitioning to `running`) for the correct-workspace token.
2026-04-17 14:50:05 +08:00
Bohan Jiang
f8c6dd505f fix(security): bind URL issueId to workspace on four issue-scoped daemon.go handlers (#1145)
GetActiveTaskForIssue, CancelTask, ListTasksByIssue, and GetIssueUsage
accepted the issueId URL parameter and queried by it without verifying
that the issue belonged to the caller's X-Workspace-ID workspace. The
RequireWorkspaceMember middleware only proves membership in the header
workspace; it does not bind the path-parameter issue to it. A member of
workspace A could therefore enumerate tasks, cancel tasks, and read
usage metadata for any issue UUID in workspace B.

Route every issueId through loadIssueForUser (matching GetIssue and the
existing comment/subscriber handlers). For CancelTask additionally
verify that the task's IssueID matches the loaded issue — the task must
not only belong to the caller's workspace but also to the specific
issue named in the URL, and the access check must run before any
mutation.

Follow-up to MUL-899 / #1112.
2026-04-16 15:08:27 +08:00
Bohan Jiang
ce52374d5d test(daemon): add cross-workspace regression for GetIssueGCCheck (#1143)
Adds TestGetIssueGCCheck_WithDaemonToken_CrossWorkspace alongside the
existing TestGetTaskStatus_WithDaemonToken_CrossWorkspace, covering:

- daemon token scoped to a different workspace → 404 (matches the
  "issue not found" status, so no UUID enumeration oracle)
- daemon token scoped to the issue's workspace → 200 with status and
  updated_at fields populated

Follow-up to #1121, which fixed the underlying IDOR reported in #1112
but did not ship a regression test. This gates the class of bug at CI
so the next handler to forget requireDaemonWorkspaceAccess will be
caught before merge.
2026-04-16 13:49:54 +08:00
Black
0427fd8cc7 fix(daemon): refresh workspace repos on checkout miss (#1085)
Co-authored-by: black-fe <black-fe@gate.me>
2026-04-15 19:10:54 +08:00
Jiayuan Zhang
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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 12:49:23 +08:00