Files
multica/server/pkg/agent/pi_test.go
Bohan Jiang 0066ab259e refactor(agent): drop unreachable inline system-prompt branches (MUL-5392) (#6050)
* refactor(agent): drop unreachable inline system-prompt branches (MUL-5392)

The daemon only populates ExecOptions.SystemPrompt for openclaw, kimi and
traecli (providerNeedsInlineSystemPrompt); every other backend receives the
runtime brief as a per-task context file in the workdir. The inline branches in
claude, codex, opencode and pi were therefore dead, and read as if they were the
live delivery path.

Probed each backend over its real launch path with a canary in the context file
and no inline delivery — claude 2.1.220 (CLAUDE.md), codex 0.144.6 via the
app-server, opencode 1.17.7, pi 0.67.2, hermes 0.18.2 via ACP — and all of them
picked the brief up from disk, so the branches were removable with no behaviour
change. An empty workdir returned no canary, confirming the probe could fail.

opencode's branch was worse than dead: `opencode run` has no --prompt flag, so
enabling inline delivery there would have made every opencode task exit 1 with a
usage dump. The DevEco backend, forked from opencode, already documents this
constraint; opencode itself never got the fix.

Regression tests pin all three arg builders against re-adding the flag, and
providerNeedsInlineSystemPrompt now documents what was verified and what is
still unprobed (grok, qoder, codebuddy).

Hermes and kiro are untouched: their exclusion is deliberate and already tested.

Co-authored-by: multica-agent <github@multica.ai>

* test(agent): pin codex developerInstructions contract, drop stale pi flag doc

Review follow-up on MUL-5392.

buildPiArgs' doc comment still advertised --append-system-prompt after the
branch that emitted it was removed — exactly the stale-signal this PR set out
to delete.

The two codex sites fixed to a literal nil had no regression test, so restoring
nilIfEmpty(opts.SystemPrompt) would still have gone green. Both thread/start and
thread/resume now run with a canary SystemPrompt and assert developerInstructions
comes through as an explicit null. Mutation-checked: reverting either site fails
its test.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <agent@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-07-28 22:02:55 +08:00

267 lines
9.1 KiB
Go

package agent
import (
"context"
"log/slog"
"path/filepath"
"runtime"
"strings"
"testing"
"time"
)
func TestBuildPiArgsNoToolAllowlist(t *testing.T) {
// Extension tools registered via Pi's registerTool() must not be
// filtered out by a hardcoded --tools allowlist. Omitting --tools
// lets Pi use its full tool registry. See #2379.
args := buildPiArgs("test prompt", "/tmp/session.jsonl", ExecOptions{}, slog.Default())
for i, arg := range args {
if arg == "--tools" {
t.Errorf("buildPiArgs emits --tools %q; should not restrict tool registry (see #2379)", args[i+1])
}
}
}
func TestBuildPiArgsBasicFlags(t *testing.T) {
args := buildPiArgs("hello world", "/tmp/s.jsonl", ExecOptions{
Model: "anthropic/claude-sonnet-4-20250514",
}, slog.Default())
joined := strings.Join(args, " ")
for _, want := range []string{"-p", "--mode json", "--session /tmp/s.jsonl", "--provider anthropic", "--model claude-sonnet-4-20250514"} {
if !strings.Contains(joined, want) {
t.Errorf("expected %q in args, got: %v", want, args)
}
}
// Prompt must be the last positional argument.
if args[len(args)-1] != "hello world" {
t.Errorf("prompt should be last arg, got %q", args[len(args)-1])
}
}
// Pi reads the per-task AGENTS.md the daemon writes into the workdir, so the
// daemon never populates SystemPrompt for it (providerNeedsInlineSystemPrompt).
// Forwarding it anyway would duplicate the whole runtime brief on every turn.
func TestBuildPiArgsIgnoresSystemPrompt(t *testing.T) {
args := buildPiArgs("hello world", "/tmp/s.jsonl", ExecOptions{
SystemPrompt: "the entire multica runtime brief",
}, slog.Default())
for _, a := range args {
if a == "--append-system-prompt" {
t.Fatalf("unexpected --append-system-prompt in args: %v", args)
}
if a == "the entire multica runtime brief" {
t.Fatalf("SystemPrompt leaked into args: %v", args)
}
}
}
func TestBuildPiArgsCustomArgsAppended(t *testing.T) {
// Users can still restrict tools via custom_args if desired.
args := buildPiArgs("prompt", "/tmp/s.jsonl", ExecOptions{
CustomArgs: []string{"--tools", "read,bash"},
}, slog.Default())
found := false
for i, arg := range args {
if arg == "--tools" && i+1 < len(args) && args[i+1] == "read,bash" {
found = true
}
}
if !found {
t.Errorf("custom --tools should pass through via custom_args, got: %v", args)
}
}
// TestPiExecuteAttachesStdinPipe verifies that the Pi backend spawns the
// child with an explicit stdin pipe (FIFO) instead of leaving cmd.Stdin
// nil. Without an explicit pipe, Pi has been observed to block under
// systemd waiting for stdin events (#2188); attaching and immediately
// closing a pipe delivers a clean EOF on a FIFO and unblocks Pi.
//
// The probe is structural rather than behavioral: a shell script in
// place of `pi` inspects /proc/self/fd/0 and only emits a valid event
// stream if stdin is a FIFO. If the fix regresses (stdin nil → /dev/null
// char device), the fake exits non-zero and the test fails.
func TestPiExecuteAttachesStdinPipe(t *testing.T) {
t.Parallel()
if runtime.GOOS != "linux" {
// /proc/self/fd/0 is Linux-specific; skipping elsewhere keeps
// the assertion portable without losing CI coverage.
t.Skip("stdin fd inspection relies on /proc/self/fd/0")
}
fakePath := filepath.Join(t.TempDir(), "pi")
script := "#!/bin/sh\n" +
"kind=$(stat -c '%F' -L /proc/self/fd/0 2>/dev/null || echo unknown)\n" +
"case \"$kind\" in\n" +
" fifo|*pipe*)\n" +
" printf '%s\\n' '{\"type\":\"agent_start\"}'\n" +
" printf '%s\\n' '{\"type\":\"turn_end\",\"message\":{\"role\":\"assistant\",\"model\":\"test\",\"usage\":{\"input\":1,\"output\":1,\"cacheRead\":0,\"cacheWrite\":0,\"totalTokens\":2}}}'\n" +
" exit 0\n" +
" ;;\n" +
"esac\n" +
"printf 'stdin was %s; expected fifo\\n' \"$kind\" >&2\n" +
"exit 1\n"
writeTestExecutable(t, fakePath, []byte(script))
backend, err := New("pi", Config{ExecutablePath: fakePath, Logger: slog.Default()})
if err != nil {
t.Fatalf("new pi backend: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
if err != nil {
t.Fatalf("execute: %v", err)
}
go func() {
for range session.Messages {
}
}()
select {
case result, ok := <-session.Result:
if !ok {
t.Fatal("result channel closed without a value")
}
if result.Status != "completed" {
t.Fatalf("expected status=completed (stdin attached as fifo), got %q (error=%q)", result.Status, result.Error)
}
case <-time.After(10 * time.Second):
t.Fatal("timeout waiting for result")
}
}
// piEventStreamScript builds a sh script that prints each JSON event on
// its own stdout line. Fixtures must not contain single quotes.
func piEventStreamScript(events []string) string {
var b strings.Builder
b.WriteString("#!/bin/sh\n")
for _, e := range events {
b.WriteString("printf '%s\\n' '")
b.WriteString(e)
b.WriteString("'\n")
}
return b.String()
}
// TestPiExecuteRetainsOnlyLastTurnOutput verifies turn_start resets the
// output buffer so Result.Output keeps only the final turn's text.
func TestPiExecuteRetainsOnlyLastTurnOutput(t *testing.T) {
t.Parallel()
if runtime.GOOS == "windows" {
t.Skip("shell-script fixture is POSIX-only")
}
events := []string{
`{"type":"agent_start"}`,
`{"type":"turn_start"}`,
`{"type":"message_update","assistantMessageEvent":{"type":"text_delta","delta":"intermediate"}}`,
`{"type":"tool_execution_start","toolCallId":"call_1","toolName":"bash","args":{"command":"echo hi"}}`,
`{"type":"tool_execution_end","toolCallId":"call_1","toolName":"bash","result":{"content":[{"type":"text","text":"hi"}]},"isError":false}`,
`{"type":"turn_end","message":{"role":"assistant","model":"test","usage":{"input":1,"output":1}}}`,
`{"type":"turn_start"}`,
`{"type":"message_update","assistantMessageEvent":{"type":"text_delta","delta":"final"}}`,
`{"type":"message_update","assistantMessageEvent":{"type":"text_delta","delta":" "}}`,
`{"type":"message_update","assistantMessageEvent":{"type":"text_delta","delta":"answer"}}`,
`{"type":"turn_end","message":{"role":"assistant","model":"test","usage":{"input":2,"output":2}}}`,
}
fakePath := filepath.Join(t.TempDir(), "pi")
writeTestExecutable(t, fakePath, []byte(piEventStreamScript(events)))
backend, err := New("pi", Config{ExecutablePath: fakePath, Logger: slog.Default()})
if err != nil {
t.Fatalf("new pi backend: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
session, err := backend.Execute(ctx, "prompt-ignored", ExecOptions{Timeout: 5 * time.Second})
if err != nil {
t.Fatalf("execute: %v", err)
}
go func() {
for range session.Messages {
}
}()
select {
case result := <-session.Result:
if result.Status != "completed" {
t.Fatalf("expected status=completed, got %q (error=%q)", result.Status, result.Error)
}
if result.Output != "final answer" {
t.Fatalf("Output: got %q, want %q", result.Output, "final answer")
}
case <-time.After(10 * time.Second):
t.Fatal("timeout waiting for result")
}
}
func TestStripPiToolCallMarkup(t *testing.T) {
tests := map[string]string{
`before call:bash{command:<|"|>cd repo/path && ls -F<|"|>}<tool_call|> after`: "before after",
`before call:read{path:<|"|>repo/path/roles/example/verify.yml<|"|>} after`: "before after",
`before response:bash{command:<|"|>multica issue comment list issue-id --all --output json<|"|>} after`: "before after",
`before call:bash{command:<|"|>printf '{"key":"value"}'<|"|>} after`: "before after",
`before <|turn>model after`: "before after",
}
for in, want := range tests {
got := stripPiToolCallMarkup(in)
if got != want {
t.Fatalf("unexpected stripped text: %q, want %q", got, want)
}
}
}
func TestDrainPiTextBufferSplitToolCall(t *testing.T) {
chunks := []string{
"before ca",
`ll:bash{command:<|"|>ls -R repo/path`,
`/roles/example<|"|>}`,
" after",
}
var buf strings.Builder
var got strings.Builder
for _, chunk := range chunks {
got.WriteString(drainPiTextBuffer(&buf, chunk))
}
got.WriteString(flushPiTextBuffer(&buf))
if got.String() != "before after" {
t.Fatalf("unexpected streamed text: %q", got.String())
}
}
func TestDrainPiTextBufferSplitControlToken(t *testing.T) {
chunks := []string{"before <|tu", "rn>model after"}
var buf strings.Builder
var got strings.Builder
for _, chunk := range chunks {
got.WriteString(drainPiTextBuffer(&buf, chunk))
}
got.WriteString(flushPiTextBuffer(&buf))
if got.String() != "before after" {
t.Fatalf("unexpected streamed text: %q", got.String())
}
}
func TestFlushPiTextBufferKeepsUnmatchedToolPrefixes(t *testing.T) {
tests := []string{
"plain response: see below",
"plain call: see below",
`plain call:bash{command:<|"|>unterminated`,
}
for _, want := range tests {
var buf strings.Builder
got := drainPiTextBuffer(&buf, want)
got += flushPiTextBuffer(&buf)
if got != want {
t.Fatalf("unexpected flushed text: %q, want %q", got, want)
}
}
}