Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions docs/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ The roadblock protocol (section 11.6) and the model edge cases (section 24.6) ar

Handler-level checks for the same conditions (e.g. the `if mode not in WRITE_MODES` guard in `_write_file`, the `if status not in STEP_STATUSES` guard in `_update`) remain in place as defense in depth; in normal flow they are unreachable for those parameters.

**`max_plan_steps` enforcement (v0.5.2).** `RuntimeSettings.max_plan_steps` (default 10) is now enforced at proposal time. A `propose_plan` call whose `steps` list exceeds the limit returns a corrective failure (`plan has N steps; max is M — consolidate related steps and propose again`) via the normal `ToolResult(success=False, ...)` path — not the malformed-call path — because the step count is a policy decision, not a schema error. The setting is threaded from `Settings.runtime.max_plan_steps` through `ConversationRuntime` to `make_plan_tools(max_plan_steps=...)` at construction time.
**`max_plan_steps` enforcement (v0.5.2).** `RuntimeSettings.max_plan_steps` (default 10) is now enforced at proposal time. A `propose_plan` call whose `steps` list exceeds the limit returns a corrective failure (`plan has N steps; max is M — consolidate related steps and propose again`) via the normal `ToolResult(success=False, ...)` path — not the malformed-call path — because the step count is a policy decision, not a schema error. The limit lives on `PlanManager.max_plan_steps` (seeded from `Settings.runtime.max_plan_steps` when the manager is constructed) and is read *live* at proposal time: `make_plan_tools` is called with no `max_plan_steps` argument, so its `_limit()` reads `manager.max_plan_steps` on every `propose_plan`. A `/config set runtime.max_plan_steps` therefore takes effect the same session — `update_settings` writes the new value onto `plan_manager.max_plan_steps` without rebuilding the plan tools.

### 10.5 Context And Output Budgeting

Expand Down Expand Up @@ -1409,6 +1409,7 @@ The project `<workspace>/AGENTS.md` is injected as standing "Project instruction
- The **project** `<workspace>/AGENTS.md` is loaded only after the user accepts it. Acceptance is recorded in `.shellpilot/state.json` as the `trusted_agents_md` SHA-256 digest of the file's raw bytes.
- On boot, the current digest is compared to the recorded one. If they match, the file loads without prompting. If it is new or its content has changed since it was last trusted (any byte change flips the digest), the user is re-prompted (default No) and the file is loaded only on acceptance.
- A **non-TTY** session fails closed: the project file is not loaded (no way to obtain consent).
- The same gate runs **mid-session on `/cwd set <path>`**: switching the workspace re-resolves trust for the *new* directory's `AGENTS.md` inside `SlashDispatcher._reload_behavior_for_workspace` before its instructions are injected. An already-trusted destination loads without prompting; a new or changed destination re-prompts (default No); a non-TTY session fails closed. Either way the previous workspace's project instructions are dropped, so a switch into an untrusted repo can never keep injecting the old rules or silently pick up the new ones. The always-trusted global `AGENTS.md` is reloaded unchanged.

Storing the digest never clobbers the recorded last-selected model; both keys coexist in `state.json` via read-merge-write.

Expand Down Expand Up @@ -1683,9 +1684,11 @@ Three slash commands manage the overrides file at runtime:
the error is printed in red and the file is never touched. This invariant
means a corrupt entry can never reach `overrides.json` via the CLI.
The new value takes effect immediately for live keys (via `update_settings`).
A subset of keys are *boot-only* (theme, model client, tool registration,
A subset of keys are *boot-only* (theme, model client, web-tool registration,
keep_alive preload, etc.); for those a dim note is appended: "takes effect
next session". For `model.default` specifically the note adds "use
next session". (Not all tool registration is boot-only: `update_settings`
re-syncs the `skill_read` tool live when `skills.enabled` becomes empty or
non-empty, registering or unregistering it without a restart.) For `model.default` specifically the note adds "use
`/model use <name>` to switch now".
Setting a **high-stakes key** (`HIGH_STAKES_KEYS` — `tools.web`,
`model.base_url`, `runtime.security_profile`, `model.allow_cloud`) first
Expand Down Expand Up @@ -3154,6 +3157,8 @@ Because a high-stakes override persists in `overrides.json` and silently outrank

