mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-08 17:36:45 +08:00
* fix(channel): refuse an inbound turn from a member who may not run the agent
The web chat checks the agent's invoke permission before it opens a session: a
private agent runs only for its owner, with no admin bypass. The shared channel
Router checked the sender's binding and workspace membership and nothing else,
so any member who had bound a chat identity could run another member's private
agent through its bot — on the owner's runtime, with the owner's credentials. A
1:1 message, a group mention, /issue and /new all got through, on every channel
the engine serves.
The Router now asks service.CanMemberInvokeAgent right after identity
resolution and before anything is stored, so a refused turn reaches no Chat, no
/issue and no later run's context. It judges the SENDER, never the installer who
owns a group's route — judging the installer is what would let this through.
MemberMayInvokeAgent is that existing function keyed by agent id, since the
Router holds the installation's agent id rather than the loaded row. An agent
that no longer exists admits nobody; a lookup that FAILED is an error rather
than a denial, so the dedup claim is released and the platform's redelivery is
still the message's chance.
A refusal is audited as invocation_not_allowed. WeCom answers a 1:1 in place; a
group trigger is answered in the sender's own 1:1 and the room hears nothing,
since a line there would tell everyone present which member was refused and
that the agent is someone's private one. Other channels stay silent, as they do
for a non-member.
* fix(channel): a failed permission lookup is not a denial
The policy swallowed two of its three query errors. CanMemberInvokeAgent
returns a plain bool, so ListAgentInvocationTargets failing came back as false,
and any error from GetMemberByUserAndWorkspace — not only pgx.ErrNoRows — read
as 'not a workspace member'. Only GetAgent's failure reached the caller.
For the scheduled triggers that shipped with it, failing closed is right: an
autopilot or an issue wakeup can wait for the next tick. For a channel turn it
is not. The Router reads false as a verdict, marks the message processed so the
platform's redelivery is discarded, and WeCom tells the sender they may not run
an agent they are in fact allowed to run — all from one transient error.
So the policy is now memberMayInvokeAgent, returning (bool, error):
- (false, nil) stays a real verdict — no user id, not the owner of a private
agent, not a target of a public_to one, and a member row that is simply not
there, which is what pgx.ErrNoRows means here.
- (false, err) is 'we do not know', and every other query failure produces it.
CanMemberInvokeAgent is now a thin wrapper that fails closed, so autopilot and
issue wakeup behave exactly as before. TaskService.MemberMayInvokeAgent returns
the error, and the Router releases its dedup claim on it.
TestRouter_RealServiceLookupFailure_ReleasesInsteadOfDenying drives the REAL
TaskService against a real database with one query failed at the SQL boundary,
because the router tests stub this at the interface and can only prove what the
Router does with an error it is handed — not whether the policy ever produces
one. It asserts Release rather than Mark, no invocation_not_allowed audit, and
no denial notice, for each of the two queries.
Also moves postPrivate's doc comment back to postPrivate: sendInvokeDenied's
was inserted above it, leaving one function documented by the other's text.
235 lines
11 KiB
Go
235 lines
11 KiB
Go
package service
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
"github.com/multica-ai/multica/server/internal/dispatch"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// TestDispatchFailReasonCode is the regression for Elon must-fix 2, case 1: a
|
|
// dispatch that fails fail-closed on attribution must be classified
|
|
// attribution_blocked, not internal_error — decided by a TYPED errors.Is check,
|
|
// not by substring-matching an English message.
|
|
func TestDispatchFailReasonCode(t *testing.T) {
|
|
if got := dispatchFailReasonCode(ErrAttributionFailClosed); got != dispatch.ReasonAttributionBlocked {
|
|
t.Errorf("bare fail-closed: got %q, want attribution_blocked", got)
|
|
}
|
|
// The real dispatch path wraps the sentinel (create_issue enqueue → %w);
|
|
// errors.Is must still see through the wrap.
|
|
wrapped := fmt.Errorf("dispatch create_issue: enqueue task for issue: %w", ErrAttributionFailClosed)
|
|
if got := dispatchFailReasonCode(wrapped); got != dispatch.ReasonAttributionBlocked {
|
|
t.Errorf("wrapped fail-closed: got %q, want attribution_blocked", got)
|
|
}
|
|
if got := dispatchFailReasonCode(errors.New("some other failure")); got != dispatch.ReasonInternalError {
|
|
t.Errorf("generic error: got %q, want internal_error", got)
|
|
}
|
|
}
|
|
|
|
// TestAgentReadinessVerdict is the regression for Elon must-fix 2, case 2: a
|
|
// runtime-availability failure must be classified from the agent's and
|
|
// runtime's own state, not from the reason text. The three runtime failures are
|
|
// deliberately distinct — an agent with NO runtime needs to be bound to one
|
|
// (agent_runtime_required); a bound-but-offline runtime needs the machine back
|
|
// (runtime_offline); a runtime whose CLI cannot be executed needs a reinstall
|
|
// on a machine that is already connected (runtime_unusable, MUL-6164).
|
|
// Collapsing any pair sends the user to fix the wrong thing.
|
|
func TestAgentReadinessVerdict(t *testing.T) {
|
|
validRuntime := pgtype.UUID{Bytes: [16]byte{1}, Valid: true}
|
|
archivedAt := pgtype.Timestamptz{Valid: true}
|
|
|
|
// The agent-only half needs no runtime row, so AgentReadiness answers before
|
|
// it queries one.
|
|
if got, _ := AgentReadiness(t.Context(), RuntimeLookup{}, db.Agent{}); got.Reason != dispatch.ReasonAgentRuntimeRequired || !got.Blocked() {
|
|
t.Errorf("no runtime bound: got %+v, want blocked/agent_runtime_required", got)
|
|
}
|
|
if got, _ := AgentReadiness(t.Context(), RuntimeLookup{}, db.Agent{ArchivedAt: archivedAt, RuntimeID: validRuntime}); got.Reason != dispatch.ReasonTargetUnavailable || !got.Blocked() {
|
|
t.Errorf("archived agent: got %+v, want blocked/target_unavailable", got)
|
|
}
|
|
|
|
// The runtime half. A private runtime with an owner admits only an agent
|
|
// owned by that same member; ownerless agents are blocked for new work.
|
|
ownerA := pgtype.UUID{Bytes: [16]byte{2}, Valid: true}
|
|
ownerB := pgtype.UUID{Bytes: [16]byte{3}, Valid: true}
|
|
if got := runtimeVerdict(db.AgentRuntime{Status: "online"}, db.Agent{OwnerID: ownerA}); !got.Ready() {
|
|
t.Errorf("online runtime: got %+v, want ready", got)
|
|
}
|
|
for name, tc := range map[string]struct {
|
|
runtime db.AgentRuntime
|
|
agent db.Agent
|
|
blocked bool
|
|
reason dispatch.ReasonCode
|
|
}{
|
|
"private mismatch": {
|
|
runtime: db.AgentRuntime{Status: "online", Visibility: "private", OwnerID: ownerA},
|
|
agent: db.Agent{OwnerID: ownerB},
|
|
blocked: true,
|
|
reason: dispatch.ReasonRuntimeAccessDenied,
|
|
},
|
|
"private ownerless agent": {
|
|
runtime: db.AgentRuntime{Status: "online", Visibility: "private", OwnerID: ownerA},
|
|
agent: db.Agent{},
|
|
blocked: true,
|
|
reason: dispatch.ReasonRuntimeAccessDenied,
|
|
},
|
|
"private same owner": {
|
|
runtime: db.AgentRuntime{Status: "online", Visibility: "private", OwnerID: ownerA},
|
|
agent: db.Agent{OwnerID: ownerA},
|
|
},
|
|
"public mismatch": {
|
|
runtime: db.AgentRuntime{Status: "online", Visibility: "public", OwnerID: ownerA},
|
|
agent: db.Agent{OwnerID: ownerB},
|
|
},
|
|
"ownerless runtime": {
|
|
runtime: db.AgentRuntime{Status: "online", Visibility: "private"},
|
|
agent: db.Agent{OwnerID: ownerB},
|
|
},
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
got := runtimeVerdict(tc.runtime, tc.agent)
|
|
if got.Blocked() != tc.blocked || (tc.blocked && got.Reason != tc.reason) {
|
|
t.Fatalf("got %+v, want blocked=%v reason=%q", got, tc.blocked, tc.reason)
|
|
}
|
|
})
|
|
}
|
|
// Offline with no explanation is the sleeping-laptop case: work waits for it,
|
|
// so this must NOT be blocked.
|
|
offline := runtimeVerdict(db.AgentRuntime{Status: "offline"}, db.Agent{OwnerID: ownerA})
|
|
if offline.Blocked() || offline.Reason != dispatch.ReasonRuntimeOffline {
|
|
t.Errorf("plain offline runtime: got %+v, want waitable/runtime_offline", offline)
|
|
}
|
|
// Offline because the CLI cannot be executed: waiting is futile, and the
|
|
// repair the daemon reported has to survive to the caller — it is the only
|
|
// thing that tells the user what to actually do.
|
|
unusable := runtimeVerdict(db.AgentRuntime{
|
|
Status: "offline",
|
|
Metadata: []byte(`{"offline_reason":{"code":"not_executable","detail":"agent CLI is not executable","repair":{"package":"@anthropic-ai/claude-code","command":"cd '/pkg' && node install.cjs"}}}`),
|
|
}, db.Agent{OwnerID: ownerA})
|
|
if !unusable.Blocked() || unusable.Reason != dispatch.ReasonRuntimeUnusable {
|
|
t.Fatalf("unusable runtime: got %+v, want blocked/runtime_unusable", unusable)
|
|
}
|
|
if unusable.Repair == nil || unusable.Repair.Command != "cd '/pkg' && node install.cjs" {
|
|
t.Errorf("unusable runtime lost the repair command: got %+v", unusable.Repair)
|
|
}
|
|
// Offline because the DSH runtime profile is missing is blocked for the same
|
|
// reason — the machine is reachable, its CLI cannot serve work, and no
|
|
// amount of waiting installs a bundle nobody configured — but under its OWN
|
|
// code. Collapsing it into runtime_unusable is what made every client tell
|
|
// the user to reinstall a CLI that runs perfectly.
|
|
profile := runtimeVerdict(db.AgentRuntime{
|
|
Status: "offline",
|
|
Metadata: []byte(`{"offline_reason":{"code":"dsh_profile","detail":"the Multica runtime profile is not installed","repair":{"package":"DeepSeek Harness runtime profile"}}}`),
|
|
}, db.Agent{OwnerID: ownerA})
|
|
if !profile.Blocked() || profile.Reason != dispatch.ReasonRuntimeProfileMissing {
|
|
t.Fatalf("missing DSH profile: got %+v, want blocked/runtime_profile_missing", profile)
|
|
}
|
|
if !RuntimeBlockedNeedsNotice(profile.Reason) {
|
|
t.Error("missing DSH profile would leave no durable trace on the issue")
|
|
}
|
|
// The notice must describe the profile, not a broken CLI, and must not hand
|
|
// the user a command with a placeholder in it to paste.
|
|
notice := RuntimeUnusableNotice("Kit", profile)
|
|
if !strings.Contains(notice, "runtime profile") {
|
|
t.Errorf("notice does not name the missing profile: %q", notice)
|
|
}
|
|
for _, wrong := range []string{"cannot be executed", "postinstall", "<bundle>", "```"} {
|
|
if strings.Contains(notice, wrong) {
|
|
t.Errorf("notice contains %q, which belongs to the unrunnable-CLI repair: %q", wrong, notice)
|
|
}
|
|
}
|
|
// The unrunnable-CLI notice keeps its own text and its fenced command.
|
|
if unusableNotice := RuntimeUnusableNotice("Kit", unusable); !strings.Contains(unusableNotice, "node install.cjs") {
|
|
t.Errorf("unusable-CLI notice lost its repair command: %q", unusableNotice)
|
|
}
|
|
// The installing claim is bounded, not trusted. A withdraw cannot be
|
|
// guaranteed — the verdict is built before the runtime ids it needs are
|
|
// known, the corrective Deregister is best-effort and runs on a cancelled
|
|
// context during shutdown, and a daemon restarted mid-install does not track
|
|
// the row at all — so a claim older than the install it describes must stop
|
|
// buying a queue, or it recreates the failure it was added to prevent.
|
|
installingRow := func(updatedAt time.Time) db.AgentRuntime {
|
|
return db.AgentRuntime{
|
|
Status: "offline",
|
|
Metadata: []byte(`{"offline_reason":{"code":"dsh_profile","installing":true,"detail":"installing the configured bundle now"}}`),
|
|
UpdatedAt: pgtype.Timestamptz{Time: updatedAt, Valid: true},
|
|
}
|
|
}
|
|
// The one exception is an install the daemon is running right now: that wait
|
|
// DOES end by itself, so the work queues instead of being refused.
|
|
fresh := runtimeVerdict(installingRow(time.Now()), db.Agent{OwnerID: ownerA})
|
|
if fresh.Blocked() {
|
|
t.Errorf("install in flight: got %+v, want the waitable verdict", fresh)
|
|
}
|
|
if fresh.Reason != dispatch.ReasonRuntimeOffline {
|
|
t.Errorf("install in flight: reason = %q, want %q", fresh.Reason, dispatch.ReasonRuntimeOffline)
|
|
}
|
|
stale := runtimeVerdict(installingRow(time.Now().Add(-runtimeInstallClaimWindow-time.Minute)), db.Agent{OwnerID: ownerA})
|
|
if !stale.Blocked() || stale.Reason != dispatch.ReasonRuntimeProfileMissing {
|
|
t.Fatalf("stale install claim: got %+v, want blocked/runtime_profile_missing", stale)
|
|
}
|
|
// No timestamp to bound it with: refuse rather than queue behind something
|
|
// that cannot be shown to be running.
|
|
unbounded := runtimeVerdict(db.AgentRuntime{
|
|
Status: "offline",
|
|
Metadata: []byte(`{"offline_reason":{"code":"dsh_profile","installing":true}}`),
|
|
}, db.Agent{OwnerID: ownerA})
|
|
if !unbounded.Blocked() {
|
|
t.Errorf("install claim with no updated_at: got %+v, want blocked", unbounded)
|
|
}
|
|
|
|
// An unrecognised or malformed reason must not invent a verdict: unknown
|
|
// causes stay in the waitable bucket they are in today.
|
|
for name, metadata := range map[string]string{
|
|
"unknown code": `{"offline_reason":{"code":"who_knows"}}`,
|
|
"malformed": `{"offline_reason":`,
|
|
"empty": ``,
|
|
} {
|
|
got := runtimeVerdict(db.AgentRuntime{Status: "offline", Metadata: []byte(metadata)}, db.Agent{OwnerID: ownerA})
|
|
if got.Blocked() {
|
|
t.Errorf("%s: got %+v, want the plain offline verdict", name, got)
|
|
}
|
|
}
|
|
}
|
|
|
|
// The notice is the only thing a user sees when an assignment or an
|
|
// agent-authored mention is refused, so it has to carry the repair command when
|
|
// there is one and stay actionable when there is not.
|
|
func TestRuntimeUnusableNotice(t *testing.T) {
|
|
withRepair := RuntimeUnusableNotice("Mika", AgentVerdict{
|
|
Repair: &RuntimeRepair{Package: "@anthropic-ai/claude-code", Command: "cd '/pkg' && node install.cjs"},
|
|
})
|
|
if !strings.Contains(withRepair, "Mika") || !strings.Contains(withRepair, "cd '/pkg' && node install.cjs") {
|
|
t.Errorf("notice must name the agent and the repair command:\n%s", withRepair)
|
|
}
|
|
// The fence is labelled with the shell the command was written for: a
|
|
// Windows repair shown as bash is how a user ends up pasting `cd 'C:\...'`
|
|
// into PowerShell.
|
|
if !strings.Contains(withRepair, "```bash") {
|
|
t.Errorf("a bash repair must be fenced as bash:\n%s", withRepair)
|
|
}
|
|
windows := RuntimeUnusableNotice("Mika", AgentVerdict{
|
|
Repair: &RuntimeRepair{Command: "Set-Location 'C:\\pkg'\nnode install.cjs", Shell: "powershell"},
|
|
})
|
|
if !strings.Contains(windows, "```powershell") {
|
|
t.Errorf("a PowerShell repair must be fenced as powershell:\n%s", windows)
|
|
}
|
|
|
|
withoutRepair := RuntimeUnusableNotice("Mika", AgentVerdict{})
|
|
if strings.Contains(withoutRepair, "```") {
|
|
t.Errorf("no repair command is known, so none may be shown:\n%s", withoutRepair)
|
|
}
|
|
if !strings.Contains(withoutRepair, "Reinstall") {
|
|
t.Errorf("notice must still say what to do:\n%s", withoutRepair)
|
|
}
|
|
|
|
accessDenied := RuntimeUnusableNotice("Mika", AgentVerdict{Reason: dispatch.ReasonRuntimeAccessDenied})
|
|
if !strings.Contains(accessDenied, "public") || !strings.Contains(accessDenied, "rebind/copy") {
|
|
t.Errorf("access-denied notice must include both recovery paths:\n%s", accessDenied)
|
|
}
|
|
}
|