Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ GitHub Releases page; `0.8.0` is the new starting line.

## Unreleased

- **designer-skill MCP bridge.** Bundled a `designer-skill` stub skill that routes frontend work to the connected designer-skill MCP tools instead of failing ReadSkill; plugin-style names like `designer-skill:designer-skill` resolve correctly, and ReadSkill falls back to a generic MCP bridge (any user-configured server name) when only the MCP server is connected.
- **Always-on best practices.** New `best_practices_always` config option folds the full `/best-practices` engineering guidance into the root session's system prompt at startup, so the guardrails apply to every new session without running the command. Default off.
- **Smarter multi-edit errors.** A `StrReplaceFile` batch that fails schema validation (e.g. edit entries collapsed by a streaming glitch) now returns a precise, actionable error naming the bad entries and steering toward single-edit calls, instead of a wall of validation errors. Valid edits are never partially applied.
- **Agent guardrails.** The default system prompt now requires absence claims ("no em-dashes", "no leftover debug", "matches the source") to be backed by an actual zero-hit scan, and to re-ask rather than act on a self-authored reading of a non-responsive clarifying answer.
- **`/recap on|off`.** Toggle turn recaps from the recap command (mirrors `/settings recap(s)`), with a grey inline autosuggest reflecting the current state.
- **Recap hygiene.** Session recaps strip `<system-reminder>` blocks so injected harness context no longer leaks into one-line recaps.
- **Login selector polish.** Configured `/login` providers render with distinct success/state styling; the background working indicator uses the braille spinner, and working tips wrap with a hanging indent under the verb.
- **Scratch cleanup on exit.** Sessions that end via an exception now clean up their scratch files instead of orphaning them.
- **Readable diff context.** Unchanged context lines in file-edit diff snippets now render in the normal body-text color instead of muted grey, so edited-file previews are easier to read; added/removed lines are unchanged.

## 0.42.0 (2026-06-12)

- **Adversarial branch review hardening.** A multi-agent review pass confirmed and fixed: committed-but-clean isolation worktrees are now retained (commits ahead of the creation base count as changes — they were previously orphaned on cleanup); foreground shell commands and relative-path file edits now resolve against the agent's work dir, so worktree isolation actually binds them (host exec gained a `cwd` argument); safe mode now also disables the read-only-command prompt elision; locally parallel-safe MCP/tools run in parallel in the same-step gate; worktree add/remove serializes per repo. Behavior notes: same-step tool calls without `supports_parallel` now serialize deterministically (previously fully concurrent), and text-mode error diagnostics moved to stderr — capture `2>&1` or use `--output-format stream-json` if you scraped stdout.
Expand Down
4 changes: 2 additions & 2 deletions src/pythinker_code/agents/default/system.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Eight rules that override convenience, speed, and every other instruction in thi

1. **Read before write.** Never edit a file you have not read this session; confirm the exact lines you are about to modify still match what you read.
2. **Complete code only.** Never write placeholders, stubs, `TODO: implement`, elided bodies, or "rest of the file unchanged" markers into files. If a change is too large for one step, split the work — never abridge the code. (Genuine `TODO:` notes for real technical debt are fine.)
3. **Evidence before claims.** Every "done", "fixed", or "works" names the command you ran and the result you observed. Verification means a passing test, a working repro, or a deterministic command that confirms the intended behavior — compiling or type-checking alone is not verification. This definition is canonical: it is what "verify" means everywhere in this prompt.
3. **Evidence before claims.** Every "done", "fixed", or "works" names the command you ran and the result you observed. Verification means a passing test, a working repro, or a deterministic command that confirms the intended behavior — compiling or type-checking alone is not verification. This definition is canonical: it is what "verify" means everywhere in this prompt. A claim that something is *absent* — no banned strings, no em-dashes, no leftover debug instrumentation, no TODOs, output matches the source — is only true after a scan that returned zero hits; never assert absence from memory.
4. **Re-verify after every edit.** An edit invalidates all prior verification; re-run the smallest check that proves the change is sound before building on top of it.
5. **Honest failure.** When verification fails, report the failing output verbatim under **BLOCKERS**. Never weaken an assertion, skip a test, widen a tolerance, swallow an error, or silently narrow scope to get to green.
6. **Match the codebase.** Existing style, granularity, naming, and idioms beat your preferences. A correct change that fights the codebase's conventions is not done.
Expand Down Expand Up @@ -57,7 +57,7 @@ State multi-step plans inline as `Step → verify: check`; substantial tasks kee

**Report** with `path:line` references over pasted blocks, concise findings, and explicit residual risk — unverified assumptions, untested paths, recommended follow-ups, and unrelated issues noticed but not touched.

