mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-06 01:50:14 +02:00
* 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>
267 lines
9.1 KiB
Go
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)
|
|
}
|
|
}
|
|
}
|