CLAUDE.md: document merge order rule for paired pgxntool/pgxntool-test PRs - #91
Open
jnasbyupgrade wants to merge 3 commits into
Open
jnasbyupgrade wants to merge 3 commits into
jnasbyupgrade wants to merge 3 commits into
Conversation
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>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
it): the pgxntool PR must merge first.
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.
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, andtest-buildordering) 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:
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-resultstoresults-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.mdconventions) states a merge-order rule for this pairing —../ai/PR.mdonly has the generic "ask what order split PRs should mergein 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-auditskill: crossref-audit is an after-the-fact checkfor 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