fix: address CodeRabbit review findings on PR #89

- live_view: render cancelled todos (✕, muted+struck) instead of dropping
  them from the pinned list, now that "cancelled" is a valid todo status
- wire/server: pop dismissed QuestionRequests from the pending map on steer
  so a late client response can't double-resolve a superseded question
- ui/shell: log (don't silently swallow) failures to write the fallback
  429 diagnostic, so that diagnostic path stays debuggable
- mcp_resource: add -> None to the public constructors per the annotation
  guideline
- tests: drop unused params, remove a duplicate @pytest.mark.asyncio, split
  chained assertions; add coverage for the three behavioral fixes above
- tasks/*.md: fix markdownlint blank-line nits

Skipped as stale or out of policy: ruff-format/typos findings already fixed
in earlier commits; an MD037 false positive on snake_case prose; narrowing
the best-effort asyncio-warning catch (would reintroduce a crash path).

Author: elkaix

src/pythinker_code/tools/mcp_resource/__init__.py

@@ -28,7 +28,7 @@ class ListMcpResources(CallableTool2[ListParams]):
     name: str = "ListMcpResources"
     params: type[ListParams] = ListParams
 
-    def __init__(self, toolset: PythinkerToolset):
+    def __init__(self, toolset: PythinkerToolset) -> None:
         super().__init__(description=load_desc(Path(__file__).parent / "list_description.md"))
         self._toolset = toolset
 
@@ -76,7 +76,7 @@ class ReadMcpResource(CallableTool2[ReadParams]):
     name: str = "ReadMcpResource"
     params: type[ReadParams] = ReadParams
 
-    def __init__(self, toolset: PythinkerToolset):
+    def __init__(self, toolset: PythinkerToolset) -> None:
         super().__init__(description=load_desc(Path(__file__).parent / "read_description.md"))
         self._toolset = toolset
 

src/pythinker_code/ui/shell/__init__.py

@@ -456,7 +456,7 @@ def _capture_unparsed_429(exc: BaseException) -> None:
         with path.open("a", encoding="utf-8") as fh:
             fh.write(line)
     except Exception:
-        pass
+        logger.debug("Failed to capture unparsed 429 payload to debug log", exc_info=True)
 
 
 def _render_429_message(detail: dict[str, str], usage_lines: list[str] | None = None) -> str:

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

@@ -683,10 +683,10 @@ def _pinned_todo_block(
         latest_todos = tuple(
             todo
             for todo in getattr(self, "_latest_todos", ())
-            if todo.status in ("done", "in_progress", "pending") and todo.title.strip()
+            if todo.status in ("done", "in_progress", "pending", "cancelled") and todo.title.strip()
         )
         active_todo = next((todo for todo in latest_todos if todo.status == "in_progress"), None)
-        status_order = {"in_progress": 0, "pending": 1, "done": 2}
+        status_order = {"in_progress": 0, "pending": 1, "cancelled": 2, "done": 3}
         ordered_todos = tuple(
             sorted(
                 enumerate(latest_todos),
@@ -743,6 +743,10 @@ def _pinned_todo_row(
             icon = "✓"
             icon_token = "muted"
             title_style = tui_rich_style("muted") + Style(strike=True)
+        elif todo.status == "cancelled":
+            icon = "✕"
+            icon_token = "muted"
+            title_style = tui_rich_style("muted") + Style(strike=True)
         elif todo.status == "in_progress":
             icon = "■"
             icon_token = "activity_verb"

src/pythinker_code/wire/server.py

@@ -780,8 +780,9 @@ async def _handle_steer(
         # question, so dismiss any in-flight QuestionRequest — the blocked tool yields
         # and the steer takes precedence, instead of the steer deferring behind a
         # manual answer.
-        for request in list(self._pending_requests.values()):
+        for msg_id, request in list(self._pending_requests.items()):
             if isinstance(request, QuestionRequest) and not request.resolved:
+                self._pending_requests.pop(msg_id, None)
                 request.resolve({})
 
         self._soul.steer(msg.params.user_input)

tasks/_gap_actionable.md

@@ -89,6 +89,7 @@ FILES: src/pythinker_code/soul/compaction.py, src/pythinker_code/soul/pythinkers
 FIT: Transfers. Pruning stale tool outputs is backend-only and orthogonal to UI. One caveat: pythinker's append-only JSONL context (context.py) makes in-place part mutation harder than kilo's SQLite part-update model, so the implementation must rewrite the context file (as clear()/revert_to() already do) rather than mutate a part. Manageable, hence effort L.
 
 ## [ctxmgmt-3] Recall is one-shot injection only; no model-invocable cross-session recall tool
+
 sev=medium effort=M risk=low verdict=partial(0.82)
 GAP: Pythinker's recall is push-only and fires once: a fact that becomes relevant mid-session (after the single injection) is not re-surfaced until compaction re-arms it, and the model has no way to actively ask 'what did I decide in the session where I set up the CI pipeline?' and read that transcript. Kilo gives the agent agency to retrieve prior-session context on demand, which is exactly what long, resumed coding tasks need.
 ACTION: Add a first-class, model-invocable Recall tool that (a) lists/searches prior sessions by title/recency/relevance and (b) returns ranked excerpts (or the full transcript) of a chosen prior session's context.jsonl on demand — i.e. give the agent agency to pull cross-session context mid-task instead of relying solely on the one-shot push injection. Scope the rec correctly: the underlying data (context.jsonl transcripts + state.json todos under ~/.pythinker/sessions/) is already durably persisted and is technically reachable today via the unsandboxed Shell tool (cat/grep), so this is NOT about making data reachable — it is about replacing a brittle raw-file escape hatch with a designed, semantically-searchable, approval-aware, sanitized affordance (reuse the existing LexicalRetriever BM25 + memory/sanitize.py threat scanning that the push path already uses). Do NOT claim the transcript is currently unreachable by the model; the accurate framing is 'no purpose-built recall tool; only an ungainly shell hatch + one-shot push injection.'

tasks/_gap_extract.md

@@ -186,6 +186,7 @@ The real gap is in the generic ToolResultBuilder truncation path (tools/utils.py
 - **refined:** Add a cheaper intermediate tier between "do nothing" and full SimpleCompaction. Concretely: (1) introduce a lower trigger threshold below the 0.85/reserved-buffer point that, instead of LLM summarization, walks history and replaces large COMPLETED tool-result message bodies in DEEP history (older than the last N turns) with a short placeholder (e.g. "[tool output elided: 40k chars, ToolName, ts]"), preserving conversational/tool-call structure and ids; (2) only escalate to full SimpleCompaction (compaction.py / pythinkersoul.py:1261) when this fidelity-preserving pruning fails to bring token_count back under the higher threshold. Reuse existing wiring: gate it in the should_auto_compact branch at pythinkersoul.py:1252-1272 and add a `prune_stale_tool_outputs(history)` helper alongside SimpleCompaction. Drop/deprioritize the separate "post-compaction pruning to reclaim subsumed tool outputs" idea — full compaction already clears everything, so that sub-step is only meaningful for the new intermediate tier, where it is the whole point.
 
 ### [ctxmgmt-3] Recall is one-shot injection only; no model-invocable cross-session recall tool
+
 - **dimension:** Context management: compaction / overflow / summary / recall
 - **severity:** medium  |  **verdict:** partial (0.82)  |  **effort:** M  |  **risk:** low
 - **pythinker now:** memory/recall.py:218-270 RecallInjectionProvider fires exactly once per context (self._injected guard) and re-arms ONLY on compaction (on_context_compacted) or explicit rearm. It BM25-ranks MEMORY/USER/JOURNAL/scratch + recent-session open todos against the last user message and injects them as a system-reminder. The model cannot proactively pull a *prior session's full transcript* mid-task: there is no recall tool in tools/ (confirmed: tools/ has agent, ask_user, background, dmail, file, memory, plan, scratchpad, shell, skill, think, todo, web — no recall). Cross-session knowledge is limited to (a) durable MEMORY/USER facts and (b) open-todo titles, surfaced passively at injection time.

tasks/pythinker-agent-enhancement-plan.md

@@ -271,6 +271,7 @@ Phases are ordered by impact×effort and by dependency. Within a phase, items ar
 ### Phase 3 — Memory & recall agency
 
 #### 3.1 — Model-invocable cross-session `Recall` tool (`memory-1` / `ctxmgmt-3`) · M · med
+
 - **Current.** Recall is push-only and fires once; the agent cannot actively ask "what did I decide in the session where I set up CI?" and read that transcript. Distilled JOURNAL recaps lose load-bearing detail (exact commands, paths, rationale). The data *is* durably persisted (`context.jsonl` under the sessions dir) and technically reachable via the unsandboxed Shell — so this replaces a brittle `cat`/`grep` escape hatch with a designed, sanitized, approval-aware affordance.
 - **Target.** The agent has agency to search and read prior sessions on demand.
 - **Change.** Add a root-agent, read-only `Recall` tool (`tools/recall/`) with two modes: (1) **search** prior sessions by topic/file/date over `wire.jsonl`/`context.jsonl` using the existing `LexicalRetriever` BM25+recency (`memory/retriever.py`), scoped to the current `project_memory.project_key`, returning id/title/ts/snippet; (2) **read** a chosen session's transcript span via `Session.list_all` (`session.py:278`) + `wire_file.iter_records`. Cap returned bytes/turns; **sanitize via `memory/sanitize.py`** (a prior transcript is untrusted input → also subject to §1's wrapping). Gate cross-workspace reads behind Approval. Register read-only in `agents/default/agent.yaml`.

tests/core/test_context_pruning.py

@@ -125,7 +125,7 @@ async def test_prune_context_rewrites_history_preserving_structure(runtime, tmp_
     assert history[-1].extract_text("") == "done"
 
 
-def _seed_prunable(context) -> list[Message]:
+def _seed_prunable() -> list[Message]:
     return [
         Message(role="user", content="go"),
         Message(role="assistant", content=[TextPart(text="working")]),
@@ -143,7 +143,7 @@ async def test_prune_context_restores_history_when_rebuild_fails(runtime, tmp_pa
     runtime.config.loop_control.prune_min_chars = 2000
     context, soul = _make_soul(runtime, tmp_path)
     await context.write_system_prompt("sys")
-    await context.append_message(_seed_prunable(context))
+    await context.append_message(_seed_prunable())
     before = list(context.history)
 
     # Fail the rebuild's append of the pruned body (it carries the "elided" placeholder);
@@ -189,13 +189,12 @@ async def on_context_compacted(self) -> None:
     soul.add_injection_provider(spy)
 
     await context.write_system_prompt("sys")
-    await context.append_message(_seed_prunable(context))
+    await context.append_message(_seed_prunable())
 
     assert await soul.prune_context() is True
     assert spy.compacted == 0  # prune is not compaction; one-shot state must survive
 
 
-@pytest.mark.asyncio
 @pytest.mark.asyncio
 async def test_prune_context_never_increases_token_count(runtime, tmp_path) -> None:
     """Pruning only removes content, so the post-prune token count must never exceed the
@@ -206,7 +205,7 @@ async def test_prune_context_never_increases_token_count(runtime, tmp_path) -> N
     runtime.config.loop_control.prune_min_chars = 2000
     context, soul = _make_soul(runtime, tmp_path)
     await context.write_system_prompt("sys")
-    await context.append_message(_seed_prunable(context))
+    await context.append_message(_seed_prunable())
     # Authoritative pre-prune count (from the LLM) below the heuristic estimate of the
     # remaining content — the case where a naive full re-estimate would grow the count.
     before = 1

tests/core/test_mcp_docker_rm.py

@@ -18,7 +18,7 @@ def test_injects_rm_after_run(cmd: str) -> None:
     assert args == ["run", "--rm", "-i", "ghcr.io/example/mcp"]
 
 
-def test_keeps_existing_rm(cmd: str = "docker") -> None:
+def test_keeps_existing_rm() -> None:
     original = ["run", "--rm", "-i", "img"]
     assert ensure_docker_rm("docker", original) == original
 

tests/core/test_project_mcp_config.py

@@ -19,7 +19,8 @@ def test_finds_project_mcp_config_at_repo_root(
     monkeypatch.chdir(tmp_path)
 
     found = _find_project_mcp_config_file()
-    assert found is not None and found.samefile(cfg)
+    assert found is not None
+    assert found.samefile(cfg)
 
 
 def test_finds_project_mcp_config_from_subdir(
@@ -34,7 +35,8 @@ def test_finds_project_mcp_config_from_subdir(
     monkeypatch.chdir(sub)
 
     found = _find_project_mcp_config_file()
-    assert found is not None and found.samefile(cfg)
+    assert found is not None
+    assert found.samefile(cfg)
 
 
 def test_no_project_mcp_config_returns_none(