Skip to content

chore: enforce one-PR-per-story in orchestration (workflow + implementer + implement skill + git-workflow) [+ gate/test ADL pointer]#326

Merged
rucka merged 1 commit into
mainfrom
chore/one-pr-per-story-rule
Jul 13, 2026
Merged

chore: enforce one-PR-per-story in orchestration (workflow + implementer + implement skill + git-workflow) [+ gate/test ADL pointer]#326
rucka merged 1 commit into
mainfrom
chore/one-pr-per-story-rule

Conversation

@rucka

@rucka rucka commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

What Changed

Encodes two owner-mandated rules into the Pair orchestration tooling and shipped process, as one PR (this change itself follows one-PR-per-story):

  • Rule 2 (primary) — One PR per story. A story is delivered on ONE branch with ONE PR, opened the first time and UPDATED for all subsequent work (further tasks/features included); never a second PR for the same story unless a human explicitly says so. The open-PR step is create-or-update.
  • Rule 1 (reference) — gate/tooling code in tested modules. A brief pointer to the ADL (.pair/adoption/decision-log/2026-07-13-gate-tooling-code-in-tested-modules.md) added where implementers see coding rules — not restated in full.

Why This Change

The convention (single-PR-per-story) is already recorded in adoption (way-of-working.md) and its ADL (2026-07-12-one-pr-per-story-default.md), but the orchestration tooling and process files that implementers/agents execute did not state it explicitly. Recent stories (#312, #313) carried standing per-task/per-batch-PR instructions in their issue bodies, which contradicted the norm. This PR makes the rule unambiguous at every point of authorship.

Story Context

Owner rules, no PM story issue. Rule 1's gate/test ADL is added on PR #315 (this PR only adds the pointer to that adoption path; it does not create the ADL file).

Changes Made

Implementation Details

  • .claude/workflows/implement-batch.js (local tooling, no dataset mirror) — Input comment block now states batch items are STORIES (never tasks); each story = ONE branch + ONE PR opened once and updated iteratively (create-or-update, reusing the existing branch/{prNumber}); a second PR for the same story is forbidden unless a human explicitly instructs it. Code behavior unchanged (it already drives one PR per input story) — contract/comment clarification only.
  • .claude/agents/implementer.md (local) — new operating rule: "One PR per story (the open-PR step is create-or-update)"; and a Rule-1 pointer to the gate/test ADL appended to the test-first rule. Frontmatter description left as-is (its "opens the PR" is correct).
  • pair-process-implement SKILL.md (dataset packages/knowledge-hub/dataset/.skills/process/implement/SKILL.md + .claude/skills/pair-process-implement/SKILL.md mirror) — single reinforcement line under the overview: one PR per story, subsequent work updates the same PR. Byte-consistent under the name-prefix transform (validated by transfer-dataset tests).
  • git-workflow development-process.md (dataset + root .pair/ mirror) — normative "PR granularity — one PR per story" bullet added to the Feature Branches (branch/PR strategy) section. Mirrors byte-identical.
  • .claude/agents/reviewer.md — reviewed, left untouched. The reviewer never opens PRs or merges, and contains no PR-granularity language, so Rule 2 requires no change there.

Per-task-PR phrasing audit

Grepped the dataset skills + collaboration templates for "each task ... own PR" / "one PR per task" / "PR per task": zero editable hits. The only PR-granularity statements in the implement skill already say the correct thing ("Single PR at end", "Single PR per story regardless of commit strategy"); commit-per-task is a commit strategy within one PR, not a PR-per-task mandate. The stale per-task/per-batch-PR instruction existed only in the issue bodies of #312/#313 (PM data), which are being corrected separately via gh issue edit — not in this PR.

Files Changed

  • Modified: .claude/workflows/implement-batch.js, .claude/agents/implementer.md, .claude/skills/pair-process-implement/SKILL.md, packages/knowledge-hub/dataset/.skills/process/implement/SKILL.md, .pair/knowledge/guidelines/technical-standards/git-workflow/development-process.md, packages/knowledge-hub/dataset/.pair/knowledge/guidelines/technical-standards/git-workflow/development-process.md

Testing

Test Results

node --test .claude/workflows/implement-batch.test.mjs   PASS 8/8 (no header/contract assertions changed)
pnpm --filter @pair/knowledge-hub test (via turbo)       PASS 80/80 + check:links clean
  transfer-dataset (dataset<->.claude mirror transform)  PASS 29/29
pnpm mdlint:check                                        PASS (exit 0)
pnpm docs:staleness                                      PASS - 35 skills, 8 commands in sync
pre-commit ts:check                                      PASS
pre-push quality-gate                                    PASS

Quality Assurance

Code Quality Checklist

  • Mirror discipline: dataset + .claude/root mirrors byte-consistent in the same commit for every shipped file touched
  • Committed only the files changed (explicit paths, no git add -A)
  • implement-batch.js behavior unchanged (comment/contract clarification only)
  • Owner Rule 1 pointer links the adoption path, does not restate the ADL

🤖 Generated with Claude Code

- implement-batch.js Input: batch items are STORIES (one branch + one PR, create-or-update), never one-PR-per-task; second PR forbidden unless human instructs
- implementer.md: add one-PR-per-story operating rule + pointer to gate/test ADL
- implement SKILL (dataset + .claude mirror): one-PR-per-story reinforcement line
- git-workflow (dataset + root mirror): normative one-PR-per-story in branch/PR strategy
- reviewer.md unchanged (never opens PRs — Rule 2 no-op)

Refs owner rules 1 (gate/test) + 2 (one-PR-per-story); gate/test ADL lands on PR #315.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rucka rucka merged commit 1ea9fab into main Jul 13, 2026
1 check passed
@rucka rucka deleted the chore/one-pr-per-story-rule branch July 13, 2026 19:24
@rucka

rucka commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review — PR #326

Review Information

PR Number: #326
Author: rucka (Gianluca Carucci)
Reviewer: Independent reviewer (automated, blind to author working context)
Review Date: 2026-07-13
Story/Epic: None — owner-rules change, no PM story. Acceptance bar = owner Rule 2 (primary: one PR per story) + Rule 1 (pointer only) + repo conventions (CLAUDE.md, mirror discipline).
Review Type: Config / Docs (process + orchestration tooling)

Review Summary

Overall Assessment

  • Approved with Comments — no blocking issues; one Minor consistency nit + one accepted forward-reference noted, can merge.

Key Changes

Encodes owner Rule 2 at every point of authorship: the implement-batch.js Input-contract comment, implementer.md operating rule, pair-process-implement SKILL.md (dataset + .claude mirror), and git-workflow development-process.md (dataset + root mirror). Adds a Rule-1 pointer to the gate/tooling ADL in implementer.md.

Business Value

Removes the ambiguity that let #312/#313 issue bodies carry contradictory per-task/per-batch-PR instructions. The one-PR-per-story rule is now unambiguous everywhere an implementer/agent reads authoring rules.

Verification vs acceptance bar

# Check Result
1 Rule consistent across all 4 targets, none implies per-task PRs PASS (see Minor below on escape-hatch wording)
2 implement-batch.js comment-only, no behavior change; smoke tests PASS — diff touches only the Input comment block; node --test implement-batch.test.mjs = 8/8
3 Mirror byte-consistency (git-workflow; implement SKILL.md) PASS — see Testing/Gates
4 reviewer.md untouched and correctly needs no change PASS — absent from diff; reviewer never opens PRs/merges
5 ADL pointer path correct; no full Rule-1 restatement PASS — path matches date-kebab convention; forward-ref only (see nonActionable)
6 No residual per-task-PR phrasing (dataset .skills + collaboration templates) PASS — zero hits
7 Scope: only intended process/doc/tooling files PASS — exactly the 6 intended files, no stray package.json/test/dist
8 Gates: smoke, mdlint, docs:staleness, mirror PASS — see Testing/Gates
+ Out-of-PR spot-check: #312 / #313 issue-body edits PASS — clean; see PM-data note

Detailed Review Comments

Positive Feedback

  • Rule stated identically in substance across all four targets; no target implies per-task PRs. The pre-existing commit-per-task language is correctly left as a commit strategy within one PR, not a PR-per-task mandate.
  • Mirror discipline correctly applied: dataset+.claude/root mirrors updated in the same commit for every shipped file; the two .claude-local orchestration files (implementer.md, implement-batch.js) correctly have no dataset mirror to update.
  • implement-batch.js change is genuinely comment-only — driveStory/parallel logic untouched, smoke suite unchanged and green.
  • Scope is tight: exactly the 6 intended files, explicit-path commit, no stray artifacts.

Minor Issues 💡

  • packages/knowledge-hub/dataset/.skills/process/implement/SKILL.md:12 (and .claude mirror) — The escape hatch "…never a new one unless explicitly requested" omits the authorized actor. The owner rule and the other three targets all name it: git-workflow "a human explicitly says so"; implement-batch.js "a human explicitly instructs it"; implementer.md "the orchestrator explicitly instructs it". In the shipped skill (consumed by adopting teams), unqualified "explicitly requested" could be read as agent- or self-initiated. Recommendation: align to "unless a human explicitly requests it" for one consistent authority across all four targets. Non-blocking.

Questions / Notes ❓

  • Escape-hatch actor differs by design: implementer.md says "the orchestrator explicitly instructs it" while the other targets name "a human". This is not a contradiction — the implementer's command channel is the orchestrator, and the orchestrator's own governing doc (implement-batch.js) requires "a human explicitly instructs it", so the human requirement is preserved up the chain. Role-appropriate; noted for transparency.

Testing Review

node --test .claude/workflows/implement-batch.test.mjs   PASS 8/8 (Input comment not asserted; behavior unchanged)
markdownlint (5 changed .md files, repo config)          PASS (exit 0, no findings)
docs:staleness (scripts/docs-staleness-check.js)         PASS — 35 skills, 8 commands in sync

Mirror byte-consistency (verified directly):

  • git-workflow development-process.md: root .pair/ vs packages/knowledge-hub/dataset/.pair/byte-identical.
  • pair-process-implement SKILL.md: .claude/skills/... vs dataset/.skills/process/implement/... → differs only by the name-prefix transform (pair-process-implementimplement, /pair-capability-*/*); the added "One PR per story" line is byte-identical in both (no skill names). This is exactly what the transfer-dataset gate asserts.
  • Full pnpm --filter @pair/knowledge-hub test (vitest 80/80 + transfer-dataset 29/29 + check:links) is reported green in the PR body; not re-run here — the throwaway detached worktree has no node_modules — but the core mirror assertion was verified directly (above) and passes.

Adoption Compliance

Rule 2 is backed by adoption: way-of-working.md ("PR granularity — one PR per story (default)") + ADL 2026-07-12-one-pr-per-story-default.md. The new git-workflow bullet is consistent with — and a slightly stricter operational framing of — the way-of-working default (which allows splitting "with an explicit reason"); the PR's "unless a human explicitly says so" is a valid operationalization, not a conflict.

Tech Debt

None introduced. Docs/comment-only change.

Risk Assessment

Low. No runtime behavior change; documentation + one JS comment block + one tooling comment block. No dependencies, no API/schema surface.

Non-actionable / Accepted findings (carried to human merge gate)

Out-of-PR PM-data note (spot-check, not in the git diff)

Issues #312 and #313 were edited separately via gh issue edit (PM data, outside this git PR). Both now carry "Delivered on a single PR per story (tasks are commits/checkboxes within it)" in the DoD checklist and task-breakdown note. Verified: markdown intact (tables, GWT sections, redirect table all render), no mangling, historical delivery narrative not rewritten, and no residual per-task/per-batch-PR instruction remains. These edits are consistent with the rule this PR encodes but are not part of the reviewable diff.


Verdict: Approved with Comments. All eight verification items pass; the rule is consistent and non-contradictory across all four targets, mirrors are byte-consistent, scope is clean, gates are green, and reviewer.md is correctly untouched. One Minor phrasing-consistency nit (shipped SKILL.md escape-hatch actor) and one accepted ADL forward-reference are noted; neither blocks merge.

rucka added a commit that referenced this pull request Jul 13, 2026
- implement-batch.js Input: batch items are STORIES (one branch + one PR, create-or-update), never one-PR-per-task; second PR forbidden unless human instructs
- implementer.md: add one-PR-per-story operating rule + pointer to gate/test ADL
- implement SKILL (dataset + .claude mirror): one-PR-per-story reinforcement line
- git-workflow (dataset + root mirror): normative one-PR-per-story in branch/PR strategy
- reviewer.md unchanged (never opens PRs — Rule 2 no-op)

Refs owner rules 1 (gate/test) + 2 (one-PR-per-story); gate/test ADL lands on PR #315.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant