diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e7320e..1498347c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **Reviewer subagents now receive deterministic Git scopes.** Structured automatic, + uncommitted, base, and commit targets resolve to full commit anchors before dispatch, reject + invalid or empty scopes explicitly, and keep repository metadata isolated from instructions + across foreground and background runs. - **Parallel streamed tool calls are now correlated safely.** Interleaved argument chunks stay attached to their indexed calls, malformed or truncated call streams stop before tool execution, and failed attempts are not retried after output has already been shown. - **Provider compatibility and Z.AI routing are now explicit.** Immutable compatibility profiles keep request-format quirks behind the chat-provider boundary, while independent Z.AI Coding Plan and API login routes use separate credentials, endpoints, model identities, catalog refresh, logout, and usage/rate-limit state. Curated GLM requests now apply exact context/output limits, thinking controls, reasoning replay, and tool-stream support without activating for local or unknown models. - **Tool execution is now supervised as a terminal batch.** A private execution engine preserves the Toolset registry and legacy per-call API while centralizing ordered results, deduplication, callbacks, and batch summaries; cancellation is bounded, late work stays owned, and new batches fail closed until timed-out cleanup drains. diff --git a/docs/en/release-notes/changelog.md b/docs/en/release-notes/changelog.md index 437dfbf5..a5f8d0f6 100644 --- a/docs/en/release-notes/changelog.md +++ b/docs/en/release-notes/changelog.md @@ -17,6 +17,10 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **Reviewer subagents now receive deterministic Git scopes.** Structured automatic, + uncommitted, base, and commit targets resolve to full commit anchors before dispatch, reject + invalid or empty scopes explicitly, and keep repository metadata isolated from instructions + across foreground and background runs. - **Parallel streamed tool calls are now correlated safely.** Interleaved argument chunks stay attached to their indexed calls, malformed or truncated call streams stop before tool execution, and failed attempts are not retried after output has already been shown. - **Provider compatibility and Z.AI routing are now explicit.** Immutable compatibility profiles keep request-format quirks behind the chat-provider boundary, while independent Z.AI Coding Plan and API login routes use separate credentials, endpoints, model identities, catalog refresh, logout, and usage/rate-limit state. Curated GLM requests now apply exact context/output limits, thinking controls, reasoning replay, and tool-stream support without activating for local or unknown models. - **Tool execution is now supervised as a terminal batch.** A private execution engine preserves the Toolset registry and legacy per-call API while centralizing ordered results, deduplication, callbacks, and batch summaries; cancellation is bounded, late work stays owned, and new batches fail closed until timed-out cleanup drains. diff --git a/docs/superpowers/plans/2026-07-15-deterministic-review-target-resolution.md b/docs/superpowers/plans/2026-07-15-deterministic-review-target-resolution.md new file mode 100644 index 00000000..494aedd9 --- /dev/null +++ b/docs/superpowers/plans/2026-07-15-deterministic-review-target-resolution.md @@ -0,0 +1,3226 @@ +# Deterministic Review Target Resolution Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development +> (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give every fresh reviewer subagent one pre-resolved, authoritative Git target across +single, fan-out, foreground, and background dispatch. + +**Architecture:** Add a strict async Git result seam beside the existing best-effort repository +context collector, then build a focused `review_target` module that validates structured targets +and renders the runtime target block. Keep the caller prompt and resolved target separate through +dispatch; `prepare_soul` alone revalidates live-target `HEAD` and composes output language, safe Git +orientation, caller task, and the final authoritative target. + +**Tech Stack:** Python 3.14, Pydantic v2, asyncio, `pythinker_host`, Git CLI, pytest, +inline-snapshot, Ruff, Pyright, ty, uv, Make, npm docs sync. + +## Global Constraints + +- Use `uv` for direct Python commands and repository `make` targets for package gates. +- Add no dependency, telemetry event, hosted endpoint, configuration key, feature flag, or database + migration. +- Reviewer types are exactly `review`, `code-reviewer`, and `security-reviewer`; keep one shared + definition. +- Target kinds are exactly `auto`, `uncommitted`, `base`, and `commit`. +- A ref is at most 1,024 characters, has no leading/trailing whitespace, does not start with `-`, + and contains no Unicode control character; validation rejects rather than normalizes it. +- Use argv execution only. Resolve untrusted refs with + `git rev-parse --verify --end-of-options ^{commit}`. +- Disable configured filesystem monitors for resolver probes and prescribe `--no-ext-diff` plus + `--no-textconv` for review diff/show commands. +- Commit and merge-base identifiers are immutable; uncommitted/base index and worktree state stays + explicitly live and must never be described as a frozen patch. +- Recheck `HEAD` immediately before model execution for live-worktree targets and fail if it moved. +- Preserve the original caller prompt for `SubagentStart`; generated target text must not consume + the existing 500-character hook preview. +- Final fresh-reviewer prompt order is output-language instruction, generic safe Git context without + its default merge-base line, caller prompt inside ``, then the authoritative + `` block. +- Neutralize every prompt-visible Git value through one shared renderer: bound length, strip + invisible/bidi smuggling characters, replace controls, and HTML-escape markup. +- Explicit targets never fall back. Only `auto` follows the documented `origin/main`, `main`, + `master`, dirty worktree, then `HEAD` policy, and its chosen path remains visible. +- Commit mode uses root-safe, single-parent, and first-parent merge semantics; it does not claim + per-parent conflict analysis. +- Keep `/review`, standalone `pythinker-review` engine behavior, range/staged-only targets, diff + embedding, and reviewer output schemas out of scope. +- Write each regression before its production change and observe the intended failure. +- Add a non-blank `CHANGELOG.md` `## Unreleased` bullet before the first shipped-code commit; update + `docs/en/release-notes/changelog.md` only through `npm run sync` from `docs/`. +- Before completion, use `superpowers:requesting-code-review` and + `superpowers:verification-before-completion` and run the full Pythinker Code gates. +- Never add Codex co-author or generated-by trailers to commits or PR text. + +## Authoritative Command Validation + +- The [Git rev-parse manual](https://git-scm.com/docs/git-rev-parse) recommends + `--end-of-options` for names from untrusted sources and documents `^{commit}` as the commit-ish + type assertion. +- The [Git status manual](https://git-scm.com/docs/git-status) guarantees porcelain v1 stability; + `-z` emits NUL-separated, unquoted paths and `--untracked-files=all` enumerates individual files. +- The [Git diff manual](https://git-scm.com/docs/git-diff) documents `--no-ext-diff`, + `--no-textconv`, and `--quiet` exit 0/1 semantics. +- The [Git show manual](https://git-scm.com/docs/git-show) defines first-parent merge output through + `--diff-merges=first-parent` / `--dd`. + +## File Map + +- Create `src/pythinker_code/subagents/review_target.py`: structured target models, strict + resolution policy, prompt templates, and live-`HEAD` revalidation. +- Create `tests/subagents/test_review_target.py`: resolver topology, validation, prompt safety, + failure, and drift tests. +- Modify `src/pythinker_code/utils/trust.py`: add one deterministic prompt-data escaping helper. +- Modify `tests/utils/test_trust.py`: pin control removal, markup escaping, and length bounds. +- Modify `src/pythinker_code/subagents/git_context.py`: strict bounded Git result seam, public base + candidates, optional merge-base orientation, and safe metadata rendering. +- Modify `tests/test_git_context.py`: characterize strict result handling and hostile metadata. +- Modify `src/pythinker_code/subagents/core.py`: shared reviewer set, resolved-target transport, + live-`HEAD` check, and final prompt composition. +- Modify `tests/subagents/test_git_context_gate.py`: pin the shared reviewer set and explorer union. +- Modify `tests/core/test_prepare_soul.py`: pin prompt order, no duplicate merge-base, resume, and + moved-`HEAD` behavior. +- Modify `src/pythinker_code/subagents/runner.py`: carry the resolved target without changing hook + input. +- Modify `src/pythinker_code/background/manager.py`: persist the serialized target beside the caller + prompt. +- Modify `src/pythinker_code/background/agent_runner.py`: restore the target into + `SubagentRunSpec`. +- Modify `tests/background/test_manager.py`: pin background payload persistence and runner transport. +- Modify `tests/background/test_task_metadata.py`: pin JSON-safe resolved-target metadata. +- Modify `src/pythinker_code/tools/agent/__init__.py`: public schemas, pre-allocation resolution, + result hints, fan-out forwarding, and orchestration fingerprinting. +- Modify `src/pythinker_code/tools/agent/description.md`: document target modes and resume/type + restrictions. +- Modify `tests/tools/test_agent_tool.py`: cover single/fan-out, foreground/background, allocation + failure, hook preservation, and fingerprints. +- Modify `tests/tools/test_tool_schemas.py`: deliberately update the Agent JSON-schema snapshot. +- Modify `tests/tools/test_tool_descriptions.py`: deliberately update Agent usage documentation. +- Modify `tests/core/test_default_agent.py`: deliberately update the default-agent tool-schema and + description snapshots. +- Modify `CHANGELOG.md`: add the shipped behavior under `## Unreleased`. +- Generated modify `docs/en/release-notes/changelog.md`: update only through docs sync. +- Modify `docs/superpowers/specs/2026-07-15-review-target-resolution-design.md`: mark the approved + implementation outcome without changing scope. +- Modify `tasks/agent-harness-adoption-plan.md`: mark only the deterministic-target item complete. +- Modify `tasks/todo.md`: track task completion and record exact verification/review evidence. +- Modify `tasks/lessons.md` only if execution produces another concrete correction or surprise. + +--- + +### Task 1: Add a strict, safe Git prompt substrate + +**Files:** +- Modify: `tests/utils/test_trust.py:1-100` +- Modify: `src/pythinker_code/utils/trust.py:1-88` +- Modify: `tests/test_git_context.py:1-430` +- Modify: `src/pythinker_code/subagents/git_context.py:1-197` +- Modify: `CHANGELOG.md:17-22` + +**Interfaces:** +- Produces: `escape_prompt_data(text: str, *, max_chars: int) -> str`. +- Produces: `DEFAULT_BASE_REFS: tuple[str, ...]` equal to + `("origin/main", "main", "master")`. +- Produces: immutable `GitCommandResult(stdout: str, stderr: str, returncode: int, + stdout_truncated: bool, stderr_truncated: bool)`. +- Produces: `GitCommandError(category: Literal["spawn", "timeout"], command: str)` with no raw + stderr in its message. +- Produces: `run_git(args: Sequence[str], cwd: str, *, timeout: float = 5.0, + max_output_bytes: int = 65536) -> GitCommandResult`. +- Preserves: `_run_git(args: list[str], cwd: str, timeout: float = 5.0) -> str | None` as the + best-effort compatibility wrapper used by generic context. +- Changes: `collect_git_context(work_dir: HostPath, *, include_merge_base: bool = True) -> str`. + +- [ ] **Step 1: Add failing trust and Git-substrate tests** + +Add these focused contracts before production changes: + +```python +# tests/utils/test_trust.py +import html + +from pythinker_code.utils.trust import escape_prompt_data + + +def test_escape_prompt_data_neutralizes_controls_markup_and_length() -> None: + rendered = escape_prompt_data( + "lead\u202e\nTAIL-TOO-LONG", + max_chars=24, + ) + + assert "\u202e" not in rendered + assert "\n" not in rendered + assert "" not in rendered + assert "</git-context>" in rendered + assert len(html.unescape(rendered)) <= 24 + assert html.unescape(rendered).endswith("…") + + +def test_escape_prompt_data_rejects_impossible_limit() -> None: + with pytest.raises(ValueError, match="max_chars"): + escape_prompt_data("value", max_chars=0) +``` + +```python +# tests/test_git_context.py +from pythinker_code.subagents.git_context import GitCommandError, GitCommandResult, run_git + + +@pytest.mark.asyncio +async def test_run_git_preserves_nonzero_exit_for_callers(tmp_path: Path) -> None: + await _init_repo(tmp_path) + + result = await run_git( + ["rev-parse", "--verify", "--end-of-options", "missing^{commit}"], + str(tmp_path), + ) + + assert result.returncode != 0 + assert result.stdout == "" + + +@pytest.mark.asyncio +async def test_collect_context_can_omit_merge_base(tmp_path: Path) -> None: + await _init_repo(tmp_path) + await _git(tmp_path, "checkout", "-b", "feature") + (tmp_path / "feature.txt").write_text("feature", encoding="utf-8") + await _git(tmp_path, "add", ".") + await _git(tmp_path, "commit", "-m", "feature") + + result = await collect_git_context(_host_path(tmp_path), include_merge_base=False) + + assert "Merge base" not in result + assert "Branch: feature" in result + + +@pytest.mark.asyncio +async def test_collect_context_neutralizes_hostile_git_metadata(tmp_path: Path) -> None: + await _init_repo(tmp_path) + await _git(tmp_path, "checkout", "-b", "feature