forked from molecule-ai/molecule-core
Merge pull request #2132 from Molecule-AI/test/comprehensive-comms-e2e
test(comms): E2E + canvas coverage for agent → user attachments
This commit is contained in:
GitHub
commit
f547c4e259
2
.github/workflows/e2e-api.yml
vendored
2
.github/workflows/e2e-api.yml
vendored
@ -97,6 +97,8 @@ jobs:
|
||||
echo "Migrations OK"
|
||||
- name: Run E2E API tests
|
||||
run: bash tests/e2e/test_api.sh
|
||||
- name: Run notify-with-attachments E2E
|
||||
run: bash tests/e2e/test_notify_attachments_e2e.sh
|
||||
- name: Dump platform log on failure
|
||||
if: failure()
|
||||
run: cat workspace-server/platform.log || true
|
||||
|
||||
@ -262,4 +262,36 @@ describe("extractFilesFromTask", () => {
|
||||
const files = extractFilesFromTask(task);
|
||||
expect(files[0]).toMatchObject({ name: "out.txt", uri: "workspace:/workspace/out.txt" });
|
||||
});
|
||||
|
||||
it("hydrates a notify-with-attachments response_body — both text caption AND file chips", () => {
|
||||
// Pins the exact wire shape the platform's Notify handler persists
|
||||
// when send_message_to_user passes attachments (activity.go writes
|
||||
// response_body = {"result": <message>, "parts": [{kind:"file",...}]}).
|
||||
// The chat history loader runs both extractors over this object on
|
||||
// reload — without this contract holding, refreshing the page after
|
||||
// an agent attached a file would lose either the caption or the chips.
|
||||
const responseBody = {
|
||||
result: "Done — see attached.",
|
||||
parts: [
|
||||
{
|
||||
kind: "file",
|
||||
file: {
|
||||
name: "build-output.zip",
|
||||
mimeType: "application/zip",
|
||||
uri: "workspace:/tmp/build-output.zip",
|
||||
size: 12345,
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
expect(extractResponseText(responseBody)).toBe("Done — see attached.");
|
||||
expect(extractFilesFromTask(responseBody)).toEqual([
|
||||
{
|
||||
name: "build-output.zip",
|
||||
mimeType: "application/zip",
|
||||
uri: "workspace:/tmp/build-output.zip",
|
||||
size: 12345,
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@ -598,6 +598,143 @@ describe("handleCanvasEvent – AGENT_MESSAGE", () => {
|
||||
|
||||
expect(set).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Attachment passthrough — the broadcast payload's `attachments` array
|
||||
// is the wire format the platform's Notify handler emits (activity.go:
|
||||
// 318-326). These tests pin the canvas-side filtering / shape coercion
|
||||
// so the chat reliably renders download chips for agent-sent files.
|
||||
|
||||
it("passes through valid attachments onto the new message", () => {
|
||||
const node = makeNode("ws-1");
|
||||
const { get, set } = makeStore([node], [], null, {});
|
||||
const att = {
|
||||
uri: "workspace:/tmp/build.zip",
|
||||
name: "build.zip",
|
||||
mimeType: "application/zip",
|
||||
size: 12345,
|
||||
};
|
||||
|
||||
handleCanvasEvent(
|
||||
makeMsg({
|
||||
event: "AGENT_MESSAGE",
|
||||
workspace_id: "ws-1",
|
||||
payload: { message: "see attached", attachments: [att] },
|
||||
}),
|
||||
get,
|
||||
set,
|
||||
);
|
||||
|
||||
const { agentMessages } = set.mock.calls[0][0] as {
|
||||
agentMessages: Record<string, Array<{ content: string; attachments?: Array<{ uri: string; name: string; mimeType?: string; size?: number }> }>>;
|
||||
};
|
||||
const msg = agentMessages["ws-1"][0];
|
||||
expect(msg.attachments).toEqual([att]);
|
||||
});
|
||||
|
||||
it("appends an attachments-only message (empty content) when at least one attachment present", () => {
|
||||
// Regression: previously the AGENT_MESSAGE handler short-circuited on
|
||||
// empty `message`, dropping a notify whose intent was "here's the
|
||||
// file" with no caption. The fix renders the bubble whenever EITHER
|
||||
// text or attachments are present.
|
||||
const node = makeNode("ws-1");
|
||||
const { get, set } = makeStore([node], [], null, {});
|
||||
|
||||
handleCanvasEvent(
|
||||
makeMsg({
|
||||
event: "AGENT_MESSAGE",
|
||||
workspace_id: "ws-1",
|
||||
payload: {
|
||||
message: "",
|
||||
attachments: [{ uri: "workspace:/x.txt", name: "x.txt" }],
|
||||
},
|
||||
}),
|
||||
get,
|
||||
set,
|
||||
);
|
||||
|
||||
expect(set).toHaveBeenCalledOnce();
|
||||
const { agentMessages } = set.mock.calls[0][0] as {
|
||||
agentMessages: Record<string, Array<{ content: string; attachments?: unknown[] }>>;
|
||||
};
|
||||
expect(agentMessages["ws-1"]).toHaveLength(1);
|
||||
expect(agentMessages["ws-1"][0].content).toBe("");
|
||||
expect(agentMessages["ws-1"][0].attachments).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("filters out attachments with empty uri or name (defence-in-depth for missing gin `dive`)", () => {
|
||||
// Server-side per-element validation rejects empty uri/name, but the
|
||||
// canvas defence-in-depth filter exists because the broadcast path
|
||||
// skips that handler — a malformed broadcast (or a future regression)
|
||||
// could still emit empty entries. Drop them rather than rendering
|
||||
// blank/broken chips.
|
||||
const node = makeNode("ws-1");
|
||||
const { get, set } = makeStore([node], [], null, {});
|
||||
|
||||
handleCanvasEvent(
|
||||
makeMsg({
|
||||
event: "AGENT_MESSAGE",
|
||||
workspace_id: "ws-1",
|
||||
payload: {
|
||||
message: "ok",
|
||||
attachments: [
|
||||
{ uri: "workspace:/good.txt", name: "good.txt" },
|
||||
{ uri: "", name: "missing-uri" },
|
||||
{ uri: "workspace:/missing-name", name: "" },
|
||||
{ uri: "workspace:/wrong-types", name: 42 }, // non-string name
|
||||
],
|
||||
},
|
||||
}),
|
||||
get,
|
||||
set,
|
||||
);
|
||||
|
||||
const { agentMessages } = set.mock.calls[0][0] as {
|
||||
agentMessages: Record<string, Array<{ attachments?: Array<{ name: string }> }>>;
|
||||
};
|
||||
const atts = agentMessages["ws-1"][0].attachments!;
|
||||
expect(atts).toHaveLength(1);
|
||||
expect(atts[0].name).toBe("good.txt");
|
||||
});
|
||||
|
||||
it("ignores non-array attachments payloads", () => {
|
||||
const node = makeNode("ws-1");
|
||||
const { get, set } = makeStore([node], [], null, {});
|
||||
|
||||
handleCanvasEvent(
|
||||
makeMsg({
|
||||
event: "AGENT_MESSAGE",
|
||||
workspace_id: "ws-1",
|
||||
payload: { message: "hi", attachments: "not-an-array" },
|
||||
}),
|
||||
get,
|
||||
set,
|
||||
);
|
||||
|
||||
const { agentMessages } = set.mock.calls[0][0] as {
|
||||
agentMessages: Record<string, Array<{ content: string; attachments?: unknown[] }>>;
|
||||
};
|
||||
expect(agentMessages["ws-1"][0].content).toBe("hi");
|
||||
// No attachments key when input was malformed (rather than [] which
|
||||
// would render an empty "0 files" header in some chat UIs).
|
||||
expect("attachments" in agentMessages["ws-1"][0]).toBe(false);
|
||||
});
|
||||
|
||||
it("is a no-op when both message and attachments are empty", () => {
|
||||
const node = makeNode("ws-1");
|
||||
const { get, set } = makeStore([node]);
|
||||
|
||||
handleCanvasEvent(
|
||||
makeMsg({
|
||||
event: "AGENT_MESSAGE",
|
||||
workspace_id: "ws-1",
|
||||
payload: { message: "", attachments: [] },
|
||||
}),
|
||||
get,
|
||||
set,
|
||||
);
|
||||
|
||||
expect(set).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
220
tests/e2e/test_notify_attachments_e2e.sh
Executable file
220
tests/e2e/test_notify_attachments_e2e.sh
Executable file
@ -0,0 +1,220 @@
|
||||
#!/usr/bin/env bash
|
||||
# E2E test: agent → user notify with attachments (PR #2130).
|
||||
#
|
||||
# Exercises the wire contract that workspace/a2a_tools.tool_send_message_to_user
|
||||
# uses to push files into the canvas chat. Pure platform test — no workspace
|
||||
# container or LLM key required, so it runs against a bare workspace-server
|
||||
# in <2s and pins the contract that real runtimes depend on.
|
||||
#
|
||||
# What this proves:
|
||||
# 1. POST /notify (no attachments) persists a chat row that survives reload.
|
||||
# 2. POST /notify with attachments persists parts[].kind=file in the SAME
|
||||
# shape extractFilesFromTask reads → chips re-render after reload.
|
||||
# 3. Per-element attachment validation rejects empty uri/name (regression
|
||||
# for the bug where gin's binding:"required" on slice elements was a
|
||||
# no-op without `dive` — see activity.go:299-306).
|
||||
# 4. Empty `attachments: []` does NOT inject a stray `parts` key (would
|
||||
# otherwise render a "0 files" header in the canvas).
|
||||
# 5. Real /chat/uploads → /notify chain round-trips the URI verbatim.
|
||||
#
|
||||
# Usage: tests/e2e/test_notify_attachments_e2e.sh
|
||||
# Prereqs: workspace-server on http://localhost:8080, MOLECULE_ENV != production
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
source "$(dirname "$0")/_lib.sh"
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
WSID=""
|
||||
|
||||
cleanup() {
|
||||
if [ -n "$WSID" ]; then
|
||||
curl -s -X DELETE "$BASE/workspaces/$WSID?confirm=true" > /dev/null || true
|
||||
fi
|
||||
}
|
||||
trap cleanup EXIT
|
||||
|
||||
assert() {
|
||||
local label="$1"
|
||||
local actual="$2"
|
||||
local expected="$3"
|
||||
if [ "$actual" = "$expected" ]; then
|
||||
echo " PASS — $label"
|
||||
PASS=$((PASS+1))
|
||||
else
|
||||
echo " FAIL — $label"
|
||||
echo " expected: $expected"
|
||||
echo " actual: $actual"
|
||||
FAIL=$((FAIL+1))
|
||||
fi
|
||||
}
|
||||
|
||||
assert_contains() {
|
||||
local label="$1"
|
||||
local haystack="$2"
|
||||
local needle="$3"
|
||||
if echo "$haystack" | grep -qF "$needle"; then
|
||||
echo " PASS — $label"
|
||||
PASS=$((PASS+1))
|
||||
else
|
||||
echo " FAIL — $label"
|
||||
echo " haystack: $haystack"
|
||||
echo " needle: $needle"
|
||||
FAIL=$((FAIL+1))
|
||||
fi
|
||||
}
|
||||
|
||||
echo "=== Setup ==="
|
||||
R=$(curl -s -X POST "$BASE/workspaces" -H "Content-Type: application/json" \
|
||||
-d '{"name":"Notify E2E","tier":1}')
|
||||
WSID=$(echo "$R" | python3 -c 'import json,sys;print(json.load(sys.stdin)["id"])' 2>/dev/null || true)
|
||||
[ -n "$WSID" ] || { echo "Failed to create workspace: $R"; exit 1; }
|
||||
echo "Created workspace $WSID"
|
||||
|
||||
# Mint a bearer token so the wsAuth-grouped endpoints (notify, activity,
|
||||
# chat/uploads) accept us. Local dev mode skips auth, but CI enforces it
|
||||
# — so we always send the header to keep the test portable. The
|
||||
# admin/test-token endpoint is only enabled when MOLECULE_ENV != production.
|
||||
TOKEN=$(e2e_mint_test_token "$WSID")
|
||||
[ -n "$TOKEN" ] || { echo "Failed to mint test token"; exit 1; }
|
||||
AUTH="Authorization: Bearer $TOKEN"
|
||||
|
||||
echo ""
|
||||
echo "=== Test 1: notify without attachments persists row ==="
|
||||
CODE=$(curl -s -o /tmp/notify1.json -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" \
|
||||
-d '{"message":"Working on it"}')
|
||||
assert "POST /notify (text only) returns 200" "$CODE" "200"
|
||||
|
||||
# Read it back via /activity. The notify handler writes a2a_receive,
|
||||
# method=notify, response_body.result=<message>.
|
||||
# Notify writes source_id=NULL → use source=canvas (matches the chat
|
||||
# panel's history loader filter). source=agent would correctly hide it.
|
||||
ACT=$(curl -s -H "$AUTH" "$BASE/workspaces/$WSID/activity?source=canvas&limit=10")
|
||||
ROW=$(echo "$ACT" | python3 -c '
|
||||
import json, sys
|
||||
rows = json.load(sys.stdin) or []
|
||||
for r in rows:
|
||||
if r.get("method") == "notify":
|
||||
print(json.dumps(r))
|
||||
break
|
||||
')
|
||||
[ -n "$ROW" ] || { echo " FAIL — could not find notify row in activity"; FAIL=$((FAIL+1)); }
|
||||
|
||||
if [ -n "$ROW" ]; then
|
||||
RESULT=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["result"])')
|
||||
assert "persisted response_body.result matches message" "$RESULT" "Working on it"
|
||||
PARTS=$(echo "$ROW" | python3 -c 'import json,sys;b=json.load(sys.stdin)["response_body"];print("parts" in b)')
|
||||
assert "no stray parts[] when message has no attachments" "$PARTS" "False"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "=== Test 2: notify with attachments persists parts[].kind=file ==="
|
||||
CODE=$(curl -s -o /tmp/notify2.json -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" \
|
||||
-d '{
|
||||
"message": "Done — see attached.",
|
||||
"attachments": [
|
||||
{"uri":"workspace:/tmp/build-output.zip","name":"build-output.zip","mimeType":"application/zip","size":12345}
|
||||
]
|
||||
}')
|
||||
assert "POST /notify (with attachment) returns 200" "$CODE" "200"
|
||||
|
||||
ACT=$(curl -s -H "$AUTH" "$BASE/workspaces/$WSID/activity?source=canvas&limit=10")
|
||||
ROW=$(echo "$ACT" | python3 -c '
|
||||
import json, sys
|
||||
rows = json.load(sys.stdin) or []
|
||||
# Most recent matching notify row
|
||||
for r in rows:
|
||||
rb = r.get("response_body") or {}
|
||||
if r.get("method") == "notify" and "parts" in rb:
|
||||
print(json.dumps(r))
|
||||
break
|
||||
')
|
||||
[ -n "$ROW" ] || { echo " FAIL — could not find notify-with-attachments row"; FAIL=$((FAIL+1)); }
|
||||
|
||||
if [ -n "$ROW" ]; then
|
||||
KIND=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["parts"][0]["kind"])')
|
||||
URI=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["parts"][0]["file"]["uri"])')
|
||||
NAME=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["parts"][0]["file"]["name"])')
|
||||
MIME=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["parts"][0]["file"]["mimeType"])')
|
||||
SIZE=$(echo "$ROW" | python3 -c 'import json,sys;print(json.load(sys.stdin)["response_body"]["parts"][0]["file"]["size"])')
|
||||
assert "parts[0].kind == file" "$KIND" "file"
|
||||
assert "parts[0].file.uri preserved" "$URI" "workspace:/tmp/build-output.zip"
|
||||
assert "parts[0].file.name preserved" "$NAME" "build-output.zip"
|
||||
assert "parts[0].file.mimeType preserved" "$MIME" "application/zip"
|
||||
assert "parts[0].file.size preserved" "$SIZE" "12345"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "=== Test 3: per-element validation rejects empty uri/name ==="
|
||||
# Critical regression: gin's binding:"required" on slice elements is a no-op
|
||||
# without `dive`. activity.go:299 explicitly loops and rejects. Keep in lock-
|
||||
# step here so a future refactor that drops the loop fails this test.
|
||||
CODE=$(curl -s -o /tmp/notify3.json -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" \
|
||||
-d '{"message":"x","attachments":[{"uri":"","name":""}]}')
|
||||
assert "empty uri/name attachment is 400" "$CODE" "400"
|
||||
ERR=$(cat /tmp/notify3.json | python3 -c 'import json,sys;print(json.load(sys.stdin).get("error",""))')
|
||||
assert_contains "error mentions attachment[0]" "$ERR" "attachment[0]"
|
||||
|
||||
CODE=$(curl -s -o /tmp/notify3b.json -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" \
|
||||
-d '{"message":"x","attachments":[{"uri":"workspace:/ok.txt","name":"ok.txt"},{"uri":"","name":"bad"}]}')
|
||||
assert "second-element empty uri rejects whole call" "$CODE" "400"
|
||||
ERR=$(cat /tmp/notify3b.json | python3 -c 'import json,sys;print(json.load(sys.stdin).get("error",""))')
|
||||
assert_contains "error mentions attachment[1]" "$ERR" "attachment[1]"
|
||||
|
||||
echo ""
|
||||
echo "=== Test 4: missing message field rejected ==="
|
||||
CODE=$(curl -s -o /dev/null -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" -d '{}')
|
||||
assert "POST /notify with no message returns 400" "$CODE" "400"
|
||||
|
||||
echo ""
|
||||
echo "=== Test 5: real /chat/uploads → /notify round-trip ==="
|
||||
TMPF=$(mktemp -t notify-e2e-XXXX.txt)
|
||||
echo "round-trip-marker-$(date +%s)" > "$TMPF"
|
||||
UP=$(curl -s -X POST "$BASE/workspaces/$WSID/chat/uploads" -H "$AUTH" -F "files=@$TMPF")
|
||||
URI=$(echo "$UP" | python3 -c '
|
||||
import json,sys
|
||||
try:
|
||||
print(json.load(sys.stdin)["files"][0]["uri"])
|
||||
except Exception:
|
||||
pass
|
||||
')
|
||||
NAME=$(basename "$TMPF")
|
||||
|
||||
if [ -z "$URI" ]; then
|
||||
# /chat/uploads requires a running container in some configs. Skip the
|
||||
# round-trip if the platform refused — the synthetic-URI tests above
|
||||
# already pin the wire contract.
|
||||
echo " SKIP — /chat/uploads not available in this env (response: $UP)"
|
||||
else
|
||||
CODE=$(curl -s -o /dev/null -w "%{http_code}" -X POST "$BASE/workspaces/$WSID/notify" \
|
||||
-H "Content-Type: application/json" -H "$AUTH" \
|
||||
-d "{\"message\":\"see file\",\"attachments\":[{\"uri\":\"$URI\",\"name\":\"$NAME\"}]}")
|
||||
assert "uploaded URI round-trips through notify" "$CODE" "200"
|
||||
|
||||
ACT=$(curl -s -H "$AUTH" "$BASE/workspaces/$WSID/activity?source=canvas&limit=10")
|
||||
STORED_URI=$(echo "$ACT" | python3 -c "
|
||||
import json, sys
|
||||
rows = json.load(sys.stdin) or []
|
||||
for r in rows:
|
||||
rb = r.get('response_body') or {}
|
||||
parts = rb.get('parts') or []
|
||||
for p in parts:
|
||||
f = p.get('file') or {}
|
||||
if f.get('name') == '$NAME':
|
||||
print(f.get('uri',''))
|
||||
sys.exit(0)
|
||||
")
|
||||
assert "stored URI matches uploaded URI" "$STORED_URI" "$URI"
|
||||
fi
|
||||
|
||||
rm -f "$TMPF"
|
||||
|
||||
echo ""
|
||||
echo "=== Results: $PASS passed, $FAIL failed ==="
|
||||
[ "$FAIL" -eq 0 ]
|
||||
Loading…
Reference in New Issue
Block a user