mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-10 18:36:45 +08:00
60408ef252
* fix(autopilot): let an agent trigger a manual run for the human it acts for MUL-6951 replaced canCreatorInvokeAgent, which judged automatic dispatch by the autopilot CREATOR and needed no trigger, with ResolveAutopilotTriggerPrincipal, which resolves the FIRING TRIGGER's creator. The manual entry point passes no trigger by construction, so the resolver returned nothing on its first line and every manual trigger from a non-member actor was refused. Scheduled runs kept working — they have a trigger row — which is what made this read as a configuration problem rather than a regression. Reported as #8078. The refusal was also self-contradicting: requireAutopilotWrite had already resolved a real human from the request (for a task token, the runtime owner the middleware binds) and confirmed they may trigger this autopilot. Three lines later memberActorUserID discarded that human for not being a "member" actor, and the empty identity fell into the automation branch. TriggerAutopilot now resolves the human the caller acts FOR: - a member acts for themselves, judged exactly as before; - an agent acts for its run's ORIGINATOR, which is how every other invoke surface judges an agent actor (MUL-3963 / canInvokeAgent), including this file's own private-leader gate on save. Reading the runtime owner instead would have been a smaller diff and the wrong grant: it makes every agent a standing proxy for its machine's owner, so anyone who can talk to the agent inherits that owner's autopilot rights. The originator must ALSO hold write access, so an agent can only press a "Run now" that the human ordering it could press themselves. No resolvable human is refused outright (403). The alternative was the workspace-broad exception invokeAgentDecision grants unattributed agent/system principals on a public_to-workspace agent, which would have admitted an originator-less chain here too. Rejected per Bohan: a manual trigger is by definition somebody's decision, so a chain arriving with no human at its top means something upstream dropped it, and that should surface. Two things this does NOT change, both deliberate: - requireAutopilotWrite still judges the request's authenticated user. That it resolves the runtime owner for an agent is pre-existing and shared by all eight autopilot write endpoints; narrowing it is a separate change with its own blast radius, filed separately. - The run's attribution stays direct_human to the resolved human rather than delegation. attributionForIssueTask treats a valid actor as direct_human by contract, shared with assign/promote/rerun, so labelling this one path delegation means threading a new parameter through TaskService's enqueue chain. The human on the hook is correct either way; only the label is coarser. Filed as follow-up rather than half-applied to one execution mode. Two independent defects the same report exposed: - The skip reason blamed "the trigger records no owner" on a path that has no trigger, sending #8078's reporter hunting a broken trigger owner for a day. It now branches on whether a trigger exists. - `multica autopilot trigger` printed "Autopilot triggered" and exited 0 for a skipped run, so the operator — and any agent running it for them — read success. It now mirrors the web client's runNowToastKind whitelist: only issue_created and running are success, everything else prints failure_reason and exits non-zero. --output json still writes the run to stdout first. This is the half that made the bug silent, and it covers every skip reason, not just this one. The MUL-4525 admission test asserted that (no actor, no trigger) must be refused and called it "automation dispatch" — but that is exactly the argument pair a manual trigger produced, so the test pinned the regression and would have blocked the fix. Rewritten to assert the no-principal phrasing and, explicitly, that the reason does not blame a trigger row that does not exist. Verification (real PostgreSQL 17): new handler suite covers agent-with- originator (dispatches), agent-without-originator (403, no run row), originator lacking write (403, no run row) and the untouched member path. The acceptance test was confirmed to fail against the pre-fix handler with exactly the reported symptom — status skipped, invocation_not_allowed. internal/service and internal/handler pass; the one internal/handler failure (TestPluginRoutes- RequireWorkspaceAdmin, a duplicate user email) and the cmd/multica failures reproduce identically on a clean tree. Refs #8078 Co-authored-by: multica-agent <[email protected]> * fix(autopilot): judge a manual trigger by the ordering human alone Addresses Elon's review of #8099. Two must-fixes, both confirmed against the code before changing it. Must-fix 1 — the endpoint still required TWO humans to hold the grant. requireAutopilotWrite ran in front of the originator check, and under a task token it judges the RUNTIME OWNER. So the effective rule was "runtime owner AND originator", not "the human who ordered it" — the opposite failure from the one #8078 reported, and it broke a case a member is plainly entitled to: U created the autopilot and may invoke an agent running on O's machine, O holds no grant, so U could press Run now themselves but could not ask the agent to. It also contradicted the docs this PR shipped, which already promised originator-based authorization. requireAutopilotTriggerInvoker is now this endpoint's only write gate. requireAutopilotWrite is not called alongside it. Workspace tenancy for the authenticated caller is unaffected: /api/autopilots sits inside the router's RequireWorkspaceMember group, so dropping the second check loses no isolation — it only stops a third party's machine ownership from vetoing the ordering human's own rights. The other seven autopilot write endpoints are untouched and stay with MUL-7108. Authorization also moved ahead of the status check, so an unauthorized caller cannot tell an active autopilot from a paused one. The positive test could not have caught this: it let testUserID be runtime owner, autopilot creator and originator at once. TestTriggerAutopilot_ RuntimeOwnerNeedsNoGrant now separates all three — the ordering human creates the autopilot, the runtime owner is a plain member with no grant — and was verified to fail against the previous commit with exactly the 403 Elon described. Must-fix 2 — the actionable refusal never reached the person running the command. The handler named the cause, but FormatError collapses every 403 into generic "no access" copy, so `calling run records no originator` died in the response body. That is the same silence #8078 was about, one layer up. Both refusals now carry a stable code (autopilot_trigger_no_originator, autopilot_trigger_forbidden) via writeErrorCode, and cli.ServerErrorCode reads it. The trigger command opts those two out of the generic copy and prints guidance. The branch is on the code, never on the English sentence: copy edits and translation must not silently disable it. The opt-out is per-code, so every 403 this command has not named keeps the deliberately vague copy that stops a refusal confirming a resource exists — pinned by a test. The two 403s deliberately share one sentence and one code whether the ordering human is a non-member or merely ungranted, so the endpoint cannot be used to probe workspace membership. Verification (real PostgreSQL 17): - TestTriggerAutopilot_RuntimeOwnerNeedsNoGrant fails against the previous commit's handler and passes now; the other four trigger tests still pass. - Both 403 tests assert the code, not just the status — the CLI's output depends on it. - New cmd/multica test runs the real cli.FormatError over both refusals, since the defect being guarded lives in FormatError's collapse, and asserts an un-opted-in 403 still gets the generic copy. - New internal/cli TestServerErrorCode covers missing code, non-JSON, malformed JSON, prose in `code`, and a non-HTTP error. - internal/service passes. Pre-existing unrelated failures reproduce identically on a clean tree: TestPluginRoutesRequireWorkspaceAdmin and TestCommentSourceContextLifecycle in internal/handler, four TestCLIConfig_* in internal/cli. - gofmt, go vet, git diff --check clean. Docs corrected at both sites to state the contract explicitly: the machine owner's grants are not consulted, only workspace membership. Refs #8078 Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: J <[email protected]> Co-authored-by: multica-agent <[email protected]>