release: add API documentation review agents to the process - #42
Merged
jnasbyupgrade merged 7 commits intoJul 27, 2026
Conversation
Adds a new early step that launches two independent review efforts via the Agent tool: a since-last-release review comparing commits against release..HEAD to catch behavior changes missing from HISTORY.asc's STABLE section, and a comprehensive review comparing README.asc's documented make targets/variables against the current code regardless of history. Both are launched in the background right after pre-flight so they have time to finish, and a new gate step requires their findings to be inspected before any release-related git change is made (the HISTORY.asc edit and commit).
|
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: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…definition The prior version pinned the review agents' scope to a fixed list of pgxntool files, which is both too specific (goes stale as files are added/removed) and doesn't reflect the real criterion. Add a 'User-facing API surface' entry to Terminology instead: all make targets and documented-worthy prerequisites (e.g. testdeps), all PGXNTOOL_-prefixed variables, and the scripts users are expected to invoke by hand (setup.sh, pgxntool-sync.sh, update-setup-files.sh, pgtle.sh). Everything else defaults to internal. Both review agents and the Step 5 gate now reference this definition and are told to flag ambiguous cases for the user rather than silently deciding, since the definition is expected to evolve.
Grepping for target definitions across .mk/.mk.sh files is liable to miss pattern rules and generated targets. pgxntool already ships a 'list' target in base.mk that parses make's own database via 'make -p', which is the authoritative source. Point the target-discovery step at that instead, run from a scratch directory that just includes base.mk, and cross-reference its output against pgxntool's own makefiles to exclude targets it merely inherits from PGXS.
jnasbyupgrade
marked this pull request as ready for review
July 27, 2026 22:14
Ran the Step 2A/2B review agents for real (against the current pgxntool state and, as a stand-in since there were no unreleased commits, the 2.0.3..2.1.0 range) to shake out problems with the process before relying on it. They surfaced real gaps (an undocumented file-pruning behavior added to pgxntool-sync, a silently renumbered DEBUG scheme) and several rough edges in the instructions themselves: - make list output includes harmless submake chatter (Makefile, make[1]) that isn't real targets, and misses targets gated behind ifeq/ifdef conditionals in a bare scratch directory. - Commit subject lines are not a reliable filter for whether a commit touches the API surface; the file-level diff must always be checked. - Every PGXNTOOL_-prefixed variable was too broad a rule (internal plumbing vars share the prefix with real user-facing toggles). - Generated-file targets (META.json, meta.mk, control.mk) are build plumbing, not something a user intentionally runs, while dev-helper targets (list, print-%) are genuinely user-facing. - DEBUG's existence can be documented, but its specific level numbers are an internal implementation detail, not a stable contract. - Doc-only changes to files that ship to consumers don't need a HISTORY.asc entry. Moves the full 'user-facing API surface' definition out of SKILL.md's Terminology section and into this repo's own CLAUDE.md (not ../pgxntool/CLAUDE.md, which is user-facing docs for extension developers, not dev/audit tooling docs), so it can evolve independently of the release process instructions that consume it. SKILL.md now points to that section and keeps only the make-list discovery mechanics and the review-effort descriptions.
- pgxntool/CLAUDE.md is AI-agent instructions for people working in consumer repos, not internal pgxntool-developer documentation -- it ships with every subtree pull the same way README.asc does, so substantive changes to it are in scope (previously exempted as 'doc-only', which was wrong). - Conditionally-gated helper targets that only exist to support another target's lifecycle (clean-test-build, the install-schedule file target) are internal, not meant to be invoked by hand -- unlike the primary target they support (test-build), which stays in scope on its own merits. Also makes explicit that finding conditionally-defined targets requires reading the source directly; make list alone won't surface them from a bare scratch directory.
2 tasks
No existing instruction (here or in the release skill's own, narrower pre-flight check) required confirming local master matches upstream before branching off it. Found the gap the hard way this session: local pgxntool master was 8 commits behind upstream/master, and one of the missed commits had already fixed one of the exact issues the API documentation review was about to redo.
jnasbyupgrade
added a commit
to jnasbyupgrade/pgxntool
that referenced
this pull request
Jul 27, 2026
git subtree pull copies pgxntool's entire tree into the consumer project, including dev-only directories that don't belong there. prune_pgxntool_dev_dirs() (lib.sh) has silently removed these after every sync since Postgres-Extensions#41, but the behavior was never documented anywhere -- surfaced by the pgxntool-test API documentation review (Postgres-Extensions/pgxntool-test#42).
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
1 participant
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.
Summary
/releaseskill, run right after pre-flight passes and in the background.releasetag andHEADin../pgxntool; flags behavior changes not called out inHISTORY.asc's STABLE section, and API items added/removed by those commits but not reflected inREADME.asc. Commit subject lines are explicitly called out as an unreliable filter — the file-level diff must always be checked.README.asc.HISTORY.ascin Step 6 onward) happens until both sets of findings are retrieved and inspected. Since-last-release findings must be fixed before continuing; comprehensive findings go to the user for a fix-now/follow-up/dismiss decision; ambiguous scope calls also go to the user, with a nudge to fold the answer back into the definition.SKILL.mdand into this repo's ownCLAUDE.md(../pgxntool/CLAUDE.mdis user-facing docs for extension developers, not dev/audit tooling), so the definition can evolve independently of the process instructions that consume it. Target discovery uses pgxntool's ownmake listtarget rather than grepping for target definitions, since pattern rules and generated targets are easy to miss that way.Validated with a real dry run
Ran both review efforts for real (Step 2B against pgxntool's current state, Step 2A as a stand-in against the
2.0.3..2.1.0range, since there are currently no unreleased commits). That surfaced real process rough edges, now folded into the instructions:make list's output includes harmless submake chatter (Makefile,make[1]) and misses targets gated behindifeq/ifdefconditionals in a bare scratch directory.PGXNTOOL_-prefixed variable" rule was too broad — internal plumbing vars share the prefix with real user-facing toggles; narrowed to variables designed for override.META.json,meta.mk,control.mk) are build plumbing; dev-helper targets (list,print-%) are genuinely user-facing.DEBUG's existence can be documented, but its specific level numbers are an internal implementation detail, not a stable contract.HISTORY.ascentry.It also surfaced two real, currently-undocumented gaps in pgxntool itself (not fixed by this PR — flagged separately):
pgxntool-syncsilently deletespgxntool/.github/pgxntool/.claudefrom consumer projects with noHISTORY.asc/README.ascmention, andpgtle.sh'sDEBUGlevel numbers were renumbered in a way that silently breaks oldDEBUG=1-4usage.Test plan
SKILL.mdandCLAUDE.mdend-to-end to confirm step numbers and cross-references are consistent./releaseon a real release to confirm the review agents produce useful, actionable findings in practice going forward.🤖 Generated with Claude Code