fix: resolve PR #155 CI build and CodeRabbit review findings

Docs build failed because an inline-code span `pythinker plugin
enable/disable <name>` wrapped across a line break, so VitePress's Vue
compiler read `<name>` as an unclosed HTML tag. Keep the span on one line
and regenerate the synced docs changelog.

Address unresolved review findings:
- Lock the plugin install / marketplace registries' read-modify-write
  cycles with a shared cross-process file_lock helper (no lost updates on
  concurrent CLI writers).
- Make MCP inventory refresh side-effect-free: _inventory_mcp_server now
  returns the discovered inventory and the caller assigns it only after
  the awaited call succeeds, so a timeout/__aexit__ failure can't leave
  the published registry inconsistent with the callable tools.
- Roll back partially materialized plugins when dependency resolution
  fails, so a failed install never leaves partial on-disk/registry state.
- Always validate ${user_config.*} placeholders (even when a manifest
  declares no userConfig) so an unresolved placeholder fails soft instead
  of reaching an executable artifact literally.
- Scope-lock plugins.options to user config; it feeds executable plugin
  MCP/hook artifacts and must not be repo-controlled.
- Only mark a reviewer ```report block parsed when findings is a list;
  raise on unknown eval-case names and wrap gate_report in the CLI error
  boundary; trim plugin dependency identifiers before exact-name matching.

Add/adjust tests for rollback, fail-soft hook substitution, the unknown
eval-case error, and the new report-parse and MCP-refresh contracts.

Author: elkaix

CHANGELOG.md

@@ -20,8 +20,8 @@ GitHub Releases page; `0.8.0` is the new starting line.
   installed for Claude Code or Codex are auto-detected (no symlink): their safe artifacts
   (skills, commands, agents) activate by default, while executable artifacts (hooks, MCP
   servers) stay opt-in. Config `plugins.discover_external`, `plugins.external_exec`,
-  `plugins.enabled`, and `plugins.disabled` — plus `pythinker plugin enable/disable
-  <name>` — control which installed plugins contribute artifacts. Hook and MCP commands
+  `plugins.enabled`, and `plugins.disabled` — plus `pythinker plugin enable/disable <name>`
+  — control which installed plugins contribute artifacts. Hook and MCP commands
   expand `${CLAUDE_PLUGIN_ROOT}`/`${PYTHINKER_PLUGIN_ROOT}` and
   `${CLAUDE_PLUGIN_DATA}`/`${PYTHINKER_PLUGIN_DATA}`.
 - **Plugin dependencies.** Plugins may declare `dependencies`; installing one pulls its

docs/en/release-notes/changelog.md

@@ -17,6 +17,48 @@ GitHub Releases page; `0.8.0` is the new starting line.
 
 ## Unreleased
 
+- **Plugin marketplaces and activation policy.** `pythinker plugin marketplace` can add,
+  refresh, install, and uninstall Claude/Codex-compatible marketplace plugins. Plugins
+  installed for Claude Code or Codex are auto-detected (no symlink): their safe artifacts
+  (skills, commands, agents) activate by default, while executable artifacts (hooks, MCP
+  servers) stay opt-in. Config `plugins.discover_external`, `plugins.external_exec`,
+  `plugins.enabled`, and `plugins.disabled` — plus `pythinker plugin enable/disable <name>`
+  — control which installed plugins contribute artifacts. Hook and MCP commands
+  expand `${CLAUDE_PLUGIN_ROOT}`/`${PYTHINKER_PLUGIN_ROOT}` and
+  `${CLAUDE_PLUGIN_DATA}`/`${PYTHINKER_PLUGIN_DATA}`.
+- **Plugin dependencies.** Plugins may declare `dependencies`; installing one pulls its
+  transitive dependencies from the same marketplace (cross-marketplace deps are blocked),
+  and a plugin whose dependencies aren't present+enabled is disabled at load instead of
+  half-activating.
+- **Plugin options (`userConfig`).** `${user_config.KEY}` in a plugin's MCP server configs
+  and hook commands is filled from `[plugins.options.<plugin>]` config; an artifact that
+  references an unconfigured option is skipped rather than run blank. (Content substitution,
+  `PYTHINKER_PLUGIN_OPTION_*` hook env vars, and keychain-backed sensitive storage are not
+  yet implemented.)
+- **MCP tool lists refresh automatically when servers change.** Connected MCP
+  sessions stay open for `tools/list_changed` (and resources/prompts) notifications;
+  inventory is re-published without a manual `/mcp refresh`.
+- **Shell live token readouts track output throughput.** The spinner and background
+  status line show session-wide output tokens produced during the current turn or
+  background stretch instead of the context-size snapshot.
+- **MCP servers can be managed without a full reload.** `/mcp disconnect`, `/mcp reconnect`, and
+  `/mcp refresh` (or `retry`) update the live toolset for one server; disconnect unregisters its
+  tools and marks the server failed until reconnect.
+- **Recall search matches session ids and plan slugs.** Prior-session keyword search now indexes
+  `session_id` and `plan_slug` in addition to titles so agents can find plan-linked sessions by slug.
+- **Wire and ACP surfaces now get max-steps handoff summaries.** When a turn hits the step ceiling,
+  wire clients receive a streamed handoff event plus a `handoff` field on the `max_steps_reached`
+  result; ACP sessions emit the same summary text before returning `max_turn_requests`.
+- **MCP CLI commands resolve normalized server names.** `mcp remove`, `mcp auth`, `mcp test`, and
+  `reset-auth` accept display names with spaces or slashes and map them to stored config keys; config
+  load applies the same normalization as add.
+- **Compaction failure circuit breaker respects thresholds above one.** A proactive compaction
+  failure below `max_compaction_failures` no longer aborts the turn; the handoff fires only after
+  the configured number of consecutive failures.
+- **AI eval gate schema is self-contained under `tests_ai/`.** Shared eval-case types live in
+  `tests_ai/eval_schema.py` so isolated `tests_ai` runs do not import from `tests_e2e`.
+- **Softer TUI chrome in the dark theme.** Panel borders (welcome banner, menus) and the input-area
+  rules now render in a mid grey (`#8a8d91`) instead of near-white, for a less glaring look.
 - **Stop-time memory extraction can now be enabled explicitly.** Added an opt-in
   `memory.harvest_on_stop` setting that stages safe assistant decisions, blockers, evidence, and
   next steps into the existing scratchpad recall flow at turn end without writing directly to
