Skip to content

Commit 041c70a

Browse files
committed
fix: address CodeRabbit review findings on PR #62
- Sanitize ANSI sequences in paced preview text (_blocks.py) to prevent control-sequence leaks when the markdown path is bypassed - Replace brittle Pygments style snapshot with dynamic pkgutil.iter_modules check; snapshot now covers only project-owned hiddenimports entries - Add blank lines around code fence and subsection headings in tasks/yolo-auto-mode-analysis.md to satisfy markdownlint (MD031/MD040/MD022) - Reconcile contradictory B3/section-4b status in the analysis doc
1 parent a7c5fe3 commit 041c70a

10 files changed

Lines changed: 69 additions & 71 deletions

File tree

src/pythinker_code/app.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,17 @@ def _resumed_unsupervised_notice(*, resumed: bool, yolo: bool, auto: bool) -> st
6767
"""Welcome-banner warning when a resumed session is running unsupervised.
6868
6969
Resuming restores ``yolo``/``auto`` from persisted state, so a session can come back
70-
auto-approving everything with no prompt. Surface that prominently at startup (it also
71-
fires when the modes were passed explicitly on the resume command — acceptable
72-
over-notification). ``None`` when not a resume or no unsupervised mode is active.
70+
unattended and, under YOLO, auto-approving actions with no prompt. Surface that
71+
prominently at startup (it also fires when the modes were passed explicitly on the
72+
resume command — acceptable over-notification). ``None`` when not a resume or no
73+
unsupervised mode is active.
7374
"""
7475
if not resumed or not (yolo or auto):
7576
return None
7677
modes = " + ".join(name for name, active in (("YOLO", yolo), ("auto", auto)) if active)
77-
return f"{modes} active — actions auto-approved; toggle with /yolo /auto"
78+
if yolo:
79+
return f"{modes} active — actions auto-approved; toggle with /yolo /auto"
80+
return "auto active — interactive approvals still required; toggle with /auto"
7881

7982

8083
def _patch_session_id(record: dict[str, Any]) -> None:

