Canary verification PR for the shared claude-code-review.yml workflow - #90
jnasbyupgrade wants to merge 2 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>
The printed layout is relative to bin/, not safe to paste into a `cd` from an arbitrary shell. Co-Authored-By: Claude Opus 5 (1M context) <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 |
|
Claude finished @jnasbyupgrade's task in 3m 24s —— View job Code Review — PR #90 (canary verification)
Scope: 6-line diff in Finding (as intended by the PR description):
No other issues found (removed-behavior, cross-file breakage, reuse/simplification/efficiency, and CLAUDE.md convention checks were all clean — the added Inline comments posted:
This confirms the canary's goal: the shared workflow ran via |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| # Resolve to an absolute path since WORKTREE_DIR above is relative to | ||
| # SCRIPT_DIR and not safe to paste into a `cd` from an arbitrary shell. | ||
| absPath="$(cd "$WORKTREE_DIR" && pwd)" |
There was a problem hiding this comment.
absPath breaks the naming convention used everywhere else in this script — WORKTREE_NAME, SCRIPT_DIR, WORKTREES_BASE, and WORKTREE_DIR are all UPPER_SNAKE_CASE. For consistency this should be ABS_PATH (or reuse/extend WORKTREE_DIR directly rather than introducing a new variable with a different case style).
| absPath="$(cd "$WORKTREE_DIR" && pwd)" | |
| ABS_PATH="$(cd "$WORKTREE_DIR" && pwd)" |
| # SCRIPT_DIR and not safe to paste into a `cd` from an arbitrary shell. | ||
| absPath="$(cd "$WORKTREE_DIR" && pwd)" | ||
| echo | ||
| echo "Absolute path: $absPath" |
There was a problem hiding this comment.
Note: if you rename the variable on line 45 to ABS_PATH per the previous comment's suggestion, this reference needs to change too ($absPath → $ABS_PATH), otherwise this becomes an unbound-variable error under set -u.
b1502e9 to
2491a57
Compare
1 participant
Deliberate canary-verification PR for #89's migration to
Postgres-Extensions/ai's shared, reusableclaude-code-review.yml— not real feature work. That migration's own CI couldn't exercise the newpull_request_targetworkflow (GitHub reads that trigger's workflow file from the base branch as it stood before the triggering PR), so this PR is the actual end-to-end proof.The change itself (
bin/create-worktree.sh) is small and genuinely useful — it prints the created worktree's absolute path, since the layout printed above it is relative tobin/and not safe to paste into acdfrom an arbitrary shell — but it also contains one deliberately findable minor issue (an inconsistent variable-naming convention) to confirm the review posts a real inline comment, not just that the job ran.Safe to close without merging once verified.