A project `AGENTS.md` is injected as standing instructions with the same authority as ShellPilot's own prompt, so cloning and running in an untrusted repo could load attacker-authored instructions every turn (and, under cloud, egress them turn one). The project (workspace) `AGENTS.md` is now gated behind per-workspace **trust-on-first-use**: its SHA-256 content digest (`project_agents_md_digest`, `memory/agents_md.py`) is recorded in program-managed workspace state (`.shellpilot/state.json`) once the user accepts it. A non-TTY session fails closed (the file is skipped), and because the gate keys on the content digest, **any change to the file re-prompts** before the new content is trusted. The global config-dir `AGENTS.md` stays trusted (it is the user's own).

The gate is not boot-only: a mid-session `/cwd set <path>` re-runs it for the new workspace (`SlashDispatcher._reload_behavior_for_workspace` → `_resolve_project_agents_trust`) before any of the destination's instructions are injected. The switch always drops the previous workspace's project instructions, then loads the new project `AGENTS.md` only if its digest is already trusted or the user accepts it this session (non-TTY fails closed). This closes the stale-instructions gap where a session that switched into an untrusted repo would otherwise keep injecting the prior workspace's rules — or silently adopt the new repo's — without a trust decision.

### 36.5 Egress Chokepoint and Audit

A single locality predicate, `is_egressing(model, base_url)` (`config/model.py`), is the one source of truth for whether a session is off-box — true for a cloud model (the Ollama cloud tag — `:cloud` or a sized `<size>-cloud` — via `is_cloud_model`) or a non-loopback `base_url` (`is_loopback_url`, `llm/ollama.py`). At the conversational chokepoint (`conversation.py` tool loop; the `/memory compact` residual is noted in section 15.1):
Expand Down
23 changes: 22 additions & 1 deletion shellpilot/cli/slash.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,32 @@ def _cwd(self, args: list[str]) -> None:
return
if self._confirm(f"Change the workspace boundary to {new_workspace}?"):
self._runtime.set_workspace(new_workspace)
self._reload_behavior_for_workspace(new_workspace)
self._console.print(f"Workspace boundary: {new_workspace}")
return
self._console.print("Usage: /cwd | /cwd set <path>")

def _reload_behavior_for_workspace(self, workspace: Path) -> None:
"""Trust-gate and reload AGENTS.md for the new workspace after ``/cwd``."""
from shellpilot.cli.terminal import _resolve_project_agents_trust
from shellpilot.memory.agents_md import BehaviorInstructions, load_behavior_instructions

