mirror of
https://github.com/Imbad0202/academic-research-skills.git
synced 2026-09-14 13:51:17 +08:00
ci: add platform-port reminder (remind, don't block) (#473)
* ci: add platform-port reminder (remind, don't block) Closes #472. Surface the Platform ports policy when a PR adds a new top-level directory (the shape a platform port takes), so contributors learn about the policy before review rather than mid-review (the #470 gap). - .github/workflows/platform-port-reminder.yml: on: pull_request, read-only. Detects a newly-added top-level dir via base-tree comparison, emits one ::warning:: + a job summary linking to CONTRIBUTING.md, exits 0. Never blocks, never comments (no pull_request_target), never re-lists the policy conditions (CONTRIBUTING.md stays the single source of truth), never verifies the human conditions. - .github/pull_request_template.md: a "Platform port?" section as a second, lower-friction surface. Detection verified against a throwaway worktree across five scenarios: new dir triggers; edit-existing-file does not; rename-within-existing-dir does not; new-dir + edit-existing reports only the new dir; a single top-level file is not a directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mi2n2xUaqmfXPSmkimt3qs * ci: harden platform-port detection (dual-track review fixes) Cross-model review of the workflow surfaced three detection bugs that the design-stage review missed. All three confirmed by first-party git testing (8 scenarios pass) before applying. - Compare from the merge base, not the moving base tip. A two-dot "$BASE_SHA" "$HEAD_SHA" diff reports a directory deleted on main (while the PR is open) as something the PR added — a false warning. merge-base isolates what the PR actually introduced. - `grep -qxF -e "$top"` so a directory name starting with `-` is not parsed as a grep flag; under set -e that would error and fail the check, turning the reminder into a red X and violating the never-block promise. - Accumulate newline-delimited and drop xargs, so a directory name containing a space is neither split nor corrupts the dedup test. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Cheng-I Wu
committed by
GitHub
GitHub
parent
610d60aa51
commit
17c518b286
@@ -34,6 +34,18 @@ gold-set metric:
|
||||
|
||||
> No eval impact.
|
||||
|
||||
## Platform port?
|
||||
|
||||
<!-- Only relevant if this PR adapts the suite to another agent platform
|
||||
(Hermes, OpenCode, Cursor, Aider, etc.) by adding a new top-level
|
||||
<platform>/ directory. Small edits to an existing port do not count. -->
|
||||
|
||||
If this PR adds a new `<platform>/` directory, read the
|
||||
[Platform ports policy](https://github.com/Imbad0202/academic-research-skills/blob/main/CONTRIBUTING.md#platform-ports-community-maintained-only)
|
||||
and **open a design issue first**. Otherwise, leave this section as "Not a platform port."
|
||||
|
||||
> Not a platform port.
|
||||
|
||||
## Checklist
|
||||
|
||||
- [ ] Tests added / updated and passing locally
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
name: Platform Port Reminder
|
||||
|
||||
# Surface the Platform ports policy when a PR adds a NEW top-level directory.
|
||||
#
|
||||
# Why: PR #470 (a Hermes platform port) violated several conditions of the
|
||||
# "Platform ports (community-maintained only)" policy in CONTRIBUTING.md
|
||||
# simply because the contributor did not know the policy existed. The repo
|
||||
# had no mechanism to surface it. This is that mechanism.
|
||||
#
|
||||
# What it does: when a PR adds a top-level directory that did not exist on
|
||||
# the base branch, it emits ONE `::warning::` plus a job summary linking to
|
||||
# CONTRIBUTING.md#platform-ports-community-maintained-only. That's it.
|
||||
#
|
||||
# What it deliberately does NOT do:
|
||||
# - It never blocks merge (always exits 0; `::warning::`, not `::error::`).
|
||||
# - It never posts a PR comment (would require `pull_request_target`, the
|
||||
# classic "pwn request" privilege-escalation shape for fork PRs).
|
||||
# - It does NOT re-list the policy conditions here — the policy text in
|
||||
# CONTRIBUTING.md is the single source of truth (a hand-copied list would
|
||||
# drift, which is the same failure that produced #470's confusion).
|
||||
# - It does NOT verify the human conditions (named maintainer / design issue
|
||||
# / real end-to-end evidence) — those are judgment calls a machine can't
|
||||
# check, and a "lint must pass" gate would give false green and be trivially
|
||||
# bypassable. Remind, don't block. The merge decision stays with the maintainer.
|
||||
#
|
||||
# A new directory is detected by base-tree comparison, not a hardcoded
|
||||
# allowlist (an allowlist drifts as the repo legitimately grows). False
|
||||
# positives (a warning the maintainer ignores) are cheap; false negatives are
|
||||
# the status quo. The effort is calibrated to that asymmetry.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened, ready_for_review]
|
||||
branches: [main]
|
||||
|
||||
concurrency:
|
||||
group: platform-port-reminder-${{ github.event.pull_request.number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
reminder:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Check out repository
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# Full history so the PR base SHA is reachable for the diff below;
|
||||
# a shallow checkout may not contain origin/main.
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Warn on new top-level directory
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
# Compare from the merge base, not the moving base tip. If main advances
|
||||
# while this PR is open (e.g. a directory is deleted on main), a two-dot
|
||||
# "$BASE_SHA" "$HEAD_SHA" diff would report that as something the PR
|
||||
# added — a false warning. The merge base isolates exactly what THIS PR
|
||||
# introduced. fetch-depth: 0 above guarantees the merge base is present.
|
||||
merge_base=$(git merge-base "$BASE_SHA" "$HEAD_SHA")
|
||||
|
||||
# Top-level directories that existed before this PR. This is the source
|
||||
# of truth for "what already exists" — no hardcoded allowlist to drift.
|
||||
# Computed at the same merge base as the diff below, so the two agree.
|
||||
base_dirs=$(git ls-tree -d --name-only "$merge_base")
|
||||
|
||||
# Files ADDED by this PR. --no-renames keeps an added platform dir
|
||||
# from being reported as a rename (R old new), which the A filter
|
||||
# would otherwise miss. --diff-filter=A restricts to additions.
|
||||
added=$(git diff --no-renames --diff-filter=A --name-only "$merge_base" "$HEAD_SHA" || true)
|
||||
|
||||
# First path segment of each added file = its top-level dir.
|
||||
# Accumulate newline-separated (not space-separated): a directory name
|
||||
# could in principle contain a space, which would corrupt both the
|
||||
# dedup test and any whitespace-splitting downstream.
|
||||
new_top_dirs=""
|
||||
while IFS= read -r path; do
|
||||
[ -z "$path" ] && continue
|
||||
top="${path%%/*}"
|
||||
# A file added at repo root (no slash) is not a new directory.
|
||||
[ "$top" = "$path" ] && continue
|
||||
# Already present on base? not new. `-e` marks $top as the pattern
|
||||
# so a dir name starting with `-` is not parsed as a grep flag
|
||||
# (which would error and, under set -e, fail the check — a red X
|
||||
# would violate the never-block promise).
|
||||
if printf '%s\n' "$base_dirs" | grep -qxF -e "$top"; then
|
||||
continue
|
||||
fi
|
||||
# Newline-delimited contains-test, robust to spaces in names.
|
||||
case $'\n'"$new_top_dirs"$'\n' in
|
||||
*$'\n'"$top"$'\n'*) ;; # already collected
|
||||
*) new_top_dirs="${new_top_dirs}${top}"$'\n' ;;
|
||||
esac
|
||||
done <<< "$added"
|
||||
|
||||
new_top_dirs=$(printf '%s' "$new_top_dirs" | sed '/^$/d' | sort -u || true)
|
||||
|
||||
if [ -z "$new_top_dirs" ]; then
|
||||
echo "No new top-level directory added. Nothing to remind."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
POLICY_URL="https://github.com/${GITHUB_REPOSITORY}/blob/main/CONTRIBUTING.md#platform-ports-community-maintained-only"
|
||||
|
||||
echo "::warning::This PR adds a new top-level directory ($(printf '%s' "$new_top_dirs" | tr '\n' ' ')). If this is a platform port, see the Platform ports policy in CONTRIBUTING.md and open a design issue first: ${POLICY_URL}"
|
||||
|
||||
{
|
||||
echo "### Platform port reminder"
|
||||
echo ""
|
||||
echo "This PR adds a new top-level directory:"
|
||||
echo ""
|
||||
printf '%s\n' "$new_top_dirs" | sed 's/^/- `/; s/$/`/'
|
||||
echo ""
|
||||
echo "If this is a **platform port** (adapting the suite to another agent"
|
||||
echo "platform), please read the [Platform ports policy](${POLICY_URL})"
|
||||
echo "and **open a design issue first**."
|
||||
echo ""
|
||||
echo "_This is a reminder, not a blocker — it does not affect whether this"
|
||||
echo "PR can merge. A new directory for an unrelated reason is fine; ignore"
|
||||
echo "this notice._"
|
||||
} >> "$GITHUB_STEP_SUMMARY"
|
||||
|
||||
# Never block: this is advisory only.
|
||||
exit 0
|
||||
Reference in New Issue
Block a user