**Ask vs. act.** Act without asking when intent is clear, the change is reversible, and it is in scope. Ask one focused question — before implementation, never after mistakes — when interpretations genuinely diverge, an action is irreversible or destructive, credentials are needed, requirements conflict, or scope grows beyond the request. Never ask what a tool call can answer.
**Ask vs. act.** Act without asking when intent is clear, the change is reversible, and it is in scope. Ask one focused question — before implementation, never after mistakes — when interpretations genuinely diverge, an action is irreversible or destructive, credentials are needed, requirements conflict, or scope grows beyond the request. Never ask what a tool call can answer. If an answer to a clarifying question does not actually resolve the ambiguity, say so and re-ask with your default stated — never act on a self-authored interpretation of a non-answer.

**Steering.** If the user interjects or redirects mid-task, stop, reconcile the new instruction with the current plan, update the todos, then continue.

Expand Down
8 changes: 8 additions & 0 deletions src/pythinker_code/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,14 @@ async def create(
)
_phase_timings_ms["mcp_ms"] = int((time.monotonic() - _phase_t) * 1000)

if runtime.config.best_practices_always:
from pythinker_code.prompts import apply_always_on_best_practices

agent = dataclasses.replace(
agent,
system_prompt=apply_always_on_best_practices(agent.system_prompt, enabled=True),
)

if startup_progress is not None:
startup_progress("Restoring conversation...")
context = Context(session.context_file)
Expand Down
8 changes: 8 additions & 0 deletions src/pythinker_code/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,14 @@ async def _reload_loop(session_id: str | None) -> tuple[str | None, int]:
# the most recent _run() call, which may have failed before returning.
# last_session is from a *previous* iteration and must not be touched.
if _latest_created_session is not None:
with contextlib.suppress(Exception):
from pythinker_code.scratchpad import cleanup_session_scratch

await cleanup_session_scratch(
_latest_created_session.work_dir,
session_id=_latest_created_session.id,
session_title=_latest_created_session.title,
)
Comment on lines +1206 to +1213

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Log best-effort cleanup failures instead of fully suppressing them.

At Line 1206, exceptions from cleanup_session_scratch(...) are swallowed without any trace. Keep it best-effort, but log failures so repeated scratch-cleanup regressions are diagnosable.

Proposed fix
-                with contextlib.suppress(Exception):
-                    from pythinker_code.scratchpad import cleanup_session_scratch
-
-                    await cleanup_session_scratch(
-                        _latest_created_session.work_dir,
-                        session_id=_latest_created_session.id,
-                        session_title=_latest_created_session.title,
-                    )
+                try:
+                    from pythinker_code.scratchpad import cleanup_session_scratch
+
+                    await cleanup_session_scratch(
+                        _latest_created_session.work_dir,
+                        session_id=_latest_created_session.id,
+                        session_title=_latest_created_session.title,
+                    )
+                except Exception:
+                    logger.debug(
+                        "Best-effort scratch cleanup failed for session {session_id}",
+                        session_id=_latest_created_session.id,
+                        exc_info=True,
+                    )