settings = self._runtime.settings
if not settings.instructions.load_agents_md:
self._runtime.set_behavior(BehaviorInstructions(global_text=None, project_text=None))
return
config_dir = self._user_config_file.parent
detected = self._runtime.budget.model_context_tokens
cap = min(1500, max(1, detected // 10))
project_trusted = _resolve_project_agents_trust(self._console, workspace, tty=self._tty)
behavior = load_behavior_instructions(
config_dir,
workspace,
max_tokens=cap,
project_trusted=project_trusted,
)
self._runtime.set_behavior(behavior)

def _doctor(self) -> None:
from shellpilot.cli.doctor import run_doctor

Expand Down Expand Up @@ -784,7 +806,6 @@ def _profile(self, args: list[str]) -> None:
self._runtime.update_settings(dataclasses.replace(settings, runtime=new_runtime))
if self._runtime.audit is not None:
self._runtime.audit.write("config_change", setting="profile", value=name)
self._runtime.audit.profile = name
self._console.print(f"Switched to profile: {name}")
return
self._console.print("Usage: /profile | /profile use <supervised|balanced>")
Expand Down
2 changes: 1 addition & 1 deletion shellpilot/cli/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ def _status_values() -> StatusValues:
return StatusValues(
workspace=st.workspace,
model=runtime.model,
profile=settings.runtime.security_profile,
profile=st.profile,
is_cloud=is_egressing(runtime.model, settings.model.base_url),
ctx_pct=ctx_percent(st.estimated_prompt_tokens, st.budget.model_context_tokens),
)
Expand Down
44 changes: 41 additions & 3 deletions shellpilot/runtime/conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,18 @@ def __init__(
self._cancel: threading.Event | None = None
self.snapshots = SnapshotStore()
self.recent_diffs: list[str] = []
self.plan_manager = PlanManager(workspace, settings.runtime.security_profile)
self.plan_manager = PlanManager(
workspace,
settings.runtime.security_profile,
max_plan_steps=settings.runtime.max_plan_steps,
)
self._last_recorded_plan_ptr: str | None | _Unset = _UNSET
self.plan_manager.on_change = self._on_plan_change
for spec in make_plan_tools(
self.plan_manager,
ui.ask_plan_approval,
lambda: self._last_user_text,
on_step_change=ui.show_plan_progress,
max_plan_steps=settings.runtime.max_plan_steps,
):
self._registry.register(spec)
self._registry.register(
Expand All @@ -184,7 +187,12 @@ def __init__(
if memory is not None:
from shellpilot.tools.memory_tools import make_memory_tools

for spec in make_memory_tools(memory):
def _live_memory() -> MemoryStores:
if self._memory is None:
raise RuntimeError("memory tools registered without live memory stores")
return self._memory

for spec in make_memory_tools(_live_memory):
self._registry.register(spec)
if settings.tools.web:
from shellpilot.tools.web import default_web_tools
Expand Down Expand Up @@ -280,6 +288,8 @@ def set_workspace(self, workspace: Path) -> None:
# so a /cwd change must rebuild the project store for the new path —
# otherwise the previous workspace's facts keep injecting (and, under
# cloud, egressing). The shared global store is preserved as-is.
# Memory tools resolve stores through a getter, so they follow this
# replacement automatically.
self._memory = dataclasses.replace(
self._memory,
project_store=MemoryStore(
Expand All @@ -296,9 +306,37 @@ def set_workspace(self, workspace: Path) -> None:
self._audit.workspace = workspace
self._audit.write("config_change", setting="workspace", value=str(workspace))

def set_behavior(self, behavior: BehaviorInstructions) -> None:
"""Replace standing AGENTS.md instructions (used after ``/cwd`` trust)."""
self._behavior = behavior

def update_settings(self, settings: Settings) -> None:
"""Apply live settings and refresh dependents that must stay in sync.

Boot-only keys (for example ``tools.web``) may still leave registered
tools unchanged by design; this method keeps the live surface coherent
for profile, plan limits, audit metadata, and skill_read availability.
"""
previous = self._settings
self._settings = settings
self.budget = self._resolve_budget()
self.plan_manager.set_profile(settings.runtime.security_profile)
self.plan_manager.max_plan_steps = settings.runtime.max_plan_steps
if self._audit is not None:
self._audit.profile = settings.runtime.security_profile
self._sync_skill_read_tool(previous.skills.enabled, settings.skills.enabled)

def _sync_skill_read_tool(self, was_enabled: tuple[str, ...], enabled: tuple[str, ...]) -> None:
"""Keep skill_read registered iff the enabled-skills list is non-empty."""
if bool(was_enabled) == bool(enabled):
return
from shellpilot.tools.skill_tools import make_skill_read_tool

if enabled:
valid_skills = tuple(s for s in self._skills if s.valid)
self._registry.replace(make_skill_read_tool(valid_skills))
else:
self._registry.unregister("skill_read")

def _endpoint_host(self) -> str:
"""Host of the model endpoint (for audit); empty when unparseable."""
Expand Down
33 changes: 27 additions & 6 deletions shellpilot/runtime/planner.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ def compact_plan_state(plan: TaskPlan) -> str:
class PlanManager:
"""Owns the active plan and its artifact on disk."""

def __init__(self, workspace: Path, profile: str) -> None:
def __init__(self, workspace: Path, profile: str, *, max_plan_steps: int = 10) -> None:
self._workspace = workspace
self._profile = profile
self._max_plan_steps = max_plan_steps
self.active: TaskPlan | None = None
self.pending_revision: str | None = None
# Transient completion-guard state (runtime-only; never persisted to
Expand All @@ -181,6 +182,18 @@ def set_workspace(self, workspace: Path) -> None:
"""New tasks use the new boundary; an active plan keeps its artifact path."""
self._workspace = workspace

def set_profile(self, profile: str) -> None:
"""New tasks stamp *profile*; an active plan keeps its recorded profile."""
self._profile = profile

@property
def max_plan_steps(self) -> int:
return self._max_plan_steps

@max_plan_steps.setter
def max_plan_steps(self, value: int) -> None:
self._max_plan_steps = value

def artifact_path(self, plan: TaskPlan) -> Path:
# Pinned to the plan's own workspace (set at create, persisted, restored),
# not the mutable self._workspace — so a mid-plan /cwd keeps PLAN.md where
Expand Down Expand Up @@ -393,9 +406,16 @@ def make_plan_tools(
ask_plan_approval: PlanApprovalAsker,
get_user_intent: UserIntentGetter,
on_step_change: PlanProgressShower | None = None,
max_plan_steps: int = 10,
max_plan_steps: int | None = None,
) -> list[ToolSpec]:
"""Plan tools close over the manager and the UI approval flow."""
"""Plan tools close over the manager and the UI approval flow.

When *max_plan_steps* is omitted, the live ``manager.max_plan_steps`` value
is used so ``/config`` changes take effect without rebuilding the tools.
"""

def _limit() -> int:
return manager.max_plan_steps if max_plan_steps is None else max_plan_steps

def _propose(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
goal = str(arguments["goal"]).strip()
Expand All @@ -405,13 +425,14 @@ def _propose(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
success=False, summary="plan needs a goal and at least one step", content=""
)

if len(steps) > max_plan_steps:
limit = _limit()
if len(steps) > limit:
n = len(steps)
return ToolResult(
success=False,
summary=f"plan has {n} steps; max is {max_plan_steps}",
summary=f"plan has {n} steps; max is {limit}",
content=(
f"plan has {n} steps; max is {max_plan_steps} — consolidate related "
f"plan has {n} steps; max is {limit} — consolidate related "
"steps and propose again"
),
)
Expand Down
13 changes: 12 additions & 1 deletion shellpilot/tools/memory_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from __future__ import annotations

from collections.abc import Callable
from typing import Any

from shellpilot.llm.messages import ToolDefinition
Expand Down Expand Up @@ -38,8 +39,16 @@ def _diff_preview(title: str, lines: list[str]) -> str:
return f"--- a/{title}\n+++ b/{title}\n@@ -1 +1 @@\n{body}\n"


def make_memory_tools(stores: MemoryStores) -> list[ToolSpec]:
def make_memory_tools(get_stores: Callable[[], MemoryStores]) -> list[ToolSpec]:
"""Build memory tools that resolve stores through *get_stores* each call.

Handlers must not close over a concrete ``MemoryStores`` instance: ``/cwd``
replaces the runtime's project store, and a captured reference would keep
reading and writing the previous workspace.
"""

def _read(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
stores = get_stores()
block = stores.render(max_tokens=PREVIEW_TOKENS)
if not block:
return ToolResult(
Expand All @@ -50,6 +59,7 @@ def _read(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
return ToolResult(success=True, summary=f"{entries} memory entries", content=block)

def _propose(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
stores = get_stores()
action = str(arguments.get("action", ""))
try:
if action == "add_preference":
Expand Down Expand Up @@ -106,6 +116,7 @@ def _propose(context: ToolContext, arguments: dict[str, Any]) -> ToolResult:
)

def _propose_preview(context: ToolContext, arguments: dict[str, Any]) -> str:
stores = get_stores()
action = str(arguments.get("action", ""))
if action == "add_preference":
scope = str(arguments.get("scope", "global"))
Expand Down
7 changes: 7 additions & 0 deletions shellpilot/tools/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ def register(self, spec: ToolSpec) -> None:
raise ValueError(f"tool already registered: {spec.name}")
self._specs[spec.name] = spec

def replace(self, spec: ToolSpec) -> None:
"""Register or overwrite a tool by name (for live settings transitions)."""
self._specs[spec.name] = spec

def unregister(self, name: str) -> None:
self._specs.pop(name, None)

def get(self, name: str) -> ToolSpec | None:
return self._specs.get(name)

Expand Down
Loading