mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 05:01:28 -10:00
* feat(agent): add PermissionModePlan for interactive read-only planning * fix(agent): drop redundant modeName branch, add plan mode regression tests string(permissionMode) already yields "plan" / "spec-draft", so the if/else recomputing modeName in the denial message was dead branching on the same values. Also add plan-mode coverage mirroring three of the four existing spec-draft regression tests: advertised tool set, and denied write_file/bash calls. The fourth (submit-and-stop review control) has no plan-mode analog, since plan mode has no submit tool. * fix(agent): deny request_permissions in plan/spec-draft even when the registry omits it request_permissions is dispatched by name in executeToolCall before the registry-based ToolAdvertised gate runs, so that gate only helps when the tool happens to be present in the caller's registry. A plan- or spec-draft-mode registry that simply omits the tool (rather than registering it as denied) let the call fall through to a real turn/session-scoped permission grant, defeating the read-only boundary. Deny it unconditionally at the top of executeRequestPermissions for both read-only modes instead. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(agent): suppress executable hooks while plan/spec-draft mode is active Plan mode promises a read-only turn, but sessionStart/sessionEnd fire on every run and beforeTool/afterTool fire around allowed read calls, and all four execute configured host commands outside the advertised-tool and sandbox gates — so a project hook could mutate the workspace or spawn a process from a session that advertises it cannot. Gate all four dispatch points on the run's permission mode, with a regression test asserting no hook command launches during a plan-mode run. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(agent): close plan-mode tool advertisement bypass toolAdvertisedInPlan whitelisted ask_user and update_plan by name alone, so a caller could register a mutating tool under either name and have it advertised and executed in plan mode. Validate every tool against its Safety() instead. Also exclude lsp_navigate, which is marked SideEffectRead but lazily spawns a real language-server process, contradicting plan mode's read-only guarantee. Add regression tests for both. * fix(agent): keep trust-gated hooks in spec-draft mode Plan mode still suppresses executable hooks so a read-only planning turn cannot spawn host processes via session or tool hooks. Spec-draft keeps the existing trust model: project hooks fire when the workspace (or its worktree trust root) is trusted. Unconditionally suppressing hooks in spec-draft broke TestExecSpecWorktreeInheritsTrustEndToEnd for trusted worktrees under --use-spec --worktree. * fix(agent): cover plan-mode spoof and lsp_navigate denials Expand the name-only spoof regression to both update_plan and ask_user, and add an execution-path denial for lsp_navigate so plan mode cannot spawn language servers even when a call still arrives. * fix(agent): filter tool_search deferred candidates by plan/spec-draft visibility tool_search resolved and ranked deferred tools by EnabledTools/DisabledTools only, never by the run's permission-mode visibility. tool_search itself is already denied at dispatch in plan/spec-draft (its Safety carries no side effect, so it fails the same SideEffect==Read advertisement gate direct calls use), so this was not reachable through the normal Run() path today. But it is a real landmine: if that outer gate is ever loosened independently (e.g. tool_search's no-side-effect Safety is judged advertisable), the loader had no gate of its own and would hand a deferred write/mutator tool's name, description, and full schema straight to a plan/spec-draft model. Mirror agent.ToolAdvertised's plan/spec-draft branches inside the tools package (toolAdvertisedForPermissionMode, next to the existing toolAllowedByFilters mirror that avoids the same import cycle) and apply it alongside the operator filters in visibleDeferredTools and visibleEagerToolNames. Added unit tests in tool_search_test.go for both modes, and an end-to-end agent test that force-calls tool_search in plan mode and asserts no schema leaks. Verified by reverting tool_search.go and confirming the new tests fail (one shows load_tools resolving to the mutator's name); restored and confirmed they pass. Also confirmed via a temporary probe that if plan mode's outer advertisement gate is loosened, this filter is what actually stops the leak. * fix(agent): require Safety for spec-draft ask_user/submit_spec Do not advertise or load re-registered control tools by name alone in spec-draft mode. ask_user must be SideEffectRead+Allow and submit_spec must be SideEffectWrite+Allow, matching the real tools. Apply the same filter in tool_search and add spoof regression tests. Refs Gitlawb/zero#642 * fix(agent): wire plan mode into TUI, CLI, and ACP entry points Address the P1 finding that PermissionModePlan was documented but never selected by /plan, zero exec, or ACP mode selectors. /plan on|off now toggles the session permission mode (restoring the prior mode on off), zero exec --plan selects plan for a run, and ACP advertises plan as a client-selectable mode. Integration coverage for each entry path. * fix(cli): reject --plan combined with --worktree Worktree preparation runs in runExec before the plan permission mode is assigned, so `zero exec --plan --worktree` could still trigger workspace mutation ahead of the read-only gate. Reject the combination during option validation, alongside the existing --use-spec/--skip-permissions- unsafe conflict checks, so no worktree prep can occur. Addresses a coderabbitai finding on PR #642. * test(tools): assert spoofed tool schema doesn't leak via tool_search The spoofed-control-tool regression only asserted on the description string; Parameters() exposed no distinctive schema marker, so a regression that leaked the schema without the description would still have passed. Add a spoofed_secret property to the test tool's schema and assert it's absent from result.Output alongside the description. Addresses a coderabbitai finding on PR #642. * fix(tui): gate local mutating commands behind plan mode /plan on only flips the agent permission mode, which gates agent tool calls. Local TUI commands that run entirely inside the TUI process bypass that gate: /rewind restores workspace files from a checkpoint, /export writes a transcript to disk, and /sandbox-setup spawns a native host process. Add a shared plan-mode guard at the start of dispatchCommand (mirroring the existing BTW-unavailable guard) that rejects these three commands while permissionMode is agent.PermissionModePlan, with regression coverage proving each is blocked with no mutation/process spawn in plan mode and unaffected outside it. Addresses a coderabbitai finding on PR #642. * fix(agent,specialist): layer plan mode system prompt and enforce read-only subagent mode * Fix jatmn review findings for PR 642 * fix(agent,cli,tui): resolve CodeRabbit review comments on active turn model field and --plan permission mode conflict * fix(agent): propagate permission mode to exec options and serialize ACP mode changes Parse permissionMode in resolveExecPermissionMode, acquire turnMu.Lock in ACP handleSetMode to serialize mode changes with active turns, and block MCP subcommands in TUI plan mode. Refs #642 * fix(agent,tui): update tests off removed tools.CoreTools/NewWriteFileTool/NewLSPNavigateTool wrappers Those were thin unscoped wrappers around the Scoped variants, deleted upstream in #706 since nothing else called them directly. Only these tests still did; switch to the Scoped calls main's own tests already use. * fix(agent): fail-closed beforeTool vetoes and permission-mode plan guards Keep beforeTool deny gates active under plan mode so hooksSuppressed no longer fails open, and stop propagating --permission-mode for auto/ask/member children so swarm members keep write tools. Apply --plan combination rejects to --permission-mode plan as well. Refs #853 * fix(agent): address CodeRabbit findings for plan-mode advertisement and entry paths Unify plan/spec-draft tool advertisement in tools.ToolAdvertisedForPermissionMode (with PermissionDeny short-circuit), cover ACP config and --permission-mode plan list-tools paths, and allow bare /mcp while blocking mutating MCP subcommands. Refs #853 --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: euxaristia <[email protected]>
102 lines
3.4 KiB
Go
102 lines
3.4 KiB
Go
package tui
|
|
|
|
import (
|
|
"fmt"
|
|
"strings"
|
|
|
|
"github.com/Gitlawb/zero/internal/agent"
|
|
"github.com/Gitlawb/zero/internal/tools"
|
|
)
|
|
|
|
type currentPlanReader interface {
|
|
CurrentPlan() []tools.PlanItem
|
|
}
|
|
|
|
// handlePlanCommand drives /plan: bare or "status" just reports the current
|
|
// plan (pre-existing behavior); "on" and "off" are the entry/exit path into
|
|
// PermissionModePlan. Unlike /spec (which drafts in a separate, forked
|
|
// session), plan mode applies to the CURRENT session, so entering/exiting it
|
|
// is a direct m.permissionMode flip rather than a run-option override.
|
|
func (m model) handlePlanCommand(args string) (model, string) {
|
|
switch strings.ToLower(strings.TrimSpace(args)) {
|
|
case "", "status":
|
|
return m, m.planText()
|
|
case "on":
|
|
if m.pending {
|
|
return m, "Cannot change plan mode while a turn is active."
|
|
}
|
|
if m.permissionMode == agent.PermissionModePlan {
|
|
return m, "Plan mode\nAlready active. Write and shell tools stay hidden until /plan off."
|
|
}
|
|
m.permissionModeBeforePlan = m.permissionMode
|
|
m.permissionMode = agent.PermissionModePlan
|
|
return m, "Plan mode\nActive: read-only planning. Write and shell tools are hidden until /plan off."
|
|
case "off":
|
|
if m.pending {
|
|
return m, "Cannot change plan mode while a turn is active."
|
|
}
|
|
if m.permissionMode != agent.PermissionModePlan {
|
|
return m, "Plan mode\nNot currently active."
|
|
}
|
|
restored := m.permissionModeBeforePlan
|
|
if restored == "" {
|
|
restored = agent.PermissionModeAuto
|
|
}
|
|
m.permissionMode = restored
|
|
m.permissionModeBeforePlan = ""
|
|
return m, "Plan mode\nExited. Permission mode restored to " + string(restored) + "."
|
|
default:
|
|
return m, "Plan mode\nUsage: /plan [status|on|off]"
|
|
}
|
|
}
|
|
|
|
// planModeCommandUnavailable reports whether a local (non-tool) TUI command
|
|
// must be blocked while plan mode is active. Plan mode's tool-advertisement
|
|
// gate only covers agent tool calls; these commands run entirely inside the
|
|
// TUI process and would mutate the workspace or spawn a host process outside
|
|
// that gate: /rewind restores files from a checkpoint, /export writes a
|
|
// transcript file to disk, /sandbox-setup runs native platform setup,
|
|
// /spec forks a drafting session, /mcp <sub> mutates server configuration, and
|
|
// /init's whole job is writing AGENTS.md (which plan mode then denies).
|
|
// Bare /mcp (empty text) only opens the read-only manager view, so it stays
|
|
// available. Modeled on btwCommandUnavailable's shape for the analogous BTW guard.
|
|
func planModeCommandUnavailable(command parsedCommand) bool {
|
|
switch command.kind {
|
|
case commandRewind, commandExport, commandSandboxSetup, commandSpec, commandInit:
|
|
return true
|
|
case commandMCP:
|
|
// Bare /mcp only opens the read-only manager view; subcommands mutate config.
|
|
return strings.TrimSpace(command.text) != ""
|
|
default:
|
|
return false
|
|
}
|
|
}
|
|
|
|
func (m model) planText() string {
|
|
tool, ok := m.registry.Get("update_plan")
|
|
if !ok {
|
|
return "No plan is active."
|
|
}
|
|
|
|
reader, ok := tool.(currentPlanReader)
|
|
if !ok {
|
|
return "No plan is active."
|
|
}
|
|
|
|
plan := reader.CurrentPlan()
|
|
if len(plan) == 0 {
|
|
return "No plan is active."
|
|
}
|
|
|
|
lines := make([]string, 0, len(plan)+1)
|
|
lines = append(lines, "Current Plan")
|
|
for index, item := range plan {
|
|
line := fmt.Sprintf("%d. [%s] %s", index+1, item.Status, item.Content)
|
|
if item.Notes != "" {
|
|
line += "\n Notes: " + item.Notes
|
|
}
|
|
lines = append(lines, line)
|
|
}
|
|
return strings.Join(lines, "\n")
|
|
}
|