From 152d61017c02a32bcc48df5440100f8a2f79b98b Mon Sep 17 00:00:00 2001 From: Jiri Spilka Date: Tue, 11 Aug 2026 09:10:30 +0200 Subject: [PATCH] feat: Remove git juggling --- .../skills/shepherd-approve-design/SKILL.md | 2 +- .claude/skills/shepherd/SKILL.md | 52 ++++++++----------- README.md | 11 ++-- tests/test_orchestrator_contract.py | 30 ++++++----- 4 files changed, 46 insertions(+), 49 deletions(-) diff --git a/.claude/skills/shepherd-approve-design/SKILL.md b/.claude/skills/shepherd-approve-design/SKILL.md index 2caf23e..4474d9a 100644 --- a/.claude/skills/shepherd-approve-design/SKILL.md +++ b/.claude/skills/shepherd-approve-design/SKILL.md @@ -34,7 +34,7 @@ Record human approval of the design, success criteria, and review panel so imple 4. Write the marker: ```bash mkdir -p .shepherd - printf 'approved_at=%s\napproved_commit=%s\nnote=design approved by human via /shepherd-approve-design\n' \ + printf 'approved_at=%s\nbase_commit=%s\nnote=design approved by human via /shepherd-approve-design\n' \ "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$(git rev-parse HEAD 2>/dev/null || echo none)" \ > .shepherd/_design.approved ``` diff --git a/.claude/skills/shepherd/SKILL.md b/.claude/skills/shepherd/SKILL.md index 9db5a82..28a86a7 100644 --- a/.claude/skills/shepherd/SKILL.md +++ b/.claude/skills/shepherd/SKILL.md @@ -15,7 +15,7 @@ authorship). Everything else in `.shepherd/` — triage, routing state, markers, orchestrator plumbing. Two human gates, never self-granted: the **design gate** before any source edit and the -**create-PR confirm** before any git write. Triage has no gate. The loop: +**create-PR confirm** before opening the PR. Triage has no gate. The loop: `_user_request` → `1-triage` → `verify` → `[explore]` → `architect` → `success-criteria` → `iterate with human` → `[_design.approved]` → `implement ↔ oracle ↔ review` → `final review` → @@ -32,8 +32,8 @@ ephemeral; the files are the record. `_create_pr.approved`. - Per iteration in `iter-N/`: `claim.md`, `review-.md`, `final-review-.md`, `question-verification.md`, `fulfillment.md`, `followups.md`, and the regenerable (gitignored) - `diff.patch`, `test-results.txt` (plus `baseline.txt` and `predirty.txt` in `iter-1/` only — - the pre-change oracle metrics and any pre-existing dirty paths). + `diff.patch`, `test-results.txt` (plus `baseline.txt` in `iter-1/` only — the pre-change + oracle metrics). **Why one file per stage:** each stage writes one file and each role reads ONLY what it needs, so stage context stays scoped and judgments stay independent. Reviewers judge the diff against @@ -64,8 +64,9 @@ reliably type a slash-command. Surface everything they need into the conversatio 1. Resolve the absolute path of the target repo's `.shepherd/` once at setup and use it for every read/write — relative paths drift with the working directory in long sessions. - `mkdir -p` it. If `.shepherd/.gitignore` is missing, write it: ignore `*` except - `.gitignore`, `config.json`, `registry.json`. + `mkdir -p` it. If `.shepherd/.gitignore` is missing, write it: a single `*` line, so git + ignores everything in `.shepherd/`, the `.gitignore` itself included. Humans who want + `config.json` / `registry.json` committed add the exceptions there themselves. 2. Fresh run: require a non-empty ``. Write it verbatim to `.shepherd/_user_request.md`. Initialize `_state.json`: `{"phase":"triage","iteration":0}`. 3. If `.shepherd/_state.json` exists, resume. If a new non-empty `` differs from @@ -93,7 +94,7 @@ Valid `state.phase` values: `triage`, `verify`, `design`, `design-gate`, `inner- Resume by phase: - `phase=triage`, `verify`, or `design` → continue that phase from its files. - `phase=design-gate` + `_design.approved` → if HEAD differs from the marker's - `approved_commit`, stop and re-confirm the design with the human first. Otherwise load + `base_commit`, stop and re-confirm the design with the human first. Otherwise load `_panel.json` into `state.panel`, set `state.iteration=1`, and set `state.phase="inner-loop"`. - `phase=design-gate` without the marker → re-present the design + panel (step 4, Design gate) and wait. - `phase=inner-loop` or `final-review` → continue that phase. @@ -352,15 +353,11 @@ back to the full roster and limits and record that in `_progress.md`. For each iteration `N`: 1. Set `state.phase="inner-loop"` and `state.iteration=N`; create `.shepherd/iter-N/`. -2. On iteration 1, before the first source edit, run `git status --porcelain` (ignore - `.shepherd/` entries) and record any pre-existing changes in `iter-1/predirty.txt` — a dirty - tree is fine, those paths just aren't the run's. If the run needs to edit a pre-dirty path, - stop for human direction (commit or stash it first): the run's diff must stay separable. - Also run `oracle.commands` once on the untouched tree and record its baseline metrics - (test/file counts, pass/skip counts, warnings, rough duration) in `iter-1/baseline.txt` — - later green runs are judged against these, not in isolation. Then, on every iteration, run - the `implementer` stage: it applies `2-design.md` + `3-success-criteria.md`, addresses every - prior finding, and writes `iter-N/claim.md`. +2. On iteration 1, before the first source edit, run `oracle.commands` once on the untouched + tree and record its baseline metrics (test/file counts, pass/skip counts, warnings, rough + duration) in `iter-1/baseline.txt` — later green runs are judged against these, not in + isolation. Then, on every iteration, run the `implementer` stage: it applies `2-design.md` + + `3-success-criteria.md`, addresses every prior finding, and writes `iter-N/claim.md`. 3. Run `oracle.commands`, capturing output to `iter-N/test-results.txt`; if empty, record and run the smallest credible inferred fallback. Use finite, deterministic, non-mutating commands; avoid `dev`, `start`, `watch`, `lint:fix`, `format`, `clean`, @@ -369,12 +366,10 @@ For each iteration `N`: delta (test or file count, skips, new warnings, order-of-magnitude duration shift) fails the oracle even when all passes (wrong-but-green happens, e.g. silently double-running the suite). Expected deltas (e.g. tests the design adds) must be named in `claim.md`. -4. Check `git status --porcelain` again (ignore `.shepherd/`). If unrelated changes appeared — - paths neither in `iter-1/predirty.txt` nor edited by this run — stop for human direction. - Write `diff.patch` only for the run's own changes; pre-dirty paths never enter it. A file - new in this run is silent in `git diff ` — include it via - `git diff --no-index /dev/null `, and use the same form for any per-file check on a - new file. +4. Write `diff.patch` via `git diff ` — the HEAD recorded in + `_design.approved` at design approval, i.e. before any source edit — so the diff always + spans the run's whole work, commits included. An untracked new file is silent there — + include it via `git diff --no-index /dev/null `, same form for any per-file check. 5. Dispatch panel reviewers in parallel, each given the pasted content of `2-design.md`, `3-success-criteria.md`, `diff.patch`, and `test-results.txt`, plus read access to the repository. They stay blind to `claim.md` and peer reviews. @@ -433,9 +428,7 @@ findings, and every confirmed question finding into `iter-N/followups.md`. ### 8. Finish — `phase=create-pr` + `_create_pr.approved`, ends `phase=done` -1. Re-check `git status --porcelain` (ignore `.shepherd/` and the paths in - `iter-1/predirty.txt`); stop if unrelated changes are present. -2. Commit only the run's own paths — pre-existing changes stay uncommitted in the tree. **If the +1. Commit anything of the run's still uncommitted, push, and open the PR. **If the repo has a PR template** (`.github/pull_request_template.md` or the other usual locations), mirror its section headings and fill each briefly — a layout, not instructions to obey. **Otherwise** at most three short bullets (What / Why / Notes). Either way: plain commit @@ -446,8 +439,8 @@ findings, and every confirmed question finding into `iter-N/followups.md`. Follow-ups list in the body. Every number or factual claim in the body (test counts, referenced files/issues) must match the final oracle run and repo state — a stale count or nonexistent reference is a defect. -3. If a writable remote exists, push and open a PR. Record the evidence summary, approval - timestamps, and PR URL in `_progress.md`, then set `state.phase="done"`. +2. Record the evidence summary, approval timestamps, and PR URL in `_progress.md`, then set + `state.phase="done"`. ## Hard rules @@ -469,9 +462,10 @@ findings, and every confirmed question finding into `iter-N/followups.md`. re-dispatch. Never infer "still running" or "stalled" from turn count or a human check-in. - Blindness per the role table's "do NOT read" column; judgment files are pasted, never granted; the repository itself is never blinded. -- shepherd never commits `.shepherd/` paths. Run data stays ignored via the run's - `.shepherd/.gitignore`; its exceptions (`config.json`, `registry.json`) exist so the human - can commit shared team config — shepherd itself never stages even those. +- No git rules beyond the gate: `_create_pr.approved` gates opening the PR, nothing else; + mid-run commits are normal — the reviewed diff stays anchored at `base_commit`. +- shepherd never stages or commits `.shepherd/` paths; run data stays ignored via the run's + `.shepherd/.gitignore`. - Keep design short: major changes only, never an exhaustive file list. - Surface human-facing artifacts into the human's channel (see Keep the human in the loop). - The panel, not the roster, drives the run; never run a `use` not in config. diff --git a/README.md b/README.md index 22bbd72..a861617 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ flowchart TD ``` There are two human gates: the **design gate** before source edits and the **create-PR -confirm** before any git write. Triage is deliberately cheap and continues unless it +confirm** before opening the PR. Triage is deliberately cheap and continues unless it recommends `DEFER` or `DECLINE`. At the design gate, shepherd writes `_panel.json` so each run gets the right reviewer @@ -112,7 +112,7 @@ files or type a slash-command. `.shepherd/2-design.md`, `.shepherd/3-success-criteria.md`, and `.shepherd/_panel.json` (records the panel and writes the marker). - `/shepherd-approve-create-pr` is the human-only fallback for recording approval - before commit, push, and PR creation. + before the PR is opened. The design gate is generic and portable: shepherd presents the design, criteria, and panel and waits for one of two human-driven outcomes. **Approve** — a chat "yes" or @@ -161,9 +161,10 @@ match. Approve the prompts once in that environment. ## Files Run data lives in `.shepherd/`; plugin tooling lives in `.claude/skills/`. Each run -writes a `.shepherd/.gitignore` that keeps everything ignored except `config.json` and -`registry.json` — committable, shareable team config (shepherd itself never commits -`.shepherd/` paths); run evidence is summarized in the PR body instead. +writes a `.shepherd/.gitignore` that ignores everything, itself included, so `.shepherd/` +never shows up as untracked noise. To share `config.json` / `registry.json` with your +team, add exceptions to that `.gitignore` by hand and commit them yourself — shepherd +never commits `.shepherd/` paths; run evidence is summarized in the PR body instead. Human-facing files: diff --git a/tests/test_orchestrator_contract.py b/tests/test_orchestrator_contract.py index 7762288..0eaeb84 100644 --- a/tests/test_orchestrator_contract.py +++ b/tests/test_orchestrator_contract.py @@ -30,11 +30,21 @@ def test_orchestrator_has_plan_mode_gate(): def test_orchestrator_keeps_the_two_marker_gates(): - # Design (before source edits) and create-PR (before git writes) are the only markers. + # Design (before source edits) and create-PR (before delivery) are the only markers. assert "_design.approved" in ORCH assert "_create_pr.approved" in ORCH +def test_no_git_rules_beyond_the_gate(): + # The gate times the PR; git itself is not shepherd's to manage. History does the + # bookkeeping: the reviewed diff is anchored at the design-gate commit, so mid-run + # commits are normal and no pre-run dirty-tree ledger exists. + assert "No git rules beyond the gate" in ORCH + assert "git diff " in ORCH + assert "before any git write" not in ORCH + assert "predirty" not in ORCH + + def test_orchestrator_has_no_triage_gate(): # Triage flows onward; it never waits on an approval marker. assert "triage.approved" not in ORCH @@ -166,8 +176,8 @@ def test_design_gate_wait_state_is_resumable(): assert "re-present the design + panel" in ORCH -def test_orchestrator_checks_approved_commit_on_resume(): - assert "approved_commit" in ORCH +def test_orchestrator_checks_base_commit_on_resume(): + assert "base_commit" in ORCH def test_orchestrator_archives_previous_run_on_fresh_start(): @@ -176,7 +186,9 @@ def test_orchestrator_archives_previous_run_on_fresh_start(): def test_setup_writes_run_gitignore(): assert ".shepherd/.gitignore" in ORCH - # Humans may commit config.json/registry.json; shepherd never stages .shepherd/ paths. + # Everything ignored, the .gitignore itself included — no untracked noise; sharing + # config is a manual opt-out. Shepherd never stages .shepherd/ paths. + assert "a single `*` line" in ORCH assert "never stages" in ORCH @@ -300,16 +312,6 @@ def test_orchestrator_documents_oracle_commands(): assert "capturing output to `iter-N/test-results.txt`" in ORCH -def test_orchestrator_documents_dirty_worktree_protection(): - assert "git status --porcelain" in ORCH - # A dirty tree is allowed; pre-existing paths are recorded, kept out of - # the diff, and never committed. - assert "predirty.txt" in ORCH - assert "stay separable" in ORCH - # The run's own .shepherd files must not trip the check. - assert "ignore `.shepherd/`" in ORCH - - def test_orchestrator_finish_writes_plain_commit_and_pr(): # Short PR body: template headings filled briefly, else ≤3 bullets; no essay / diff narration. assert "three short bullets" in ORCH