Skip to content

Commit 5e50508

Browse files
authored
feat: agent harness enhancements (#122)
* fix(tools): wrap FetchURL output as untrusted after truncation FetchURL wrote pre-rendered <untrusted_data> envelopes into the ToolResultBuilder, so when a page exceeded the builder's character limit, truncation cut off the closing </untrusted_data> tag. The model then received an unterminated untrusted-data envelope (the exact failure mode the wrapper exists to prevent), and strip_untrusted_envelope could not strip the torn envelope, so the raw envelope leaked into display/UI paths. Write the raw text and call builder.mark_untrusted() instead (the same idiom SearchWeb uses), so wrapping happens after truncation in ok() and the closing tag can never be cut. Applied to all three write sites: the verbatim text/plain+markdown path, the trafilatura extraction path, and the fetch-service path. The spill file now holds raw unwrapped output, matching the documented spill contract. Regression tests cover all three paths with >50k-char pages that trigger builder truncation, asserting the envelope stays well-formed and strip_untrusted_envelope round-trips. * refactor(soul): expose public turn() contract for external drivers External drivers (FlowRunner._flow_turn, the /goal and /learn slash handlers) were calling the private PythinkerSoul._turn directly, each carrying a pyright reportPrivateUsage suppression. Add a public turn() method that documents the single-turn contract: one user message in, one full agent turn out (model steps plus tool calls until the model stops), with TurnOutcome conveying stop_reason (no_tool_calls / tool_rejected / stuck), the final assistant message, and step count. turn() does not emit TurnBegin/TurnEnd wire framing; callers frame the turn themselves, as run() does. turn() is a thin delegate to _turn on purpose: many tests monkeypatch soul._turn to stub turn execution, so _turn stays the single implementation/patch point and those patches keep intercepting turns started through turn(). The three external call sites now use turn() with the suppressions removed; internal self._turn calls are unchanged, and the runtime_checkable Soul protocol is deliberately untouched. * refactor(soul): replace string-matched tool gates with declarative flags Two gates identified special tool classes by mechanisms that break invisibly on rename/move: - check_tool_call_allowed (permission.py) and _is_external_side_effect_tool (toolset.py) matched external adapters (MCPTool, WireExternalTool, PluginTool) by module/qualname strings. This is a permission gate that FAILS OPEN: moving or renaming any of these classes silently drops them out of external-tool permission gating with no test failure. A planned refactor moves MCPTool out of toolset.py, so the trap is defused first. - _tool_defers_execution_started duck-typed on the private _approval attribute to decide whether ToolExecutionStarted is deferred until after approval. Both gates now read explicit class-level flags instead: - external_side_effect_tool (ClassVar) on MCPTool, WireExternalTool, and PluginTool, documented as a security contract and pinned by tests so a future move/rename that loses the flag fails CI instead of failing open. - emits_tool_execution_started_after_approval on every approval-gated tool class (Shell, WriteFile, StrReplaceFile, TaskInput, TaskStop, Terminal, PluginTool), matching the existing RunAgentsTool precedent; the hasattr(_approval) fallback is removed. Routing is behavior-preserving: the same tool classes pass through the same gates before and after, and the Shell/network branches keep priority in check_tool_call_allowed. * docs(tasks): record design adoption blueprint and branch task log Add the verified design-adoption blueprint (ranked, review-caveated refactor plan for cleaner agent/runtime layering) and update the task log with this branch's three landed tasks, their review outcomes, and the deferred follow-ups, including the known machine-local PTY shell-cancel test failure verified pre-existing on main. * feat: sharpen orchestration guidance and genericize design comments Add a root-only OrchestrationInjectionProvider that nudges substantial normal-mode tasks toward the lightest effective work shape (direct tools, SetTodoList, foreground RunAgents, verification), throttled via a history-scanned reminder marker and suppressed under plan/auto/goal/subagent modes. Sharpen the matching system-prompt guidance, refresh a feature tip to promote /goal, and rephrase design-source comment attributions as generic agent-enhancement notes. * feat(acp): stop advertising the question tool to ACP clients ACP sessions cannot present interactive questions (the session loop signals QuestionNotSupported), so advertising AskUserQuestion invites a wasted model step per question. replace_tools now hides the tool from the model-facing list while keeping it registered, so a stray call still resolves through the graceful textual fallback. Harmonize the task log after merging refactor/agent-contract-and-tool-metadata. * fix: apply review-deferred fixes from the contract/metadata work - FetchURL: await spill_to_disk() at the trafilatura and fetch-service sites so large pages spill off the event loop instead of falling back to the synchronous spill inside ok(). - MCPTool: declare emits_tool_execution_started_after_approval so the ToolExecutionStarted event defers until approval resolves, matching every other approval-gated tool; the old _approval duck-typing missed this class because it requests via runtime.approval. Pinned in test_toolset.py. - spinner_words: genericize remaining external credit wording. - tasks/todo.md: record the fixes; document why structural flag enforcement for future adapters is deferred (no shared adapter base until the toolset split lands). * docs(tasks): add verified agent-harness adoption plan (124 ranked items) Synthesized from a 14-cluster map+adversarial-verify workflow comparing the local reference agent harness against src/pythinker_code. Each item records current state, verifier evidence, an adoption sketch fitted to pythinker's design, effort/value, and target files; three refuted claims are pinned so they are not re-implemented. Includes execution discipline for the multi-writer branch (checkpoint = TDD + clean-code-guard + green gates, hot-file serialization). * feat(soul): repair tool call/result pairing at context restore A crash between persisting an assistant tool-call message and its tool results leaves context.jsonl with a dangling call or an orphaned result; after resume every provider request then fails with a pairing error. restore() and revert_to() now run a pure repair pass that synthesizes an explicit lost-result message for unpaired calls and drops orphaned or duplicate tool results, logging each repair. The file is never rewritten, so re-repair on each restore stays idempotent. Plan item: context-mgmt/history-invariant-repair (Tier 1). * feat(soul): decision-complete planning protocol in plan-mode reminders Plan mode now teaches a two-kinds-of-unknowns rule (explore repo-discoverable facts yourself; surface preference/scope decisions early via AskUserQuestion with a recommended default), records unanswered defaults under an Assumptions section, gates ExitPlanMode on a decision-complete plan (no decisions left to the implementer), and adds a plan-file brevity rubric (3-5 short sections, subsystem-grouped bullets). Phrase pins lock the new clauses into all three reminder variants. Plan item: prompts-instructions/decision-complete-plan-mode (Tier 1). * feat(print): strict stdout/stderr channel discipline on failure paths Headless failure diagnostics (provider errors, max-steps + handoff, interrupt, unknown errors) printed plain text to stdout, corrupting the stream-json channel for machine parsers. All diagnostics now route to the pre-redirect stderr fd (falling back to sys.stderr), and stream-json mode additionally emits one structured error record — a Notification with category=run, type=error, the failure class, and the exit code — written via raw stdout so rich cannot soft-wrap the JSON line. Plan item: protocol-headless/channel-discipline (Tier 1). * feat(subagents): inject merge-base-scoped git context into review agents Review-class subagents (review, code_reviewer, security_reviewer) received scope purely via the parent's prompt text and burned their first turns rediscovering branch, dirty files, and the merge base. The git-context prefix now also resolves the merge base against the first existing base ref (origin/main, main, master), names the exact review scope (git diff <sha>...HEAD), omits it when HEAD is the base, and is injected for reviewer-class agents alongside explore. Plan item: review-mode/deterministic-review-target-resolution (Tier 1, agent-dispatch slice). * feat(soul): same-step concurrency policy for parallel tool calls Provider-emitted parallel tool calls all executed concurrently — two mutating tools (WriteFile + Shell from one assistant message) could race with no ordering guarantee. Tool dispatch now runs through a reader-writer gate: tools declaring supports_parallel (read-only builtins: ReadFile, ReadMediaFile, Glob, Grep, SmartSearch, Think, Recall, ListMcpResources, ReadMcpResource, SearchWeb, FetchURL) overlap freely, while everything else — including unflagged plugin/MCP tools, the safe default — executes exclusively in dispatch order. Writers drain in-flight readers and cannot be starved. Plan item: tools-registry-codemode/concurrency-policy (Tier 1). * feat(soul): reactive context-overflow recovery (compact-and-retry) A provider context-length 400 was telemetry-classified but treated as fatal: the step raised and the turn died, even though the proactive prune/compact thresholds run on heuristic counts that can undercount. Two recovery layers, both bounded: - Agent loop: on a context_overflow-classified step error, prune (best-effort), force a full compaction, and retry the step — once per turn; telemetry records recovered vs failed. - SimpleCompaction: the compaction request itself carries the whole to-compact slice and can overflow too; on a context-length rejection it drops the oldest half and retries, terminally falling back to the preserved tail plus an explicit dropped-context note. classify_api_error moves to soul/api_errors.py (re-exported from pythinkersoul) so compaction can classify without a circular import. Plan items: core-loop/reactive-overflow-recovery + context-mgmt/context-overflow-recovery (Tier 1). * docs(tasks): record Tier-1 adoption progress and next M-item queue * feat(config): gate project-scope hooks behind durable per-project trust A cloned repository's .pythinker/config.toml merged unconditionally, so its [[hooks]] shell commands auto-executed at session start — arbitrary code execution from cloning a repo. Project/local-scope hooks now load only after the user records trust: - New project_trust store (user-scope trusted_projects.json, atomic writes, fail-closed on corruption) keyed by the resolved repo root, so the repo itself can never grant its own trust. - _load_scoped strips hooks from untrusted project/local scopes with a warning naming /trust as the fix; broken TOML in an untrusted project degrades to an empty scope instead of blocking startup (trusted projects keep the loud error). - /trust on|off persists the per-project decision alongside the session flags and points at /reload for hook activation. - find_project_root promoted to public API (the /trust path needs it). Plan item: config-features/per-project-trust-gating (Tier 1). Out of scope (own plan item): sanitize-and-warn for scope-locked keys in untrusted scopes — they keep the existing loud ConfigError. * feat(config): warn on unknown config keys with source-located diagnostics Config models ignore extra keys, so a typo'd key silently vanished and changed behavior with no signal. After merge, the raw dict is now diffed against the model field tree (aliases and AliasChoices honored; recursion follows provable shapes only — nested models, dict-of-model maps, lists of models — so unmodellable values can never false- positive). Each finding warns with the dotted path and the scope file it came from via the existing provenance map; PYTHINKER_STRICT_CONFIG=1 escalates to ConfigError for CI use. Plan item: config-features/unknown-config-key-detection (Tier 1). * docs(tasks): record trust-gating and unknown-key checkpoints * feat(shell): carry a conversation summary across /model switches /model discarded the entire conversation by starting a fresh session. The switch now summarizes the outgoing session with the OUTGOING model — only plain text crosses the model boundary, so the incoming provider never sees foreign thinking blocks or tool-call schemas — and seeds the new session's context with it before Reload. Best-effort with a start-fresh fallback on empty history, summarization failure, or model_switch_carryover=false. SimpleCompaction gains summarize_all() (no preserved tail) atop the extracted overflow-halving summarizer. Plan item: core-loop/model-switch-context-continuity (Tier 1). * docs(tasks): record model-switch carry-over checkpoint * feat(shell): elide approval prompts for provably read-only commands The first ls or git status of a session always interrupted the user with an approval dialog. soul/permission.py gains is_known_safe_command(): a positive allowlist, fail closed — the mutation guard's hidden-command/substitution/newline, write-redirection, and network/mutation rejections run first, then every ;/&&/||/| segment must start with an allowlisted read-only binary or read-only git subcommand (--output rejected). Wrappers (sudo/env/time) are never unwrapped, and absolute command paths must live in a system bin dir so a workspace-local fake git cannot ride its basename onto the allowlist. Shell consults it only in the root agent (subagent approval requests stay — they are part of the unattended-denial defense surface) and only after the deny gate, so elision can never override a deny. Elisions are tracked in telemetry; the started event fires at the elision point. Plan item: exec-safety/known-safe-command-auto-approval (Tier 1). * docs(tasks): record safe-command elision checkpoint * fix(security): allowlist inline env prefixes on the elision path The safe-command elision accepted any KEY=VALUE prefix, so PATH=/tmp/evil ls would resolve ls from the attacker directory — defeating the system-bin pinning — and LD_PRELOAD/DYLD_*/GIT_PAGER prefixes could inject code into otherwise read-only commands. Only harmless locale/timezone assignments (LANG/LC_*/TZ) may now prefix an elidable command; every other assignment fails closed to the normal approval prompt. Flagged by automated security review. * test(e2e): pin shell approval round-trip with non-elidable commands The approval-protocol e2e tests drove Shell with 'echo ok', which the new known-safe elision now runs without a prompt — the round-trip these tests exist to pin never started. 'env echo ok' keeps stdout identical while the wrapper prefix disqualifies elision, so the approval exchange still exercises request/approve/reject. Fallout from 5dc87aa (caught by the full tests_e2e scope). * feat(mcp): per-server startup timeout with actionable failure diagnostics A hung MCP connect left the server in 'connecting' forever and blocked every agent turn (the loop awaits MCP loading with no bound). Connect + inventory is now wrapped in asyncio.wait_for governed by a new mcp.client.startup_timeout_ms (default 30s), and every connect failure is classified into one short actionable line — timeout names the config knob, 401/unauthorized names the exact 'pythinker mcp auth' command, ENOENT names the missing binary — carried on MCPServerInfo and MCPServerSnapshot and rendered by /mcp instead of a bare 'failed'. Plan item: mcp/per-server-startup-timeout-diagnostics (Tier 1). * test: refresh wire snapshots for the MCP server error field The serde and e2e snapshots pin wire-model dumps; the new optional MCPServerSnapshot.error field appears as null in them. Applied via --inline-snapshot=fix (deliberate, follows 05f8642). * docs(tasks): record elision and MCP-timeout checkpoints * feat(mcp): per-server tool allow/deny filtering (enabledTools/disabledTools) A server listing 30 tools floods the model tool list with all of them. mcp.json server entries now accept optional enabledTools (exclusive allowlist) and disabledTools (denylist, wins on conflict): filtered tools are skipped at connect time — never registered in the toolset or runtime.mcp_tools — and MCPTool re-checks membership at call time as defense in depth for tool maps shared with subagents and future live tool-list updates. No filter fields keeps today's permissive behavior. Plan item: mcp/per-server-tool-allow-deny-filtering (Tier 1). * docs(tasks): record MCP tool-filtering checkpoint * feat(subagents): spawn-time context fork for foreground agents New children started blank, relying on the orchestrator hand-writing a context packet into every prompt. Agent(fork_context=true) now seeds a new foreground child with the parent's conversational spine — user requests and assistant text, with tool traffic (whose call/result pairing would dangle), thinking parts, injected reminders/notifications, and checkpoint markers all filtered out. The fork reads the persisted parent context (inheriting the restore-time pairing repair) and seeds the child's own context file, so resume keeps working unchanged. Invalid with resume or run_in_background (background fork is a tracked follow-up); read failures degrade to a blank child rather than failing the spawn. Tool/agent schema snapshots refreshed. Plan item: multi-agent/spawn-time-context-fork (Tier 1, foreground slice). * docs(tasks): record context-fork checkpoint * docs(tasks): worktree-isolation design note (re-sized M→L, phased seam plan) The audit found ~94 work-dir consumer sites and shared session/ builtin_args across child runtimes; honoring isolation=worktree without a single work-dir seam first would yield false isolation. Phases: P1 mechanical Runtime.work_dir seam, P2 worktree lifecycle in the background runner (create/redirect/report/cleanup, non-git rejection), P3 RunAgents batch reuse. * refactor(soul): Runtime.work_dir seam for worktree isolation (P1) Operational cwd/path-resolution sites (26 across tools, soul, permission, app, UI) now read runtime.work_dir — work_dir_override or the session's — instead of reaching through runtime.session.work_dir. copy_for_subagent accepts work_dir_override (re-rendering the child's PYTHINKER_WORK_DIR/_LS prompt args) and propagates it to grandchildren; the shared session keeps owning persistence paths. Behavior-preserving with no override set; full suite green (5381 local + e2e, the two TimeoutError wire tests verified pre-existing/machine-local on the stashed tree). Phase P1 of tasks/worktree-isolation-design.md; P2 wires the worktree lifecycle into the background runner. * docs(tasks): record work-dir seam (P1) checkpoint * feat(subagents): enforce worktree isolation for background write agents (P2) isolation='worktree' only recorded intent; parallel coder/implementer children shared one working tree and could clobber each other. The background runner now creates a detached git worktree of HEAD per write-profile child under <session>/worktrees/<agent_id>, points the child runtime at it through the P1 work_dir seam (prompt work-dir args re-rendered), and on completion appends the worktree path plus a diff summary to the final report so the orchestrator merges deliberately. Clean worktrees are removed; changed or failed ones are retained. Non-git roots fail before any model spend with an actionable error; read-profile children log and ignore the request; resume reuses the existing worktree. Local subprocesses are safe here — the manager enforces a local backend for agent tasks. Phase P2 of tasks/worktree-isolation-design.md; P3 (RunAgents batch) remains. * docs(tasks): record worktree-isolation P2 checkpoint * feat(subagents): document enforced isolation on RunAgents batches (P3) The RunAgents → Agent → create_agent_task → BackgroundAgentRunner chain already threads isolation per child, so P2 enforcement covers batch fan-outs; the parameter description now states the enforced semantics (per-child worktrees, diff-summary reports, deliberate merging) instead of 'records an intent'. Closes tasks/worktree-isolation-design.md. * docs(tasks): close worktree-isolation item (P1-P3 complete) * feat(tools): graduated fuzzy-matching ladder for edit-location recovery Whitespace drift or smart-punctuation mismatch in StrReplaceFile's old string hard-failed with 'not found', burning a re-read + retry turn — the drift is invisible in numbered ReadFile output. After the exact match and CRLF fallback miss, a line-window seek now retries with graduated relaxations (trailing-whitespace -> indentation -> unicode-punctuation); the first firing tier replaces the ACTUAL matched file slice — never the needle text — adopting the slice's CRLF style and trailing newline, and the tool message names the relaxation. Ambiguity contract preserved: multiple fuzzy hits without replace_all error with the tier named. Deferred (low value): opt-in final-newline normalization for whole-file writes. Plan item: patch-file-tools/graduated-fuzzy-matching-ladder (Tier 1). * docs(tasks): record fuzzy edit-ladder checkpoint * feat(soul): live permissions-state injection (posture-fingerprinted) Enforcement was rich (profiles, safe mode, yolo/auto, session approvals, shlex-based command classification) but invisible prompt-side — the model discovered policy through denied tool calls. A new PermissionsInjectionProvider renders the enforced profile, posture flags, mutation/network allowances, session-approved actions, and the command-shaping rules the gate can actually classify. Fingerprinted on (profile, yolo, auto, safe_mode, approvals): re-emits exactly on posture changes, after compaction, and on auto toggles; root-only (subagent overlays already document their constraints). Approval gains read accessors is_safe_mode/session_approved_actions. History-shape test pins scoped to their subject; wire-session e2e snapshots refreshed. Plan item: prompts-instructions/dynamic-permissions-state (Tier 1). * docs(tasks): record permissions-state checkpoint * fix: apply external review findings across recent checkpoints - CRITICAL: GIT_CONTEXT_AGENT_TYPES used underscored reviewer names while registered type names are dashed (code-reviewer/security-reviewer), so reviewer agents silently missed the git-context injection; names fixed and a pin added asserting every gate name is a real profile key. - Foreground isolation requests now fail fast on Agent AND RunAgents instead of warning-and-proceeding unisolated (degraded behavior was presented as authoritative); warning pin updated to the new contract. - Unknown-config-key diagnostics now also run for explicit loads (--config-file / --config text) via single-source provenance. - Failure/timeout/cancel paths name the retained isolation worktree in the task output (retention is deliberate for resume, never silent). - Best-effort prune in overflow recovery logs its failure instead of contextlib.suppress. - supports_parallel flags annotated (: bool); test helpers cleaned (fail-fast _git asserts, unused _ListingClient params dropped). * fix: harden isolation, elision, and concurrency per adversarial arc review A 16-agent adversarial review (4 dimensions, every finding refuted-or- confirmed against live code) confirmed 11 findings; all fixed except one deliberate deferral (exclusive gate held across approval waits — needs the approval-split refactor; recorded in tasks/todo.md). - DATA LOSS (high): a child that committed its work left a clean worktree, so cleanup removed it and orphaned the commits. Creation now records a base-SHA sidecar (next to the worktree, never inside it); commits ahead of base count as changes and force retention, with unknown provenance failing closed to retention. - FALSE ISOLATION (high): foreground shell inherited the process cwd and relative file-tool paths resolved against it, so isolated children mutated the original repo. Host exec (protocol, local, ssh, ACP fallback) gained a cwd argument; foreground shell passes the runtime work dir, and write/replace/read resolve relative paths against it while preserving the relative-escape error contract. - REGRESSION (high): safe mode now disables the read-only-command prompt elision — users who disabled auto-approval keep every checkpoint. - REGRESSION (high): untrusted-project hook stripping now publishes a session notification (web/ACP visible), not just a shell log line. - MCP readOnlyHint annotations enable supports_parallel via property; worktree add/remove serializes per repo; CHANGELOG documents the same-step serialization and stderr-diagnostics behavior changes. * fix: resolve PR #122 review findings (CodeRabbit, CodeQL, typos) Address all bot review feedback on the agent-harness branch: Security / CI gates: - project_trust: store SHA-256 digests instead of clear-text paths (CodeQL clear-text-storage) and serialize read-modify-write behind a cross-process file lock; read legacy clear-text stores for compat. - typos: rename intentional config-key fixtures to validly-spelled unknown keys; fix `unparsable`/`default_yolo_typo` prose in planning doc. - api_errors: drop redundant `400 <= status < 500` guard (always true after the >=500 early return). Correctness: - ssh: resolve relative cwd against the host's tracked cwd, not the SSH login dir. - config: read untrusted project/local scopes independently so one bad file no longer discards the other. - context: re-repair the post-usage slice so token accounting reflects the repaired history; keep tool messages with no call id. - soul/agent + subagents/builder: recompute AGENTS.md payload for a child worktree override instead of inheriting the parent's. - subagents/core: import TextPart/ThinkPart from pythinker_core.message; collect git context from the effective child work dir. - subagents/runner: fail an explicit context fork loudly instead of silently degrading to a blank child. - subagents/worktree: validate a pre-existing dest is a registered worktree before reusing it. - background/agent_runner: report retained isolation worktrees on the early failure/empty-output exits too. - slash: persist the carry-over summary as a system turn, not a user turn. Safety hardening: - permission: refuse prompt elision for read-only commands with path-bearing operands (cat /etc/shadow, git -C /other, ../secret). - toolset: keep MCP tools exclusive in the same-step gate (ignore untrusted remote readOnlyHint); stage MCP inventory locally until connect succeeds. - read_media: keep ReadMediaFile serialized (large in-memory payloads). - permissions_state: include agent_execution_profile in the injection fingerprint so profile switches reinject. plan_mode: complete the truncated exit-rule sentence and wrap multi-line reminder literals in parentheses (fixes the implicit-concat warning without splitting sentences across rendered lines). Tests: cover the trust-store hashing/legacy path, the work_dir and runtime.work_dir seams by behavior not identity, the MCP exclusive default, legacy StatusUpdate deserialization, and the new unsafe path-operand commands; stop pinning full reminder text in loop tests. * fix(tui): hide thinking shimmer while a foreground tool runs The verb spinner (shimmering "Working…/Thinking…") was gated only on `_active_turn_depth > 0`, i.e. the whole turn. When the agent started a long-running foreground command — a dev server via npm/docker, a watch task — the agent coroutine just awaits the subprocess, but the shimmer kept animating for the full turn, falsely signalling active agent cognition. The tool card already shows an animated running marker plus streaming output, so the shimmer was redundant and misleading. Suppress the working indicator while any foreground tool is mid-execution (execution started, no result yet, not a detached background agent) on both render surfaces — the non-interactive Rich Live path and the interactive pinned status tail. The shimmer now means "the agent is thinking" and reappears the moment the command returns. Platform-agnostic: the root cause was turn-level gating, not Windows-specific. Adds `_ToolCallBlock.is_executing` and `_LiveView._foreground_tool_executing()`, and a test pinning that the pinned tail is empty mid-execution and returns once the tool finishes. * fix(api-errors): keep 4xx lower bound; drop redundant 500 check The earlier `if status < 500` simplification was behaviour-changing: after the `status >= 500` early return the upper bound is always true (CodeQL "redundant comparison" + "unreachable code" on the `return "api"` fallback), but dropping the `>= 400` lower bound also routed sub-400 statuses (e.g. the `status=0` default for non-HTTP-ish errors) into `4xx_client` instead of the generic `api` bucket. Use `if status >= 400` — equivalent to the original `400 <= status < 500` given the preceding return, with the `api` fallback reachable again for status < 400. Also parenthesize the remaining sparse plan-mode reminder concatenation so CodeQL's implicit-string-concatenation check stays quiet without splitting the line across the rendered output. * fix: address CodeRabbit re-review on harness changes - soul/agent: when overriding a child's work dir, only replace PYTHINKER_AGENTS_MD when an explicit value is provided; None now keeps the parent payload instead of silently clearing inherited context. - soul/context: drop tool results with no tool_call_id during pairing repair. Keeping them left malformed history that re-broke the next provider request — the exact failure the repair exists to prevent. - subagents/worktree: a failed `git worktree list` no longer collapses to "not a registered worktree" (which could send the operator to delete a path holding the child's only work); raise WorktreeError with the git stderr instead. - tests/memory: annotate the `_runtime` helper return type (ANN202). * test(context): use well-formed tool pairs in pending-token fixtures The drop of id-less tool results during pairing repair (previous commit) correctly removes malformed history, but three pending-token tests fed bare `tool` messages (no tool_call_id, no opening assistant tool call) as token ballast through the restore/repair path, so they now under-counted. Production tool results always carry the originating tool_call_id, so model the fixtures realistically: an assistant message that opens a tool call plus a paired tool result, both after the last `_usage`. They survive pairing repair and keep the pending estimate intact — exercising the post-`_usage` slice accounting without depending on malformed history.
1 parent 199cbe9 commit 5e50508

120 files changed

Lines changed: 6916 additions & 322 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,22 @@ GitHub Releases page; `0.8.0` is the new starting line.
1515

1616
## Unreleased
1717

18+
- **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.
19+
- **The agent now knows its own permission posture.** A live permissions-state reminder renders the enforced profile, safe-mode/yolo/auto flags, mutation/network allowances, session-approved actions, and the shell gate's command-shaping rules — re-emitted exactly when the posture changes (/yolo, /auto, /trust, new approvals) instead of the model discovering policy through denied tool calls.
20+
- **Edits recover from whitespace and smart-punctuation drift.** StrReplaceFile no longer hard-fails with "old string not found" when the only mismatch is trailing whitespace, indentation, or smart quotes/dashes: a graduated line-window ladder relocates the edit, replaces the actual file slice (preserving CRLF endings), and names the relaxation it used in the tool message. Multiple fuzzy hits without replace_all still error, so ambiguity is never silently resolved.
21+
- **`isolation="worktree"` is now enforced for background write agents.** Previously it only recorded intent, so parallel coders shared one working tree and could clobber each other. A write-profile child now runs in its own git worktree of HEAD; its final report names the worktree path with a diff summary so changes merge deliberately, clean worktrees are removed, non-git roots fail with an actionable error, and read-profile children ignore the request.
22+
- **MCP servers can be scoped to specific tools.** Optional `enabledTools` (exclusive allowlist) and `disabledTools` (denylist, wins on conflict) arrays per server in `mcp.json` keep a noisy server from flooding the model's tool list — filtered tools are never registered, and a call-time re-check guards shared tool maps.
23+
- **A hung MCP server can no longer stall the whole session.** Server connects are bounded by a new `mcp.client.startup_timeout_ms` (default 30s) — previously a hung connect blocked every agent turn. `/mcp` now shows one actionable line per failed server (timeout → the config knob, 401 → the exact auth command, missing binary → the command path) instead of a bare "failed".
24+
- **Provably read-only commands no longer prompt for approval.** The first `ls` or `git status` of a session used to interrupt with an approval dialog. A tight positive allowlist (read-only binaries and git subcommands, with hidden-command, write-redirection, wrapper, and fake-path rejections, fail closed) now elides the prompt in the root agent; subagents keep requesting approval as their unattended defense surface, and deny-profile decisions are never overridden.
25+
- **Switching models keeps your conversation.** `/model` used to start a fresh session, discarding all context. The switch now seeds the new session with a plain-text summary written by the outgoing model (so provider-specific message formats never cross the boundary), falling back to the old fresh start if summarization fails; disable with `model_switch_carryover = false`.
26+
- **Typo'd config keys are no longer silently ignored.** Loading now diffs the merged config against the model schema and warns with the dotted path and originating scope file for every unrecognized key (`default_yolo_typo = true` names itself and its file instead of silently changing nothing); `PYTHINKER_STRICT_CONFIG=1` escalates the findings to a startup error for CI.
27+
- **Project-scope hooks now require trusting the project.** A cloned repository's `.pythinker/config.toml` could previously auto-execute its shell hooks at session start. Hooks from project and local scopes now load only after `/trust` records a durable per-project decision (stored user-side in `trusted_projects.json`, keyed by the resolved repo root); until then they are stripped with a warning naming the fix. Broken TOML in an untrusted project no longer blocks startup — the scope is treated as empty with a warning, while trusted projects keep the loud error.
28+
- **Agent orchestration guidance is sharper for substantial tasks.** The default prompt now sharpens work-shaping guidance, and a new root-only runtime reminder nudges substantial normal-mode tasks toward the lightest effective path — direct tools, `SetTodoList`, foreground `RunAgents`, or verification — while backing off for plan mode, `/goal`, auto mode, and subagents.
1829
- **The in-app updater recovers from Homebrew's untrusted-tap refusal.** Homebrew 5.0 (`HOMEBREW_REQUIRE_TAP_TRUST`) refuses to load formulas from third-party taps until `brew trust <tap>` is run once, which made the in-app `brew upgrade` fail with only a generic "run manually" hint. The updater now detects both the hard `Refusing to load … from untrusted tap` refusal and the soft `Skipping … not trusted` warning, offers to run `brew trust pythoughts-labs/pythinker` and retry the upgrade once on an interactive terminal, and otherwise prints the exact remediation. It also catches the silent no-op where an untrusted tap is skipped during `brew update` and `brew upgrade` exits 0 without advancing the version.
1930
- **Session exports redact secrets surfaced by tool output.** A tool result (e.g. `grep`/`cat` over a `.env`) could write a secret value into an exported transcript in plaintext. `/export` now redacts the value of secret-named keys (`password`, `token`, `api_key`, `secret`, …) to `[REDACTED]` in both the markdown and YAML formats, while leaving non-secret keys such as `token_count`, usernames, and ports intact.
2031
- **The welcome logo's antenna blinks a fixed number of times on launch, then settles.** Replaces the terminal's indefinite slow-blink with a bounded boot animation — the antenna ball blinks seven times after the banner prints and then holds steady. It is skipped under reduced motion, on non-interactive output, and when the terminal is too short to keep the antenna row on screen.
2132
- **Inline `/command` references get acted on, not just explained away.** When a message mentions a slash command mid-sentence (e.g. "your `/goal` today is to `/plan` and build the page"), the command doesn't auto-run — but the agent no longer leads its reply by reporting it as failed. The per-turn reminder and the system prompt now steer the agent to act on the intent: call the real `EnterPlanMode` tool for `/plan` (clarified as a genuine, callable tool so models stop doubting it exists), pursue the described objective for `/goal`, load `/skill:<name>` via `ReadSkill`, and apply equivalent guidance for other commands — only surfacing how to invoke the literal command when genuinely needed.
33+
- **The "thinking" shimmer no longer runs while a foreground command does.** When the agent started a long-running foreground process — a dev server via `npm`/`docker`, a watch task — the shimmering verb spinner ("Working…/Thinking…") kept animating for the whole turn, implying the agent was busy when it was really just awaiting the subprocess. The spinner is now suppressed while any foreground tool is mid-execution; the tool card's own animated running marker (and its streaming output) carries the liveness, so the shimmer means "the agent is thinking" again and reappears the moment the command returns.
2234

2335
## 0.41.0 (2026-06-11)
2436

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ Pythinker loads [Model Context Protocol](https://modelcontextprotocol.io/) tools
562562
### 🛠️ Manage persistent MCP servers
563563

564564
```sh
565-
# 📚 Context7 stdio server (Codex-style: NAME -- COMMAND)
565+
# 📚 Context7 stdio server (positional form: NAME -- COMMAND)
566566
pythinker mcp add context7 -- npx -y @upstash/context7-mcp --api-key YOUR-API-KEY
567567
# Added MCP server 'context7' to ~/.pythinker/mcp.json
568568

packages/pythinker-host/src/pythinker_host/__init__.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,14 +220,18 @@ async def mkdir(
220220
"""Create a directory at the given path."""
221221
...
222222

223-
async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostProcess:
223+
async def exec(
224+
self, *args: str, env: Mapping[str, str] | None = None, cwd: str | None = None
225+
) -> HostProcess:
224226
"""
225227
Execute a command with arguments and return the running process.
226228
227229
Args:
228230
*args: Command and its arguments.
229231
env: Environment variables for the subprocess. If None, inherits
230232
from the parent process.
233+
cwd: Working directory for the subprocess. If None, inherits the
234+
backend's current working directory (process cwd locally).
231235
"""
232236
...
233237

@@ -347,8 +351,10 @@ async def mkdir(path: StrOrHostPath, parents: bool = False, exist_ok: bool = Fal
347351
return await get_current_host().mkdir(path, parents=parents, exist_ok=exist_ok)
348352

349353

350-
async def exec(*args: str, env: Mapping[str, str] | None = None) -> HostProcess:
351-
return await get_current_host().exec(*args, env=env)
354+
async def exec(
355+
*args: str, env: Mapping[str, str] | None = None, cwd: str | None = None
356+
) -> HostProcess:
357+
return await get_current_host().exec(*args, env=env, cwd=cwd)
352358

353359

354360
from pythinker_host._current import current_host as current_host # noqa: E402

packages/pythinker-host/src/pythinker_host/local.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ async def mkdir(
190190
local_path = path.unsafe_to_local_path() if isinstance(path, HostPath) else Path(path)
191191
await asyncio.to_thread(local_path.mkdir, parents=parents, exist_ok=exist_ok)
192192

193-
async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostProcess:
193+
async def exec(
194+
self, *args: str, env: Mapping[str, str] | None = None, cwd: str | None = None
195+
) -> HostProcess:
194196
if not args:
195197
raise ValueError("At least one argument (the program to execute) is required.")
196198

@@ -208,6 +210,7 @@ async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostPr
208210
stdout=asyncio.subprocess.PIPE,
209211
stderr=asyncio.subprocess.PIPE,
210212
env=env,
213+
cwd=cwd,
211214
**process_options,
212215
)
213216
return self.Process(process)

packages/pythinker-host/src/pythinker_host/ssh.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,9 @@ async def mkdir(
303303
raise FileExistsError(f"{path} already exists")
304304
await self._sftp.mkdir(str(path))
305305

306-
async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostProcess:
306+
async def exec(
307+
self, *args: str, env: Mapping[str, str] | None = None, cwd: str | None = None
308+
) -> HostProcess:
307309
if not args:
308310
raise ValueError("At least one argument (the program to execute) is required.")
309311
command = " ".join(shlex.quote(arg) for arg in args)
@@ -313,8 +315,15 @@ async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostPr
313315
# cwd before running the command.
314316
#
315317
# This is intentionally strict: if cwd doesn't exist, the command fails.
316-
if self._cwd:
317-
command = f"cd {shlex.quote(self._cwd)} && {command}"
318+
if cwd is None:
319+
effective_cwd = self._cwd
320+
elif posixpath.isabs(cwd):
321+
effective_cwd = cwd
322+
else:
323+
base_cwd = self._cwd or "/"
324+
effective_cwd = posixpath.normpath(posixpath.join(base_cwd, cwd))
325+
if effective_cwd:
326+
command = f"cd {shlex.quote(effective_cwd)} && {command}"
318327
process = await self._connection.create_process(command, encoding=None, env=env)
319328
return self.Process(process)
320329

src/pythinker_code/acp/host.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,10 @@ async def mkdir(
296296
) -> None:
297297
await self._fallback.mkdir(path, parents=parents, exist_ok=exist_ok)
298298

299-
async def exec(self, *args: str, env: Mapping[str, str] | None = None) -> HostProcess:
300-
return await self._fallback.exec(*args, env=env)
299+
async def exec(
300+
self, *args: str, env: Mapping[str, str] | None = None, cwd: str | None = None
301+
) -> HostProcess:
302+
return await self._fallback.exec(*args, env=env, cwd=cwd)
301303

302304
def _abs_path(self, path: StrOrHostPath) -> str:
303305
host_path = path if isinstance(path, HostPath) else HostPath(path)

src/pythinker_code/acp/tools.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from pythinker_code.soul.approval import Approval
1111
from pythinker_code.soul.permission import check_shell_command_allowed
1212
from pythinker_code.soul.toolset import PythinkerToolset
13+
from pythinker_code.tools.ask_user import AskUserQuestion
1314
from pythinker_code.tools.shell import Params as ShellParams
1415
from pythinker_code.tools.shell import Shell
1516
from pythinker_code.tools.utils import ToolResultBuilder
@@ -28,6 +29,11 @@ def replace_tools(
2829
# Only replace tools when running locally or under ACPHost.
2930
return
3031

32+
# ACP clients get no interactive question UI (the session loop signals
33+
# QuestionNotSupported), so don't advertise the tool — a hallucinated call
34+
# still resolves through the registered tool's graceful fallback.
35+
toolset.hide(AskUserQuestion.name)
36+
3137
if client_capabilities.terminal and (shell_tool := toolset.find(Shell)):
3238
# Replace the Shell tool with the ACP Terminal tool if supported.
3339
toolset.add(
@@ -48,6 +54,8 @@ class HideOutputDisplayBlock(DisplayBlock):
4854

4955

5056
class Terminal(CallableTool2[ShellParams]):
57+
emits_tool_execution_started_after_approval = True
58+
5159
def __init__(
5260
self,
5361
shell_tool: Shell,

src/pythinker_code/agents/default/system.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ For research or multimedia tasks (images, video, PDFs, docs, spreadsheets, prese
105105

106106
**Act with tools; prose is not action.** Code that appears only in your reply is not saved — use `WriteFile` to create or overwrite, `StrReplaceFile` to edit, `Shell` to run and verify; iterate on failures. Follow each tool's parameter spec exactly. Don't narrate routine tool calls. Do not re-read a file after a successful edit tool call.
107107

108-
**Parallelize.** Before every tool response, ask whether another independent read/search/check can run in the same turn — you may emit any number of tool calls in one response; batch non-interfering calls. Serializing independent operations wastes time and grows context. This is very important to your performance.
108+
**Parallelize.** Before every tool response, ask whether another independent read/search/check can run in the same turn — you may emit any number of tool calls in one response; batch non-interfering calls. Choose the lightest effective work shape: direct tools for known-path checks, `SetTodoList` once a substantial approach is clear, foreground `RunAgents` when independent children feed immediate synthesis, and background agents only when you can make other progress while they run. Serializing independent operations wastes time and grows context. This is very important to your performance.
109109

110110
**Spend context deliberately.** The context window is a finite budget: read targeted ranges instead of whole files when the region is known, distill long command output to what the task needs, and push bulky exploration into subagents that return summaries rather than raw dumps.
111111

src/pythinker_code/app.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,30 @@ async def create(
388388
from pythinker_code.hooks.engine import HookEngine
389389

390390
hook_engine = HookEngine(config.hooks, cwd=str(session.work_dir))
391+
if config.disabled_project_hooks:
392+
# The load-time logger.warning only reaches shell users; publish a
393+
# notification so web/ACP frontends also learn why their project
394+
# hooks did not run and how to enable them.
395+
from pythinker_code.notifications.models import NotificationEvent
396+
397+
runtime.notifications.publish(
398+
NotificationEvent(
399+
id=f"project-hooks-disabled:{session.id}",
400+
category="system",
401+
type="project_hooks_disabled",
402+
source_kind="config",
403+
source_id="project_trust",
404+
title="Project hooks disabled (untrusted project)",
405+
body=(
406+
"Hooks defined in "
407+
+ ", ".join(config.disabled_project_hooks)
408+
+ " are disabled until you trust this project. Run /trust to "
409+
"enable them (takes effect on /reload or next start)."
410+
),
411+
severity="warning",
412+
dedupe_key=f"project-hooks-disabled:{session.id}",
413+
)
414+
)
391415
soul.set_hook_engine(hook_engine)
392416
runtime.hook_engine = hook_engine
393417

@@ -603,7 +627,7 @@ async def await_bg_tasks_shutdown(self, timeout: float = 2.0) -> None:
603627
async def _env(self) -> AsyncGenerator[None]:
604628
async with _CWD_LOCK:
605629
original_cwd = HostPath.cwd()
606-
await pythinker_host.chdir(self._runtime.session.work_dir)
630+
await pythinker_host.chdir(self._runtime.work_dir)
607631
try:
608632
# to ignore possible warnings from dateparser
609633
warnings.filterwarnings("ignore", category=DeprecationWarning)
@@ -782,7 +806,7 @@ async def run_shell(
782806
"""Run the Pythinker CLI instance with shell UI."""
783807
from pythinker_code.ui.shell import Shell, WelcomeInfoItem
784808

785-
work_dir = self._runtime.session.work_dir
809+
work_dir = self._runtime.work_dir
786810
welcome_info = [
787811
WelcomeInfoItem(name="Directory", value=str(shorten_home(work_dir))),
788812
]

0 commit comments

Comments
 (0)