From 7877bc864e987cae96f84eae7279b7d142c7befb Mon Sep 17 00:00:00 2001 From: Lavindeep Dhillon Date: Sat, 4 Jul 2026 08:05:26 -0400 Subject: [PATCH] feat: add dynamic tool guide and cap memory stores --- docs/DESIGN.md | 10 ++- pyproject.toml | 2 +- shellpilot/__init__.py | 2 +- shellpilot/cli/slash.py | 28 ++++++- shellpilot/memory/store.py | 112 +++++++++++++++++++------ shellpilot/prompts/system.py | 73 +++++++++++++++- shellpilot/runtime/context.py | 14 +++- shellpilot/runtime/conversation.py | 11 ++- shellpilot/tools/memory_tools.py | 17 +++- tests/test_context.py | 33 ++++++-- tests/test_conversation.py | 34 ++++++++ tests/test_memory_store.py | 125 ++++++++++++++++++++++++++- tests/test_memory_tools.py | 130 ++++++++++++++++++++++++++++- tests/test_system_prompt.py | 39 ++++++++- 14 files changed, 576 insertions(+), 54 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 9b09980..3be4310 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -553,7 +553,7 @@ The runtime should expose `/compact status` so the user can see current context #### ContextAssembler -The system prompt is assembled from a fixed, ordered set of blocks — base prompt, behavior instructions, memory, the conditional skills group (a `skills index` block plus per-skill bodies and triggered references, section 23), and (when a plan is live) a compact plan-state block. A pure `ContextAssembler` (no file or model I/O) captures that assembly as a structured `ContextSnapshot` of `ContextBlock`s, each carrying a block name, source, token estimate, an `injected` flag, and an optional skip reason. The snapshot is the single source of truth: the same structure produces both the live model prompt (joining injected blocks with `\n\n`) and the `/context` breakdown, so the figures shown to the user cannot drift from what the model receives. Block order is load-bearing; the base prompt is always present, while behavior, memory, skills, and plan state are injected only when their trigger or non-empty condition holds. +The system prompt is assembled from a fixed, ordered set of blocks — base prompt, dynamic tool guide, behavior instructions, memory, the conditional skills group (a `skills index` block plus per-skill bodies and triggered references, section 23), and (when a plan is live) a compact plan-state block. A pure `ContextAssembler` (no file or model I/O) captures that assembly as a structured `ContextSnapshot` of `ContextBlock`s, each carrying a block name, source, token estimate, an `injected` flag, and an optional skip reason. The snapshot is the single source of truth: the same structure produces both the live model prompt (joining injected blocks with `\n\n`) and the `/context` breakdown, so the figures shown to the user cannot drift from what the model receives. Block order is load-bearing; the base prompt is always present, while the tool guide, behavior, memory, skills, and plan state are injected only when their trigger or non-empty condition holds. Skill activation uses an explicit `TriggerContext(plan_status, web_enabled, enabled)` rendered by the runtime. `plan_status` is the live plan status when a plan sidecar exists, `web_enabled` is true only when both `web_search` and `web_fetch` are registered in the `ToolRegistry`, and `enabled` is the `[skills] enabled` tuple. This keeps boot-only settings drift from accidentally activating web guidance. @@ -1361,8 +1361,9 @@ Implemented in v0.3.0 (settled 2026-06-11) following this section's design, with - Stores: global `memory.json` in the user config dir, project `.shellpilot/memory.json` with `project_id`. Versioned schema, explicit validation (unknown versions rejected), atomic writes, secrets redacted before disk. - Facts carry an `id` (e.g. `fact_001`) — an addition to the 16.5 example schema — so `/memory forget` can address them. -- Tools: `memory_read` (read-only, auto) and `memory_propose_update` (the model's only write path; MEDIUM risk, approval required in every profile, with a diff-style preview). The runtime injects a budget-capped Memory block into the system prompt each turn. -- `/memory compact` optimization (16.4) covers preferences; facts are structured and excluded for now. A deterministic guard rejects any optimization that drops or invents ids, or drops a `source: user` entry — regardless of what the model returns. +- Tools: `memory_read` (read-only, auto) and `memory_propose_update` (the model's only write path; MEDIUM risk, approval required in every profile, with a diff-style preview). The runtime injects a budget-capped Memory block into the system prompt each turn, grouping preferences into global and project sections so project preferences can override global ones inside the workspace. +- Memory store files are capped at 1800 serialized characters per file (global and project separately). Writes that would exceed the cap are rejected with a clear instruction to forget or compact existing entries, forcing stored memory to stay selective rather than relying on prompt truncation. Legacy over-cap files can still shrink: the cap ratchets against normalized current payload size and rejects growth until the store complies. +- `/memory compact` optimization (16.4) covers preferences; facts are structured and excluded for now. A deterministic guard rejects any optimization that drops or invents ids, or drops a `source: user` entry — regardless of what the model returns. Replacements are preflighted for every store before any file is written, so a rejected compact cannot partially mutate global or project memory. - AGENTS.md remains read-only and is loaded alongside memory; `trusted-local` auto-save (16.3) remains moot until that profile exists. V1 behavior instructions were static AGENTS.md only; that path still works unchanged. @@ -1910,7 +1911,8 @@ Prompt principles: - **Proposal-time rules live in the base prompt** (`shellpilot/prompts/system.py`, `_BASE`): call `propose_plan` once for real multi-step work (3+ distinct steps), include all known setup/work, do not plan trivial one-step command/edit/inspection tasks, and never write plans as prose. The base prompt keeps a single bridge sentence ("After a plan is approved, keep working in this same turn…") and carries no `update_plan` mechanics. The base prompt is always present. - **Plan-mode discipline is the builtin `planning` skill** (`shellpilot/skills/builtin/planning/`): `SKILL.md` is only a tiny harness-managed-plan preamble, and mode-specific guidance lives in `references/proposed.md`, `references/active.md`, and `references/blocked.md`. The loader assigns those three references to `PLAN_PROPOSED`, `PLAN_ACTIVE`, and `PLAN_BLOCKED` by filename convention; templates are discovered metadata only. -- **Rationale.** The 8K-context target model pays for every system-prompt token on every turn, and system blocks are never compacted (section 20.2). `update_plan` mechanics are dead weight before an active/blocked plan, while expanded proposal guidance is dead weight outside proposal mode. Selecting planning references by exact plan status keeps plan-free turns lean while guaranteeing the relevant discipline is present exactly when it can be acted on. The split is enforced by tests: the base prompt must contain compact proposal rules and no `update_plan`; planning references must carry the mode mechanics. `PROMPT_VERSION` is bumped to 4 for the Skills v2 builtin-resource layout. +- **Dynamic tool guide (v0.11.1).** A compact `Tool guide` block is rendered from the same profile-filtered registry definitions offered to the model. Optional tools (`memory_*`, `web_*`, `skill_read`) appear only when registered for the session. `update_plan` is omitted on plan-free/proposed turns and appears only when a plan is active or blocked, preserving the proposal-vs-execution split. +- **Rationale.** The 8K-context target model pays for every system-prompt token on every turn, and system blocks are never compacted (section 20.2). `update_plan` mechanics are dead weight before an active/blocked plan, while expanded proposal guidance is dead weight outside proposal mode. Selecting planning references by exact plan status keeps plan-free turns lean while guaranteeing the relevant discipline is present exactly when it can be acted on. The split is enforced by tests: the base prompt must contain compact proposal rules and no `update_plan`; planning references must carry the mode mechanics. The dynamic guide bumps `PROMPT_VERSION` to 6. ### 19.1 Unified System Prompt Themes diff --git a/pyproject.toml b/pyproject.toml index 24d995c..c530460 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "shellpilot" -version = "0.11.0" +version = "0.11.1" description = "A local-first AI shell harness for your terminal, powered by Ollama." readme = "README.md" requires-python = ">=3.11" diff --git a/shellpilot/__init__.py b/shellpilot/__init__.py index d517410..da3ee2d 100644 --- a/shellpilot/__init__.py +++ b/shellpilot/__init__.py @@ -1,3 +1,3 @@ """ShellPilot: a local-first AI shell harness.""" -__version__ = "0.11.0" +__version__ = "0.11.1" diff --git a/shellpilot/cli/slash.py b/shellpilot/cli/slash.py index 9b2c0a3..1a57445 100644 --- a/shellpilot/cli/slash.py +++ b/shellpilot/cli/slash.py @@ -34,6 +34,7 @@ ) from shellpilot.config.overrides import load_overrides, overrides_path, save_overrides from shellpilot.llm.client import LLMClient +from shellpilot.memory.store import MemoryFormatError from shellpilot.runtime.conversation import ConversationRuntime from shellpilot.skills.model import SkillTrigger @@ -851,7 +852,13 @@ def _memory(self, args: list[str]) -> None: self._console.print("Usage: /memory add ") return if self._confirm(f'Add global preference "{text}"?'): - preference = stores.global_store.add_preference(text, scope="global", source="user") + try: + preference = stores.global_store.add_preference( + text, scope="global", source="user" + ) + except MemoryFormatError as exc: + self._console.print(f"[red]Memory update rejected:[/red] {exc}") + return self._audit_memory(f"add {preference.id}") self._console.print(f"Saved {preference.id}.") return @@ -865,7 +872,11 @@ def _memory(self, args: list[str]) -> None: self._console.print(f"[red]No memory entry {entry_id}.[/red] See /memory show.") return if self._confirm(f"Forget {entry_id}?"): - store.remove(entry_id) + try: + store.remove(entry_id) + except MemoryFormatError as exc: + self._console.print(f"[red]Memory update rejected:[/red] {exc}") + return self._audit_memory(f"forget {entry_id}") self._console.print(f"Removed {entry_id}.") return @@ -942,7 +953,8 @@ def _memory_compact(self, stores: object) -> None: kept = len(final_ids) if not self._confirm(f"Apply optimization: {len(all_preferences)} preferences -> {kept}?"): return - for name, store in by_store.items(): + replacements: dict[str, list[Preference]] = {} + for name in by_store: new_preferences = [] for entry in final_entries: if not isinstance(entry, dict): @@ -961,7 +973,15 @@ def _memory_compact(self, stores: object) -> None: updated_at=original.updated_at, ) ) - store.replace_all(new_preferences, list(store.facts)) + replacements[name] = new_preferences + try: + for name, store in by_store.items(): + store.validate_replacement(replacements[name], list(store.facts)) + except MemoryFormatError as exc: + self._console.print(f"[red]Optimization rejected:[/red] {exc}") + return + for name, store in by_store.items(): + store.replace_all(replacements[name], list(store.facts)) self._audit_memory(f"compact {len(all_preferences)} -> {kept}") self._console.print(f"Memory compacted: {len(all_preferences)} -> {kept} preferences.") diff --git a/shellpilot/memory/store.py b/shellpilot/memory/store.py index 52b3e3d..7461b23 100644 --- a/shellpilot/memory/store.py +++ b/shellpilot/memory/store.py @@ -22,6 +22,7 @@ from shellpilot.runtime.budget import truncate_to_tokens SCHEMA_VERSION = 1 +MAX_MEMORY_FILE_CHARS = 1800 VALID_SCOPES = ("global", "project") VALID_CONFIDENCE = ("observed", "stated", "inferred") @@ -141,6 +142,54 @@ def _next_id(prefix: str, existing: list[str]) -> str: def _clean(self, text: str) -> str: return redact_secrets(text) if self._redact else text + def _payload( + self, + preferences: list[Preference] | None = None, + facts: list[ProjectFact] | None = None, + ) -> dict[str, Any]: + stored_preferences = self._preferences if preferences is None else preferences + stored_facts = self._facts if facts is None else facts + payload: dict[str, Any] = { + "version": SCHEMA_VERSION, + "preferences": [asdict(p) for p in stored_preferences], + "facts": [asdict(f) for f in stored_facts], + } + if self.project_id is not None: + payload["project_id"] = self.project_id + return payload + + def _payload_text(self, payload: dict[str, Any]) -> str: + return json.dumps(payload, indent=2, sort_keys=True) + "\n" + + def _current_payload_size(self) -> int: + return len(self._payload_text(self._payload())) + + def _ensure_within_file_cap(self, payload: dict[str, Any]) -> None: + size = len(self._payload_text(payload)) + current_size = self._current_payload_size() + if size > MAX_MEMORY_FILE_CHARS and size <= current_size: + return + if size > MAX_MEMORY_FILE_CHARS: + raise MemoryFormatError( + f"{self.path}: memory file would be {size} characters; " + f"limit is {MAX_MEMORY_FILE_CHARS} characters. " + "Forget or compact existing memories before adding more." + ) + + def validate_replacement(self, preferences: list[Preference], facts: list[ProjectFact]) -> None: + """Raise if replacing this store would violate the memory file cap.""" + self._ensure_within_file_cap(self._payload(preferences=preferences, facts=facts)) + + def _save_payload( + self, + *, + preferences: list[Preference] | None = None, + facts: list[ProjectFact] | None = None, + ) -> None: + payload = self._payload(preferences=preferences, facts=facts) + self._ensure_within_file_cap(payload) + atomic_write_json(self.path, payload) + def add_preference(self, text: str, *, scope: str, source: str) -> Preference: if scope not in VALID_SCOPES: raise MemoryFormatError(f"invalid scope {scope!r}; use one of {VALID_SCOPES}") @@ -151,8 +200,9 @@ def add_preference(self, text: str, *, scope: str, source: str) -> Preference: source=source, updated_at=_now_iso(), ) - self._preferences.append(preference) - self.save() + preferences = [*self._preferences, preference] + self._save_payload(preferences=preferences) + self._preferences = preferences return preference def add_fact( @@ -176,34 +226,33 @@ def add_fact( confidence=confidence, source=source, ) - self._facts.append(fact) - self.save() + facts = [*self._facts, fact] + self._save_payload(facts=facts) + self._facts = facts return fact def remove(self, entry_id: str) -> bool: before = len(self._preferences) + len(self._facts) - self._preferences = [p for p in self._preferences if p.id != entry_id] - self._facts = [f for f in self._facts if f.id != entry_id] - if len(self._preferences) + len(self._facts) == before: + preferences = [p for p in self._preferences if p.id != entry_id] + facts = [f for f in self._facts if f.id != entry_id] + if len(preferences) + len(facts) == before: return False - self.save() + self._save_payload(preferences=preferences, facts=facts) + self._preferences = preferences + self._facts = facts return True def replace_all(self, preferences: list[Preference], facts: list[ProjectFact]) -> None: """Wholesale replacement used by /memory compact after approval.""" - self._preferences = list(preferences) - self._facts = list(facts) - self.save() + next_preferences = list(preferences) + next_facts = list(facts) + self.validate_replacement(next_preferences, next_facts) + atomic_write_json(self.path, self._payload(preferences=next_preferences, facts=next_facts)) + self._preferences = next_preferences + self._facts = next_facts def save(self) -> None: - payload: dict[str, Any] = { - "version": SCHEMA_VERSION, - "preferences": [asdict(p) for p in self._preferences], - "facts": [asdict(f) for f in self._facts], - } - if self.project_id is not None: - payload["project_id"] = self.project_id - atomic_write_json(self.path, payload) + self._save_payload() @dataclass(frozen=True) @@ -216,18 +265,29 @@ class MemoryStores: def render(self, max_tokens: int, *, meta: bool = False) -> str: """Render the memory block. ``meta`` annotates each preference with its (scope, source) for the ``/memory show`` view; the default is the - injected prompt format and must stay byte-identical.""" - preferences = list(self.global_store.preferences) + list(self.project_store.preferences) + injected prompt format, grouped by global/project scope.""" + global_preferences = list(self.global_store.preferences) + project_preferences = list(self.project_store.preferences) facts = list(self.global_store.facts) + list(self.project_store.facts) - if not preferences and not facts: + if not global_preferences and not project_preferences and not facts: return "" lines = ["## Memory"] - if preferences: - lines.append("Preferences:") + if global_preferences: + lines.append("Global preferences:") + if meta: + lines.extend( + f"- [{p.id}] ({p.scope}, {p.source}) {p.text}" for p in global_preferences + ) + else: + lines.extend(f"- [{p.id}] {p.text}" for p in global_preferences) + if project_preferences: + lines.append("Project preferences:") if meta: - lines.extend(f"- [{p.id}] ({p.scope}, {p.source}) {p.text}" for p in preferences) + lines.extend( + f"- [{p.id}] ({p.scope}, {p.source}) {p.text}" for p in project_preferences + ) else: - lines.extend(f"- [{p.id}] {p.text}" for p in preferences) + lines.extend(f"- [{p.id}] {p.text}" for p in project_preferences) if facts: lines.append("Project facts:") lines.extend(f"- [{f.id}] ({f.kind}) {f.label}: {f.value}" for f in facts) diff --git a/shellpilot/prompts/system.py b/shellpilot/prompts/system.py index de61d2c..63ad06e 100644 --- a/shellpilot/prompts/system.py +++ b/shellpilot/prompts/system.py @@ -2,12 +2,67 @@ from __future__ import annotations +from collections.abc import Iterable from pathlib import Path # Tracks behavioral prompt revisions: v1 = initial, v2 = plan-discipline hardening, # v3 = proposal/execution split, v4 = Skills v2 builtin resources/triggers, -# v5 = conditional locality line (honest under opt-in cloud egress). -PROMPT_VERSION = 5 +# v5 = conditional locality line (honest under opt-in cloud egress), +# v6 = dynamic registry-derived tool guide. +PROMPT_VERSION = 6 + +_TOOL_GUIDE_GROUPS = ( + ( + "Context", + ( + ("env_info", "env_info for OS, Python, and workspace facts"), + ("list_dir", "list_dir for directory entries"), + ("read_file", "read_file for bounded file windows"), + ("search_text", "search_text for exact workspace text"), + ), + ), + ( + "Action", + ( + ("run_command", "run_command for argv commands"), + ("patch_file", "patch_file for anchored edits after reading the file"), + ("write_file", "write_file for new files, appends, or whole-file rewrites"), + ), + ), + ( + "Planning", + ( + ("propose_plan", "propose_plan for real work with 3 or more steps"), + ("update_plan", "update_plan for active-plan progress or blockers"), + ), + ), + ( + "Memory", + ( + ("memory_read", "memory_read for stored preferences and project facts"), + ( + "memory_propose_update", + "memory_propose_update to request approved memory changes " + "(global=user facts/preferences; project=workspace facts/preferences)", + ), + ), + ), + ( + "Images", + (("view_image", "view_image to inspect a workspace image in vision-capable sessions"),), + ), + ( + "Web", + ( + ("web_search", "web_search for current or external leads"), + ("web_fetch", "web_fetch to verify a specific page"), + ), + ), + ( + "Skills", + (("skill_read", "skill_read to open on-demand docs for loaded skills"),), + ), +) # Local (non-egressing) opening: byte-identical to the pre-v0.10.0 prompt so the # gemma4 baseline session is unchanged. @@ -58,6 +113,20 @@ """ +def build_tool_guide(tool_names: Iterable[str], *, plan_active: bool = False) -> str: + available = set(tool_names) + if not plan_active: + available.discard("update_plan") + lines: list[str] = [] + for group, entries in _TOOL_GUIDE_GROUPS: + parts = [text for name, text in entries if name in available] + if parts: + lines.append(f"- {group}: {'; '.join(parts)}.") + if not lines: + return "" + return "Tool guide:\n" + "\n".join(lines) + + def build_system_prompt( *, workspace: Path, diff --git a/shellpilot/runtime/context.py b/shellpilot/runtime/context.py index 81cc0f5..1878e93 100644 --- a/shellpilot/runtime/context.py +++ b/shellpilot/runtime/context.py @@ -153,6 +153,7 @@ def assemble( self, *, base_prompt: str, + tool_guide: str = "", behavior_block: str, memory_block: str, skills: Sequence[Skill], @@ -163,9 +164,10 @@ def assemble( """Build the snapshot from already-rendered block texts plus the discovered skills. - Order is load-bearing: base prompt, behavior, memory, the skills group - (skills-index block then skill bodies), plan state. Behavior, memory, - and plan state are injected only when non-empty. + Order is load-bearing: base prompt, dynamic tool guide, behavior, + memory, the skills group (skills-index block then skill bodies), plan + state. Tool guide, behavior, memory, and plan state are injected only + when non-empty. Skill injection is deterministic. Only valid skills get blocks (planning first, then alphabetical). A skill is injected when any of its triggers @@ -301,6 +303,12 @@ def assemble( text=base_prompt, injected=True, ), + ContextBlock( + name="tool guide", + source="tools", + text=tool_guide, + injected=bool(tool_guide), + ), ContextBlock( name="behavior", source="behavior:AGENTS.md", diff --git a/shellpilot/runtime/conversation.py b/shellpilot/runtime/conversation.py index c86a26c..08345ff 100644 --- a/shellpilot/runtime/conversation.py +++ b/shellpilot/runtime/conversation.py @@ -23,7 +23,7 @@ from shellpilot.persistence.sessions import SessionStore from shellpilot.persistence.snapshots import SnapshotStore from shellpilot.policy.risk import SideEffect -from shellpilot.prompts.system import build_system_prompt +from shellpilot.prompts.system import build_system_prompt, build_tool_guide from shellpilot.runtime.budget import ContextBudget, estimate_tokens, resolve_budget from shellpilot.runtime.context import ContextAssembler, ContextSnapshot from shellpilot.runtime.events import RuntimeUI, TurnStats @@ -356,9 +356,11 @@ def _context_snapshot(self) -> ContextSnapshot: """Structured system-prompt snapshot — single source for the live prompt and the /context breakdown. Renders each block here (the assembler stays pure) in the order the legacy concatenation used.""" + profile = self._settings.runtime.security_profile + tool_definitions = self._registry.definitions_for_profile(profile) base_prompt = build_system_prompt( workspace=self._workspace, - profile=self._settings.runtime.security_profile, + profile=profile, is_egressing=self._is_egressing(), ) memory_block = "" @@ -371,6 +373,7 @@ def _context_snapshot(self) -> ContextSnapshot: if plan is not None and plan.status in ("active", "blocked") else "" ) + plan_active = plan is not None and plan.status in ("active", "blocked") trigger_ctx = TriggerContext( plan_status=plan.status if plan is not None else None, web_enabled=( @@ -381,6 +384,10 @@ def _context_snapshot(self) -> ContextSnapshot: ) return self._assembler.assemble( base_prompt=base_prompt, + tool_guide=build_tool_guide( + (definition.name for definition in tool_definitions), + plan_active=plan_active, + ), behavior_block=self._behavior.as_prompt_block(), memory_block=memory_block, skills=self._skills, diff --git a/shellpilot/tools/memory_tools.py b/shellpilot/tools/memory_tools.py index 580b62e..4ac66e3 100644 --- a/shellpilot/tools/memory_tools.py +++ b/shellpilot/tools/memory_tools.py @@ -18,6 +18,20 @@ PREVIEW_TOKENS = 2000 +MEMORY_SCOPE_POLICY = ( + "Global memory: durable user facts and cross-project preferences, including name, " + "timezone, role, preferred languages/tools, collaboration style, and defaults the " + "user wants everywhere. Project memory: current workspace facts/preferences, " + "including repo commands, paths, architecture, dependencies, conventions, " + "team/product facts, and preferences that apply only here. Facts about the user's " + "skills or identity are global unless explicitly project-scoped. Project memory " + "overrides global memory only inside this workspace. Do not store secrets, " + "credentials, one-off task details, guesses, temporary context, or file contents " + "the user did not ask to remember. If scope is ambiguous, prefer project memory " + "when global storage could pollute other projects; ask only when the distinction " + "matters." +) + def _diff_preview(title: str, lines: list[str]) -> str: body = "\n".join(lines) @@ -138,7 +152,8 @@ def _propose_preview(context: ToolContext, arguments: dict[str, Any]) -> str: "Propose a memory update for user approval. " "action=add_preference (text, optional scope: global|project), " "action=add_fact (kind, label, value), or action=forget (id). " - "The user sees and approves every update; never assume it was saved." + "The user sees and approves every update; never assume it was saved. " + f"{MEMORY_SCOPE_POLICY}" ), parameters={ "action": { diff --git a/tests/test_context.py b/tests/test_context.py index d9be88e..e29aedd 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -67,24 +67,37 @@ def _planning_reference(name: str) -> str: def _expected_system_text(runtime: ConversationRuntime) -> str: """Inline reconstruction of the expected system-prompt concatenation. - Order: base prompt, behavior, memory, skills-index + skill bodies/references, - plan state. + Order: base prompt, tool guide, behavior, memory, + skills-index + skill bodies/references, plan state. """ - from shellpilot.prompts.system import build_system_prompt + from shellpilot.prompts.system import build_system_prompt, build_tool_guide settings = runtime.settings prompt = build_system_prompt( workspace=runtime.status().workspace, profile=settings.runtime.security_profile, - behavior_block=runtime._behavior.as_prompt_block(), ) + plan = runtime.plan_manager.active + plan_mode = plan.status if plan is not None else None + guide = build_tool_guide( + ( + definition.name + for definition in runtime.registry.definitions_for_profile( + settings.runtime.security_profile + ) + ), + plan_active=plan_mode in ("active", "blocked"), + ) + if guide: + prompt = f"{prompt}\n\n{guide}" + behavior = runtime._behavior.as_prompt_block() + if behavior: + prompt = f"{prompt}\n\n{behavior}" if runtime._memory is not None: memory_cap = max(200, runtime.budget.model_context_tokens // 16) memory_block = runtime._memory.render(max_tokens=memory_cap) if memory_block: prompt = f"{prompt}\n\n{memory_block}" - plan = runtime.plan_manager.active - plan_mode = plan.status if plan is not None else None if plan_mode in ("active", "blocked"): prompt = f"{prompt}\n\nLoaded skills: planning, context-management." prompt = f"{prompt}\n\n## Skill: planning\n{_planning_body()}" @@ -267,6 +280,7 @@ def _template(name: str) -> SkillResource: def _assemble( *, base_prompt: str = "BASE", + tool_guide: str = "", behavior_block: str = "", memory_block: str = "", skills: tuple[Skill, ...] = (), @@ -282,6 +296,7 @@ def _assemble( ) return ContextAssembler().assemble( base_prompt=base_prompt, + tool_guide=tool_guide, behavior_block=behavior_block, memory_block=memory_block, skills=skills, @@ -295,6 +310,7 @@ def test_assembler_block_names_and_order_no_skills() -> None: snapshot = _assemble() assert [block.name for block in snapshot.blocks] == [ "base prompt", + "tool guide", "behavior", "memory", "skills index", @@ -309,6 +325,11 @@ def test_assembler_empty_behavior_and_memory_excluded() -> None: assert snapshot.system_text() == "BASE" +def test_assembler_includes_nonempty_tool_guide() -> None: + snapshot = _assemble(tool_guide="TOOLS") + assert snapshot.system_text() == "BASE\n\nTOOLS" + + def test_assembler_includes_nonempty_behavior_and_memory() -> None: snapshot = _assemble(behavior_block="BEHAVE", memory_block="MEM") assert snapshot.system_text() == "BASE\n\nBEHAVE\n\nMEM" diff --git a/tests/test_conversation.py b/tests/test_conversation.py index 4f489d2..db82d8e 100644 --- a/tests/test_conversation.py +++ b/tests/test_conversation.py @@ -16,6 +16,7 @@ from shellpilot.llm.client import GenerationCancelled from shellpilot.llm.messages import Message, ToolCall, ToolDefinition, assistant from shellpilot.memory.agents_md import BehaviorInstructions +from shellpilot.memory.store import MemoryStore, MemoryStores, project_id_for from shellpilot.persistence.audit_store import AuditLogger from shellpilot.persistence.sessions import SessionStore from shellpilot.policy.risk import RiskLevel, SideEffect @@ -1446,6 +1447,39 @@ def test_skill_read_registered_when_skills_enabled(tmp_path: Path) -> None: assert runtime.registry.get("skill_read") is not None +def test_tool_guide_tracks_registered_optional_tools(tmp_path: Path) -> None: + """The prompt guide names optional tools only when those tools are registered.""" + default_runtime = make_runtime(FakeLLM(script=[]), FakeUI(), tmp_path, settings=Settings()) + default_guide = _injected_block_texts(default_runtime)["tool guide"] + assert "read_file" in default_guide + assert "memory_read" not in default_guide + assert "web_search" not in default_guide + assert "skill_read" not in default_guide + + global_store = MemoryStore(tmp_path / "global-memory.json") + project_store = MemoryStore( + tmp_path / ".shellpilot" / "memory.json", project_id=project_id_for(tmp_path) + ) + configured = ConversationRuntime( + llm=FakeLLM(script=[]), + settings=Settings( + tools=ToolSettings(web=True), + skills=SkillSettings(enabled=("skill-authoring",)), + ), + workspace=tmp_path, + behavior=BehaviorInstructions(global_text=None, project_text=None), + ui=FakeUI(), + memory=MemoryStores(global_store=global_store, project_store=project_store), + skills=tuple(discover_skills(user_skills_dir=Path("/nonexistent/skills"), max_tokens=800)), + ) + configured_guide = _injected_block_texts(configured)["tool guide"] + assert "memory_read" in configured_guide + assert "memory_propose_update" in configured_guide + assert "web_search" in configured_guide + assert "web_fetch" in configured_guide + assert "skill_read" in configured_guide + + # --------------------------------------------------------------------------- # Group E: egress chokepoint (locality signal, outbound redaction, model_request) # --------------------------------------------------------------------------- diff --git a/tests/test_memory_store.py b/tests/test_memory_store.py index d9126e2..8260e2c 100644 --- a/tests/test_memory_store.py +++ b/tests/test_memory_store.py @@ -10,6 +10,7 @@ from shellpilot.config.loader import load_config from shellpilot.memory.agents_md import BehaviorInstructions from shellpilot.memory.store import ( + MAX_MEMORY_FILE_CHARS, MemoryFormatError, MemoryStore, MemoryStores, @@ -72,6 +73,113 @@ def test_secrets_never_stored(tmp_path: Path) -> None: assert "ghp_abcdefghijklmnopqrstuvwxyz0123456789" not in raw +def test_memory_file_write_is_capped_to_force_selective_storage(tmp_path: Path) -> None: + store = MemoryStore(tmp_path / "memory.json") + too_large = "x" * MAX_MEMORY_FILE_CHARS + + with pytest.raises(MemoryFormatError, match=f"{MAX_MEMORY_FILE_CHARS} characters"): + store.add_preference(too_large, scope="global", source="user") + + assert not (tmp_path / "memory.json").exists() + + +def test_project_memory_file_write_is_capped_separately(tmp_path: Path) -> None: + store = MemoryStore( + tmp_path / ".shellpilot" / "memory.json", project_id=project_id_for(tmp_path) + ) + too_large = "x" * MAX_MEMORY_FILE_CHARS + + with pytest.raises(MemoryFormatError, match=f"{MAX_MEMORY_FILE_CHARS} characters"): + store.add_fact(kind="note", label="oversized", value=too_large, source="assistant") + + assert not (tmp_path / ".shellpilot" / "memory.json").exists() + + +def test_over_cap_legacy_store_can_shrink_by_removing_entry(tmp_path: Path) -> None: + path = tmp_path / "memory.json" + legacy_entries = [ + { + "id": f"pref_{index:03d}", + "scope": "global", + "text": f"legacy preference {index} " + ("x" * 220), + "source": "user", + "updated_at": "2026-07-04T00:00:00Z", + } + for index in range(1, 21) + ] + path.write_text( + json.dumps({"version": 1, "preferences": legacy_entries, "facts": []}, indent=2) + "\n", + encoding="utf-8", + ) + before_size = len(path.read_text(encoding="utf-8")) + assert before_size > MAX_MEMORY_FILE_CHARS + + store = MemoryStore(path) + + assert store.remove("pref_001") is True + after_size = len(path.read_text(encoding="utf-8")) + assert after_size < before_size + assert after_size > MAX_MEMORY_FILE_CHARS + assert "pref_001" not in {preference.id for preference in MemoryStore(path).preferences} + + +def test_over_cap_legacy_store_still_rejects_growth(tmp_path: Path) -> None: + path = tmp_path / "memory.json" + legacy_entries = [ + { + "id": f"pref_{index:03d}", + "scope": "global", + "text": f"legacy preference {index} " + ("x" * 220), + "source": "user", + "updated_at": "2026-07-04T00:00:00Z", + } + for index in range(1, 21) + ] + path.write_text( + json.dumps({"version": 1, "preferences": legacy_entries, "facts": []}, indent=2) + "\n", + encoding="utf-8", + ) + before = path.read_text(encoding="utf-8") + store = MemoryStore(path) + + with pytest.raises(MemoryFormatError, match="Forget or compact"): + store.add_preference("new entry", scope="global", source="user") + + assert path.read_text(encoding="utf-8") == before + + +def test_minified_over_cap_legacy_store_can_shrink_by_removing_entry(tmp_path: Path) -> None: + path = tmp_path / "memory.json" + legacy_entries = [ + { + "id": f"pref_{index:03d}", + "scope": "global", + "text": f"legacy preference {index} " + ("x" * 220), + "source": "user", + "updated_at": "2026-07-04T00:00:00Z", + } + for index in range(1, 21) + ] + path.write_text( + json.dumps( + {"version": 1, "preferences": legacy_entries, "facts": []}, separators=(",", ":") + ), + encoding="utf-8", + ) + raw_size = len(path.read_text(encoding="utf-8")) + store = MemoryStore(path) + normalized_size = len(store._payload_text(store._payload())) + assert raw_size > MAX_MEMORY_FILE_CHARS + assert normalized_size > raw_size + + assert store.remove("pref_001") is True + + after_size = len(path.read_text(encoding="utf-8")) + assert after_size < normalized_size + assert after_size > raw_size + assert "pref_001" not in {preference.id for preference in MemoryStore(path).preferences} + + def test_render_block_lists_entries_and_caps_tokens(tmp_path: Path) -> None: global_store = MemoryStore(tmp_path / "global.json") project_store = MemoryStore(tmp_path / "project.json", project_id="ShellPilot:abc") @@ -89,10 +197,25 @@ def test_render_block_lists_entries_and_caps_tokens(tmp_path: Path) -> None: assert len(tiny) <= 10 * 4 + 40 # truncated to roughly the cap +def test_render_block_keeps_global_and_project_preferences_distinct(tmp_path: Path) -> None: + global_store = MemoryStore(tmp_path / "global.json") + project_store = MemoryStore(tmp_path / "project.json", project_id="ShellPilot:abc") + global_store.add_preference("Use concise answers.", scope="global", source="user") + project_store.add_preference("Use detailed release notes.", scope="project", source="user") + stores = MemoryStores(global_store=global_store, project_store=project_store) + + block = stores.render(max_tokens=500) + + assert "Global preferences:" in block + assert "- [pref_001] Use concise answers." in block + assert "Project preferences:" in block + assert "- [pref_001] Use detailed release notes." in block + + def test_render_meta_adds_scope_source_for_display_only(tmp_path: Path) -> None: """meta=True annotates preference lines with (scope, source) for the /memory show view (folding in what /prefs used to print); the default - (injected) format stays byte-identical and never leaks the tag.""" + injected format shows scope by section, not by source tag.""" global_store = MemoryStore(tmp_path / "global.json") project_store = MemoryStore(tmp_path / "project.json", project_id="ShellPilot:abc") global_store.add_preference("Prefer concise answers.", scope="global", source="user") diff --git a/tests/test_memory_tools.py b/tests/test_memory_tools.py index 2a4771d..94afa80 100644 --- a/tests/test_memory_tools.py +++ b/tests/test_memory_tools.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path from rich.console import Console @@ -10,7 +11,7 @@ from shellpilot.cli.theme import SHELLPILOT_THEME from shellpilot.config.loader import LoadedConfig, load_config from shellpilot.memory.agents_md import BehaviorInstructions -from shellpilot.memory.store import MemoryStore, MemoryStores, project_id_for +from shellpilot.memory.store import MAX_MEMORY_FILE_CHARS, MemoryStore, MemoryStores, project_id_for from shellpilot.runtime.conversation import ConversationRuntime from tests.fakes.fake_llm import FakeLLM, answer, tool_call from tests.fakes.fake_ui import FakeUI @@ -55,6 +56,20 @@ def test_memory_read_runs_without_approval(tmp_path: Path) -> None: assert any(success for _, success, _ in ui.tool_results) +def test_memory_propose_update_schema_explains_scope_policy(tmp_path: Path) -> None: + stores = make_stores(tmp_path) + runtime, _, _ = make_runtime(tmp_path, stores, []) + tool = runtime.registry.get("memory_propose_update") + assert tool is not None + description = tool.definition.description + + assert "Global memory: durable user facts" in description + assert "preferred languages/tools" in description + assert "Project memory: current workspace" in description + assert "repo commands, paths, architecture" in description + assert "If scope is ambiguous" in description + + def test_propose_add_preference_requires_approval_and_persists(tmp_path: Path) -> None: stores = make_stores(tmp_path) runtime, ui, _ = make_runtime( @@ -169,6 +184,47 @@ def test_slash_memory_show_add_forget(tmp_path: Path) -> None: assert harness.stores.global_store.preferences == () +def test_slash_memory_add_reports_file_cap(tmp_path: Path) -> None: + harness = MemoryHarness(tmp_path, []) + harness.dispatcher.handle(f"/memory add {'x' * MAX_MEMORY_FILE_CHARS}") + + output = harness.output() + assert "limit is" in output + assert str(MAX_MEMORY_FILE_CHARS) in output + assert harness.stores.global_store.preferences == () + + +def test_slash_memory_forget_shrinks_over_cap_legacy_store(tmp_path: Path) -> None: + harness = MemoryHarness(tmp_path, []) + legacy_entries = [ + { + "id": f"pref_{index:03d}", + "scope": "global", + "text": f"legacy preference {index} " + ("x" * 220), + "source": "user", + "updated_at": "2026-07-04T00:00:00Z", + } + for index in range(1, 21) + ] + harness.stores.global_store.path.write_text( + json.dumps({"version": 1, "preferences": legacy_entries, "facts": []}, indent=2) + "\n", + encoding="utf-8", + ) + before_size = len(harness.stores.global_store.path.read_text(encoding="utf-8")) + assert before_size > MAX_MEMORY_FILE_CHARS + harness.stores.global_store.reload() + + harness.dispatcher.handle("/memory forget pref_001") + + after = harness.stores.global_store.path.read_text(encoding="utf-8") + assert len(after) < before_size + assert len(after) > MAX_MEMORY_FILE_CHARS + assert "Removed pref_001." in harness.output() + assert "pref_001" not in { + preference.id for preference in harness.stores.global_store.preferences + } + + def test_memory_show_folds_in_scope_source_and_prefs_retired(tmp_path: Path) -> None: """/prefs was retired into /memory show (v0.10.0): the preference view now carries the (scope, source) tag /prefs printed, and /prefs is gone.""" @@ -207,3 +263,75 @@ def test_memory_compact_refuses_to_drop_user_entries(tmp_path: Path) -> None: harness.dispatcher.handle("/memory compact") assert len(harness.stores.global_store.preferences) == 2 # unchanged assert "user entries" in harness.output() + + +def test_memory_compact_reports_file_cap(tmp_path: Path) -> None: + oversized = "x" * MAX_MEMORY_FILE_CHARS + script = [answer(json.dumps([{"id": "pref_001", "text": oversized}]))] + harness = MemoryHarness(tmp_path, script) + harness.stores.global_store.add_preference( + "First assistant entry.", scope="global", source="assistant" + ) + harness.stores.global_store.add_preference( + "Second assistant entry.", scope="global", source="assistant" + ) + + harness.dispatcher.handle("/memory compact") + + output = harness.output() + assert "limit is" in output + assert str(MAX_MEMORY_FILE_CHARS) in output + assert [p.text for p in harness.stores.global_store.preferences] == [ + "First assistant entry.", + "Second assistant entry.", + ] + + +def test_memory_compact_rejects_atomically_when_later_store_exceeds_cap(tmp_path: Path) -> None: + oversized = "x" * MAX_MEMORY_FILE_CHARS + script = [ + answer( + json.dumps( + [ + {"id": "pref_001", "text": "Global changed."}, + {"id": "pref_900", "text": oversized}, + ] + ) + ) + ] + harness = MemoryHarness(tmp_path, script) + harness.stores.global_store.add_preference( + "Global original.", scope="global", source="assistant" + ) + harness.stores.project_store.path.parent.mkdir(parents=True, exist_ok=True) + harness.stores.project_store.path.write_text( + json.dumps( + { + "version": 1, + "project_id": project_id_for(tmp_path), + "preferences": [ + { + "id": "pref_900", + "scope": "project", + "text": "Project original.", + "source": "assistant", + "updated_at": "2026-07-04T00:00:00Z", + } + ], + "facts": [], + }, + indent=2, + sort_keys=True, + ) + + "\n", + encoding="utf-8", + ) + harness.stores.project_store.reload() + + harness.dispatcher.handle("/memory compact") + + output = harness.output() + assert "limit is" in output + assert str(MAX_MEMORY_FILE_CHARS) in output + assert harness.stores.global_store.preferences[0].text == "Global original." + assert harness.stores.project_store.preferences[0].text == "Project original." diff --git a/tests/test_system_prompt.py b/tests/test_system_prompt.py index 66d9af3..694fcd2 100644 --- a/tests/test_system_prompt.py +++ b/tests/test_system_prompt.py @@ -2,7 +2,7 @@ from pathlib import Path -from shellpilot.prompts.system import _BASE, PROMPT_VERSION, build_system_prompt +from shellpilot.prompts.system import _BASE, PROMPT_VERSION, build_system_prompt, build_tool_guide from shellpilot.skills.loader import discover_skills from shellpilot.skills.model import Skill, SkillTrigger @@ -37,6 +37,41 @@ def test_prompt_includes_core_themes() -> None: assert phrase in prompt, f"missing theme: {phrase}" +def test_tool_guide_lists_only_available_tools() -> None: + guide = build_tool_guide( + ( + "env_info", + "read_file", + "search_text", + "memory_read", + "memory_propose_update", + "web_search", + "web_fetch", + "update_plan", + ) + ) + + assert guide.startswith("Tool guide:") + assert "env_info" in guide + assert "read_file" in guide + assert "search_text" in guide + assert "memory_read" in guide + assert "global=user facts/preferences" in guide + assert "project=workspace facts/preferences" in guide + assert "web_search" in guide + assert "web_fetch" in guide + assert "update_plan" not in guide + assert "patch_file" not in guide + assert "skill_read" not in guide + + +def test_tool_guide_names_update_plan_only_when_plan_active() -> None: + guide = build_tool_guide(("propose_plan", "update_plan"), plan_active=True) + + assert "propose_plan" in guide + assert "update_plan" in guide + + def test_prompt_appends_behavior_block() -> None: prompt = build_system_prompt( workspace=Path("/work"), @@ -99,7 +134,7 @@ def test_base_prompt_retains_proposal_rules() -> None: def test_prompt_version_bumped() -> None: - assert PROMPT_VERSION == 5 + assert PROMPT_VERSION == 6 def test_prompts_planning_module_has_no_live_content() -> None: