Skip to content

Fix orchestrator loopholes, zero-findings convergence, and compress the skill spec#6

Merged
jirispilka merged 16 commits into
mainfrom
fix/loopholes
Jul 16, 2026
Merged

Fix orchestrator loopholes, zero-findings convergence, and compress the skill spec#6
jirispilka merged 16 commits into
mainfrom
fix/loopholes

Conversation

@jirispilka

Copy link
Copy Markdown
Collaborator

What we're solving

A full scrutiny of the shepherd skill found state-machine gaps, doc contradictions, and under-specified plumbing that a literal orchestrator would get wrong.

Changes

Pipeline logic

  • Architect reads _design_feedback.md whenever it exists — verify-stage human decisions are no longer invisible to the first draft
  • Review-only runs and "abandon" now terminate at phase=done; "fix the findings" is an explicit reopen
  • Dirty tree allowed at start: pre-existing paths recorded in iter-1/predirty.txt, kept out of diff.patch and the final commit
  • Zero-findings convergence: every finding gets fixed, nits included — no implementer skips; any final-review finding triggers a fix round
  • Review-only runs: PR branch checked out at triage, oracle runs on it, success-criteria stage skipped (nothing gets built)
  • Resume at phase=create-pr without the marker re-establishes fulfillment before the confirm
  • Final-review fix rounds advance to a fresh iter-N — evidence is never overwritten
  • Oracle output captured to iter-N/test-results.txt; fulfillment added to model tiering

Docs & config

  • Resolved README ↔ hard-rules contradiction: config.json/registry.json are committable team config; shepherd itself never stages .shepherd/ paths
  • Frontmatter description rewritten: trigger-only, explicitly human-invoked
  • Vendored ponytail-review/SKILL.mdENGINE.md (no stray slash command); shallow-merge warning in config docs; dead head_sha field removed

Spec size

  • SKILL.md compressed 450 → 424 lines with rule-for-rule semantic verification (three drifts found and restored)
  • ~20 new contract-test assertions pin the changed rules; 84 tests green

🤖 Generated with Claude Code

@jirispilka
jirispilka requested a review from Pijukatel July 16, 2026 09:16

@Pijukatel Pijukatel left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok, but I realized that the step referencing seems a little bit loose. What is making the model correctly understand that the steps mentioned are from the top-level flow and not some substeps inside some smaller task? Maybe I just missed the part that makes it clear

For example, step 2 could be several things in different contexts:
### 2. Verify... <- this is the desired target, right?
2. On iteration 1, before...
2. Commit only the run...
2. Fresh run: require a non-empt...

@jirispilka

Copy link
Copy Markdown
Collaborator Author

Seems ok, but I realized that the step referencing seems a little bit loose. What is making the model correctly understand that the steps mentioned are from the top-level flow and not some substeps inside some smaller task? Maybe I just missed the part that makes it clear

For example, step 2 could be several things in different contexts: ### 2. Verify... <- this is the desired target, right? 2. On iteration 1, before... 2. Commit only the run... 2. Fresh run: require a non-empt...

Great point! Nothing guaranteed that; fixed in the latest commits: every cross-reference now names its target ("step 4 (Design gate)"), each top-level heading declares its _state.json phase

@jirispilka
jirispilka merged commit 4548c3e into main Jul 16, 2026
2 checks passed
@jirispilka
jirispilka deleted the fix/loopholes branch July 16, 2026 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants