Skip to content

CLAUDE.md: document merge order rule for paired pgxntool/pgxntool-test PRs - #91

Open
jnasbyupgrade wants to merge 3 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:docs/paired-pr-merge-order
Open

jnasbyupgrade wants to merge 3 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:docs/paired-pr-merge-order

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Docs-only. No paired pgxntool PR.

What

Adds a "Merge Order for Paired PRs" section to CLAUDE.md's "PR-Based
Workflow" section, split by direction:

  • Addition (pgxntool adds behavior, pgxntool-test adds coverage for
    it): the pgxntool PR must merge first.
  • Removal/rename (pgxntool removes or renames something
    pgxntool-test's existing tests already reference): no unilateral order
    is safe — land near-atomically or keep a deprecation window, and flag
    sequencing to the maintainer rather than picking blind.
  • If pgxntool-test's tests only cover already-existing pgxntool behavior,
    order doesn't matter.

Why

Once a pgxntool-test PR has no paired pgxntool branch (matched by branch
name and account), its CI falls back to running against pgxntool's
actual master (per README.md's CI section). That's the mechanism behind
both directions:

Addition evidence: pgxntool-test PR #79 (tests for
check-test-install-error-stop.sh, build-results, and test-build
ordering) merged 2026-09-08, eight days before its paired pgxntool PR #109
— which actually added the script and gating those tests exercise —
merged 2026-09-16. In that window, unrelated pgxntool-test PRs with no
paired pgxntool branch failed CI with a misleading error rather than an
obvious "wrong repo" signal — confirmed via the actual run logs for PRs
#84 and #85:

.../pgxntool/test/bin/check-test-install-error-stop.sh: No such file or directory

Removal/rename evidence: pgxntool PR #93 (renamed 5 internal
PGXNTOOL_* variables to _PGXNTOOL_*) and its paired pgxntool-test PR
#72 merged ~2h8m apart same day — no unrelated pgxntool-test PR happened
to run CI in that gap, but the rename still broke something else with no
transition period: pgxntool PR #95 (open, unrelated), whose own new code
referenced the pre-rename name, broke the moment its branch merged master
and picked up #93's rename (fixed in commit 42e0903). pgxntool #123
(renames build-results to results-build) and its paired pgxntool-test
#88 are open as of this writing, in the same unresolved category.

No prior doc anywhere (CLAUDE.md, the crossref-audit skill, or the shared
../ai/PR.md conventions) states a merge-order rule for this pairing —
../ai/PR.md only has the generic "ask what order split PRs should merge
in if it's not obvious." This documents the concrete, repo-specific rule
the incidents above establish.

Placement

Landed under "PR-Based Workflow: Merges Happen Outside AI Control" rather
than the crossref-audit skill: crossref-audit is an after-the-fact check
for a different problem (a missing cross-reference in an already-merged
commit message); this is a before/at-merge-time sequencing decision, so it
sits with the rest of the PR-workflow section rather than inside the audit
skill's own scope.

🤖 Generated with Claude Code

jnasbyupgrade and others added 2 commits September 17, 2026 15:33
Replaces the hand-maintained review workflow with a thin caller into Postgres-Extensions/ai's reusable `workflow_call` version, so a future fix lands here without a manual copy. pgxntool-test is the permanent canary for that shared workflow, pinned to `@main` rather than `@v1`, so a change runs here for real before the `v1` tag protecting the other consuming repos is ever moved to include it. This also picks up the `--comment` fix for the silent-review-failure bug that the old per-repo copy still carried.

**This PR cannot be fully validated by its own CI.** A `pull_request_target` workflow change only takes effect after merging to the base branch, and even then it's only exercised by a *subsequent* PR event against this repo — this PR's own CI run still uses the old workflow file. What's verified here is structural correctness only: the YAML parses, `Postgres-Extensions/ai/.github/workflows/claude-code-review.yml@main` resolves (`gh api repos/Postgres-Extensions/ai/contents/.github/workflows/claude-code-review.yml?ref=main` returns the file), the repo's default workflow permissions (`read`) and secret availability match what every other caller relies on via `secrets: inherit`, and `git merge-tree` against `upstream/master` is clean.

Once merged, the real proof is the next PR opened against this repo.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t PRs

pgxntool-test PR Postgres-Extensions#79 merged eight days before its paired pgxntool PR #109,
which actually added the behavior Postgres-Extensions#79's new tests exercised
(check-test-install-error-stop.sh, test-build/installcheck gating). Since a
pgxntool-test PR with no paired pgxntool branch runs CI against pgxntool
master directly, this broke CI for every unrelated pgxntool-test PR in that
window (e.g. Postgres-Extensions#84, Postgres-Extensions#85) with a misleading "No such file or directory" error.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c2307f70-7dfe-4339-8700-d5da90a7df7b

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

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.

The addition case (pgxntool-test's new tests reference not-yet-existing
pgxntool behavior) has one safe order: pgxntool merges first, per Postgres-Extensions#79/#109.

Removal/rename is different: both orders leave one master's tests broken
against the other's, since there's no version of the behavior both sides
agree on simultaneously. #93/Postgres-Extensions#72 (a variable rename) landed ~2 hours apart
with no unrelated PR caught in the gap, but the rename still broke #95's
own in-flight code via a plain merge from master (fixed in 42e0903) --
confirming a rename has no free lunch even when landed close together.
#123/Postgres-Extensions#88 (open, unmerged) is a live example of this same unresolved case.

Co-Authored-By: Claude Sonnet 5 <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