opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: add safeguards to prevent /plan from executing without a complete plan

Browse source 
The AI could transition to "executing" status without writing phases,
commands, or context to MEMORY.md, causing empty plan execution.

Adds multiple defense layers:
- Block non-read/non-MEMORY.md tool calls during interview and review
- Cap LLM iterations to 3 during pre-execution phases
- Validate phases exist before /plan start allows transition
- Auto-revert to interviewing if executing with no phases
- Hijack interviewing→executing transition to "review" status,
  showing the plan to the user for approval via /plan start
- Inject staleness nudge after 2 consecutive turns without MEMORY.md update
- Add GetReviewContext() for review-phase-only system prompt injection

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
dj-oyu 2026-02-21 02:20:16 +09:00
parent d5330b9f07
commit 103693851f
5 changed files with 204 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
pkg/agent/context.go
View file

@ -83,8 +83,8 @@ Your workspace is at: %s
3. **Memory & Plans**
- Use memory/MEMORY.md for structured plans.
- If Status is "interviewing": Ask clarifying questions.
Update Context with answers via edit_file.
When ready, organize into Phases and set Status to "executing".
After each answer, use edit_file to save findings to ## Context in memory/MEMORY.md.
When you have enough information, write ## Phase and ## Commands sections into MEMORY.md, then set Status to "executing".
- If Status is "executing": Work through the current Phase's steps.
Mark each [x] via edit_file. The system will auto-advance phases.
- Plan format:
@ -339,6 +339,11 @@ func (cb *ContextBuilder) GetCurrentPhase() int {
return cb.memory.GetCurrentPhase()
}
// GetTotalPhases returns the total number of phases in the plan.
func (cb *ContextBuilder) GetTotalPhases() int {
return cb.memory.GetTotalPhases()
}
// FormatPlanDisplay returns a user-facing display of the full plan.
func (cb *ContextBuilder) FormatPlanDisplay() string {
return cb.memory.FormatPlanDisplay()

4
pkg/agent/instance.go
View file

@ -30,6 +30,10 @@ type AgentInstance struct {
Subagents *config.SubagentsConfig
SkillsFilter []string
Candidates []providers.FallbackCandidate
// Interview staleness tracking: consecutive turns where MEMORY.md was not updated.
interviewStaleCount int
interviewMemoryLen int
}
// NewAgentInstance creates an agent instance from config.

126
pkg/agent/loop.go
View file

@ -492,6 +492,23 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
opts.ChatID,
)
// 2b. Interview staleness nudge: if MEMORY.md hasn't been updated for
// several consecutive turns, inject a reminder so the AI writes its findings.
const interviewStaleThreshold = 2
if agent.ContextBuilder.GetPlanStatus() == "interviewing" && agent.interviewStaleCount >= interviewStaleThreshold {
messages = append(messages, providers.Message{
Role: "user",
Content: "[System] You have been interviewing for several turns without updating memory/MEMORY.md. Please use edit_file now to save your findings to the ## Context section, or organize the plan into Phases if you have enough information.",
})
}
// 2c. Snapshot plan status and MEMORY.md size before LLM iteration.
preStatus := agent.ContextBuilder.GetPlanStatus()
var preMemoryLen int
if preStatus == "interviewing" {
preMemoryLen = len(agent.ContextBuilder.ReadMemory())
}
// 3. Save user message to session (use compact form if available)
historyMsg := opts.UserMessage
if opts.HistoryMessage != "" {
@ -514,8 +531,33 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
// This is controlled by the tool's Silent flag and ForUser content
// 5a. Auto-advance plan phases after LLM iteration
if agent.ContextBuilder.HasActivePlan() && agent.ContextBuilder.GetPlanStatus() == "executing" {
if agent.ContextBuilder.IsPlanComplete() {
postStatus := agent.ContextBuilder.GetPlanStatus()
if agent.ContextBuilder.HasActivePlan() && postStatus == "executing" {
// Intercept: if AI changed status from interviewing to executing,
// hijack to "review" and show the plan for user approval.
if preStatus == "interviewing" {
if agent.ContextBuilder.GetTotalPhases() == 0 {
_ = agent.ContextBuilder.SetPlanStatus("interviewing")
logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
map[string]interface{}{"agent_id": agent.ID})
} else {
_ = agent.ContextBuilder.SetPlanStatus("review")
if !constants.IsInternalChannel(opts.Channel) {
planDisplay := agent.ContextBuilder.FormatPlanDisplay()
al.bus.PublishOutbound(bus.OutboundMessage{
Channel: opts.Channel,
ChatID: opts.ChatID,
Content: planDisplay + "\n\nUse /plan start to approve, or continue chatting to refine.",
SkipPlaceholder: true,
})
}
}
} else if agent.ContextBuilder.GetTotalPhases() == 0 {
// Safeguard: executing but no phases (shouldn't happen, but be safe).
_ = agent.ContextBuilder.SetPlanStatus("interviewing")
logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
map[string]interface{}{"agent_id": agent.ID})
} else if agent.ContextBuilder.IsPlanComplete() {
_ = agent.ContextBuilder.ClearMemory()
if !constants.IsInternalChannel(opts.Channel) {
al.bus.PublishOutbound(bus.OutboundMessage{
@ -540,7 +582,21 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
}
}
// 5b. Handle empty response
// 5b. Interview staleness detection: compare MEMORY.md size after iteration.
if agent.ContextBuilder.GetPlanStatus() == "interviewing" {
postMemoryLen := len(agent.ContextBuilder.ReadMemory())
if postMemoryLen == preMemoryLen {
agent.interviewStaleCount++
} else {
agent.interviewStaleCount = 0
}
agent.interviewMemoryLen = postMemoryLen
} else {
// Reset counter when not interviewing.
agent.interviewStaleCount = 0
}
// 5c. Handle empty response
if finalContent == "" {
finalContent = opts.DefaultResponse
}
@ -616,14 +672,24 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
var finalContent string
lastReminderIdx := -1
for iteration < agent.MaxIterations {
// During pre-execution plan modes (interviewing/review), cap iterations
// to prevent runaway tool loops.
maxIter := agent.MaxIterations
if isPlanPreExecution(agent.ContextBuilder.GetPlanStatus()) {
const interviewMaxIter = 3
if maxIter > interviewMaxIter {
maxIter = interviewMaxIter
}
}
for iteration < maxIter {
iteration++
logger.DebugCF("agent", "LLM iteration",
map[string]interface{}{
"agent_id": agent.ID,
"iteration": iteration,
"max": agent.MaxIterations,
"max": maxIter,
})
// Build tool definitions
@ -770,7 +836,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
al.bus.PublishOutbound(bus.OutboundMessage{
Channel: opts.Channel,
ChatID: opts.ChatID,
Content: fmt.Sprintf("🔧 %s (%d/%d)", strings.Join(toolNames, ", "), iteration, agent.MaxIterations),
Content: fmt.Sprintf("🔧 %s (%d/%d)", strings.Join(toolNames, ", "), iteration, maxIter),
IsStatus: true,
})
}
@ -825,7 +891,14 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
}
}
toolResult := agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback)
// Block non-allowed tools during plan interview mode.
// Only read-type tools and MEMORY.md writes are permitted.
var toolResult *tools.ToolResult
if isPlanPreExecution(agent.ContextBuilder.GetPlanStatus()) && !isToolAllowedDuringInterview(tc.Name, tc.Arguments) {
toolResult = tools.ErrorResult("Interview mode: only read tools and MEMORY.md edits are allowed. Focus on asking questions and updating the plan.")
} else {
toolResult = agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback)
}
// Send ForUser content to user immediately if not Silent
if !toolResult.Silent && toolResult.ForUser != "" && opts.SendResponse {
@ -884,7 +957,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
// If max iterations exhausted with tool calls still pending,
// make one final LLM call without tools to force a text response.
if finalContent == "" && iteration >= agent.MaxIterations {
if finalContent == "" && iteration >= maxIter {
logger.WarnCF("agent", "Max iterations reached, forcing final response without tools",
map[string]interface{}{
"agent_id": agent.ID,
@ -1465,13 +1538,20 @@ func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) {
if !agent.ContextBuilder.HasActivePlan() {
return "No active plan.", true
}
if agent.ContextBuilder.GetPlanStatus() != "interviewing" {
status := agent.ContextBuilder.GetPlanStatus()
if status == "executing" {
return "Plan is already executing.", true
}
if status != "interviewing" && status != "review" {
return fmt.Sprintf("Cannot start from status %q.", status), true
}
if agent.ContextBuilder.GetTotalPhases() == 0 {
return "Cannot start: no phases defined yet. Complete the interview first.", true
}
if err := agent.ContextBuilder.SetPlanStatus("executing"); err != nil {
return fmt.Sprintf("Error: %v", err), true
}
return "Plan status changed to executing.", true
return "Plan approved. Executing.", true
case "next":
if !agent.ContextBuilder.HasActivePlan() {
@ -1495,6 +1575,32 @@ func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) {
}
}
// isPlanPreExecution returns true if the plan is in a pre-execution state
// (interviewing or review) where tool restrictions and iteration caps apply.
func isPlanPreExecution(status string) bool {
return status == "interviewing" || status == "review"
}
// isToolAllowedDuringInterview checks whether a tool call is permitted while the
// plan is in a pre-execution state. Read-type tools are always allowed. Write-type
// tools (edit_file, append_file, write_file) are only allowed when targeting MEMORY.md.
func isToolAllowedDuringInterview(toolName string, args map[string]interface{}) bool {
// Read-type tools: always allowed
switch toolName {
case "read_file", "list_dir", "web_search", "web_fetch":
return true
}
// Write-type tools: allowed only when targeting MEMORY.md
switch toolName {
case "edit_file", "append_file", "write_file":
path, _ := args["path"].(string)
return strings.HasSuffix(path, "MEMORY.md")
}
return false
}
// expandPlanCommand detects "/plan <task>" (new plan start) and:
// - writes the interview seed to MEMORY.md
// - rewrites the message content for the LLM

61
pkg/agent/loop_test.go
View file

@ -995,18 +995,60 @@ func TestPlanCommand_Start(t *testing.T) {
al, cleanup := newTestAgentLoop(t)
defer cleanup()
// Create interviewing plan
agent := al.registry.GetDefaultAgent()
// Create interviewing plan with phases (start requires phases)
plan := "# Active Plan\n\n> Task: Test task\n> Status: interviewing\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
_ = agent.ContextBuilder.WriteMemory(plan)
// Transition to executing via /plan start
response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
if !strings.Contains(response, "approved") {
t.Errorf("expected 'approved', got %q", response)
}
if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
t.Errorf("expected 'executing', got %q", status)
}
}
func TestPlanCommand_StartFromReview(t *testing.T) {
al, cleanup := newTestAgentLoop(t)
defer cleanup()
agent := al.registry.GetDefaultAgent()
// Create a plan in review status
plan := "# Active Plan\n\n> Task: Test task\n> Status: review\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
_ = agent.ContextBuilder.WriteMemory(plan)
// Approve via /plan start
response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
if !strings.Contains(response, "approved") {
t.Errorf("expected 'approved', got %q", response)
}
if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
t.Errorf("expected 'executing', got %q", status)
}
}
func TestPlanCommand_StartNoPhases(t *testing.T) {
al, cleanup := newTestAgentLoop(t)
defer cleanup()
// Create interviewing plan without phases
al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"})
// Transition to executing
// Should be blocked because no phases exist
response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
if !strings.Contains(response, "executing") {
t.Errorf("expected 'executing', got %q", response)
if !strings.Contains(response, "no phases") {
t.Errorf("expected 'no phases' error, got %q", response)
}
agent := al.registry.GetDefaultAgent()
if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
t.Errorf("expected 'executing', got %q", status)
if status := agent.ContextBuilder.GetPlanStatus(); status != "interviewing" {
t.Errorf("expected status to remain 'interviewing', got %q", status)
}
}
@ -1014,8 +1056,11 @@ func TestPlanCommand_StartAlreadyExecuting(t *testing.T) {
al, cleanup := newTestAgentLoop(t)
defer cleanup()
// Create interviewing plan then start
al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"})
agent := al.registry.GetDefaultAgent()
// Create interviewing plan with phases, then start
plan := "# Active Plan\n\n> Task: Test task\n> Status: interviewing\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
_ = agent.ContextBuilder.WriteMemory(plan)
al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
// Try start again

27
pkg/agent/memory.go
View file

@ -375,7 +375,11 @@ func (ms *MemoryStore) GetInterviewContext() string {
sb.WriteString("- Environment (OS, language, runtime versions)\n")
sb.WriteString("- Tooling preferences (test framework, linter, formatter, CI)\n")
sb.WriteString("- Key commands the user already runs (build, test, deploy)\n")
sb.WriteString("When ready, organize into 2-5 phases with 3-5 steps each.\n")
sb.WriteString("\n### Rules\n")
sb.WriteString("- After each answer, use edit_file to append findings to the ## Context section of memory/MEMORY.md.\n")
sb.WriteString("- When you have enough information, use edit_file to write ## Phase, ## Commands, and ## Context sections into memory/MEMORY.md.\n")
sb.WriteString("- Organize into 2-5 phases with 3-5 steps each.\n")
sb.WriteString("- After writing Phases, set Status to executing. The system will handle the rest.\n")
sb.WriteString("\n### Target Format\n")
sb.WriteString("```\n")
sb.WriteString("## Phase 1: <title>\n")
@ -392,6 +396,20 @@ func (ms *MemoryStore) GetInterviewContext() string {
return sb.String()
}
// GetReviewContext returns context for injection during the review phase.
// Shows the full plan and instructs the AI to wait for user approval.
func (ms *MemoryStore) GetReviewContext() string {
content := ms.ReadLongTerm()
var sb strings.Builder
sb.WriteString("## Active Plan (awaiting approval)\n\n")
sb.WriteString(content)
sb.WriteString("\n\nThe plan is awaiting user approval.\n")
sb.WriteString("- If the user requests changes, update memory/MEMORY.md via edit_file.\n")
sb.WriteString("- Do NOT change Status yourself. The user will run /plan start to approve.\n")
return sb.String()
}
// GetPlanContext returns context for injection during the executing phase.
// Only the current phase is shown in detail; completed phases are compressed
// to one-line summaries; future phases are omitted.
@ -572,9 +590,12 @@ func (ms *MemoryStore) GetMemoryContext() string {
if longTerm != "" {
if ms.HasActivePlan() {
status := ms.GetPlanStatus()
if status == "interviewing" {
switch status {
case "interviewing":
parts = append(parts, ms.GetInterviewContext())
} else {
case "review":
parts = append(parts, ms.GetReviewContext())
default:
parts = append(parts, ms.GetPlanContext())
}
} else {