Simplification pass: remove redundant tests and single-use indirection

docs/DESIGN.md

@@ -1280,7 +1280,7 @@ An approval prompt has three outcomes, not two — `[y]es / [e]dit / [n]o` (`App
 
 This is *reject-and-steer*, not inline-edit-and-run: the user never edits a command that then executes under the badge it was approved under. **Safety is automatic** because the un-approved action is dropped and the correction is a fresh tool call that is independently classified and re-gated — steering can never smuggle a higher-risk action past the prompt. The outcome is uniform across every approval-gated tool (commands and file writes alike); no per-tool editing logic exists because nothing is edited in place.
 
-**A plain decline ends the turn; a steer continues it.** The declined outcome carries `stop_turn = not bool(steer)` (`runtime/executor.py`). On a plain `[n]o` the tool loop truncates any remaining tool calls in that reply, records the declined result, and returns without re-invoking the model — so a declined action cannot be silently retried later in the same turn. When a plan is active, the decline surfaces a status line `Action declined; plan paused on step N.` (the active step index, via `_active_plan_step`) and leaves the plan paused for the user's next instruction. A steered `[e]dit` decline does **not** stop the turn: `stop_turn` is `False` whenever steer text is present, so the correction is fed back and the loop continues so the model can re-propose.
+**A plain decline ends the turn; a steer continues it.** The declined outcome carries `stop_turn = not bool(steer)` (`runtime/executor.py`). On a plain `[n]o` the tool loop truncates any remaining tool calls in that reply, records the declined result, and returns without re-invoking the model — so a declined action cannot be silently retried later in the same turn. When a plan is active, the decline surfaces a status line `Action declined; plan paused on step N.` (the active step index, via `_plan_step(active_only=True)`) and leaves the plan paused for the user's next instruction. A steered `[e]dit` decline does **not** stop the turn: `stop_turn` is `False` whenever steer text is present, so the correction is fed back and the loop continues so the model can re-propose.
 
 **The HIGH-risk typed-`run` confirm is unchanged.** A HIGH-risk command still requires typing the literal `run` to execute; `[e]` is an additional option at that same prompt that steers without running, and Enter still cancels. Empty guidance after `[e]` is treated as a plain decline (nothing runs). A steered approval is audited as `decision="steered"` on the `approval` event (alongside `approved`/`rejected`).
 
@@ -2463,7 +2463,7 @@ The rebuild should stay light. The goal is a reliable local harness, not a frame
 | Memory system | Behavior/project memory, proposals, and optimization move to v2. V1 only reads `AGENTS.md`. Scheduled for v0.3.0 (settled 2026-06-11). |
 | Token-budget compaction | V1 uses oldest-first truncation; selective compaction is v2. Scheduled for v0.3.0 (settled 2026-06-11). |
 | `trusted-local` profile | Deferred from v1, and deferred again at the 2026-06-11 v2 scoping. Revisit for v3. |
-| Session resume | Shipped in v0.3.0 (settled 2026-06-11): append-only JSONL transcripts at `.shellpilot/sessions/<session-id>.jsonl`, written incrementally with secrets redacted; compaction trims memory, never the transcript. `shellpilot --resume [id]` restores the latest (or named) session's history; snapshots are never restored, so read-before-write forces fresh reads. `/export` renders the transcript to markdown. Tool-call arguments are redacted recursively (matching the audit log's `_redact_value` logic, now unified in `redact_structure` in `shellpilot/memory/redaction.py`) before they reach the JSONL transcript; `/export` inherits redaction by re-reading the transcript from disk. Fixed in v0.5.2. `session_markdown` re-applies redaction at export time so transcripts written before v0.5.2 (which may contain raw secrets on disk) cannot leak through `/export`; on-disk history is deliberately left untouched. Fixed in v0.5.2 review wave. Plan state now also restores on `--resume` (v0.6.0): an `active_plan` pointer in the transcript is read at boot; if the referenced plan sidecar is live (`proposed`/`active`/`blocked`), `PlanManager.restore` reinstates it (section 11.3). **Read-side traversal guard (v0.10.1):** `SessionStore.find` now rejects any session id whose resolved parent differs from the sessions directory, closing the `--resume ../../../../etc/x` path-traversal vector; the write path was already safe via `path.stem`. **Reconciliation records:** the transcript stays append-only, so mid-turn corrections are records rather than rewrites — on load, `replace_last_message` replaces the last *assistant* record (a mid-batch decline truncates the reply's remaining tool calls, section 14.6), `truncate_last_turn` deletes from the last assistant record to the end (a mid-tool cancel, section 31.15), and `discard_last_message` pops the single trailing record (a user message that was written to the transcript and then refused by the hard context-limit gate, so `--resume` does not restore a stuck user turn with no reply). A `replace_last_message`/`truncate_last_turn` record with no assistant message present, a `discard_last_message` with an empty transcript, or an unknown record kind, is ignored. |
+| Session resume | Shipped in v0.3.0 (settled 2026-06-11): append-only JSONL transcripts at `.shellpilot/sessions/<session-id>.jsonl`, written incrementally with secrets redacted; compaction trims memory, never the transcript. `shellpilot --resume [id]` restores the latest (or named) session's history; snapshots are never restored, so read-before-write forces fresh reads. `/export` renders the transcript to markdown. Tool-call arguments are redacted recursively (matching the audit log, where each record value passes through `redact_structure` in `shellpilot/memory/redaction.py`) before they reach the JSONL transcript; `/export` inherits redaction by re-reading the transcript from disk. Fixed in v0.5.2. `session_markdown` re-applies redaction at export time so transcripts written before v0.5.2 (which may contain raw secrets on disk) cannot leak through `/export`; on-disk history is deliberately left untouched. Fixed in v0.5.2 review wave. Plan state now also restores on `--resume` (v0.6.0): an `active_plan` pointer in the transcript is read at boot; if the referenced plan sidecar is live (`proposed`/`active`/`blocked`), `PlanManager.restore` reinstates it (section 11.3). **Read-side traversal guard (v0.10.1):** `SessionStore.find` now rejects any session id whose resolved parent differs from the sessions directory, closing the `--resume ../../../../etc/x` path-traversal vector; the write path was already safe via `path.stem`. **Reconciliation records:** the transcript stays append-only, so mid-turn corrections are records rather than rewrites — on load, `replace_last_message` replaces the last *assistant* record (a mid-batch decline truncates the reply's remaining tool calls, section 14.6), `truncate_last_turn` deletes from the last assistant record to the end (a mid-tool cancel, section 31.15), and `discard_last_message` pops the single trailing record (a user message that was written to the transcript and then refused by the hard context-limit gate, so `--resume` does not restore a stuck user turn with no reply). A `replace_last_message`/`truncate_last_turn` record with no assistant message present, a `discard_last_message` with an empty transcript, or an unknown record kind, is ignored. |
 | Agent raw shell | Do not expose `raw_shell` as an agent tool in v1. Keep Manual Shell for direct user-controlled `shell=True`. |
 | Capability packs (Skills v2) | v0.6.0 shipped instruction-only SKILL.md discovery; v0.7.0 extends it with deterministic trigger selection, four markdown-only builtins, read-only references/templates, script manifest discovery without execution, and enriched `/skills` + `/context` visibility (section 23). |
 | Capability packs (heavier: tools/handlers/permissions) | Design later after core tools are stable. v3 candidate (2026-06-11). |

shellpilot/cli/app_ui.py

@@ -42,7 +42,7 @@
 from shellpilot.cli.theme import SHELLPILOT_THEME, UNICODE_GLYPHS, Glyphs
 from shellpilot.memory.redaction import redact_structure
 from shellpilot.runtime.budget import CHARS_PER_TOKEN
-from shellpilot.tools.base import workspace_display
+from shellpilot.tools.base import make_workspace_path_display
 
 if TYPE_CHECKING:
     from shellpilot.policy.approvals import ApprovalRequest
@@ -167,8 +167,7 @@ def __init__(
         # workspace_fn (preferred in production) is called at render time so a
         # mid-session /cwd change is immediately reflected; workspace is the
         # static fallback for test doubles that construct without a live runtime.
-        self._workspace = workspace
-        self._workspace_fn = workspace_fn
+        self._path_display = make_workspace_path_display(workspace, workspace_fn)
         self._width_fn = width_fn
         # Gate for the reasoning-token readout (settings.ui.show_reasoning_summary,
         # design section 31.14): when False, the live/done lines show plane+phrase+
@@ -601,15 +600,6 @@ def show_tool_call(self, name: str, arguments: dict[str, object]) -> None:
         ):
             self._add_renderable(renderable)
 
-    def _path_display(self, path: str) -> str:
-        # Resolve a `path` argument to its workspace-relative target (§14.5).
-        # Prefer the live workspace (workspace_fn, set in production) so a
-        # mid-session /cwd is honoured; fall back to the build-time workspace,
-        # then verbatim (a test-double with neither set — production always wires
-        # workspace_fn, so the path display never drifts from the action).
-        workspace = self._workspace_fn() if self._workspace_fn is not None else self._workspace
-        return workspace_display(workspace, path) if workspace is not None else path
-
     def show_tool_result(self, name: str, success: bool, summary: str) -> None:
         self._add_renderable(render_tool_result(success, summary, self._glyphs))
 

shellpilot/cli/streaming.py

@@ -332,10 +332,6 @@ def _frame(self, tick: int) -> Text:
             (f"{self._glyphs.ellipsis} {int(elapsed)}s", "sp.dim"),
         )
 
-    def _current_label_text(self) -> str:
-        """Return the plain text of the most recent frame (for tests)."""
-        return self._frame(0).plain
-
     def _spin(self) -> None:
         tick = 0
         while not self._stop_event.wait(_REFRESH_SECONDS):

shellpilot/cli/terminal.py

@@ -91,7 +91,7 @@
 from shellpilot.runtime.events import RuntimeUI, TurnStats
 from shellpilot.runtime.planner import TaskPlan
 from shellpilot.skills.loader import discover_skills
-from shellpilot.tools.base import workspace_display
+from shellpilot.tools.base import make_workspace_path_display
 
 
 def should_discard_interrupt(
@@ -217,8 +217,7 @@ def __init__(
         # workspace_fn (preferred in production) is called at render time so a
         # mid-session /cwd change is immediately reflected; workspace is the
         # static fallback for test doubles that construct without a live runtime.
-        self._workspace = workspace
-        self._workspace_fn = workspace_fn
+        self._path_display = make_workspace_path_display(workspace, workspace_fn)
         self._stream = ResponseStream(console)
         self._spinner = AviationSpinner(console, glyphs, enabled=spinner)
         # The diff-reveal animation rides the same motion toggle as the spinner.
@@ -272,15 +271,6 @@ def show_tool_call(self, name: str, arguments: dict[str, object]) -> None:
         label = Text.assemble(("running ", "sp.dim"), (_sanitize_line(name), "sp.emph"))
         self._spinner.start(label=label)
 
-    def _path_display(self, path: str) -> str:
-        # Resolve a `path` argument to its workspace-relative target (§14.5).
-        # Prefer the live workspace (workspace_fn, set in production) so a
-        # mid-session /cwd is honoured; fall back to the build-time workspace,
-        # then verbatim (a test-double with neither set — production always wires
-        # workspace_fn, so the path display never drifts from the action).
-        workspace = self._workspace_fn() if self._workspace_fn is not None else self._workspace
-        return workspace_display(workspace, path) if workspace is not None else path
-
     def show_tool_result(self, name: str, success: bool, summary: str) -> None:
         self._spinner.stop()
         self._console.print(render_tool_result(success, summary, self._glyphs))

shellpilot/persistence/audit_store.py

@@ -14,12 +14,6 @@
 AUDIT_VERSION = 1
 
 
-def _redact_value(value: Any, enabled: bool) -> Any:
-    if not enabled:
-        return value
-    return redact_structure(value)
-
-
 @dataclass
 class AuditLogger:
     """Append-only JSONL audit events; secrets redacted before write."""
@@ -39,7 +33,12 @@ def write(self, event: str, **fields: Any) -> None:
             "profile": self.profile,
             "event": event,
         }
-        record.update({key: _redact_value(value, self.redact) for key, value in fields.items()})
+        record.update(
+            {
+                key: redact_structure(value) if self.redact else value
+                for key, value in fields.items()
+            }
+        )
         self.path.parent.mkdir(parents=True, exist_ok=True, mode=0o700)
         fd = os.open(self.path, os.O_WRONLY | os.O_CREAT | os.O_APPEND, 0o600)
         with os.fdopen(fd, "a", encoding="utf-8") as handle:

shellpilot/policy/command_policy.py

@@ -426,21 +426,17 @@ def _option_present(argv: list[str], names: frozenset[str]) -> str | None:
     return None
 
 
-def _short_option_letters(names: frozenset[str]) -> frozenset[str]:
-    return frozenset(
-        name[1:]
-        for name in names
-        if name.startswith("-") and not name.startswith("--") and len(name) == 2
-    )
-
-
 def _split_option_value(argv: list[str], names: frozenset[str]) -> str | None:
     """Value of a space-separated, ``=``-attached, glued, or clustered option.
 
     Clustered short options are supported when the value-taking letter is last
     in the cluster (``-ao out.txt``) or followed by a glued value (``-aofoo``).
     """
-    short_letters = _short_option_letters(names)
+    short_letters = frozenset(
+        name[1:]
+        for name in names
+        if name.startswith("-") and not name.startswith("--") and len(name) == 2
+    )
     index = 1
     while index < len(argv):
         token = argv[index]
@@ -545,14 +541,6 @@ def _git_output_path(tokens: list[str]) -> str | None:
     return None
 
 
-def _git_external_helper(flags: list[str]) -> str | None:
-    for flag in flags:
-        name = flag.partition("=")[0]
-        if name in GIT_EXTERNAL_HELPER_OPTIONS:
-            return f"{name} can execute configured external helpers"
-    return None
-
-
 def _classify_git(argv: list[str], workspace: Path) -> CommandRisk:
     verb, verb_args, conservative_global = _scan_git_verb(argv)
     flags = [token for token in argv[1:] if token.startswith("-")]
@@ -590,7 +578,12 @@ def _classify_git(argv: list[str], workspace: Path) -> CommandRisk:
             return CommandRisk(RiskLevel.MEDIUM, (f"git {verb} changes repository state",))
     if conservative_global:
         return CommandRisk(RiskLevel.MEDIUM, ("git uses a non-benign global option",))
-    helper = _git_external_helper(flags)
+    helper = None
+    for flag in flags:
+        name = flag.partition("=")[0]
+        if name in GIT_EXTERNAL_HELPER_OPTIONS:
+            helper = f"{name} can execute configured external helpers"
+            break
     if helper:
         return CommandRisk(RiskLevel.MEDIUM, (helper,))
     if verb in GIT_READONLY_VERBS or (
@@ -663,10 +656,6 @@ def _classify_tree(argv: list[str], workspace: Path) -> CommandRisk | None:
     return None
 
 
-def _format_field_list(value: str) -> list[str]:
-    return [part.strip().lower() for part in value.replace(" ", ",").split(",") if part.strip()]
-
-
 def _ps_format_exposes_environment(argv: list[str]) -> bool:
     """True when ``-o``/``-O``/``--format`` selects env/environ columns."""
     format_names = frozenset({"-o", "-O", "--format", "--Format"})
@@ -699,7 +688,9 @@ def _ps_format_exposes_environment(argv: list[str]) -> bool:
             index += 1
         if value is None:
             continue
-        fields = _format_field_list(value)
+        fields = [
+            part.strip().lower() for part in value.replace(" ", ",").split(",") if part.strip()
+        ]
         if any(field in {"env", "environ", "environment"} for field in fields):
             return True
     return False

shellpilot/runtime/conversation.py

@@ -669,14 +669,15 @@ def _turn_stats(self, elapsed_s: float) -> TurnStats:
             output_tokens=self._turn_output_tokens,
         )
 
-    def _pending_plan_step(self) -> tuple[int, str] | None:
-        """First unfinished step of the active plan, as (1-based index, title).
-
-        Returns the first step whose status is "active", else the first
-        "pending" step. Returns None when there is no plan, the plan is not yet
-        active (e.g. still "proposed" awaiting approval, or blocked/completed),
-        or every step is already in a terminal state. Used by the tool loop to
-        decide whether a no-tool-call reply should be nudged to keep executing.
+    def _plan_step(self, *, active_only: bool = False) -> tuple[int, str] | None:
+        """Relevant step of the active plan, as (1-based index, title).
+
+        Returns the first step whose status is "active". Unless *active_only* is
+        set, falls back to the first "pending" step. Returns None when there is
+        no plan, the plan is not yet active (e.g. still "proposed" awaiting
+        approval, or blocked/completed), or no matching step remains. The tool
+        loop uses the fallback form to nudge stalled execution and the
+        active-only form to report where a declined action paused the plan.
         """
         plan = self.plan_manager.active
         if plan is None or plan.status != "active":
@@ -687,6 +688,8 @@ def _pending_plan_step(self) -> tuple[int, str] | None:
         )
         if active is not None:
             return active, plan.steps[active - 1].title
+        if active_only:
+            return None
         pending = next(
             (i for i, step in enumerate(plan.steps, start=1) if step.status == "pending"),
             None,
@@ -695,19 +698,6 @@ def _pending_plan_step(self) -> tuple[int, str] | None:
             return pending, plan.steps[pending - 1].title
         return None
 
-    def _active_plan_step(self) -> tuple[int, str] | None:
-        """Currently active plan step, without falling back to pending steps."""
-        plan = self.plan_manager.active
-        if plan is None or plan.status != "active":
-            return None
-        active = next(
-            (i for i, step in enumerate(plan.steps, start=1) if step.status == "active"),
-            None,
-        )
-        if active is None:
-            return None
-        return active, plan.steps[active - 1].title
-
     def _tool_loop(self) -> Message:
         """Model call loop with tool dispatch, budgets, and recovery (section 10.4)."""
         executor = ToolExecutor(
@@ -798,7 +788,7 @@ def _tool_loop(self) -> Message:
             history_before_reply = len(self._history)
             self._record(reply)
             if not reply.tool_calls:
-                pending = self._pending_plan_step()
+                pending = self._plan_step()
                 if pending is not None and tools and nudges_used < MAX_PLAN_NUDGES:
                     nudges_used += 1
                     index, title = pending
@@ -921,7 +911,7 @@ def _tool_loop(self) -> Message:
                             self._session.replace_last_message(reply)
                 self._record(tool_result(outcome.model_text))
                 if outcome.stop_turn:
-                    active_step = self._active_plan_step()
+                    active_step = self._plan_step(active_only=True)
                     if active_step is not None:
                         index, _title = active_step
                         self._ui.show_status(f"Action declined; plan paused on step {index}.")