diff --git a/examples/demo/main.go b/examples/demo/main.go index 590cb8e..5e8c046 100644 --- a/examples/demo/main.go +++ b/examples/demo/main.go @@ -139,7 +139,8 @@ var ( // renders the checklist immediately — without this, plan mode would leave // the panel empty because every tool except exit_plan_mode is gated. func buildConfirmPlan() agent.ConfirmPlanFunc { - return func(ctx context.Context, plan string) bool { + return func(ctx context.Context, proposal agent.PlanProposal) bool { + plan := proposal.Plan approvalID := fmt.Sprintf("%d", uniqueID()) ch := make(chan bool, 1) diff --git a/pkg/agent/plan_mode.go b/pkg/agent/plan_mode.go index b497694..1987d17 100644 --- a/pkg/agent/plan_mode.go +++ b/pkg/agent/plan_mode.go @@ -2,6 +2,7 @@ package agent import ( "context" + "encoding/json" "strings" "github.com/hung12ct/gopheragent/pkg/history" @@ -29,10 +30,22 @@ const planModeHint = planModeSentinel + " Do not call any tool except exit_plan_ "When the plan is complete, call exit_plan_mode with the plan as the `plan` argument. " + "The user must approve the plan before any tool runs; if they deny, you will receive their feedback and must revise." -// ConfirmPlanFunc receives the assistant's proposed plan text and returns -// true to approve (loop exits plan mode and resumes normal execution) or -// false to deny (the model is told to revise via the tool result). -type ConfirmPlanFunc func(ctx context.Context, plan string) bool +// PlanProposal is the payload handed to ConfirmPlanFunc when the model calls +// exit_plan_mode. Plan carries the markdown text from the built-in tool's +// `plan` argument — the common case. RawArgs is the untouched JSON of the +// tool call, so a host that registers its own exit_plan_mode tool with a +// structured argument schema can json.Unmarshal typed steps directly instead +// of parsing them back out of markdown. Plan is empty when the structured +// schema has no top-level string `plan` field; RawArgs is always populated. +type PlanProposal struct { + Plan string + RawArgs json.RawMessage +} + +// ConfirmPlanFunc receives the assistant's proposed plan and returns true to +// approve (loop exits plan mode and resumes normal execution) or false to +// deny (the model is told to revise via the tool result). +type ConfirmPlanFunc func(ctx context.Context, plan PlanProposal) bool // planModeTool is the tool definition injected into the LLM's tool list // when PlanMode is active. It is never executed — the loop intercepts calls diff --git a/pkg/agent/plan_mode_gate.go b/pkg/agent/plan_mode_gate.go index c3bafa6..a303cf4 100644 --- a/pkg/agent/plan_mode_gate.go +++ b/pkg/agent/plan_mode_gate.go @@ -38,7 +38,7 @@ func (al *AgentLoop) runPlanModeGate(ctx context.Context, sessionKey string, str al.emit(ctx, sessionKey, streamChan, Event(ThoughtEvent{Message: "Plan proposed — awaiting human approval."})) approved := false if al.ConfirmPlan != nil { - approved = al.ConfirmPlan(ctx, pa.Plan) + approved = al.ConfirmPlan(ctx, PlanProposal{Plan: pa.Plan, RawArgs: json.RawMessage(tc.ArgsJSON)}) } else { al.emit(ctx, sessionKey, streamChan, Event(ActionRequiredEvent{Tool: ExitPlanModeToolName, Args: pa.Plan})) } diff --git a/pkg/agent/plan_mode_gate_test.go b/pkg/agent/plan_mode_gate_test.go index 8bf8d05..ce30c2e 100644 --- a/pkg/agent/plan_mode_gate_test.go +++ b/pkg/agent/plan_mode_gate_test.go @@ -2,6 +2,7 @@ package agent import ( "context" + "encoding/json" "strings" "testing" @@ -52,7 +53,7 @@ func TestRunPlanModeGate_BlocksNonExitToolInPlanMode(t *testing.T) { func TestRunPlanModeGate_ApprovedExitsPlanModeAndPublishesResult(t *testing.T) { al := &AgentLoop{Tools: tools.NewRegistry()} al.SetPlanMode("s1", true) - al.ConfirmPlan = func(_ context.Context, _ string) bool { return true } + al.ConfirmPlan = func(_ context.Context, _ PlanProposal) bool { return true } ws := newWaveState(1) tc := PendingToolCall{ID: "tp", Name: ExitPlanModeToolName, ArgsJSON: `{"plan":"step a"}`} @@ -71,10 +72,50 @@ func TestRunPlanModeGate_ApprovedExitsPlanModeAndPublishesResult(t *testing.T) { } } +func TestRunPlanModeGate_PassesRawArgsToConfirmPlan(t *testing.T) { + // A host that registers a structured exit_plan_mode tool gets the + // untouched tool-call JSON on PlanProposal.RawArgs, so it can unmarshal + // typed steps without parsing markdown. Plan stays empty when the schema + // has no top-level string `plan` field. + al := &AgentLoop{Tools: tools.NewRegistry()} + al.SetPlanMode("s1", true) + var got PlanProposal + al.ConfirmPlan = func(_ context.Context, p PlanProposal) bool { + got = p + return true + } + ws := newWaveState(1) + const structured = `{"goal":"reel","steps":[{"op":"cut","at":1.5}]}` + tc := PendingToolCall{ID: "tp", Name: ExitPlanModeToolName, ArgsJSON: structured} + + if !al.runPlanModeGate(context.Background(), "s1", silentChan(t), ws, tc) { + t.Fatal("expected handled=true on approval") + } + if string(got.RawArgs) != structured { + t.Fatalf("RawArgs not passed through verbatim: got %q want %q", got.RawArgs, structured) + } + if got.Plan != "" { + t.Fatalf("Plan should be empty for a structured schema with no `plan` field; got %q", got.Plan) + } + var parsed struct { + Goal string `json:"goal"` + Steps []struct { + Op string `json:"op"` + At float64 `json:"at"` + } `json:"steps"` + } + if err := json.Unmarshal(got.RawArgs, &parsed); err != nil { + t.Fatalf("unmarshal RawArgs: %v", err) + } + if parsed.Goal != "reel" || len(parsed.Steps) != 1 || parsed.Steps[0].Op != "cut" { + t.Fatalf("structured plan did not round-trip from RawArgs: %+v", parsed) + } +} + func TestRunPlanModeGate_DeniedKeepsPlanModeAndFlagsError(t *testing.T) { al := &AgentLoop{Tools: tools.NewRegistry()} al.SetPlanMode("s1", true) - al.ConfirmPlan = func(_ context.Context, _ string) bool { return false } + al.ConfirmPlan = func(_ context.Context, _ PlanProposal) bool { return false } ws := newWaveState(1) tc := PendingToolCall{ID: "tp", Name: ExitPlanModeToolName, ArgsJSON: `{"plan":"x"}`} diff --git a/pkg/agent/plan_mode_test.go b/pkg/agent/plan_mode_test.go index 223ec2c..a769149 100644 --- a/pkg/agent/plan_mode_test.go +++ b/pkg/agent/plan_mode_test.go @@ -97,8 +97,8 @@ func TestPlanMode_ApprovedPlanExitsModeAndContinues(t *testing.T) { loop.SetPlanMode("s1", true) var capturedPlan string - loop.ConfirmPlan = func(_ context.Context, plan string) bool { - capturedPlan = plan + loop.ConfirmPlan = func(_ context.Context, plan PlanProposal) bool { + capturedPlan = plan.Plan return true } @@ -138,7 +138,7 @@ func TestPlanMode_DeniedPlanStaysInModeAndBlocksTools(t *testing.T) { reg := tools.NewRegistry() loop := NewAgentLoop(sm, reg, provider) loop.SetPlanMode("s1", true) - loop.ConfirmPlan = func(_ context.Context, _ string) bool { return false } + loop.ConfirmPlan = func(_ context.Context, _ PlanProposal) bool { return false } if _, err := loop.RunIteration(context.Background(), "s1", "hi"); err != nil { t.Fatalf("RunIteration: %v", err) @@ -179,7 +179,7 @@ func TestPlanMode_BlocksOtherToolsAndAllowsExitPlanMode(t *testing.T) { reg.Register(runTool) loop := NewAgentLoop(sm, reg, provider) loop.SetPlanMode("s1", true) - loop.ConfirmPlan = func(_ context.Context, _ string) bool { return true } + loop.ConfirmPlan = func(_ context.Context, _ PlanProposal) bool { return true } if _, err := loop.RunIteration(context.Background(), "s1", "start"); err != nil { t.Fatalf("RunIteration: %v", err) @@ -275,7 +275,7 @@ func TestPlanMode_MultiSessionLoopIsolatesApproval(t *testing.T) { loop := NewAgentLoop(sm, reg, provider) loop.SetPlanMode("alice", true) loop.SetPlanMode("bob", true) - loop.ConfirmPlan = func(_ context.Context, _ string) bool { return true } + loop.ConfirmPlan = func(_ context.Context, _ PlanProposal) bool { return true } // Alice runs to completion first — approves plan, then runs do_work. if _, err := loop.RunIteration(context.Background(), "alice", "go"); err != nil {