diff --git a/docs/bootstrap-review-path.md b/docs/bootstrap-review-path.md new file mode 100644 index 0000000..33293d2 --- /dev/null +++ b/docs/bootstrap-review-path.md @@ -0,0 +1,190 @@ +# Bootstrap Review Path for Self-Hosted MCP Workflow Fixes (#557) + +## Purpose + +Gitea-Tools MCP workflow fixes can create a **bootstrap deadlock**: the live +MCP daemons still run the broken code from `master`, so canonical reviewer / +merger tools cannot complete the review that would land the fix. + +This document defines a **narrow, controller-authorized bootstrap path** so +such fixes can land without weakening normal review/merge gates. + +It is **not** a general bypass. Normal PRs must still use the full +reviewer → merger MCP workflow. + +## When this path applies + +All of the following must be true: + +1. The PR changes **Gitea-Tools MCP workflow / preflight / lease / gate** code + that the live daemon must load to review itself (self-hosted control plane). +2. Canonical review or merge tools **fail closed** because of that defect + (documented tool errors, not “inconvenience”). +3. A **controller** records an explicit bootstrap authorization on the PR or + linked issue (see below). +4. The PR source is clean, testable, and free of unrelated scope. + +Example (historical): PR #553 / Issue #546 (reviewer preflight capability/lease +deadlock). Live daemons on unpatched `master` could not complete canonical +review of the fix PR. + +## Durable authorization (required) + +**No manual remote merge, force-merge, or API merge of a bootstrap PR is +allowed** unless a controller has posted a durable authorization record on +Gitea. + +### Authorization record (issue or PR comment) + +The controller comment **must** include a machine-readable marker and the +fields below: + +```text +## BOOTSTRAP REVIEW AUTHORIZATION (#557) + +Status: APPROVED +PR: +Issue: +Controller: +Reason: live MCP runtime cannot canonically review this self-hosted workflow fix +Scope: narrow bootstrap only — does not weaken normal PR gates + +Validation evidence: +- git diff --check: clean +- targeted pytest: → pass +- full suite attribution (if non-zero): baseline compared to prgs/master + +Duplicate-PR audit: +Mutation ledger audit: +Contamination audit: + +Allowed verification actions used: +Forbidden actions NOT used: root checkout edits; raw curl mutation; direct +module import of gitea_mcp_server for mutations; in-memory gate restoration + +Post-land plan: +- restart MCP daemons for author/reviewer/merger/reconciler profiles +- re-verify with canonical tools (see Post-bootstrap steps) +``` + +Without this record, bootstrap merge is **forbidden**. Agents must stop with +`BLOCKED + DIAGNOSE` rather than improvise a merge. + +## Review gates and proof (still required) + +Bootstrap does **not** skip technical review. It only allows verification and +landing when the **live MCP mutation path** cannot complete the review. + +Before authorization, the controller (or a delegated read-only verifier in an +isolated worktree) must have: + +| Gate | Requirement | +|------|-------------| +| Diff hygiene | `git diff --check` clean on the PR tip vs `prgs/master` | +| Tests | Targeted tests for the fix pass; full suite either green or failures attributed to baseline `prgs/master` | +| Duplicate PR | Open-PR inventory shows no competing open PR for the same issue (or supersession is documented) | +| Mutation ledger | Any claimed mutations are ledger-consistent; no silent root edits | +| Contamination audit | PR/issue comments and leases classified as clean vs workflow-contaminated | +| Scope | Diff limited to the bootstrap issue; no drive-by refactors | + +### Contamination audit (comments / leases) + +Comments or leases created via **raw curl**, **direct Python import of MCP +modules**, or **token extraction** are **workflow-contaminated**. They must be +listed and must **not** be treated as canonical review proof. + +Only comments posted via **sanctioned MCP reviewer tools** (after the fix is +landed and daemons restarted, or during a clean path that did not bypass gates) +count as canonical review state. + +Historical example on PR #553: + +| Comment | Disposition | +|---------|-------------| +| #7247 test / raw API | contaminated | +| #7251 lease metadata / raw API | contaminated | +| #7252 lease metadata / direct import | contaminated | +| #7265 lease metadata / MCP reviewer tools | workflow-clean | + +## Allowed verification actions + +These may run **outside** the broken live mutation path to establish facts: + +- Read-only MCP tools (`gitea_view_pr`, `gitea_list_prs`, `gitea_whoami`, + `gitea_get_profile`, inventory, eligibility **reads**). +- Isolated `branches/` worktree checkout of the PR head (never root). +- `git fetch`, `git log`, `git diff`, `git merge-tree` (read-only analysis). +- Targeted `pytest` / `py_compile` inside that worktree. +- Controller posting of the bootstrap authorization comment via a healthy + MCP profile **if available**; if comment mutation is also deadlocked, a + **human operator** posts the authorization in the Gitea UI (still durable on + the thread). + +## Forbidden actions (always) + +Even under bootstrap: + +- Editing or committing in the **project root / control checkout**. +- Treating root as a worktree for implementation or review mutations. +- Raw `curl` / REST mutation with extracted tokens as a normal workflow. +- `import gitea_mcp_server` (or sibling modules) from a shell to call mutation + helpers and bypass preflight. +- In-memory restoration of capability / lease / decision state to “finish” + a blocked chain. +- Force-push, history rewrite, or deleting evidence comments. +- Weakening normal gates in code “temporarily” without a tracked issue/PR. +- Self-review or self-merge by the PR author identity. + +## Landing procedure (after APPROVED authorization) + +1. Confirm the authorization record is present and complete on the PR or issue. +2. Merge **only** with an independent merger identity when possible. + - Preferred: restart/replace the MCP merger daemon with a build that includes + the fix (or a one-shot process started from the PR worktree binary) so + `gitea_merge_pr` can run under normal gates. + - If the live merger daemon still cannot load the fix, a **human operator** + may merge via the Gitea UI **only** after the authorization record exists. +3. Never merge from contaminated proof alone. + +## Post-bootstrap steps + +### 1. Restart MCP daemons + +After the fix lands on `prgs/master`: + +1. Stop author / reviewer / merger / reconciler MCP server processes for this + repo (IDE MCP pool + any long-lived terminals). +2. Confirm no stale PID still serves the old code. +3. Restart each profile so it loads the updated `master` tree (or installed + package path). +4. Call `gitea_whoami` / `gitea_get_profile` on each namespace and record + profile name + identity. + +### 2. Re-verify with canonical tools + +Example pattern after PR #553-class fixes (lease release / #550-style follow-up): + +1. From a **reviewer** MCP session: acquire/release or inspect the relevant + lease with canonical tools only. +2. Confirm no direct-import or raw-API path is required. +3. Post a short controller or reconciler note: bootstrap complete; normal gates + restored. + +If verification still requires a bypass, open a new issue — do **not** extend +this bootstrap authorization silently. + +## Explicit non-goals + +- This path does **not** authorize skipping tests, duplicate audits, or + contamination audits. +- This path does **not** authorize root checkout implementation work. +- This path does **not** replace BLOCKED + DIAGNOSE when a non-bootstrap + failure occurs (#552). +- This path does **not** grant permanent “operator exception” culture. + +## Related + +- Root checkout policy: `docs/llm-workflow-runbooks.md` (Global LLM Worktree Rule, #475) +- BLOCKED + DIAGNOSE: issue #552 / skill workflows +- Reviewer lease / preflight deadlock class: issues #546, #548, #550 +- Durable session proofs across daemon pools: issue #559 diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index d0e472d..1e0366b 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -408,6 +408,28 @@ The guard blocks when the **control checkout** (repository root) is: `branches/...` directories are disposable role worktrees; the root checkout is the stable orchestration surface only. +## Bootstrap Review Path for self-hosted MCP fixes (#557) + +When a PR fixes Gitea-Tools MCP workflow code that the **live daemon** still +runs from broken `master`, canonical review can deadlock on itself. + +Do **not** improvise raw API, direct imports, root edits, or gate bypasses. + +Use the narrow controller-authorized path documented in: + +- [`docs/bootstrap-review-path.md`](bootstrap-review-path.md) + +Hard rules: + +- No merge without a durable `BOOTSTRAP REVIEW AUTHORIZATION (#557)` record on + the PR or linked issue. +- Validation, duplicate-PR, mutation-ledger, and contamination audits remain + mandatory. +- Root checkout stays read-only orchestration only; verification runs in + `branches/` worktrees. +- After land: restart MCP daemons and re-verify with canonical tools only. +- This never weakens normal reviewer/merger gates for ordinary PRs. + ## Shell Spawn Hard-Stop Rule Symptom: a shell tool call returns `exit_code: -1` with empty stdout/stderr. diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index dc2f961..73b9f8d 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -182,4 +182,17 @@ Ready-to-copy task prompts live in [`templates/`](templates/): Releases follow SemVer from remote `master` only, after full test suite passes. See [`templates/release-tag.md`](templates/release-tag.md) and -`scripts/release-tag`. \ No newline at end of file +`scripts/release-tag`. + +## Bootstrap Review Path (#557) + +Self-hosted MCP workflow fixes can deadlock live review daemons. Do not bypass +gates with raw API, direct imports, or root checkout edits. + +If and only if a controller posts a durable `BOOTSTRAP REVIEW AUTHORIZATION +(#557)` record, follow: + +`docs/bootstrap-review-path.md` + +Otherwise stop with BLOCKED + DIAGNOSE. Bootstrap never weakens normal PR gates. + diff --git a/tests/test_bootstrap_review_path_docs.py b/tests/test_bootstrap_review_path_docs.py new file mode 100644 index 0000000..71c5c14 --- /dev/null +++ b/tests/test_bootstrap_review_path_docs.py @@ -0,0 +1,81 @@ +"""Doc-contract checks for the bootstrap review path (#557). + +Self-hosted MCP workflow fixes can deadlock live review daemons. These tests +pin the narrow controller-authorized bootstrap path so it cannot silently +regress into a general gate bypass. +""" + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +DOC = REPO_ROOT / "docs" / "bootstrap-review-path.md" +RUNBOOK = REPO_ROOT / "docs" / "llm-workflow-runbooks.md" +SKILL = REPO_ROOT / "skills" / "llm-project-workflow" / "SKILL.md" + + +def _normalize(text: str) -> str: + return " ".join(text.split()) + + +def test_bootstrap_doc_exists_and_names_authorization_marker(): + text = DOC.read_text(encoding="utf-8") + assert "BOOTSTRAP REVIEW AUTHORIZATION (#557)" in text + assert "bootstrap deadlock" in text.lower() or "Bootstrap deadlock" in text + + +def test_bootstrap_doc_requires_validation_and_audits(): + text = _normalize(DOC.read_text(encoding="utf-8")) + for phrase in ( + "git diff --check", + "Duplicate-PR audit", + "Mutation ledger audit", + "Contamination audit", + "workflow-contaminated", + ): + assert phrase in text, f"bootstrap doc missing: {phrase!r}" + + +def test_bootstrap_doc_lists_allowed_and_forbidden_actions(): + text = _normalize(DOC.read_text(encoding="utf-8")) + lower = text.lower() + for phrase in ( + "Allowed verification actions", + "Forbidden actions", + "project root", + "direct Python import", + "branches/", + ): + assert phrase in text, f"bootstrap doc missing: {phrase!r}" + assert "curl" in lower + assert "raw" in lower + + +def test_bootstrap_doc_forbids_general_gate_weakening(): + text = _normalize(DOC.read_text(encoding="utf-8")).lower() + assert "never weakens normal" in text or "does not weaken normal" in text + assert "general bypass" in text or "general gate" in text + + +def test_bootstrap_doc_post_land_restart_and_verify(): + text = _normalize(DOC.read_text(encoding="utf-8")) + for phrase in ( + "Restart MCP daemons", + "canonical tools", + "gitea_whoami", + ): + assert phrase in text, f"post-bootstrap guidance missing: {phrase!r}" + + +def test_runbook_links_bootstrap_path(): + text = _normalize(RUNBOOK.read_text(encoding="utf-8")) + assert "Bootstrap Review Path for self-hosted MCP fixes (#557)" in text + assert "bootstrap-review-path.md" in text + assert "BOOTSTRAP REVIEW AUTHORIZATION (#557)" in text + + +def test_skill_links_bootstrap_path(): + text = _normalize(SKILL.read_text(encoding="utf-8")) + assert "Bootstrap Review Path (#557)" in text + assert "bootstrap-review-path.md" in text + assert "BLOCKED + DIAGNOSE" in text or "BLOCKED" in text