diff --git a/.gitignore b/.gitignore index 036b266..832c07a 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,5 @@ htmlcov/ # Local harness / session scratch (never commit) .superpowers/ -thinking-trail-preview.html +# Feature mockups / design prototypes — local-only ideas, never tracked +prototype/ diff --git a/docs/DESIGN.md b/docs/DESIGN.md index a9293aa..00dd3b3 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -1261,6 +1261,8 @@ The boundary must be clear in the UI. Relative paths are resolved against the wo **Display-integrity invariant (v0.10.0).** Every user-facing path display — the tool-call line, the approval-panel head, and the write/patch diff-panel title — is derived from the *same* `resolve_in_workspace` result the tool acts on, never from the raw model argument. A spoofing path (`..` segments, `./x/../y`, symlink, trailing junk) is shown as its resolved, workspace-relative target, so the file the user approves is always the file actually touched; a path that escapes the boundary renders an honest `` marker rather than a fabricated-looking in-workspace path. The single helper `workspace_display(workspace, raw_path)` (`tools/base.py`) is the one display formatter, layered over the canonical resolver — there is no second, divergent resolution. This is a display-only change: the boundary checks and the resolution the action uses are unchanged. +**Live workspace (v0.10.1).** `AppUI` and `TerminalUI` resolve the tool-call path against the *live* workspace via a `workspace_fn: Callable[[], Path]` injected at construction (`lambda: runtime.status().workspace`), so a mid-session `/cwd set` is immediately honoured in the tool-call summary line. The approval-panel path display (`executor._display_value`) was already live; this closes the one remaining surface that used a stale build-time capture. + ### 14.6 Approval Outcomes And Reject-And-Steer An approval prompt has three outcomes, not two — `[y]es / [e]dit / [n]o` (`ApprovalReply`, `policy/approvals.py`): diff --git a/shellpilot/cli/app_ui.py b/shellpilot/cli/app_ui.py index a4eb8af..bcc44fe 100644 --- a/shellpilot/cli/app_ui.py +++ b/shellpilot/cli/app_ui.py @@ -112,6 +112,7 @@ def __init__( *, glyphs: Glyphs = UNICODE_GLYPHS, workspace: Path | None = None, + workspace_fn: Callable[[], Path] | None = None, width_fn: Callable[[], int], show_reasoning: bool = True, time_fn: Callable[[], float] = time.monotonic, @@ -120,7 +121,11 @@ def __init__( # Workspace for display-integrity (design section 14.5): when set, a # `path` argument in the tool-call line is shown as its resolved, # workspace-relative target — the SAME resolution the tool acts on. + # 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._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+ @@ -428,10 +433,15 @@ def show_tool_call(self, name: str, arguments: dict[str, object]) -> None: def _tool_call_value(self, key: str, value: object) -> str: # A `path` argument is shown as its resolved, workspace-relative target - # (display-integrity, design section 14.5). Without a workspace the value - # renders verbatim — matching TerminalUI._tool_call_value exactly. - if key == "path" and isinstance(value, str) and self._workspace is not None: - return repr(workspace_display(self._workspace, value)) + # (display-integrity, design section 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. NOTE: the verbatim + # fallback only happens with neither set — a test-double construction; in + # production workspace_fn is always wired, so the path display never drifts + # from the action. + workspace = self._workspace_fn() if self._workspace_fn is not None else self._workspace + if key == "path" and isinstance(value, str) and workspace is not None: + return repr(workspace_display(workspace, value)) return repr(value) def show_tool_result(self, name: str, success: bool, summary: str) -> None: diff --git a/shellpilot/cli/terminal.py b/shellpilot/cli/terminal.py index cf93385..37e1162 100644 --- a/shellpilot/cli/terminal.py +++ b/shellpilot/cli/terminal.py @@ -198,13 +198,18 @@ def __init__( glyphs: Glyphs = UNICODE_GLYPHS, spinner: bool = True, workspace: Path | None = None, + workspace_fn: Callable[[], Path] | None = None, ) -> None: self._console = console self._glyphs = glyphs # Workspace for display-integrity (design section 14.5): when set, a # `path` argument in the tool-call line is shown as its resolved, # workspace-relative target — the SAME resolution the tool acts on. + # 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._stream = ResponseStream(console) self._spinner = AviationSpinner(console, glyphs, enabled=spinner) # The diff-reveal animation rides the same motion toggle as the spinner. @@ -262,10 +267,15 @@ def show_tool_call(self, name: str, arguments: dict[str, object]) -> None: def _tool_call_value(self, key: str, value: object) -> str: # A `path` argument is shown as its resolved, workspace-relative target - # (display-integrity, design section 14.5). Without a workspace (legacy - # callers) the value renders verbatim. - if key == "path" and isinstance(value, str) and self._workspace is not None: - return repr(workspace_display(self._workspace, value)) + # (display-integrity, design section 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. NOTE: the verbatim + # fallback only happens with neither set — a test-double construction; in + # production workspace_fn is always wired, so the path display never drifts + # from the action. + workspace = self._workspace_fn() if self._workspace_fn is not None else self._workspace + if key == "path" and isinstance(value, str) and workspace is not None: + return repr(workspace_display(workspace, value)) return repr(value) def show_tool_result(self, name: str, success: bool, summary: str) -> None: @@ -632,6 +642,7 @@ def _preload(model_name: str) -> None: app_ui = AppUI( glyphs=glyphs, workspace=workspace, + workspace_fn=lambda: runtime.status().workspace, width_fn=lambda: get_app().output.get_size().columns, show_reasoning=settings.ui.show_reasoning_summary, ) @@ -641,7 +652,13 @@ def _preload(model_name: str) -> None: approval_gate = ApprovalGate(ui=app_ui, schedule=app_runner.schedule, glyphs=glyphs) ui = ThreadedUI(inner=app_ui, schedule=app_runner.schedule, approval_gate=approval_gate) else: - ui = TerminalUI(console, glyphs=glyphs, spinner=settings.ui.spinner, workspace=workspace) + ui = TerminalUI( + console, + glyphs=glyphs, + spinner=settings.ui.spinner, + workspace=workspace, + workspace_fn=lambda: runtime.status().workspace, + ) runtime = ConversationRuntime( llm=client, settings=settings, diff --git a/tests/test_app_ui.py b/tests/test_app_ui.py index cb76158..f57bb48 100644 --- a/tests/test_app_ui.py +++ b/tests/test_app_ui.py @@ -835,3 +835,36 @@ def test_toggle_finished_trail_while_idle_rerenders() -> None: assert "idle14" not in plain(ui) # collapsed: 15th line hidden assert ui.toggle_thinking_trail() is True assert "idle14" in plain(ui) # expanded render reflects the toggle (cache busted) + + +# --------------------------------------------------------------------------- +# Live workspace (workspace_fn): mid-session /cwd staleness fix +# --------------------------------------------------------------------------- + + +def test_tool_call_path_uses_live_workspace(tmp_path: Path) -> None: + """workspace_fn is re-evaluated at each show_tool_call, so a mid-session + /cwd change is honoured immediately in the tool-call summary line.""" + from shellpilot.tools.base import OUTSIDE_WORKSPACE_DISPLAY + + ws_a = tmp_path + ws_b = tmp_path / "sub" + holder: dict[str, Path] = {"ws": ws_a} + + # workspace_fn returns the live workspace from the holder. + ui = AppUI(glyphs=GLYPHS, workspace_fn=lambda: holder["ws"], width_fn=lambda: 80) + + # The absolute path is inside ws_a but outside ws_b. + abs_path = str(ws_a / "file.txt") + ui.show_tool_call("read_file", {"path": abs_path}) + out_before = plain(ui) + # Under ws_a: resolves to the relative "file.txt" — inside workspace. + assert "file.txt" in out_before + assert OUTSIDE_WORKSPACE_DISPLAY not in out_before + + # Simulate /cwd set to ws_b — the same abs_path is now outside the workspace. + holder["ws"] = ws_b + ui.show_tool_call("read_file", {"path": abs_path}) + out_after = plain(ui) + # The SECOND tool-call line must reflect the live workspace, not the stale one. + assert OUTSIDE_WORKSPACE_DISPLAY in out_after diff --git a/tests/test_terminal_ui.py b/tests/test_terminal_ui.py index 6860fb5..a6f9b02 100644 --- a/tests/test_terminal_ui.py +++ b/tests/test_terminal_ui.py @@ -956,3 +956,36 @@ def test_stream_thinking_is_noop() -> None: ui.stream_thinking("I am reasoning about this…") captured = console.end_capture() assert captured == "" + + +# --------------------------------------------------------------------------- +# Live workspace (workspace_fn): mid-session /cwd staleness fix +# --------------------------------------------------------------------------- + + +def test_tool_call_path_uses_live_workspace(tmp_path: Path) -> None: + """workspace_fn is re-evaluated at each show_tool_call, so a mid-session + /cwd change is honoured immediately in the tool-call summary line.""" + from shellpilot.tools.base import OUTSIDE_WORKSPACE_DISPLAY + + ws_a = tmp_path + ws_b = tmp_path / "sub" + holder: dict[str, Path] = {"ws": ws_a} + + console = make_console() + ui = TerminalUI(console, glyphs=GLYPHS, spinner=False, workspace_fn=lambda: holder["ws"]) + + # The absolute path is inside ws_a but outside ws_b. + abs_path = str(ws_a / "file.txt") + ui.show_tool_call("read_file", {"path": abs_path}) + out_before = console.export_text() + # Under ws_a: resolves to the relative "file.txt" — inside workspace. + assert "file.txt" in out_before + assert OUTSIDE_WORKSPACE_DISPLAY not in out_before + + # Simulate /cwd set to ws_b — the same abs_path is now outside the workspace. + holder["ws"] = ws_b + ui.show_tool_call("read_file", {"path": abs_path}) + out_after = console.export_text() + # The SECOND tool-call line must reflect the live workspace, not the stale one. + assert OUTSIDE_WORKSPACE_DISPLAY in out_after