Files
multica/server/internal/daemon/execenv/runtime_config_delivery_test.go
Naiyuan Qing 1507997272 fix(agent): stop agents shipping local-path links, make Desktop 404 recoverable (MUL-4899) (#5557)
Agents were writing runtime-local paths into deliverables as clickable
links (`[screenshot](/Users/agent/work/shot.png)`). Two root causes, both
fixed here.

A. The brief never stated the delivery contract. Add an always-on delivery
invariant (outside writeOutput's kind switch, so no task kind can inherit
none) plus a per-surface file-delivery line for each of the five surfaces.
Chat splits into two: `attachment upload` works only on web/mobile chat,
never on an IM channel, so ChatChannelType is now threaded into
TaskContextForEnv.

The claim path only ever looked up Slack bindings, so a Feishu session
reported as a web chat and got upload guidance for a channel that cannot
carry attachments. Probe every channel type. The chat policy is two
independent layers and stays that way: delivery keys off "is there a
channel at all"; the `chat history` / `chat thread` commands stay
Slack-only because both endpoints are hardwired to h.SlackHistory and
there is no Feishu reader — ChatInThread only selects between those two
commands, so it stays Slack-only too.

Add a CLI hard-fail lint on `issue comment add` / `issue create` /
`issue update` as the enforcement backstop. Scoped narrowly, since a false
positive blocks a real deliverable: agent task context only (a human's PAT
run is untouched), real CommonMark link/image/autolink destinations only
via goldmark (a path in a code span or fence — how an agent quotes a path
it is discussing — is structurally invisible), and three high-confidence
signals only (`file://`, inside the workdir, or an existing local file).
A bare `/foo` is a valid origin-relative URI and is deliberately allowed.
`issue update` has no --attachment flag, so its hint redirects to
`comment add` rather than naming an argument it rejects.

B. Desktop presented the resulting router 404 as an unknown crash. 8 of 18
desktop_route_error reports were users clicking such a link and being told
the app broke and to file a bug. Split the 404 into a first-class Not Found
view: no crash framing, no Report error. Its recovery entry comes from the
tab store's active workspace, never from the failed pathname — deriving a
slug from `/Users/me/shot.png` yields "Users" and a button to `/Users/issues`,
a second 404.

Also add a will-navigate trusted-origin guard via the shared loadRenderer
(main + issue windows). This is origin hardening only, NOT the mechanism for
in-app links: client-side routing never fires will-navigate, so app paths
never reach it. Issue windows need no 404 work — their router only accepts
paths validated by parseIssueWindowPath and they do not listen for
multica:navigate, so a bad path cannot reach them.

Server-side completion observation is metric/log only and never blocks: it
is lexical (`file://` + task work_dir prefix) because the server cannot stat
the daemon's filesystem, and the metric label is a closed enum so no path or
reply text reaches Prometheus.

Verified: pnpm typecheck/lint/test (3582 tests), go vet, full Go suite
including new claim-path integration tests. cmd/multica was verified outside
the daemon workdir — inside one, 93 of its tests fail identically on
origin/main because the suite walks up and finds the runtime's own task marker.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-17 13:40:23 +08:00

161 lines
5.6 KiB
Go

package execenv
import (
"strings"
"testing"
)
// The MUL-4899 delivery contract. Two orthogonal properties are pinned here and
// must not be collapsed:
//
// - The invariant ("never link a local path") is ALWAYS-ON — every task kind,
// no exceptions. It lives outside writeOutput's kind switch so a future kind
// cannot silently inherit no invariant at all; this test is what keeps that
// true.
// - The surface policy ("here is how a file actually gets delivered HERE") is
// PER-KIND, and there are five surfaces, not four — web/mobile chat and IM
// chat are the same taskKind but opposite answers.
// deliveryInvariantFixtures covers all five task kinds. The chat kind appears
// twice because ChatChannelType splits it into two surfaces.
func deliveryInvariantFixtures() map[string]TaskContextForEnv {
return map[string]TaskContextForEnv{
"comment": {IssueID: "i-1", TriggerCommentID: "tc-1", AgentName: "Eve", AgentID: "eve-1"},
"assignment": {IssueID: "i-1", AgentName: "Eve", AgentID: "eve-1"},
"autopilot": {AutopilotRunID: "r-1", AgentName: "Eve", AgentID: "eve-1"},
"quickcreate": {QuickCreatePrompt: "p", AgentName: "Eve", AgentID: "eve-1"},
"chat_direct": {ChatSessionID: "c-1", AgentName: "Eve", AgentID: "eve-1"},
"chat_slack": {ChatSessionID: "c-1", ChatChannelType: ChannelTypeSlack, AgentName: "Eve", AgentID: "eve-1"},
"chat_feishu": {ChatSessionID: "c-1", ChatChannelType: ChannelTypeFeishu, AgentName: "Eve", AgentID: "eve-1"},
}
}
func TestBriefDeliveryInvariantIsAlwaysOn(t *testing.T) {
t.Parallel()
// Phrases every kind must carry, whatever its surface can or cannot deliver.
wantAll := []string{
"Runtime-local paths are never deliverables",
"NEVER write an absolute path or a `file://` URL as a clickable link",
"`path/to/file.ts:42`",
}
for name, ctx := range deliveryInvariantFixtures() {
out := buildMetaSkillContent("claude", ctx)
for _, want := range wantAll {
if !strings.Contains(out, want) {
t.Errorf("kind=%s: brief is missing always-on delivery invariant %q", name, want)
}
}
}
}
func TestBriefSurfaceDeliveryPolicy(t *testing.T) {
t.Parallel()
cases := map[string]struct {
mustHave []string
mustNot []string
}{
// Issue surfaces: files ride the comment.
"comment": {
mustHave: []string{"`--attachment <path>` to `multica issue comment add`"},
mustNot: []string{"multica attachment upload"},
},
"assignment": {
mustHave: []string{"`--attachment <path>` to `multica issue comment add`"},
mustNot: []string{"multica attachment upload"},
},
// Direct chat is the ONLY surface where `attachment upload` works.
"chat_direct": {
mustHave: []string{"`multica attachment upload <local-path>`"},
mustNot: []string{"text-only"},
},
// IM surfaces are text-only. The upload command must not appear: it binds
// to a Multica chat reply, which an IM reply is not, so suggesting it
// would have the agent upload a file and report it as delivered.
"chat_slack": {
mustHave: []string{"Slack conversation is text-only", "does NOT apply"},
mustNot: []string{"run `multica attachment upload"},
},
"chat_feishu": {
mustHave: []string{"Feishu/Lark conversation is text-only", "does NOT apply"},
mustNot: []string{"run `multica attachment upload"},
},
"autopilot": {
mustHave: []string{"this surface is text-only"},
mustNot: []string{"multica attachment upload"},
},
"quickcreate": {
mustHave: []string{"your stdout is text-only", "`multica issue create` call itself via `--attachment <path>`"},
mustNot: []string{"multica attachment upload"},
},
}
fixtures := deliveryInvariantFixtures()
for name, want := range cases {
ctx, ok := fixtures[name]
if !ok {
t.Fatalf("no fixture for surface %q", name)
}
out := buildMetaSkillContent("claude", ctx)
for _, phrase := range want.mustHave {
if !strings.Contains(out, phrase) {
t.Errorf("surface=%s: brief missing surface policy %q\n--- Output section ---\n%s",
name, phrase, outputSection(out))
}
}
for _, phrase := range want.mustNot {
if strings.Contains(out, phrase) {
t.Errorf("surface=%s: brief must NOT carry %q (wrong surface's delivery mechanism)\n--- Output section ---\n%s",
name, phrase, outputSection(out))
}
}
}
}
// TestBriefInboundAttachmentIsNotADeliverable locks the inbound half: a
// downloaded attachment's local path is a private working copy, and the most
// tempting one to echo back because it arrived from the conversation.
func TestBriefInboundAttachmentIsNotADeliverable(t *testing.T) {
t.Parallel()
out := buildMetaSkillContent("claude", TaskContextForEnv{
IssueID: "i-1", TriggerCommentID: "tc-1", AgentName: "Eve", AgentID: "eve-1",
})
for _, want := range []string{
"private working copy",
"Never echo it back into a deliverable as a link",
} {
if !strings.Contains(out, want) {
t.Errorf("Attachments section missing %q\n---\n%s", want, out)
}
}
}
func TestChannelDisplayName(t *testing.T) {
t.Parallel()
cases := map[string]string{
ChannelTypeSlack: "Slack",
ChannelTypeFeishu: "Feishu/Lark",
"": "",
// An unmapped channel names itself rather than reading as "unknown".
"discord": "discord",
}
for in, want := range cases {
if got := ChannelDisplayName(in); got != want {
t.Errorf("ChannelDisplayName(%q) = %q, want %q", in, got, want)
}
}
}
// outputSection extracts the brief's `## Output` section for readable failures.
func outputSection(brief string) string {
idx := strings.Index(brief, "\n## Output\n")
if idx < 0 {
return "<no ## Output section>"
}
return brief[idx:]
}