Skip to content

fix: ensure all PR-creating workflows target $BASE_BRANCH#1453

Closed
ericsoriano wants to merge 1 commit intocoleam00:mainfrom
ericsoriano:archon/task-archon-plan-to-pr-1777343459568
Closed

fix: ensure all PR-creating workflows target $BASE_BRANCH#1453
ericsoriano wants to merge 1 commit intocoleam00:mainfrom
ericsoriano:archon/task-archon-plan-to-pr-1777343459568

Conversation

@ericsoriano
Copy link
Copy Markdown
Contributor

Summary

Three bundled default workflows produced PRs targeting the repo's GitHub default branch instead of the configured $BASE_BRANCH. This PR adds --base $BASE_BRANCH to the three offending prompts/commands and adds a defensive verify-pr-base bash node to all seven PR-creating workflows that auto-corrects via gh pr edit if the AI ever drops the flag.

Changes

File Action Description
.archon/workflows/defaults/archon-architect.yaml UPDATE Add --base $BASE_BRANCH to create-pr prompt + add verify-pr-base node
.archon/workflows/defaults/archon-refactor-safely.yaml UPDATE Add --base $BASE_BRANCH to create-pr prompt + add verify-pr-base node
.archon/commands/defaults/archon-implement-issue.md UPDATE Add --base $BASE_BRANCH to gh pr create invocation
.archon/workflows/defaults/archon-fix-github-issue.yaml UPDATE Add verify-pr-base node, rewire review-scope dependency
.archon/workflows/defaults/archon-feature-development.yaml UPDATE Add verify-pr-base node
.archon/workflows/defaults/archon-plan-to-pr.yaml UPDATE Add verify-pr-base node, rewire review-scope dependency
.archon/workflows/defaults/archon-idea-to-pr.yaml UPDATE Add verify-pr-base node, rewire review-scope dependency
.archon/workflows/defaults/archon-issue-review-full.yaml UPDATE Add verify-pr-base node, rewire review-scope dependency
packages/workflows/src/defaults/bundled-defaults.generated.ts UPDATE Regenerated via bun run generate:bundled
CHANGELOG.md UPDATE Added Fixed entry under [Unreleased]

Tests

No new test files — the existing bundled-defaults.test.ts self-counts and round-trips content automatically, covering all modified files.

Validation

  • Bundled defaults check passes (36 commands, 20 workflows)
  • Type check passes (all 10 packages)
  • Lint passes (0 errors, 0 warnings)
  • Format check passes
  • All tests pass (3451 tests)

Implementation Notes

Deviations from Plan

  • No fallback gh pr create in archon-implement-issue: Plan mentioned updating a fallback at line ~384 — only one gh pr create invocation exists in the file. The single instance was updated correctly.

Plan: .claude/archon/plans/fix-pr-base-branch-targeting.md
Workflow ID: e77646a5cfb868783688d8a8f228ff5d

- Add --base $BASE_BRANCH to gh pr create in archon-architect,
  archon-refactor-safely, and archon-implement-issue
- Add verify-pr-base bash node to all 7 PR-creating workflows
  that auto-corrects via gh pr edit if the AI mis-targets
- Rewire downstream depends_on edges through verify-pr-base
- Regenerate bundled-defaults.generated.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0161dbc3-40ab-457f-bcbb-3e8696a0d14f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ericsoriano
Copy link
Copy Markdown
Contributor Author

Comprehensive PR Review

PR: #1453 — fix: ensure all PR-creating workflows target $BASE_BRANCH
Reviewed by: 5 specialized agents
Date: 2026-04-27


Summary

This PR adds --base $BASE_BRANCH to three previously-missing gh pr create invocations and introduces a defensive verify-pr-base bash node across all seven PR-creating default workflows. Changes are well-structured, consistent, correctly dependency-wired, and follow project conventions.

Verdict: APPROVE

Severity Count
CRITICAL 0
HIGH 0
MEDIUM 0
LOW 7

All 5 agents recommend approval. No blocking issues found.


LOW Issues (For Consideration)

View 7 low-priority suggestions (all recommended: keep as-is)
# Issue Agent Recommendation
1 Duplicated verify-pr-base bash block across 7 workflows Code Review Keep as-is: small, stable, self-contained duplication
2 Two gh pr view calls could be combined into one Error Handling Keep as-is: functionally correct, set -e catches failures
3 Mismatch diagnostic only on stderr, not captured in node output Error Handling Keep as-is: stderr is conventional, no downstream consumer
4 No test assertion for verify-pr-base node presence Test Coverage No action: defense-in-depth, scope excludes new tests
5 No test assertion for --base $BASE_BRANCH in prompts Test Coverage No action: verify-pr-base provides runtime safety net
6 No inline YAML comment on verify-pr-base nodes Comment Quality Keep as-is: node ID and echo messages are self-documenting
7 authoring-commands.md example omits --base Docs Impact No change: example teaches a different pattern

What's Good

  • Consistent pattern: All 7 workflows get identical verify-pr-base nodes with correct dependency wiring
  • Defense-in-depth: Bash node acts as safety net even if AI prompt adherence drifts
  • Correct dependency chains: Downstream review nodes properly rewired through verify-pr-base
  • Strict error handling: All bash nodes use set -euo pipefail
  • Clear diagnostics: Stderr messages include expected vs actual values
  • Excellent CHANGELOG entry: Root cause, fix, and defense-in-depth clearly documented
  • Scope discipline: Minimal fix, no unnecessary engine-layer changes
  • Adequate test coverage: Existing round-trip and variable substitution tests cover changes

Next Steps

No auto-fix step needed. PR is ready to merge.


Reviewed by Archon comprehensive-pr-review workflow
Artifacts: review/consolidated-review.md

@ericsoriano
Copy link
Copy Markdown
Contributor Author

⚡ Auto-Fix Report

Status: COMPLETE (no fixes needed)


Review Summary

The comprehensive review (5 agents) found 0 CRITICAL, 0 HIGH, and 0 MEDIUM issues. All 7 findings are LOW severity with "keep as-is" recommendations.

Severity Found Fixed Skipped
🔴 CRITICAL 0 0 0
🟠 HIGH 0 0 0
🟡 MEDIUM 0 0 0
🔵 LOW 7 0 7 (all "keep as-is")

🔵 LOW Issues (All "Keep As-Is")

# Issue Recommendation
1 Duplicated verify-pr-base across 7 workflows Appropriate duplication per DRY guidance
2 Duplicate gh pr view calls Functionally correct, minor optimization only
3 Stderr-only diagnostic on mismatch Conventional for diagnostics
4 No test for verify-pr-base node existence Defense-in-depth node, not primary fix
5 No test for --base $BASE_BRANCH in prompts Runtime safety net sufficient
6 No inline YAML comment on verify-pr-base Node ID and echo messages are descriptive
7 Docs example omits --base Example teaches different pattern

📋 Suggested Follow-up Issues

None required.


Verdict: PR is clean — no code changes needed. Ready to merge.


Auto-reviewed by Archon comprehensive-pr-review workflow

@ericsoriano
Copy link
Copy Markdown
Contributor Author

Workflow Summary

Plan: .claude/archon/plans/fix-pr-base-branch-targeting.md
Status: ✅ Implementation complete, PR ready for review


Implementation vs Plan

Metric Planned Actual
Files updated 10 10
Files created 0 0
Tests added 0 0
Deviations - 1
📋 Deviations from Plan (1)

No fallback gh pr create in archon-implement-issue: Plan mentioned updating a fallback at line ~384 — only one gh pr create invocation exists in the file. The single instance was updated correctly. No impact.


Review Summary

Severity Found Fixed Remaining
CRITICAL 0 0 0
HIGH 0 0 0
MEDIUM 0 0 0
LOW 7 0 7 (all "keep as-is")

All 5 review agents recommend APPROVE. All 7 LOW findings are intentional design choices (appropriate duplication, conventional stderr diagnostics, existing test coverage adequate).


Validation

  • ✅ Bundled defaults check (36 commands, 20 workflows)
  • ✅ Type check (all 10 packages)
  • ✅ Lint (0 errors, 0 warnings)
  • ✅ Format check
  • ✅ All tests pass (3451 tests)

Follow-Up

No follow-up issues, documentation updates, or quick wins needed. PR is clean and ready to merge.

ℹ️ Deferred Items (NOT Building)

These were intentionally excluded from scope:

  • No engine-layer code changes (existing $BASE_BRANCH substitution already correct)
  • No new TypeScript test files (existing bundled-defaults.test.ts self-counts and round-trips automatically)
  • No new packages, dependencies, or interface changes
  • No DB migrations

Artifacts: ~/.archon/workspaces/ericsoriano/Archon/artifacts/runs/e77646a5cfb868783688d8a8f228ff5d/

@ericsoriano
Copy link
Copy Markdown
Contributor Author

Closing — this PR was opened against the wrong base repository by accident. gh pr create defaults base repo to the parent fork; it should have been targeted at my fork (ericsoriano/Archon) for review first. Will reopen there.

@ericsoriano ericsoriano deleted the archon/task-archon-plan-to-pr-1777343459568 branch April 28, 2026 06:40
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