Skip to content

Fix #744: state one-PR-per-spec convention in SPIR builder-prompt#747

Merged
waleedkadous merged 7 commits into
mainfrom
builder/bugfix-744
May 17, 2026
Merged

Fix #744: state one-PR-per-spec convention in SPIR builder-prompt#747
waleedkadous merged 7 commits into
mainfrom
builder/bugfix-744

Conversation

@waleedkadous
Copy link
Copy Markdown
Contributor

@waleedkadous waleedkadous commented May 14, 2026

Summary

The SPIR/ASPIR builder-prompt templates did not state any PR-strategy convention. Builders interpreted the plan's "each phase commits independently" as "each phase gets its own PR" and shipped per-phase PRs that the architect then had to close (Shannon Spec 1353 → PR #1373).

Fixes #744

Root Cause

Two issues stacked:

  1. codev/protocols/spir/builder-prompt.md (and the ASPIR equivalent) had no PR-strategy guidance at all.
  2. The codev-skeleton mirror had a "Multi-PR Workflow" section that explicitly said "Continue to the next phase, open another PR. Repeat." This pattern is intended for architect-requested slice splits of large specs (e.g. Spec Better handling of builders that stop mid-protocol and create premature PRs #653), but the template framed it as the default workflow.

Fix

Adds a ## PR Strategy section to all four SPIR/ASPIR builder-prompt files (codev + skeleton) prohibiting only the builder's autonomous decision to open a PR per implementation phase — while explicitly preserving the architect's ability to request a PR at any point.

Key wording (per architect review on this PR):

  • "Do not autonomously open a PR per implementation phase." Plan phases ship as git commits within a single PR.
  • By default, the PR is opened during/after the final implement phase, with all phase-commits already on the branch.
  • Architect-requested PRs (new subsection): the architect MAY request a PR at any point — for spec review, mid-implementation feedback, slicing a large spec into shippable PRs. The prohibition is specifically on the builder autonomously deciding to open per-phase PRs without architect request.

In the skeleton, the existing multi-PR mechanics block (cut branch → push → merge → fetch → next branch) is preserved and re-scoped as "Multi-PR Mechanics (when the architect requests sequential PRs)" — the how-to for architect-driven slice splits.

Test Plan

  • Regression test (bugfix-744-spir-pr-strategy.test.ts) — asserts all four builder-prompt files contain (a) the autonomous-PR prohibition, (b) the git-commits-vs-PRs clarification, (c) the architect-override carve-out, and (d) the default PR-timing statement
  • Build passes
  • All tests pass (2616 passed | 13 skipped — same baseline as main)

Add explicit "PR Strategy" section to SPIR and ASPIR builder-prompt
templates (and their skeleton mirrors) stating that all plan phases ship
in ONE PR opened during the review phase. Clarifies that the plan's
"each phase commits independently" refers to git commits, not separate
PRs.

The skeleton's existing "Multi-PR Workflow" section is preserved but
demoted to a subsection labeled "exception, architect-requested only,"
since multi-PR shipping is reserved for specs the architect has
explicitly decided to split into slices.

Adds a regression test asserting all four builder-prompt files contain
the one-PR-per-spec statement and the git-commits-not-PRs clarification.

Fixes #744
@waleedkadous
Copy link
Copy Markdown
Contributor Author

Architect requesting changes — the current text is too rigid.

What you have now:

ONE PR per spec, opened at the end of the implement phase

This would block a real workflow: the architect MAY request a PR early (e.g., for spec review, mid-implementation feedback, or to slice a large spec into shippable PRs). The current wording forbids that.

What we actually want to prohibit: the builder autonomously opening a separate PR for each implementation phase. NOT "one PR per spec, ever, no matter what."

Suggested wording:

## PR Strategy

**Do not autonomously open a PR per implementation phase.** Plan phases ship as git commits within a single PR, not as separate PRs. The plan's instruction that 'each phase commits independently' refers to git commits, not PRs.

By default, the PR is opened during/after the final implement phase, with all phase-commits already on the branch.

### Architect-requested PRs

The architect MAY request a PR at any point — for spec review, mid-implementation feedback, slicing a large spec into shippable PRs, etc. When the architect explicitly asks for a PR earlier (or for additional PRs), follow that direction. The prohibition is specifically on the *builder* autonomously deciding to open per-phase PRs without architect request.

Please update the four prompt files (SPIR + ASPIR × codev/ + skeleton/) and adjust the regression test if it pins the old wording. Same triple-APPROVE CMAP cycle when ready.

…us per-phase PRs

Per architect feedback on PR #747: the previous "ONE PR per spec" wording
was too rigid. It would block legitimate workflows where the architect
explicitly requests a PR earlier — for spec review, mid-implementation
feedback, or slicing a large spec into shippable PRs.

The actual constraint is on the *builder* autonomously deciding to open
a separate PR per implementation phase, not on PRs themselves.

Rewords all four SPIR/ASPIR builder-prompt files (codev + skeleton):

- Replace "ONE PR per spec" with "Do not autonomously open a PR per
  implementation phase" framing.
- Add explicit "Architect-requested PRs" subsection making it clear the
  architect may request a PR at any point.
- In the skeleton, reframe the existing multi-PR mechanics section as
  "Multi-PR Mechanics (when the architect requests sequential PRs)" so
  the loop is preserved as a how-to for architect-driven slice splits.

Updates the regression test to assert all three properties: autonomous
prohibition, git-commits-vs-PRs clarification, and architect-override
carve-out.
Codex review on PR #747 flagged that the regression test covered the
prohibition, git-commits clarification, and architect-override carve-out,
but did not assert the new default PR-timing statement ("By default, the
PR is opened during/after the final implement phase"). That line is the
direct replacement for the old "ONE PR per spec, opened at the end of
the implement phase" wording — without it, builders may revert to per-
phase PRs by default in the absence of architect direction.

Adds a fourth assertion per file (4 files × 4 assertions = 16 total).
@waleedkadous
Copy link
Copy Markdown
Contributor Author

Architect approval — the revised wording captures the intent exactly. The three-part structure (autonomous prohibition / default timing / architect-override carveout) is the shape we want as a pattern for all future prompt restrictions. Approved — please merge with gh pr merge 747 --merge --admin.

@waleedkadous waleedkadous merged commit 42d8d83 into main May 17, 2026
6 checks passed
@waleedkadous waleedkadous deleted the builder/bugfix-744 branch May 17, 2026 00:28
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.

SPIR: builder-prompt ambiguous on one-PR-per-spec vs per-phase PRs

1 participant