@@ -25,6 +67,28 @@ GitHub Releases page; `0.8.0` is the new starting line.
   `ToolSearch` plus root-session `EnterWorktree` and `ExitWorktree` tools so agents can find
   currently available capabilities by keyword and isolate a session's operational working directory
   in a git worktree without deleting user work on exit.
+- **Root sessions now get a bounded git snapshot in the prompt.** When `git_status_injection` is
+  enabled (default), the agent receives branch, dirty-file summary, and recent commits as an
+  explicitly stale point-in-time reminder; disable via config or set `git_status_injection = false`.
+- **Context compaction and MCP tool registration now fail more predictably.** Proactive compaction
+  failures hand back with an explicit `compaction_failed` stop instead of bubbling an unstructured
+  loop error, and MCP duplicate tool-name resolution now follows configured server order instead of
+  connection completion order. Tool hooks also retain the original model input even if a tool
+  mutates a nested argument object during execution.
+- **Recall can now read bounded transcript windows.** `Recall(mode="read")` accepts
+  `message_offset` and `max_messages` so agents can inspect a precise, sanitized slice of a prior
+  workspace session without pulling the whole transcript into context.
+- **MCP prompt templates can now be invoked from connected servers.** `InvokeMcpPrompt` renders a
+  server-published prompt with structured arguments and wraps the returned messages as untrusted
+  input for the model.
+- **Telemetry, MCP config, and shell UX hardening.** Tool spans and metrics sanitize MCP/plugin
+  names; `mcp.json` load paths inject docker `--rm` and normalize server keys with collision
+  errors; shell suggestions accept via Alt+S into the prompt; markdown agent frontmatter maps
+  `max_turns`/`disallowed_tools`; `ReadMediaFile` enforces per-kind byte/pixel caps; written plans
+  without a Verification section get a soft warning; AI eval budgets can gate `tests_ai` reports.
+- **Plan-mode exit guidance now requires verification.** The `ExitPlanMode` tool now tells agents
+  that written plans must include a Verification section with the smallest command, test, or check
+  for each meaningful change.
 - **Agent-loop observability now emits explicit Wire events for key runtime state.** Added
   `TodoListUpdated`, `SubagentToolFallback`, `AgentListDelta`, `ToolUseSkipped`, and
   `ContextOverflowRecovered` events, with todo updates, subagent launch fallbacks, same-step tool

