-
Notifications
You must be signed in to change notification settings - Fork 1
Cross-reference rules: allow GitHub URLs, add crossref-audit skill for website merges #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d1382d1
d48aa9f
971376b
5a5a0f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| --- | ||
| name: crossref-audit | ||
| description: | | ||
| Audit master in both pgxntool and pgxntool-test for paired commits (a code | ||
| change plus its corresponding test coverage) that are missing a | ||
| cross-reference to each other. Fixes safely when it's a single, recent, | ||
| tip-of-master commit; otherwise stops and asks. | ||
|
|
||
| Use when: at the end of each round of work in this project (not just | ||
| session start — sessions run long), before rebasing a branch onto a fresh | ||
| master fetch, or when asked to check/audit cross-references. | ||
| allowed-tools: Bash(git:*), Bash(gh:*), Bash(bash .claude/skills/crossref-audit/scripts/audit.sh:*), Read | ||
| --- | ||
|
|
||
| # /crossref-audit | ||
|
|
||
| Check whether commits that landed on master in pgxntool and pgxntool-test | ||
| since the last release properly cross-reference their paired counterpart in | ||
| the other repo, per `.claude/skills/commit/guides/commit-message-format.md`. | ||
|
|
||
| ## Why this exists, and how it differs from `/commit` | ||
|
|
||
| PRs merge via the GitHub website, by the user, outside AI control — Claude | ||
| doesn't see the merge happen, so a missing cross-reference can only be | ||
| caught *after the fact* by auditing what actually landed on master. This is | ||
| a different problem from `/commit`, which only covers composing a PR | ||
| branch's own commits *before* merge. | ||
|
|
||
| ## Running it cheaply, every round | ||
|
|
||
| Run `bash .claude/skills/crossref-audit/scripts/audit.sh <pgxntool-dir> <pgxntool-test-dir>`. | ||
| This does steps 1 and most of step 3 below as a plain script — no LLM | ||
| tokens — and caches the last-checked master SHAs in | ||
| `/tmp/pgxntool-crossref-audit-state`, updated only on a clean result. That | ||
| means most rounds cost nothing more than reading one line of output: | ||
|
|
||
| - `crossref-audit: no new commits on either master since last clean check.` — done, nothing to interpret | ||
| - `crossref-audit: clean. Checked N / M commit(s)...` — done | ||
| - `crossref-audit: FLAGGED -- ...` — read the flagged list and apply the judgment in Step 2/Step 4 below | ||
|
|
||
| A flagged result is a heuristic, not a verdict — the script correlates | ||
| commits via the "(issue #N)" phrasing this project's commits use for | ||
| cross-repo issue references (not bare "(#N)", which is usually just the | ||
| local PR number), and only checks for the *presence* of a plausible | ||
| hash/URL pattern. It can both over-flag (a real pairing that already has a | ||
| reference in an unusual phrasing) and under-flag (a pairing it didn't | ||
| correlate). Treat "clean" as "nothing obviously wrong", not a guarantee. | ||
|
|
||
| ## Step 2: Identify genuinely paired commits (only needed when something is flagged, or you're doing a manual review) | ||
|
|
||
| Not every commit needs a cross-reference. It's only expected when a commit | ||
| in one repo is a *functional* code+test pairing with a commit in the | ||
| other — e.g. a pgxntool bug fix with dedicated pgxntool-test BATS coverage | ||
| for it. It is NOT expected for: | ||
| - Single-repo, doc-only changes with no test implications | ||
| - Two commits that happen to fix similar-sounding problems independently in | ||
| each repo (e.g. each repo has its own separate `claude-code-review.yml` — | ||
| fixing both is two unrelated commits, not a pairing) | ||
|
|
||
| Use issue numbers, PR descriptions, and commit content to judge whether a | ||
| pairing is real. When genuinely unsure, ask the user rather than guessing. | ||
|
|
||
| ## Step 3: Check for a cross-reference (the script does this automatically; use this if reviewing manually) | ||
|
|
||
| For each side of a genuine pairing, confirm the commit message references | ||
| the other repo — either a raw commit hash or (preferred, per the | ||
| commit-message-format guide) a GitHub PR/commit URL. Note: by convention | ||
| the pgxntool side only needs to describe related pgxntool-test changes in | ||
| prose (no hash required); the pgxntool-test side is expected to reference | ||
| pgxntool via hash or URL. | ||
|
|
||
| ## Step 4: Handle what you find | ||
|
|
||
| **Nothing missing:** report that briefly and move on. | ||
|
|
||
| **Exactly one commit missing a reference, and it's the current tip of | ||
| master, and it's newer than the last release tag:** you may fix it | ||
| directly: | ||
| 1. Create an isolated worktree tracking `upstream/master` detached — do NOT | ||
| edit the shared checkout. | ||
| 2. `git commit --amend` to add the missing cross-reference. Verify | ||
| `git diff <old-sha> <new-sha>` is empty (message-only change, no content | ||
| drift) before pushing. | ||
| 3. `git fetch upstream master` again immediately before pushing, to catch | ||
| any race, then | ||
| `git push --force-with-lease=master:<old-sha> upstream HEAD:master`. | ||
| 4. Report the old and new SHA clearly. | ||
| 5. Check whether any open PR branch (in either repo) was already rebased | ||
| onto the old, now-superseded SHA. If so, it needs re-rebasing onto the | ||
| new tip — git will typically recognize the old commit as | ||
| patch-equivalent and skip re-applying it cleanly, but re-run the full | ||
| test suite on the result before pushing the re-rebase. | ||
|
|
||
| **More than one commit missing a reference:** STOP. Do not fix anything | ||
| automatically. Report the full list to the user and wait for direction — | ||
| amending multiple non-contiguous commits requires real history surgery | ||
| (interactive rebase), which is much higher-risk than touching a single tip | ||
| commit. | ||
|
|
||
| **Any affected commit is at or before the last release tag:** NEVER amend | ||
| it, regardless of how many commits are affected, unless the user explicitly | ||
| tells you to for that specific commit. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - This audit only ever touches already-merged master commits, and only ever | ||
| the single tip commit under the conditions above. It never touches an | ||
| open PR's own commits as part of the audit itself — fixing an open PR's | ||
| commit (not yet shared/protected history) is a normal, low-risk edit and | ||
| doesn't need this skill's caution, just do it directly. | ||
| - Always re-run the full test suite after any amend-and-force-push, and | ||
| after any resulting PR-branch rebase, before considering the fix done. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,139 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # audit.sh [pgxntool-dir] [pgxntool-test-dir] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Cheap, stateful audit for missing cross-references between paired commits | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # on master in pgxntool and pgxntool-test. Meant to run every round of a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # long session, not just once at startup, without burning real tokens on | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # repeat rounds: it fetches both masters, and if neither has moved since the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # last CLEAN (nothing-flagged) check, exits immediately. State is only | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # updated on a clean result -- a flagged issue keeps resurfacing every round | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # until it's actually fixed, rather than being silently forgotten. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This is a heuristic aid, not authoritative: it correlates commits via the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # "(issue #N)" phrasing this project's commits use for cross-repo issue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # references (not bare "(#N)", which is usually just the local PR number). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # A flagged result still needs a real judgment call before acting -- see | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # SKILL.md. Absence of a flag means "nothing obviously wrong found", not a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # guarantee. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Arguments: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # pgxntool-dir : path to a pgxntool checkout (default: ../pgxntool) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # pgxntool-test-dir : path to a pgxntool-test checkout (default: .) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Exit codes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 0 : clean (nothing new since last clean check, or nothing flagged) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 1 : one or more candidate pairings flagged for review | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 2 : usage/environment error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PGXNTOOL_DIR="${1:-${PGXNTOOL_DIR:-../pgxntool}}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PGXNTOOL_TEST_DIR="${2:-.}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| STATE_FILE="${CROSSREF_AUDIT_STATE:-/tmp/pgxntool-crossref-audit-state}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for d in "$PGXNTOOL_DIR" "$PGXNTOOL_TEST_DIR"; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # -e (not -d): a linked worktree's .git is a FILE pointing at the main | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # repo's git-dir, not a directory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -e "$d/.git" ] || { echo "ERROR: '$d' is not a git checkout" >&2; exit 2; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| git -C "$PGXNTOOL_DIR" fetch upstream master --tags -q | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| git -C "$PGXNTOOL_TEST_DIR" fetch upstream master --tags -q | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PGXN_HEAD=$(git -C "$PGXNTOOL_DIR" rev-parse upstream/master) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TEST_HEAD=$(git -C "$PGXNTOOL_TEST_DIR" rev-parse upstream/master) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Derive the org/repo path from the configured remote rather than hardcoding | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # it, so this keeps working under a fork or rename. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PGXN_REPO_PATH=$(git -C "$PGXNTOOL_DIR" remote get-url upstream | sed -E 's#^(https://github\.com/|git@github\.com:)##; s#\.git$##') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -f "$STATE_FILE" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| read -r LAST_PGXN LAST_TEST < "$STATE_FILE" || true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "${LAST_PGXN:-}" = "$PGXN_HEAD" ] && [ "${LAST_TEST:-}" = "$TEST_HEAD" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "crossref-audit: no new commits on either master since last clean check. Nothing to do." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| last_release_tag() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| git -C "$1" tag --sort=-creatordate | grep -vi '^release$' | head -1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PGXN_TAG=$(last_release_tag "$PGXNTOOL_DIR") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TEST_TAG=$(last_release_tag "$PGXNTOOL_TEST_DIR") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # An empty tag would make "$TAG"..upstream/master collapse to plain | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ..upstream/master, which git quietly reinterprets as HEAD..upstream/master | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # instead of the intended unbounded range. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -z "$PGXN_TAG" ] || [ -z "$TEST_TAG" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "ERROR: no release tag found in one or both repos" >&2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pgxn_commits=$(git -C "$PGXNTOOL_DIR" log --format='%H %s' "$PGXN_TAG"..upstream/master) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test_commits=$(git -C "$PGXNTOOL_TEST_DIR" log --format='%H %s' "$TEST_TAG"..upstream/master) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -z "$pgxn_commits" ] && [ -z "$test_commits" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "crossref-audit: no commits since last release ($PGXN_TAG / $TEST_TAG) in either repo." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| printf '%s %s\n' "$PGXN_HEAD" "$TEST_HEAD" > "$STATE_FILE" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win No-release-tag case silently falls back to If 🔧 Suggested guard PGXN_TAG=$(last_release_tag "$PGXNTOOL_DIR")
TEST_TAG=$(last_release_tag "$PGXNTOOL_TEST_DIR")
+
+if [ -z "$PGXN_TAG" ] || [ -z "$TEST_TAG" ]; then
+ echo "ERROR: no release tag found in one or both repos" >&2
+ exit 2
+fi📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Extract issue numbers from "(issue #N)"-style phrasing specifically (not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # bare "#N", which is usually just the local repo's own PR number and would | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # cause false cross-repo pairings since PR numbering is independent per repo). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # `|| true` is required here, not cosmetic: under `set -e`, a `grep` (or a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # pipeline ending in one) that matches nothing exits 1, and since this | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # function's return status is that pipeline's status, calling it for a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # subject line with no "issue #N" (the common case -- most commits don't | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # reference one) would otherwise abort the whole script immediately. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extract_issue_refs() { grep -oiE 'issue #[0-9]+' <<<"$1" | grep -oE '[0-9]+' | sort -u || true; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| flagged=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| report="" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while IFS= read -r line; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -z "$line" ] && continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sha=${line%% *} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subj=${line#* } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issues=$(extract_issue_refs "$subj") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -z "$issues" ] && continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for issue in $issues; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| match=$(grep -iE "issue #${issue}([^0-9]|$)" <<<"$test_commits" || true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -z "$match" ] && continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body=$(git -C "$PGXNTOOL_DIR" log -1 --format='%b' "$sha") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ! grep -qi 'pgxntool-test' <<<"$body"; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| report+=$'\n'" PGXNTOOL $sha (issue #$issue): \"$subj\" -- no mention of pgxntool-test in body" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| flagged=1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Reuse $match (already bounded to "issue #$issue" exactly, not a numeric | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # prefix of it) rather than re-deriving test_sha with a looser pattern. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test_sha=$(head -1 <<<"$match" | cut -d' ' -f1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -n "$test_sha" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test_body=$(git -C "$PGXNTOOL_TEST_DIR" log -1 --format='%b' "$test_sha") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ! grep -qiE "[0-9a-f]{7,40}|github\.com/${PGXN_REPO_PATH}/(pull|commit)/" <<<"$test_body"; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| report+=$'\n'" PGXNTOOL-TEST $test_sha (issue #$issue): no hash or pgxntool PR/commit URL found in body" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| flagged=1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+103
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Unbounded regex in Line 91's guard correctly uses a boundary ( 🐛 Proposed fix: reuse the bounded pattern- test_sha=$(awk -v i="issue #$issue" 'BEGIN{IGNORECASE=1} $0 ~ i {print $1; exit}' <<<"$test_commits")
+ test_sha=$(grep -iE "issue #${issue}([^0-9]|\$)" <<<"$test_commits" | head -1 | cut -d' ' -f1)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done <<<"$pgxn_commits" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "$flagged" -eq 1 ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "crossref-audit: FLAGGED -- possible missing cross-reference(s):" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "$report" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Confirm each is a genuine code+test pairing before acting (a shared 'issue #N' string is a strong signal but still worth a sanity check) -- see SKILL.md for the fix-vs-ask rules. State file NOT updated; this will resurface every round until resolved." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pgxn_count=$(grep -c . <<<"$pgxn_commits" || true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test_count=$(grep -c . <<<"$test_commits" || true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "crossref-audit: clean. Checked $pgxn_count pgxntool / $test_count pgxntool-test commit(s) since last release ($PGXN_TAG / $TEST_TAG)." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| printf '%s %s\n' "$PGXN_HEAD" "$TEST_HEAD" > "$STATE_FILE" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # vi: expandtab ts=2 sw=2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift No automated test coverage for the correlation/heuristic logic. The issue-correlation and boundary-matching logic (the exact area with the bug flagged above) has no BATS coverage despite this repo already hosting a BATS test harness. A small fixture-based test (synthetic git repos with numerically-overlapping issue numbers) would have caught the line 100 regression. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Step 4's auto-fix pushes directly to master, contradicting the project's "never commit directly to master" rule.
The documented fix flow amends an already-merged commit and force-pushes it straight to
upstream/master(line 86), enabled by the unrestrictedBash(git:*)grant inallowed-tools(line 12). This is safeguarded (single tip commit, message-only diff check,--force-with-leasewith expected old SHA, immediate refetch), but it still bypasses the PR review process entirely for a write to the shared/protected branch. The repo-wide guideline is unconditional: "never commit directly to master, and treat merging as an operation performed outside AI control." If this narrow exception is intentional, the top-level guideline should be updated to document the carve-out explicitly; as written, the skill's own documented behavior conflicts with it. SkillSpector's flag on the--force-with-leaseat line 86 is a secondary signal here — the lease/expected-SHA check mitigates the classic force-push data-loss risk, but doesn't address the direct-to-master write itself.Also applies to: 76-98
🧰 Tools
🪛 SkillSpector (2.3.11)
[error] 86: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.
(Tool Misuse (TM1))
🤖 Prompt for AI Agents
Source: Coding guidelines