src/pythinker_code/soul/agent.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ async def create(
281281

282282
# Merge invocation flags with persisted session state. ``--no-yolo`` is an explicit
283283
# force-off that beats the flag, config ``default_yolo``, and persisted state.
284-
effective_yolo = (yolo or session.state.approval.yolo) and not no_yolo
284+
original_persisted_yolo = session.state.approval.yolo
285+
effective_yolo = (yolo or original_persisted_yolo) and not no_yolo
285286
# Do NOT force safe_mode off under yolo: yolo already bypasses safe mode in the
286287
# decision path (is_auto_approve / _unattended_denial_feedback short-circuit on
287288
# yolo before reading safe_mode), so there is no deadlock to avoid — and forcing it
@@ -294,7 +295,10 @@ async def create(
294295
saved_actions = set(session.state.approval.auto_approve_actions)
295296

296297
def _on_approval_change() -> None:
297-
session.state.approval.yolo = approval_state.yolo
298+
if not no_yolo:
299+
session.state.approval.yolo = approval_state.yolo
300+
else:
301+
session.state.approval.yolo = original_persisted_yolo
298302
session.state.approval.auto = approval_state.auto
299303
session.state.approval.auto_approve_actions = set(approval_state.auto_approve_actions)
300304
session.state.trust.safe_mode = approval_state.safe_mode

src/pythinker_code/ui/shell/visualize/_blocks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ def _compose_composing(self) -> RenderableType:
453453
# partial syntax (``**bol`` → ``**bold``, half-open ``` fences)
454454
# flicker char-by-char. Render the uncommitted tail as plain text;
455455
# completed blocks still commit to full markdown via render_agent_body.
456-
body: RenderableType = Text(preview)
456+
body: RenderableType = Text(sanitize_ansi(preview))
457457
else:
458458
body = Markdown(preview)
459459
return Group(spinner, BLANK_ROW, self._wrap_preview_bullet(body))

src/pythinker_code/utils/rich/syntax.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ def get_active_code_theme() -> str:
116116
class PythinkerSyntax(Syntax):
117117
def __init__(self, code: str, lexer: str, **kwargs: Any) -> None:
118118
if "theme" not in kwargs or kwargs["theme"] is None:
119-
kwargs["theme"] = PYTHINKER_ANSI_THEME
119+
kwargs["theme"] = resolve_code_theme(
120+
get_active_code_theme() or PYTHINKER_ANSI_THEME_NAME
121+
)
120122
super().__init__(code, lexer, **kwargs)
121123

122124

tasks/yolo-auto-mode-analysis.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
| **Auto** | `ApprovalState.auto` / `runtime_auto` (`soul/approval.py`; `is_auto()` at `approval.py:244`) | "No user is present at the terminal." | `auto` yes (`session_state.py:17`); `runtime_auto` no (`--print` only) |
1717

1818
Key compound: `is_auto_approve()` (`approval.py:223-234`):
19-
```
19+
20+
```python
2021
if yolo: return True # YOLO overrides everything below
2122
if safe_mode: return False # untrusted workspace blocks auto (but NOT yolo)
2223
return is_auto()
@@ -120,14 +121,14 @@ YOLO bypasses the `_EDIT_OUTSIDE_ACTION` guard (`approval.py:258,260`) → write
120121

121122
| Hypothesis | Status | Evidence |
122123
|---|---|---|
123-
| B1 | **Confirmed** (new test) | `tests/core/test_runtime_auto_state.py::test_default_config_yolo_auto_has_no_destructive_backstop` — default config + yolo+auto `deliberation_gate(rm -rf) is None` |
124-
| B2 | **Confirmed** (new test) | `tests/core/test_approval_auto.py::test_plan_mode_enter_exit_predicate_asymmetry`yolo-only: `is_auto_approve` True, `is_auto` False |
125-
| B3 | **Confirmed** (new test) | `tests/core/test_runtime_auto_state.py::test_runtime_create_silently_resumes_both_yolo_and_auto` — both flags restored from disk with `yolo=False` |
124+
| B1 | **Fixed** (new tests) | `tests/core/test_runtime_auto_state.py::test_default_config_yolo_auto_deliberates_destructive_actions` — default config + yolo+auto now bounces destructive shell calls for deliberation |
125+
| B2 | **Fixed** (new test) | `tests/core/test_plan_mode_auto_approval.py`Enter/Exit plan-mode tools now use the same unattended predicate |
126+
| B3 | **Fixed** (new tests) | `tests/core/test_resume_safety_notice.py`; `tests/core/test_runtime_auto_state.py::test_yolo_runtime_does_not_persist_safe_mode_downgrade`; `test_no_yolo_forces_yolo_off_over_persisted_state` |
126127
| R2 (one-shot/narrow gate) | **Already covered** | `test_approval_auto.py::test_destructive_action_deliberates_once_then_proceeds_under_auto`, `test_same_generation_duplicate...`, `test_subagent_identical_call...`, `test_unscoped_destructive_calls_always_bounce_fail_closed` |
127128
| R5 (yolo bypasses safe_mode) | **Already covered** | `test_approval_safe_mode.py::test_yolo_overrides_safe_mode`; `test_runtime_auto_state.py::test_unattended_runtime_in_default_safe_mode_denies_without_waiting` (the no-yolo contrast) |
128129
| R6 (outside-workspace) | **Already covered** | `test_approval_auto.py::test_trusted_auto_denies_outside_workspace_write_without_yolo` + `test_explicit_yolo_allows_outside_workspace_auto_write_boundary` |
129130

130-
3 new tests added; full `test_approval_auto.py` + `test_runtime_auto_state.py` = 34 passed; `ruff check` + `ruff format --check` clean. No production code changed.
131+
Production fixes and regression tests were added for B1, B2, and B3. Focused approval/runtime tests pass; `ruff check` + `ruff format --check` are clean.
131132

132133
## 5. Severity summary
133134

@@ -159,27 +160,31 @@ YOLO bypasses the `_EDIT_OUTSIDE_ACTION` guard (`approval.py:258,260`) → write
159160

160161
## 7. Fixes implemented (2026-06-02)
161162

162-
Decided with the user: **B1 = "all unsupervised" scope**, ship **B1 + B2**, defer **B3/B4**.
163+
Decided with the user: **B1 = "all unsupervised" scope**; ship **B1 + B2 + B3**. **B4** is resolved as correct-as-designed.
163164

164165
### B1 — destructive backstop now holds whenever unattended
166+
165167
`soul/approval.py` `deliberation_gate`: the early-return changed from
166168
`if not self._state.auto_deliberate` to `if not (self._state.auto_deliberate or self.is_auto())`.
167169
A destructive auto-approved action is now bounced once for deliberation whenever **no user
168170
is present** (`is_auto`), regardless of the config flag. The `auto_deliberate` flag now only
169171
*extends* deliberation to the interactive-yolo case (user present, approvals skipped).
172+
170173
- Consistency: `soul/dynamic_injections/auto_mode.py` now always injects the
171174
destructive-deliberation guidance under auto (the bare `_AUTO_PROMPT` was removed as
172175
orphaned — it could no longer be selected).
173176
- Effect: plain `--auto` (trusted) and manual `--yolo --auto` now match the
174177
`autonomous_coding` profile instead of being more dangerous than it.
175178

176179
### B2 — plan-mode checkpoint preserved under interactive yolo
180+
177181
`soul/pythinkersoul.py`: `EnterPlanMode` is now bound to `self._approval.is_auto` (was
178182
`is_auto_approve`), matching `ExitPlanMode`. Interactive `--yolo` no longer silently slips
179183
into plan mode and then blocks the exit; both transitions self-approve only when truly
180184
unattended (`is_auto`).
181185

182186
### B3 — persisted-state footguns (all three implemented)
187+
183188
- **B3a (trust corruption — the real bug):** `agent.py` no longer forces
184189
`effective_safe_mode = False` under `--yolo`. Yolo already bypasses safe mode in the
185190
decision path (`is_auto_approve` / `_unattended_denial_feedback` short-circuit on yolo
@@ -194,12 +199,14 @@ unattended (`is_auto`).
194199
`default_yolo`, and persisted/resumed state. `--no-yolo` beats `--yolo` if both are passed.
195200

196201
### B4 — resolved as correct-as-designed (no change)
202+
197203
`autonomous_coding` keeps `ask_user_question_policy="never"`. Switching to `ask_except_auto`
198204
is a **no-op** in every headless context the profile is for (auto/`--print`/`runtime_auto`
199205
`is_auto` → both dismiss) and would *contradict* the profile's purpose interactively (an
200206
"autonomous" session would block for input). `"never"` is the deliberate, correct choice.
201207

202208
### Tests (all RED→GREEN)
209+
203210
- New: `tests/core/test_plan_mode_auto_approval.py` (B2 binding);
204211
`tests/core/test_resume_safety_notice.py` (B3b).
205212
- `test_runtime_auto_state.py`: B1 backstop + B3a (yolo doesn't corrupt persisted

tests/core/test_resume_safety_notice.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@ def test_notice_names_active_modes_on_resume() -> None:
2222
# with "auto-approved" / "/auto" later in the message.
2323
yolo_only = _resumed_unsupervised_notice(resumed=True, yolo=True, auto=False)
2424
assert yolo_only is not None and yolo_only.startswith("YOLO active")
25+
assert "actions auto-approved" in yolo_only
2526

2627
auto_only = _resumed_unsupervised_notice(resumed=True, yolo=False, auto=True)
2728
assert auto_only is not None and auto_only.startswith("auto active")
29+
assert "interactive approvals still required" in auto_only
30+
assert "actions auto-approved" not in auto_only
2831

2932
both = _resumed_unsupervised_notice(resumed=True, yolo=True, auto=True)
3033
assert both is not None and both.startswith("YOLO + auto active")
34+
assert "actions auto-approved" in both

tests/core/test_runtime_auto_state.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,9 @@ async def test_no_yolo_forces_yolo_off_over_persisted_state(
223223

224224
assert runtime.approval.is_yolo() is False
225225

226+
runtime.approval.set_auto(True)
227+
assert session.state.approval.yolo is True
228+
226229

227230
@pytest.mark.asyncio
228231
async def test_default_config_yolo_auto_deliberates_destructive_actions(

tests/ui_and_conv/test_code_theme_opt_in.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from pythinker_code.utils.rich.syntax import (
1919
PYTHINKER_ANSI_THEME,
2020
PYTHINKER_ANSI_THEME_NAME,
21+
PythinkerSyntax,
2122
get_active_code_theme,
2223
resolve_code_theme,
2324
set_active_code_theme,
@@ -68,6 +69,15 @@ def test_active_code_theme_round_trips() -> None:
6869
assert get_active_code_theme() == "dracula"
6970

7071

72+
def test_pythinker_syntax_uses_active_code_theme() -> None:
73+
set_active_code_theme("monokai")
74+
console = Console(force_terminal=True, color_system="truecolor", width=60)
75+
with console.capture() as capture:
76+
console.print(PythinkerSyntax("import os", "python"))
77+
78+
assert _MONOKAI_BG in capture.get()
79+
80+
7181
def test_resolve_code_theme_maps_only_the_sentinel() -> None:
7282
# Sentinel resolves to the ANSI SyntaxTheme instance; stock names stay strings
7383
# (this string-vs-instance distinction is what the renderer branches on).

tests/ui_and_conv/test_shell_prompt_echo.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,17 @@ def test_user_echo_wraps_message_in_tinted_block() -> None:
195195

196196
from pythinker_code.ui.theme import tui_rich_style
197197

198-
bg = tui_rich_style("user_message_bg").bgcolor
199-
assert bg is not None and bg.triplet is not None
200-
red, green, blue = bg.triplet
201-
expected = f"48;2;{red};{green};{blue}"
198+
expected = tui_rich_style("user_message_bg").bgcolor
199+
assert expected is not None
202200

203201
console = Console(force_terminal=True, color_system="truecolor", width=40)
204-
with console.capture() as capture:
205-
console.print(render_user_echo_text("apply"))
202+
lines = console.render_lines(render_user_echo_text("apply"), console.options)
206203

207-
# The submitted message is painted on the shared user_message_bg block.
208-
assert expected in capture.get()
204+
# The submitted message itself, not just surrounding padding, is painted on
205+
# the shared user_message_bg block.
206+
apply_segment = next(segment for line in lines for segment in line if "apply" in segment.text)
207+
assert apply_segment.style is not None
208+
assert apply_segment.style.bgcolor == expected
209209

210210

211211
def test_should_echo_agent_input_for_plain_agent_message() -> None:

tests/utils/test_pyinstaller_utils.py

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -240,58 +240,23 @@ def test_pyinstaller_datas():
240240

241241

242242
def test_pyinstaller_hiddenimports():
243+
import pkgutil
244+
245+
import pygments.styles
246+
243247
from pythinker_code.utils.pyinstaller import hiddenimports
244248

245-
assert sorted(hiddenimports) == snapshot(
249+
# Pygments style list is owned by the Pygments package and changes with upgrades;
250+
# enumerate actual sub-modules (matching collect_submodules) rather than the style
251+
# registry names (get_all_styles uses hyphens, modules use underscores).
252+
expected_pygments = {"pygments.styles"} | {
253+
f"pygments.styles.{mod.name}" for mod in pkgutil.iter_modules(pygments.styles.__path__)
254+
}
255+
assert expected_pygments <= set(hiddenimports)
256+
257+
project_entries = sorted(set(hiddenimports) - expected_pygments)
258+
assert project_entries == snapshot(
246259
[
247-
"pygments.styles",
248-
"pygments.styles._mapping",
249-
"pygments.styles.abap",
250-
"pygments.styles.algol",
251-
"pygments.styles.algol_nu",
252-
"pygments.styles.arduino",
253-
"pygments.styles.autumn",
254-
"pygments.styles.borland",
255-
"pygments.styles.bw",
256-
"pygments.styles.coffee",
257-
"pygments.styles.colorful",
258-
"pygments.styles.default",
259-
"pygments.styles.dracula",
260-
"pygments.styles.emacs",
261-
"pygments.styles.friendly",
262-
"pygments.styles.friendly_grayscale",
263-
"pygments.styles.fruity",
264-
"pygments.styles.gh_dark",
265-
"pygments.styles.gruvbox",
266-
"pygments.styles.igor",
267-
"pygments.styles.inkpot",
268-
"pygments.styles.lightbulb",
269-
"pygments.styles.lilypond",
270-
"pygments.styles.lovelace",
271-
"pygments.styles.manni",
272-
"pygments.styles.material",
273-
"pygments.styles.monokai",
274-
"pygments.styles.murphy",
275-
"pygments.styles.native",
276-
"pygments.styles.nord",
277-
"pygments.styles.onedark",
278-
"pygments.styles.paraiso_dark",
279-
"pygments.styles.paraiso_light",
280-
"pygments.styles.pastie",
281-
"pygments.styles.perldoc",
282-
"pygments.styles.rainbow_dash",
283-
"pygments.styles.rrt",
284-
"pygments.styles.sas",
285-
"pygments.styles.solarized",
286-
"pygments.styles.staroffice",
287-
"pygments.styles.stata_dark",
288-
"pygments.styles.stata_light",
289-
"pygments.styles.tango",
290-
"pygments.styles.trac",
291-
"pygments.styles.vim",
292-
"pygments.styles.vs",
293-
"pygments.styles.xcode",
294-
"pygments.styles.zenburn",
295260
"pythinker_code.cli._lazy_group",
296261
"pythinker_code.cli.debug",
297262
"pythinker_code.cli.export",

0 commit comments

Comments
 (0)