src/pythinker_code/config.py

@@ -57,6 +57,9 @@ def find_project_root(cwd: Path) -> Path | None:
         ("providers",),  # contains api_key per provider — must stay in user scope
         ("services",),  # contains api_key fields — must stay in user scope
         ("feedback", "api_key"),  # only the key, not the whole feedback section
+        # Substituted into executable plugin artifacts (MCP server configs and hook
+        # commands); a repo-controlled project config must not steer those values.
+        ("plugins", "options"),
         # Auto-executed when the shell starts — a repo-controlled project config
         # must never be able to choose the binary that runs (`command`), nor to
         # trigger or extend its execution (`enabled`/`segments` flip the command

src/pythinker_code/plugin/install.py

@@ -33,6 +33,7 @@
     load_installed_plugins,
     plugin_identifier,
     record_install,
+    remove_install,
 )
 from pythinker_code.plugin.manifest import (
     MarketplaceEntry,
@@ -280,12 +281,48 @@ def install_plugin_from_marketplace(
 
     manifest, marketplace_root = _load_marketplace(marketplace_name, known[marketplace_name])
     installed: dict[str, InstalledRecord] = {}
-    _install_with_deps(
-        plugin_name, marketplace_name, manifest, marketplace_root, scope, installed, []
-    )
+    # Records materialized so far, oldest first. If dependency resolution fails
+    # partway through (e.g. a cross-marketplace dep), every plugin already written
+    # to disk/registry is rolled back so a failed install never leaves partial state.
+    materialized: list[tuple[str, InstalledRecord]] = []
+    try:
+        _install_with_deps(
+            plugin_name,
+            marketplace_name,
+            manifest,
+            marketplace_root,
+            scope,
+            installed,
+            [],
+            materialized,
+        )
+    except Exception:
+        _rollback_installs(marketplace_name, materialized)
+        raise
     return installed[plugin_name]
 
 
+def _rollback_installs(
+    marketplace_name: str, materialized: list[tuple[str, InstalledRecord]]
+) -> None:
+    """Undo partially completed installs (best-effort), newest first.
+
+    Runs while unwinding a failed install, so a cleanup error must not mask the
+    original failure — each step is logged and continued rather than raised.
+    """
+    for name, record in reversed(materialized):
+        ident = plugin_identifier(name, marketplace_name)
+        try:
+            _remove_path(Path(record.install_path))
+            remove_install(name, marketplace_name, scope=record.scope)
+        except Exception as exc:
+            logger.warning(
+                "Could not fully roll back partial install of {id}: {error}",
+                id=ident,
+                error=exc,
+            )
+
+
 def _materialize_and_record(
     plugin_name: str,
     marketplace_name: str,
@@ -333,6 +370,7 @@ def _install_with_deps(
     scope: str,
     installed: dict[str, InstalledRecord],
     in_progress: list[str],
+    materialized: list[tuple[str, InstalledRecord]],
 ) -> None:
     """Install a plugin and its transitive dependencies from the same marketplace.
 
@@ -352,6 +390,7 @@ def _install_with_deps(
     record = _materialize_and_record(
         plugin_name, marketplace_name, manifest, marketplace_root, scope
     )
+    materialized.append((plugin_name, record))
     for dep in _installed_manifest_deps(Path(record.install_path)):
         dep_name, dep_marketplace = parse_plugin_identifier(dep)
         if dep_marketplace is not None and dep_marketplace != marketplace_name:
@@ -365,7 +404,14 @@ def _install_with_deps(
         ):
             continue
         _install_with_deps(
-            dep_name, marketplace_name, manifest, marketplace_root, scope, installed, in_progress
+            dep_name,
+            marketplace_name,
+            manifest,
+            marketplace_root,
+            scope,
+            installed,
+            in_progress,
+            materialized,
         )
     in_progress.remove(plugin_name)
     installed[plugin_name] = record

src/pythinker_code/plugin/installed.py

@@ -15,6 +15,7 @@
 from pydantic import BaseModel, ConfigDict, Field
 
 from pythinker_code.plugin.directories import installed_plugins_file
+from pythinker_code.utils.io import file_lock
 from pythinker_code.utils.logging import logger
 
 INSTALLED_SCHEMA_VERSION = 2
@@ -90,37 +91,38 @@ def save_installed_plugins(plugins: dict[str, list[InstalledRecord]]) -> None:
 def record_install(name: str, marketplace: str, record: InstalledRecord) -> None:
     """Add or replace an install record for ``name@marketplace`` in its scope.
 
-    ponytail: unlocked read-modify-write. ``save_installed_plugins`` writes
-    atomically, so a single writer never corrupts the file; the residual risk is
-    two concurrent ``pythinker plugin`` processes losing one update — rare for a
-    CLI. Add cross-process file locking here (and in ``marketplace.py``) if
-    concurrent installs become a real workflow.
+    The load → mutate → save runs under a cross-process lock so concurrent
+    ``pythinker plugin`` invocations (or a session overlapping a CLI install)
+    can't drop each other's registry updates.
     """
-    plugins = load_installed_plugins()
-    ident = plugin_identifier(name, marketplace)
-    existing = [r for r in plugins.get(ident, []) if r.scope != record.scope]
-    plugins[ident] = [*existing, record]
-    save_installed_plugins(plugins)
+    with file_lock(installed_plugins_file()):
+        plugins = load_installed_plugins()
+        ident = plugin_identifier(name, marketplace)
+        existing = [r for r in plugins.get(ident, []) if r.scope != record.scope]
+        plugins[ident] = [*existing, record]
+        save_installed_plugins(plugins)
 
 
 def remove_install(name: str, marketplace: str, *, scope: str | None = None) -> bool:
     """Remove install records for a plugin (optionally only one scope).
 
-    Returns True if anything was removed.
+    Returns True if anything was removed. The read-modify-write runs under the
+    same cross-process lock as :func:`record_install`.
     """
-    plugins = load_installed_plugins()
-    ident = plugin_identifier(name, marketplace)
-    if ident not in plugins:
-        return False
-    if scope is None:
-        del plugins[ident]
-    else:
-        kept = [r for r in plugins[ident] if r.scope != scope]
-        if len(kept) == len(plugins[ident]):
+    with file_lock(installed_plugins_file()):
+        plugins = load_installed_plugins()
+        ident = plugin_identifier(name, marketplace)
+        if ident not in plugins:
             return False
-        if kept:
-            plugins[ident] = kept
-        else:
+        if scope is None:
             del plugins[ident]
-    save_installed_plugins(plugins)
-    return True
+        else:
+            kept = [r for r in plugins[ident] if r.scope != scope]
+            if len(kept) == len(plugins[ident]):
+                return False
+            if kept:
+                plugins[ident] = kept
+            else:
+                del plugins[ident]
+        save_installed_plugins(plugins)
+        return True

src/pythinker_code/plugin/integration.py

@@ -134,22 +134,24 @@ def plugin_mcp_servers(policy: PluginPolicy | None = None) -> dict[str, object]:
     """
     servers: dict[str, object] = {}
     for plugin in _enabled_plugins(policy, include_external=_exec_external(policy)):
-        options = _plugin_options(plugin, policy) if plugin.manifest.user_config else None
+        # Always validate ${user_config.*} references, even when the manifest
+        # declares no userConfig: an unresolved placeholder must fail-soft (skip
+        # the artifact) rather than reach an executable command literally.
+        options = _plugin_options(plugin, policy)
         for key, value in artifacts.mcp_servers(plugin).items():
             expanded = _map_strings(value, partial(_expand_plugin_vars, plugin=plugin))
-            if options is not None:
-                try:
-                    expanded = _map_strings(
-                        expanded, partial(substitute_user_config_vars, values=options)
-                    )
-                except UserConfigError as exc:
-                    logger.warning(
-                        "Skipping MCP server {key} from {plugin}: unconfigured user_config {error}",
-                        key=key,
-                        plugin=plugin.name,
-                        error=exc,
-                    )
-                    continue
+            try:
+                expanded = _map_strings(
+                    expanded, partial(substitute_user_config_vars, values=options)
+                )
+            except UserConfigError as exc:
+                logger.warning(
+                    "Skipping MCP server {key} from {plugin}: unconfigured user_config {error}",
+                    key=key,
+                    plugin=plugin.name,
+                    error=exc,
+                )
+                continue
             servers.setdefault(key, expanded)
     return servers
 
@@ -173,7 +175,7 @@ def _hooks_payload(plugin: LoadedPlugin) -> dict[str, Any] | None:
 
 
 def _translate_hook_defs(
-    plugin: LoadedPlugin, payload: dict[str, Any], options: dict[str, object] | None
+    plugin: LoadedPlugin, payload: dict[str, Any], options: dict[str, object]
 ) -> list[HookDef]:
     """Translate a Claude-style hooks mapping into pythinker ``HookDef`` entries.
 
@@ -206,17 +208,16 @@ def _translate_hook_defs(
                 if entry_d.get("type", "command") != "command" or not isinstance(command, str):
                     continue
                 expanded = _expand_plugin_vars(command, plugin)
-                if options is not None:
-                    try:
-                        expanded = substitute_user_config_vars(expanded, options)
-                    except UserConfigError as exc:
-                        logger.warning(
-                            "Skipping {event} hook in {plugin}: unconfigured user_config {error}",
-                            event=event,
-                            plugin=plugin.name,
-                            error=exc,
-                        )
-                        continue
+                try:
+                    expanded = substitute_user_config_vars(expanded, options)
+                except UserConfigError as exc:
+                    logger.warning(
+                        "Skipping {event} hook in {plugin}: unconfigured user_config {error}",
+                        event=event,
+                        plugin=plugin.name,
+                        error=exc,
+                    )
+                    continue
                 timeout = entry_d.get("timeout")
                 try:
                     defs.append(
@@ -245,6 +246,6 @@ def plugin_hook_defs(policy: PluginPolicy | None = None) -> list[HookDef]:
     for plugin in _enabled_plugins(policy, include_external=_exec_external(policy)):
         payload = _hooks_payload(plugin)
         if payload is not None:
-            options = _plugin_options(plugin, policy) if plugin.manifest.user_config else None
+            options = _plugin_options(plugin, policy)
             defs.extend(_translate_hook_defs(plugin, payload, options))
     return defs

src/pythinker_code/plugin/manifest.py

@@ -82,14 +82,24 @@ def _normalize_dependencies(value: Any) -> list[str]:
     out: list[str] = []
     for entry in cast("list[object]", value):
         if isinstance(entry, str):
-            out.append(_DEP_VERSION_SUFFIX.sub("", entry))
+            # Trim surrounding whitespace: downstream matching is exact-name, so a
+            # stray " lib " would be mismatched and falsely demoted as missing.
+            dep = _DEP_VERSION_SUFFIX.sub("", entry).strip()
+            if dep:
+                out.append(dep)
         elif isinstance(entry, dict):
             entry_d = cast("dict[str, object]", entry)
             name = entry_d.get("name")
             if not isinstance(name, str):
                 continue
+            name = name.strip()
+            if not name:
+                continue
             marketplace = entry_d.get("marketplace")
-            out.append(f"{name}@{marketplace}" if isinstance(marketplace, str) else name)
+            if isinstance(marketplace, str) and marketplace.strip():
+                out.append(f"{name}@{marketplace.strip()}")
+            else:
+                out.append(name)
     return out