As per coding guidelines, exception handlers that silently swallow errors should be logged or re-raised.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pythinker_code/cli/__init__.py` around lines 1206 - 1213, Replace the
silent contextlib.suppress around the cleanup call so failures are still
best-effort but logged: instead of "with contextlib.suppress(Exception): await
cleanup_session_scratch(...)" wrap the await in a try/except Exception as e and
log the exception (e.g., via logging.getLogger(__name__).exception(...) or the
module's existing logger) including context (_latest_created_session.id,
.work_dir, .title) so cleanup_session_scratch failures are recorded for
diagnosis.

Source: Coding guidelines

_print_resume_hint(_latest_created_session)
if _latest_created_session.is_empty():
with contextlib.suppress(Exception):
Expand Down
8 changes: 8 additions & 0 deletions src/pythinker_code/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,14 @@ class Config(BaseModel):
),
)
default_yolo: bool = Field(default=False, description="Default yolo (auto-approve) mode")
best_practices_always: bool = Field(
default=False,
description=(
"When true, fold the full /best-practices engineering guidance into the root "
"session's system prompt at startup, so the guardrails apply without running "
"/best-practices manually. Applies to new sessions; costs context tokens each session."
),
)
ask_user_question_policy: Literal["always", "ask_except_auto", "never", "auto_deliberate"] = (
Field(
default="ask_except_auto",
Expand Down
85 changes: 76 additions & 9 deletions src/pythinker_code/project_memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
from pythinker_code.soul.pythinkersoul import PythinkerSoul

ENTRY_DELIMITER = "\n§\n"
MEMORY_CHAR_LIMIT = 2200
USER_CHAR_LIMIT = 1375
MEMORY_CHAR_LIMIT = 5000
USER_CHAR_LIMIT = 2500
INJECTION_BUDGET_BYTES = 8 * 1024
_JOURNAL_MAX_ENTRIES = 100

Expand Down Expand Up @@ -92,6 +92,9 @@ async def project_key(work_dir: HostPath, *, git_runner: GitRunner | None = None
class MemoryOpResult:
ok: bool
message: str
# True only when the op failed because the store is at capacity. Lets the UI
# layer attach user-facing guidance without string-matching the message.
full: bool = False


class ProjectMemoryStore:
Expand Down Expand Up @@ -130,6 +133,31 @@ def _filename(self, target: Target) -> str:
def _char_limit(self, target: Target) -> int:
return self._user_limit if target == "user" else self._memory_limit

@staticmethod
def _used(entries: list[str]) -> int:
return len(ENTRY_DELIMITER.join(entries))

@staticmethod
def _append_overhead(entries: list[str]) -> int:
"""Chars an appended entry costs beyond its own text (the joining delimiter)."""
return len(ENTRY_DELIMITER) if entries else 0

@staticmethod
def _inventory(entries: list[str]) -> str:
"""Compact, model-readable listing: index, size, and a one-line preview.

The preview doubles as a copy-paste ``old_text`` substring for remove/replace,
turning a space-rejection into a guided one-step fix instead of a guessing game.
"""
if not entries:
return " (none)"
lines: list[str] = []
for i, entry in enumerate(entries):
preview = " ".join(entry.split())
clipped = preview[:60] + ("…" if len(preview) > 60 else "")
lines.append(f" [{i}] {len(entry)} chars — {clipped}")
return "\n".join(lines)

async def _path_for(self, target: Target) -> Path:
root = await self._ensure_dir()
return root / "memory" / self._filename(target)
Expand Down Expand Up @@ -219,13 +247,19 @@ async def add(self, target: Target, content: str) -> MemoryOpResult:
if content in entries:
return MemoryOpResult(True, "Entry already exists (no duplicate added).")
limit = self._char_limit(target)
new_total = len(ENTRY_DELIMITER.join([*entries, content]))
if new_total > limit:
current = len(ENTRY_DELIMITER.join(entries))
if len(ENTRY_DELIMITER.join([*entries, content])) > limit:
used = self._used(entries)
overhead = self._append_overhead(entries)
free = max(0, limit - used - overhead)
need = len(content) + overhead
return MemoryOpResult(
False,
f"Memory at {current}/{limit} chars; this entry ({len(content)}) "
"exceeds the limit. Replace or remove entries first.",
f"Not enough room: this entry needs {need} chars "
f"(content {len(content)} + {overhead} separator), but only {free} free "
f"({used}/{limit} used). Remove or replace an entry to free space, "
f"or shorten this entry to ≤{free} chars.\n"
f"Current entries:\n{self._inventory(entries)}",
full=True,
)
await self._write_entries(target, [*entries, content])
return MemoryOpResult(True, "Entry added.")
Expand Down Expand Up @@ -265,8 +299,16 @@ async def replace(self, target: Target, old_text: str, new_content: str) -> Memo
limit = self._char_limit(target)
candidate = list(entries)
candidate[idx] = new_content
if len(ENTRY_DELIMITER.join(candidate)) > limit:
return MemoryOpResult(False, f"Replacement would exceed the {limit}-char limit.")
projected = self._used(candidate)
if projected > limit:
over = projected - limit
return MemoryOpResult(
False,
f"Replacement too large by {over} chars: result would be "
f"{projected}/{limit}. Shorten the new text by ≥{over} chars, or remove "
f"another entry first.\nCurrent entries:\n{self._inventory(entries)}",
full=True,
)
await self._write_entries(target, candidate)
return MemoryOpResult(True, "Entry replaced.")

Expand All @@ -289,6 +331,31 @@ async def remove(self, target: Target, old_text: str) -> MemoryOpResult:
await self._write_entries(target, entries)
return MemoryOpResult(True, "Entry removed.")

async def capacity(self, target: Target) -> tuple[int, int, int]:
"""Return ``(used, limit, free)`` chars for ``target`` (free accounts for the
delimiter a new entry would cost). Used by the UI to show/explain capacity."""
entries = await self.read_entries(target)
limit = self._char_limit(target)
used = self._used(entries)
free = max(0, limit - used - self._append_overhead(entries))
return used, limit, free

async def status(self, target: Target) -> str:
"""Read-only capacity + inventory snapshot for mid-session introspection.

Lets the agent see what is stored, at what size, and exactly how much room
is free before attempting a write — so it can remove/consolidate instead of
repeatedly retrying an over-budget add.
"""
entries = await self.read_entries(target)
limit = self._char_limit(target)
used = self._used(entries)
free = max(0, limit - used - self._append_overhead(entries))
return (
f"{self._filename(target)}: {used}/{limit} chars across {len(entries)} "
f"entries; {free} free for a new entry.\nCurrent entries:\n{self._inventory(entries)}"
)

async def append_journal(self, recap: str) -> MemoryOpResult:
"""Prepend one stable session recap to ``JOURNAL.md`` if it is new."""
recap = recap.strip()
Expand Down
15 changes: 15 additions & 0 deletions src/pythinker_code/prompts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,18 @@
GOAL_SET = (Path(__file__).parent / "goal_set.md").read_text(encoding="utf-8")
GOAL_CONTINUATION = (Path(__file__).parent / "goal_continuation.md").read_text(encoding="utf-8")
GOAL_WRAP_UP = (Path(__file__).parent / "goal_wrap_up.md").read_text(encoding="utf-8")


def apply_always_on_best_practices(system_prompt: str, *, enabled: bool) -> str:
"""Append the full best-practices guidance to a system prompt when enabled.

The `/best-practices` profile is normally opt-in per session. When the
``best_practices_always`` config flag is set, the root session folds it into
the system prompt at startup so the guardrails apply without running the
command. The profile's opening line is phrased for the manual command ("The
user ran ``/best-practices``."), so strip that lead-in for the always-on path.
"""
if not enabled:
return system_prompt
guidance = BEST_PRACTICES.replace("The user ran `/best-practices`. ", "", 1)
return f"{system_prompt}\n\n{guidance}"
4 changes: 3 additions & 1 deletion src/pythinker_code/session_recap.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,13 @@ def _last_substantive_thread(items: list[SessionRecapItem]) -> str:
re.IGNORECASE,
)
_PARENTHETICAL_RE = re.compile(r"\([^)]*\)")
_SYSTEM_REMINDER_BLOCK_RE = re.compile(r"<system-reminder>.*?</system-reminder>", re.DOTALL)


def _recap_source_text(text: str) -> str:
"""Return one-line recap input with bulky structured blocks removed."""
without_fences = _strip_fenced_blocks(text)
without_reminders = _SYSTEM_REMINDER_BLOCK_RE.sub("", text)
without_fences = _strip_fenced_blocks(without_reminders)
without_tables = _strip_markdown_tables(without_fences)
without_structure = _strip_markdown_structure(without_tables)
without_ticks = without_structure.replace("`", "")
Expand Down
43 changes: 43 additions & 0 deletions src/pythinker_code/skills/designer-skill/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
name: designer-skill
description: Prescriptive frontend design guidance via the designer-skill MCP server. Use when the user asks to use designer-skill, improve UI/UX, run the anti-slop ship gate, apply a design system, or enhance pages/components with MCP-backed design references — especially for Pythinker docs and marketing surfaces with DESIGN.md/PRODUCT.md.
---

# designer-skill (MCP)

`designer-skill` is **not** a filesystem workflow skill. It is delivered by the connected **designer-skill MCP server**. After reading this stub, call the MCP tools below — do **not** call `ReadSkill` again for the same name.

## Required MCP tools

Call these by their registered tool keys (`mcp__designer-skill__<tool>`):

| Step | Tool | Purpose |
| --- | --- | --- |
| 1 | `get_design_system` | Session preflight, precedence rules, routing map, ship-gate overview |
| 2 | `get_reference` | Load the specific reference file the task needs |
| 3 | `dispatch_intent` | Route ambiguous design requests to the right reference/workflow |
| 4 | `apply_designer` | Apply prescriptive design moves to the scoped surface |
| 5 | `anti_slop_checklist` | Mandatory ship gate before declaring frontend work done |

If a tool is missing, check `/mcp` — the server may still be connecting.

## Session preflight

Before editing UI:

1. Read project design sources when present (`DESIGN.md`, `PRODUCT.md`, or equivalent).
2. Call `get_design_system` first.
3. Pull only the references the scoped surface needs via `get_reference`.
4. Implement changes; run `anti_slop_checklist` before finishing.

## Common aliases

These names refer to the same MCP integration — always use MCP tools, not `ReadSkill`:

- `designer-skill`
- `designer-skill:designer-skill` (plugin-style slash autocomplete)
- "use designer-skill mcp" / "designer mcp"

## When MCP is unavailable

If `/mcp` shows designer-skill disconnected, run `pythinker mcp` to configure/auth it. Do not substitute ad-hoc design advice for the MCP workflow when the user explicitly requested designer-skill.
Loading
Loading