BOHICA-LABS / BOHICA-LABS/vsdd-factory

pr-manager: enforce PR base == trunk for story PRs (P1)

Open
#358 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2
Forks
1
Avg merge
6h 43m
Merged PRs (30d)
29

Description

## Summary

The `pr-manager` skill does not validate that per-story PRs target the configured trunk branch (`main` for ArcavenAE/akey). When an upstream branch is supplied as base, the PR merges into that branch — silently producing orphan content trees that look "merged" in the GitHub UI but never reach the trunk.

## Severity: P1

This is a silent correctness defect. CI passes, GitHub reports `state: MERGED, mergedBy: `, and the merge commit is signed — but `git log origin/main` does not contain the commit. The factory orchestrator only catches this with a post-merge `git branch -r --contains ` audit, which is not part of the standard convergence checklist.

## Concrete instance — ArcavenAE/akey, Phase 3 Wave 1

PR #14 (STORY-008-serializer→STORY-004-impl) merged cleanly:

\`\`\`
$ gh pr view 14 -R ArcavenAE/akey --json state,mergedAt,mergeCommit,baseRefName
{
"state": "MERGED",
"mergedAt": "2026-06-30T16:11:17Z",
"mergeCommit": {"oid": "ac105b89817ed4b5646f733ec61d80059b3d6033"},
"baseRefName": "phase-3/wave-1-STORY-004-impl" ← NOT main
}
\`\`\`

Post-merge verification:

\`\`\`
$ git branch -r --contains ac105b8
origin/phase-3/wave-1-STORY-004-impl
$ gh api repos/ArcavenAE/akey/branches/main -q .commit.sha
51ef1e0… ← still PR #12's merge, ac105b8 is NOT here
\`\`\`

The STORY-008-serializer content was orphaned on a feature branch. Recovery required:
1. Open PR with feature-branch→main base (PR #15) — CONFLICTING due to stale base
2. Cherry-pick the three orphan commits onto current main on a fresh branch (PR #16)
3. Re-merge cleanly

Time cost: ~3 round-trips of CI + manual git surgery in a temp worktree.

## How pr-manager allowed this

The orchestrator dispatch for PR #14 specified the *head* branch but did not lock the *base*. pr-manager invoked \`gh pr create\` without an explicit \`--base main\` flag (or with a base sourced from a stale local variable). GitHub defaulted to the repo's default branch only when no base was supplied locally — and when \`gh pr create\` is run inside a worktree that has another feature branch as its tracking upstream, GitHub will infer base from upstream tracking rather than the repo default. This is a subtle interaction between gh CLI defaults and branch tracking that pr-manager does not guard against.

## Proposed fix

In \`pr-manager\` skill (or wherever \`gh pr create\` is invoked for story PRs):

1. **Always pass \`--base \` explicitly.** Never rely on inference. Resolve \`trunk\` from \`.factory/STATE.md\` or the orchestrator's project config.
2. **Post-create assertion.** Immediately after \`gh pr create\`, run \`gh pr view --json baseRefName\` and assert \`baseRefName == trunk\`. Hard-fail the burst if not.
3. **Post-merge assertion.** After \`gh pr merge\`, run \`git fetch origin && git merge-base --is-ancestor origin/\`. If not an ancestor, raise immediately — content did not actually land.

(3) is the load-bearing check. It would have caught PR #14 immediately and avoided ~30 minutes of forensics.

## Cross-references

- akey orchestration session 2026-06-30: full PR #13/#14/#15/#16 chain recorded in \`.factory/cycles/v0.1.0-phase1-spec/burst-log.md\` P3-03
- Related: #357 (Red Gate / CI paradox — forced tests+impl shape), #349 (protection-context mismatch — adjacent pr-manager guard gap)

## Acceptance

- Story PR created with explicit \`--base \`
- pr-manager refuses to proceed past create if base mismatches expected trunk
- pr-manager refuses to mark a story as "delivered" until \`git merge-base --is-ancestor\` confirms main ancestry of the merge commit

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.