feat: add /plan start clear and filter interview tool definitions
Add history-clear option to plan approval so interview conversation history (already distilled into MEMORY.md) can be discarded on transition to executing, saving context window for the execution phase. Filter tool definitions sent to the LLM during interview/review phases using a shared interviewAllowedTools whitelist, preventing wasted tokens and reject-retry cycles from disallowed tool calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
dj-oyu
parent
2759bee2e3
commit
9e34a140f0
4 changed files with 267 additions and 67 deletions
|
|
@ -14,6 +14,11 @@ go vet ./...
|
|||
|
||||
Lint: `golangci-lint run`
|
||||
|
||||
## Plan Mode
|
||||
|
||||
- **Interview tool filtering**: `interviewAllowedTools` in `pkg/agent/loop.go` is the single source of truth for tools available during interview/review phases. Both `filterInterviewTools` (strips definitions before LLM call) and `isToolAllowedDuringInterview` (argument-level gating) reference this map.
|
||||
- **History clear**: `/plan start clear` wipes session history and summary on transition to executing. The Mini App review UI offers two sliders: standard approve and approve-with-clear.
|
||||
|
||||
## Security TODOs
|
||||
|
||||
- **Log Fields masking**: `LogEntry.Fields` (`map[string]any`) is exposed via WebSocket (`/miniapp/api/logs/ws`) and snapshots (`/miniapp/api/logs/snapshot`). If any code logs sensitive values (tokens, API keys, passwords) in Fields, they will be visible to Mini App users. Add a sanitizer in `RecentLogs()` and `wsLogs()` that masks values for keys matching patterns like `token`, `key`, `secret`, `password`, `authorization`. Track in: `pkg/logger/logger.go` (RecentLogs), `pkg/miniapp/miniapp.go` (wsLogs stream).
|
||||
|
|
|
|||
|
|
@ -86,6 +86,7 @@ type AgentLoop struct {
|
|||
channelManager *channels.Manager
|
||||
providerCache map[string]providers.LLMProvider
|
||||
planStartPending bool // set by /plan start to trigger LLM execution
|
||||
planClearHistory bool // set by /plan start clear to wipe history on transition
|
||||
sessionLocks sync.Map // sessionKey → *sessionSemaphore
|
||||
activeTasks sync.Map // sessionKey → *activeTask
|
||||
sessions *SessionTracker
|
||||
|
|
@ -287,6 +288,17 @@ func (al *AgentLoop) Run(ctx context.Context) error {
|
|||
// the LLM worker actually begins executing the plan.
|
||||
if al.planStartPending {
|
||||
al.planStartPending = false
|
||||
clearHistory := al.planClearHistory
|
||||
al.planClearHistory = false
|
||||
|
||||
if clearHistory {
|
||||
if agent := al.registry.GetDefaultAgent(); agent != nil {
|
||||
agent.Sessions.SetHistory(msg.SessionKey, nil)
|
||||
agent.Sessions.SetSummary(msg.SessionKey, "")
|
||||
_ = agent.Sessions.Save(msg.SessionKey)
|
||||
}
|
||||
}
|
||||
|
||||
syntheticMeta := map[string]string{"echoed": "1"}
|
||||
for k, v := range msg.Metadata {
|
||||
if k != "source" {
|
||||
|
|
@ -1550,6 +1562,12 @@ func (al *AgentLoop) runLLMIteration(
|
|||
// Build tool definitions
|
||||
providerToolDefs := agent.Tools.ToProviderDefs()
|
||||
|
||||
// Interview mode: strip tool definitions the LLM must not use,
|
||||
// reducing token cost and preventing wasted reject-retry cycles.
|
||||
if isPlanPreExecution(planSnapshot) {
|
||||
providerToolDefs = filterInterviewTools(providerToolDefs)
|
||||
}
|
||||
|
||||
// Log LLM request details
|
||||
logger.DebugCF("agent", "LLM request",
|
||||
map[string]any{
|
||||
|
|
@ -2772,6 +2790,11 @@ func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) {
|
|||
return fmt.Sprintf("Error: %v", err), true
|
||||
}
|
||||
al.planStartPending = true
|
||||
clearHistory := len(args) > 1 && args[1] == "clear"
|
||||
al.planClearHistory = clearHistory
|
||||
if clearHistory {
|
||||
return "Plan approved. Executing with clean history.", true
|
||||
}
|
||||
return "Plan approved. Executing.", true
|
||||
|
||||
case "next":
|
||||
|
|
@ -2802,34 +2825,56 @@ func isPlanPreExecution(status string) bool {
|
|||
return status == "interviewing" || status == "review"
|
||||
}
|
||||
|
||||
// interviewAllowedTools is the single source of truth for tool names that may
|
||||
// be sent to the LLM (and subsequently invoked) during the interview phase.
|
||||
// filterInterviewTools uses this to strip tool *definitions* before the LLM call,
|
||||
// while isToolAllowedDuringInterview adds argument-level checks as a second gate.
|
||||
var interviewAllowedTools = map[string]bool{
|
||||
"readfile": true,
|
||||
"listdir": true,
|
||||
"websearch": true,
|
||||
"webfetch": true,
|
||||
"message": true,
|
||||
"editfile": true,
|
||||
"appendfile": true,
|
||||
"writefile": true,
|
||||
"exec": true,
|
||||
"logs": true,
|
||||
}
|
||||
|
||||
// filterInterviewTools removes tool definitions that are not in the
|
||||
// interviewAllowedTools whitelist, reducing token usage and preventing the
|
||||
// LLM from attempting disallowed tool calls during the interview phase.
|
||||
func filterInterviewTools(defs []providers.ToolDefinition) []providers.ToolDefinition {
|
||||
filtered := make([]providers.ToolDefinition, 0, len(defs))
|
||||
for _, d := range defs {
|
||||
if interviewAllowedTools[tools.NormalizeToolName(d.Function.Name)] {
|
||||
filtered = append(filtered, d)
|
||||
}
|
||||
}
|
||||
return filtered
|
||||
}
|
||||
|
||||
// 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.
|
||||
// Uses normalized names so "readfile" matches "read_file", etc.
|
||||
// plan is in a pre-execution state. Uses the shared interviewAllowedTools map for
|
||||
// name-level gating, then applies argument-level constraints for write-type tools
|
||||
// (MEMORY.md only) and exec (read-only commands only).
|
||||
func isToolAllowedDuringInterview(toolName string, args map[string]interface{}) bool {
|
||||
norm := tools.NormalizeToolName(toolName)
|
||||
|
||||
// Read-type tools and communication: always allowed
|
||||
switch norm {
|
||||
case "readfile", "listdir", "websearch", "webfetch", "message":
|
||||
return true
|
||||
if !interviewAllowedTools[norm] {
|
||||
return false
|
||||
}
|
||||
|
||||
// Write-type tools: allowed only when targeting MEMORY.md
|
||||
// Argument-level constraints
|
||||
switch norm {
|
||||
case "editfile", "appendfile", "writefile":
|
||||
path, _ := args["path"].(string)
|
||||
return strings.HasSuffix(path, "MEMORY.md")
|
||||
}
|
||||
|
||||
// exec: allow read-only commands
|
||||
switch norm {
|
||||
case "exec":
|
||||
cmd, _ := args["command"].(string)
|
||||
return isReadOnlyCommand(cmd)
|
||||
}
|
||||
|
||||
return false
|
||||
return true
|
||||
}
|
||||
|
||||
// isReadOnlyCommand returns true when cmd is a safe, read-only shell command
|
||||
|
|
|
|||
|
|
@ -2504,3 +2504,139 @@ func TestAgentLoop_PlanModel_NotUsedDuringExecuting(t *testing.T) {
|
|||
t.Errorf("Expected normal model 'normal-model' during executing, got %q", provider.models[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlanCommand_StartClear(t *testing.T) {
|
||||
al, cleanup := newTestAgentLoop(t)
|
||||
defer cleanup()
|
||||
|
||||
agent := al.registry.GetDefaultAgent()
|
||||
|
||||
// Create a plan in review status with phases
|
||||
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)
|
||||
|
||||
// Seed session history so we can verify it gets cleared
|
||||
agent.Sessions.AddMessage("test-session", "user", "hello")
|
||||
agent.Sessions.AddMessage("test-session", "assistant", "world")
|
||||
agent.Sessions.SetSummary("test-session", "some summary")
|
||||
|
||||
// Approve with clear
|
||||
response, handled := al.handleCommand(context.Background(), bus.InboundMessage{
|
||||
Content: "/plan start clear",
|
||||
SessionKey: "test-session",
|
||||
})
|
||||
if !handled {
|
||||
t.Fatal("expected /plan start clear to be handled")
|
||||
}
|
||||
if !strings.Contains(response, "clean history") {
|
||||
t.Errorf("expected 'clean history' in response, got %q", response)
|
||||
}
|
||||
if !al.planStartPending {
|
||||
t.Error("expected planStartPending to be true")
|
||||
}
|
||||
if !al.planClearHistory {
|
||||
t.Error("expected planClearHistory to be true")
|
||||
}
|
||||
|
||||
// Simulate what Run() does when planStartPending is set
|
||||
al.planStartPending = false
|
||||
clearHistory := al.planClearHistory
|
||||
al.planClearHistory = false
|
||||
if clearHistory {
|
||||
agent.Sessions.SetHistory("test-session", nil)
|
||||
agent.Sessions.SetSummary("test-session", "")
|
||||
_ = agent.Sessions.Save("test-session")
|
||||
}
|
||||
|
||||
// Verify history and summary are cleared
|
||||
history := agent.Sessions.GetHistory("test-session")
|
||||
if len(history) != 0 {
|
||||
t.Errorf("expected empty history after clear, got %d messages", len(history))
|
||||
}
|
||||
summary := agent.Sessions.GetSummary("test-session")
|
||||
if summary != "" {
|
||||
t.Errorf("expected empty summary after clear, got %q", summary)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlanCommand_StartWithoutClear_PreservesHistory(t *testing.T) {
|
||||
al, cleanup := newTestAgentLoop(t)
|
||||
defer cleanup()
|
||||
|
||||
agent := al.registry.GetDefaultAgent()
|
||||
|
||||
// Create a plan in review status with phases
|
||||
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)
|
||||
|
||||
// Seed session history
|
||||
agent.Sessions.AddMessage("test-session", "user", "hello")
|
||||
agent.Sessions.AddMessage("test-session", "assistant", "world")
|
||||
agent.Sessions.SetSummary("test-session", "some summary")
|
||||
|
||||
// Approve without clear
|
||||
response, _ := al.handleCommand(context.Background(), bus.InboundMessage{
|
||||
Content: "/plan start",
|
||||
SessionKey: "test-session",
|
||||
})
|
||||
if strings.Contains(response, "clean history") {
|
||||
t.Errorf("did not expect 'clean history' in response, got %q", response)
|
||||
}
|
||||
if al.planClearHistory {
|
||||
t.Error("planClearHistory should be false for /plan start without clear")
|
||||
}
|
||||
|
||||
// Verify history is preserved
|
||||
history := agent.Sessions.GetHistory("test-session")
|
||||
if len(history) != 2 {
|
||||
t.Errorf("expected 2 history messages preserved, got %d", len(history))
|
||||
}
|
||||
summary := agent.Sessions.GetSummary("test-session")
|
||||
if summary != "some summary" {
|
||||
t.Errorf("expected summary preserved, got %q", summary)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterInterviewTools(t *testing.T) {
|
||||
allDefs := []providers.ToolDefinition{
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "read_file"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "list_dir"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "web_search"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "web_fetch"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "message"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "edit_file"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "append_file"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "write_file"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "exec"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "logs"}},
|
||||
// These should be filtered out:
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "spawn_subagent"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "skills_search"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "skills_install"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "bg_monitor"}},
|
||||
{Function: protocoltypes.ToolFunctionDefinition{Name: "i2c_transfer"}},
|
||||
}
|
||||
|
||||
filtered := filterInterviewTools(allDefs)
|
||||
|
||||
// Should keep exactly the 10 allowed tools
|
||||
if len(filtered) != 10 {
|
||||
names := make([]string, len(filtered))
|
||||
for i, d := range filtered {
|
||||
names[i] = d.Function.Name
|
||||
}
|
||||
t.Errorf("expected 10 allowed tools, got %d: %v", len(filtered), names)
|
||||
}
|
||||
|
||||
// Verify none of the disallowed tools slipped through
|
||||
disallowed := map[string]bool{
|
||||
"spawnsubagent": true, "skillssearch": true,
|
||||
"skillsinstall": true, "bgmonitor": true, "ictransfer": true,
|
||||
}
|
||||
for _, d := range filtered {
|
||||
norm := tools.NormalizeToolName(d.Function.Name)
|
||||
if disallowed[norm] {
|
||||
t.Errorf("disallowed tool %q should have been filtered out", d.Function.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -995,10 +995,16 @@ function renderPlanFromData(data) {
|
|||
}
|
||||
if (data.status === 'review') {
|
||||
html += `<div class="slide-approve-wrap">
|
||||
<div class="slide-approve-track glass glass-interactive">
|
||||
<div class="slide-approve-track glass glass-interactive" data-cmd="/plan start">
|
||||
<div class="slide-approve-thumb"><svg viewBox="0 0 24 24"><path d="M5 12h14m-6-6 6 6-6 6" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" fill="none"/></svg></div>
|
||||
<div class="slide-approve-label">Slide to Approve</div>
|
||||
</div>
|
||||
</div>
|
||||
<div class="slide-approve-wrap">
|
||||
<div class="slide-approve-track glass glass-interactive" data-cmd="/plan start clear" style="border-color:var(--warn,#ff9800)">
|
||||
<div class="slide-approve-thumb" style="background:var(--warn,#ff9800)"><svg viewBox="0 0 24 24"><path d="M5 12h14m-6-6 6 6-6 6" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" fill="none"/></svg></div>
|
||||
<div class="slide-approve-label">Approve & Clear History</div>
|
||||
</div>
|
||||
</div>`;
|
||||
}
|
||||
} else {
|
||||
|
|
@ -1017,61 +1023,69 @@ function setupSlideApprove() {
|
|||
slideApproveAC = new AbortController();
|
||||
var signal = slideApproveAC.signal;
|
||||
|
||||
var track = document.querySelector('.slide-approve-track');
|
||||
if (!track) return;
|
||||
var thumb = track.querySelector('.slide-approve-thumb');
|
||||
var label = track.querySelector('.slide-approve-label');
|
||||
var dragging = false;
|
||||
var startX = 0;
|
||||
var thumbStartLeft = 0;
|
||||
var tracks = document.querySelectorAll('.slide-approve-track');
|
||||
if (!tracks.length) return;
|
||||
|
||||
function getMaxLeft() {
|
||||
return track.offsetWidth - thumb.offsetWidth - 6; // 3px padding each side
|
||||
}
|
||||
tracks.forEach(function(track) {
|
||||
var thumb = track.querySelector('.slide-approve-thumb');
|
||||
var label = track.querySelector('.slide-approve-label');
|
||||
var cmd = track.getAttribute('data-cmd') || '/plan start';
|
||||
var dragging = false;
|
||||
var startX = 0;
|
||||
var thumbStartLeft = 0;
|
||||
|
||||
function onStart(e) {
|
||||
if (track.classList.contains('approved')) return;
|
||||
dragging = true;
|
||||
thumb.classList.add('dragging');
|
||||
var clientX = e.touches ? e.touches[0].clientX : e.clientX;
|
||||
startX = clientX;
|
||||
thumbStartLeft = thumb.offsetLeft - 3; // subtract initial 3px offset
|
||||
e.preventDefault();
|
||||
}
|
||||
|
||||
function onMove(e) {
|
||||
if (!dragging) return;
|
||||
var clientX = e.touches ? e.touches[0].clientX : e.clientX;
|
||||
var dx = clientX - startX;
|
||||
var newLeft = Math.max(0, Math.min(thumbStartLeft + dx, getMaxLeft()));
|
||||
thumb.style.left = (newLeft + 3) + 'px';
|
||||
e.preventDefault();
|
||||
}
|
||||
|
||||
function onEnd(e) {
|
||||
if (!dragging) return;
|
||||
dragging = false;
|
||||
thumb.classList.remove('dragging');
|
||||
var currentLeft = thumb.offsetLeft - 3;
|
||||
var maxLeft = getMaxLeft();
|
||||
if (currentLeft >= maxLeft * 0.8) {
|
||||
// Approved
|
||||
track.classList.add('approved');
|
||||
label.textContent = 'Approved!';
|
||||
thumb.classList.add('hidden');
|
||||
sendCommand('/plan start');
|
||||
} else {
|
||||
// Snap back
|
||||
thumb.style.left = '3px';
|
||||
function getMaxLeft() {
|
||||
return track.offsetWidth - thumb.offsetWidth - 6;
|
||||
}
|
||||
}
|
||||
|
||||
thumb.addEventListener('touchstart', onStart, { passive: false, signal: signal });
|
||||
thumb.addEventListener('mousedown', onStart, { signal: signal });
|
||||
document.addEventListener('touchmove', onMove, { passive: false, signal: signal });
|
||||
document.addEventListener('mousemove', onMove, { signal: signal });
|
||||
document.addEventListener('touchend', onEnd, { signal: signal });
|
||||
document.addEventListener('mouseup', onEnd, { signal: signal });
|
||||
function markAllApproved() {
|
||||
tracks.forEach(function(t) {
|
||||
t.classList.add('approved');
|
||||
t.querySelector('.slide-approve-label').textContent = 'Approved!';
|
||||
t.querySelector('.slide-approve-thumb').classList.add('hidden');
|
||||
});
|
||||
}
|
||||
|
||||
function onStart(e) {
|
||||
if (track.classList.contains('approved')) return;
|
||||
dragging = true;
|
||||
thumb.classList.add('dragging');
|
||||
var clientX = e.touches ? e.touches[0].clientX : e.clientX;
|
||||
startX = clientX;
|
||||
thumbStartLeft = thumb.offsetLeft - 3;
|
||||
e.preventDefault();
|
||||
}
|
||||
|
||||
function onMove(e) {
|
||||
if (!dragging) return;
|
||||
var clientX = e.touches ? e.touches[0].clientX : e.clientX;
|
||||
var dx = clientX - startX;
|
||||
var newLeft = Math.max(0, Math.min(thumbStartLeft + dx, getMaxLeft()));
|
||||
thumb.style.left = (newLeft + 3) + 'px';
|
||||
e.preventDefault();
|
||||
}
|
||||
|
||||
function onEnd(e) {
|
||||
if (!dragging) return;
|
||||
dragging = false;
|
||||
thumb.classList.remove('dragging');
|
||||
var currentLeft = thumb.offsetLeft - 3;
|
||||
var maxLeft = getMaxLeft();
|
||||
if (currentLeft >= maxLeft * 0.8) {
|
||||
markAllApproved();
|
||||
sendCommand(cmd);
|
||||
} else {
|
||||
thumb.style.left = '3px';
|
||||
}
|
||||
}
|
||||
|
||||
thumb.addEventListener('touchstart', onStart, { passive: false, signal: signal });
|
||||
thumb.addEventListener('mousedown', onStart, { signal: signal });
|
||||
document.addEventListener('touchmove', onMove, { passive: false, signal: signal });
|
||||
document.addEventListener('mousemove', onMove, { signal: signal });
|
||||
document.addEventListener('touchend', onEnd, { signal: signal });
|
||||
document.addEventListener('mouseup', onEnd, { signal: signal });
|
||||
});
|
||||
}
|
||||
|
||||
function renderSimpleMarkdown(text) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue