core: add fork maintenance policy, sync helper, and audit CI - #19
Merged
Conversation
Personal-fork governance: a priority matrix (sync > hooks > plugin isolation > direct core edits), commit-labeling rules, and a decision tree, plus the tooling to actually enforce it — scripts/fork-sync.sh sets up the missing upstream remote and reports/merges drift, and .github/workflows/fork-audit.yml fails new PR commits that touch core paths without a core:/hook:/sync: label (existing history is left alone) and warns when the branch falls far behind got-feedback/feedBack:main. Kept as new files rather than edits to ci.yml/ship-ci.yml/CLAUDE.md so the policy doesn't itself create the exact conflicts it's meant to avoid.
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a fork maintenance policy, an upstream synchronization script, and a GitHub Actions workflow. The workflow audits core commit prefixes and reports upstream drift. ChangesFork maintenance controls
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant CommitHistory
participant UpstreamMain
PullRequest->>GitHubActions: trigger fork-audit
GitHubActions->>CommitHistory: inspect introduced commits
CommitHistory-->>GitHubActions: return core paths and commit subjects
GitHubActions->>UpstreamMain: compare pull-request base
UpstreamMain-->>GitHubActions: return commit divergence
sequenceDiagram
participant ForkBranch
participant ForkSync
participant UpstreamRemote
ForkSync->>UpstreamRemote: fetch upstream/main
UpstreamRemote-->>ForkSync: return upstream/main
ForkSync->>ForkBranch: compare branch divergence
ForkSync->>ForkBranch: merge upstream/main when required
🚥 Pre-merge checks | ✅ 22✅ Passed checks (22 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/fork-audit.yml:
- Line 24: Pin both actions/checkout uses in .github/workflows/fork-audit.yml at
lines 24 and 55 to the immutable commit SHA
11d5960a326750d5838078e36cf38b85af677262 instead of `@v4`.
In `@scripts/fork-sync.sh`:
- Around line 23-26: Update the upstream-remote setup in the fork-sync script to
read the existing upstream URL before fetching or merging. Add the remote only
when absent; if present, require it to match the canonical got-feedback/feedBack
repository URL and stop with an actionable error otherwise. Preserve the
existing fetch and merge flow only after validation succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6cce138-9bcb-48bc-9d14-3072d82233a8
📒 Files selected for processing (3)
.github/workflows/fork-audit.ymldocs/fork-maintenance.mdscripts/fork-sync.sh
Pin both actions/checkout uses in fork-audit.yml to the immutable commit SHA (v4.4.0) instead of the mutable @v4 tag. fork-sync.sh: validate a pre-existing 'upstream' remote's URL before fetching/merging, instead of trusting any remote named 'upstream'. An unrelated remote with that name would otherwise get silently fetched and merged into the branch.
Collaborator
Author
|
Addressed both CodeRabbit findings in bee4c7a:
Generated by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/fork-audit.yml (1)
62-64: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep upstream drift advisory when the fetch fails.
A failed
git fetchorgit rev-listexits this step before the warning logic runs. A transient network failure can therefore fail the PR check, despite this job being documented as advisory-only.Handle both failures explicitly, emit a warning, and exit successfully when drift cannot be measured.
Proposed fix
- git fetch --no-tags https://github.com/got-feedback/feedBack.git main:refs/remotes/canonical-upstream/main + if ! git fetch --no-tags https://github.com/got-feedback/feedBack.git main:refs/remotes/canonical-upstream/main; then + echo "::warning::Unable to fetch got-feedback/feedBack:main; skipping upstream drift check." + exit 0 + fi BASE_SHA="${{ github.event.pull_request.base.sha }}" - behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main) + if ! behind=$(git rev-list --count "$BASE_SHA"..canonical-upstream/main); then + echo "::warning::Unable to calculate upstream drift; skipping check." + exit 0 + fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/fork-audit.yml around lines 62 - 64, Update the upstream drift calculation around the git fetch and rev-list commands in the workflow so both failures are handled explicitly. When either command fails, emit a warning and exit successfully without failing the advisory check; retain the existing drift warning behavior when the count is measured successfully.scripts/fork-sync.sh (1)
29-35: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFetch
mainintoupstream/main.When
remote.upstream.fetchis missing or custom,git fetch upstream mainmay not updateupstream/main. The drift checks and merge then use a stale or missing ref.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/fork-sync.sh` around lines 29 - 35, Update the upstream fetch step in scripts/fork-sync.sh to explicitly map the fetched main branch to the upstream/main remote-tracking ref, regardless of the configured remote.upstream.fetch refspec. Preserve the existing upstream remote setup and ensure subsequent drift checks and merge operations use the freshly fetched upstream/main.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/fork-sync.sh`:
- Around line 23-26: Update the upstream remote mismatch handling around
upstream_url so the stderr message never prints the configured remote URL or
embedded credentials; report only that the upstream remote is non-canonical,
while preserving the refusal and remediation guidance.
---
Outside diff comments:
In @.github/workflows/fork-audit.yml:
- Around line 62-64: Update the upstream drift calculation around the git fetch
and rev-list commands in the workflow so both failures are handled explicitly.
When either command fails, emit a warning and exit successfully without failing
the advisory check; retain the existing drift warning behavior when the count is
measured successfully.
In `@scripts/fork-sync.sh`:
- Around line 29-35: Update the upstream fetch step in scripts/fork-sync.sh to
explicitly map the fetched main branch to the upstream/main remote-tracking ref,
regardless of the configured remote.upstream.fetch refspec. Preserve the
existing upstream remote setup and ensure subsequent drift checks and merge
operations use the freshly fetched upstream/main.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 155aeb02-2057-4e74-ad3f-3b3aa3d3688e
📒 Files selected for processing (2)
.github/workflows/fork-audit.ymlscripts/fork-sync.sh
scripts/fork-sync.sh: - Don't print the mismatched 'upstream' remote URL to stderr — it could contain embedded credentials (https://user:token@...). Report only that it isn't canonical. - Fetch explicitly into refs/remotes/upstream/main instead of relying on the default refspec, so a pre-existing 'upstream' remote with a customized fetch refspec can't leave upstream/main stale. fork-audit.yml upstream-drift job: - GitHub Actions run steps default to `bash -eo pipefail`, so a failed fetch/rev-list would fail the job despite it being documented as advisory-only. Handle both failures explicitly and exit 0 with a warning instead of hard-failing on a transient network error.
Collaborator
Author
|
Addressed this round's 3 findings in 0c2ebee:
Generated by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/fork-audit.yml:
- Around line 66-67: Move the pull request base and head SHA values from inline
shell assignments/interpolation into the appropriate workflow env blocks, then
update the audit commands to reference those environment variables in quoted
shell arguments. Apply this to the SHA values used around the fork-audit logic,
including the BASE_SHA assignment near the canonical-upstream comparison and the
corresponding HEAD_SHA usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94cdc7bc-d670-405d-80b7-3a9074c7eb29
📒 Files selected for processing (2)
.github/workflows/fork-audit.ymlscripts/fork-sync.sh
github.event.pull_request.{base,head}.sha were interpolated directly
into run: shell blocks. zizmor flags this as template-injection
regardless of whether the specific field is attacker-controlled — pass
them through env: and reference as shell variables instead, per
GitHub's Actions hardening guidance.
Collaborator
Author
|
Addressed in 5a2c6c0: CI was fully green on the prior commit (0c2ebee) before this trivial hardening pass. Generated by Claude Code |
carochacs
added a commit
that referenced
this pull request
Aug 5, 2026
* core: add fork maintenance policy, sync helper, and audit CI Personal-fork governance: a priority matrix (sync > hooks > plugin isolation > direct core edits), commit-labeling rules, and a decision tree, plus the tooling to actually enforce it — scripts/fork-sync.sh sets up the missing upstream remote and reports/merges drift, and .github/workflows/fork-audit.yml fails new PR commits that touch core paths without a core:/hook:/sync: label (existing history is left alone) and warns when the branch falls far behind got-feedback/feedBack:main. Kept as new files rather than edits to ci.yml/ship-ci.yml/CLAUDE.md so the policy doesn't itself create the exact conflicts it's meant to avoid. * core: address CodeRabbit review — pin checkout, validate upstream remote Pin both actions/checkout uses in fork-audit.yml to the immutable commit SHA (v4.4.0) instead of the mutable @v4 tag. fork-sync.sh: validate a pre-existing 'upstream' remote's URL before fetching/merging, instead of trusting any remote named 'upstream'. An unrelated remote with that name would otherwise get silently fetched and merged into the branch. * core: fix credential leak, fail-safety, and fetch-refspec issues scripts/fork-sync.sh: - Don't print the mismatched 'upstream' remote URL to stderr — it could contain embedded credentials (https://user:token@...). Report only that it isn't canonical. - Fetch explicitly into refs/remotes/upstream/main instead of relying on the default refspec, so a pre-existing 'upstream' remote with a customized fetch refspec can't leave upstream/main stale. fork-audit.yml upstream-drift job: - GitHub Actions run steps default to `bash -eo pipefail`, so a failed fetch/rev-list would fail the job despite it being documented as advisory-only. Handle both failures explicitly and exit 0 with a warning instead of hard-failing on a transient network error. * core: pass PR SHAs through env instead of template interpolation github.event.pull_request.{base,head}.sha were interpolated directly into run: shell blocks. zizmor flags this as template-injection regardless of whether the specific field is attacker-controlled — pass them through env: and reference as shell variables instead, per GitHub's Actions hardening guidance. --------- Co-authored-by: Claude <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
2 participants
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.
What
Adds a personal-fork maintenance policy and the tooling to enforce it, so keeping this fork in sync with
got-feedback/feedBackstays cheap.docs/fork-maintenance.md— a priority matrix (upstream sync > hook injection > plugin isolation > direct core edits), commit-labeling rules (core:/hook:/sync:prefixes for anything touching a core path), and a decision tree. Includes an audit of this fork's current state as a baseline: noupstreamremote was configured, and three existing fork-only commits are direct, unlabeled core edits (recorded for reference, not retroactively flagged).scripts/fork-sync.sh— sets up the missingupstreamremote, reports how far ahead/behindorigin/HEADis vsgot-feedback/feedBack:main, and merges it in (--report-onlyto just check)..github/workflows/fork-audit.yml— a new, fork-only CI workflow (intentionally not added as a job insideci.yml/ship-ci.yml, so pulling upstream changes to those files never conflicts with this policy):core-commit-labelingfails a PR if any commit it introduces touches a core path without acore:/hook:/sync:subject prefix. Only diffs new commits against the PR base, so existing history isn't retroactively broken.upstream-driftis advisory-only — warns (never fails) when the PR's base branch is more than 50 commits behindgot-feedback/feedBack:main.No behavior change to the app itself — this is process/tooling only.
feedpak surface
Checklist
CHANGELOG.md[Unreleased]updated — skipped; this is internal fork tooling, not a user-visible app changegit commit -s) — not signed off; this PR targets this fork's ownmain, notgot-feedback/feedBack, so DCO wasn't applied. Happy to add sign-off if this fork wants to hold itself to the same standard.Generated by Claude Code