feat(workflow): enforce fresh base branch and add local governance check - #62
Conversation
|
Warning Review limit reached
Next review available in: 1 minute 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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Comment |
startTask now rejects task starts where the local base branch trails origin/<branch> when policy.worktree.staleBaseBranch=enforce, preventing CI from seeing files the local branch point never saw (root cause behind PR #59's post-merge boundary gaps). Also adds governance:pr:local, a CLI wrapper around the existing validatePullRequest that lets PR title/body/files be checked locally before gh pr create, instead of only after CI runs validate-pr.mjs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed PR/issue pushes governance:pr:local didn't check branch name, so a non-compliant branch name (fix/codex-session-findings, missing an issue number) only surfaced after CI ran validate-branch-name.mjs. It now also runs validateBranchName before reporting success. There was no local equivalent of validate-issue.mjs at all, so an issue body missing the exact "Implementation notes" heading only surfaced after CI ran validate-issue.mjs on a linked issue. Adds governance:issue:local as the same kind of CLI wrapper governance:pr:local already is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ory mode
checkStaleBaseBranch() returned { stale: false } whenever the underlying
git call didn't complete (missing ref, timeout, spawn failure, output
limit), conflating "check failed to run" with "branch is not stale."
Under staleBaseBranch=enforce this fail-opened exactly the git errors
enforce is meant to guard against, contrary to the fail-closed pattern
already established in resolveRepoState()'s usable/ok distinction.
checkStaleBaseBranch() now returns a four-state result (fresh/stale/
unavailable/unknown) that separates "no tracking ref" from "git call
did not complete." startTask() blocks on both stale and unknown when
staleBaseBranch=enforce, and no longer treats a failed git call as
equivalent to a fresh branch.
staleBaseBranch=advisory was also a no-op — only the enforce branch
ran the check at all, so advisory behaved identically to off despite
the standard preset setting it explicitly. advisory now runs the same
check and surfaces stale/unknown results as structured warnings on
StartTaskResult without blocking the task start.
4a92145 to
ef38660
Compare
Summary
Fix two of the four performance issues found in today's codex sessions (Issue #48, #25, mottainai config): stale base branches slipping past task start, and PR governance checks only surfacing after CI runs.
Linked issue
Closes #61
Scope
Included
policy.worktree.staleBaseBranchpolicy flag (defaultoff,enforceonstrict-worktree,advisoryonstandard) pluscheckStaleBaseBranchinstartTaskthat rejects task starts when the local base branch trailsorigin/<branch>.scripts/governance-check-local.mjs(pnpm run governance:pr:local): runs the existingvalidatePullRequestandvalidateBranchNameagainst local git state (title/body/diff/branch) beforegh pr create, instead of only after CI.scripts/governance-check-issue-local.mjs(pnpm run governance:issue:local): local equivalent ofvalidate-issue.mjsfor checking a draft issue body beforegh issue create. Added after this PR's own first two pushes failed CI on exactly the gaps these two commands close (non-compliant branch name, then a missing "Implementation notes" section on the linked issue) — see PR discussion.Excluded
architecture:checklocal/CI unification — already solved on PR feat(ci): enforce coding standards and architecture boundaries #59 viaverify:standards, no change needed here.~/.codex/config.toml, not per-worktree, and the actual failure (unresolved global bin) was already fixed in a separate session.Implementation
checkStaleBaseBranchcompares localbaseCommitagainstorigin/<baseBranch>viagit merge-base --is-ancestor; only rejects when local is a strict ancestor of the remote tip (avoids false positives on diverged/ahead branches). No implicitgit fetch— it only checks what's already known locally.governance-check-local.mjsreusesvalidatePullRequestandvalidateBranchNamefromgovernance-lib.mjsunchanged; only the input-resolution layer (title from--titleor last commit subject, body from--body-fileor last commit body, files from--filesorgit diff --name-only, branch from--branchorgit branch --show-current) is new.governance-check-issue-local.mjsreusesvalidateIssuefromgovernance-lib.mjsunchanged; same input-resolution pattern via--body-file/--body.Behavioral changes
unsupported-repo-statewhenstaleBaseBranch=enforceand the local branch is behind origin. Only affectsstrict-worktreepreset by default; existingstandard/minimalpresets are unaffected (advisory/off).pnpm run governance:pr:localandpnpm run governance:issue:local; no change to existinggovernance:pr/governance:issue(CI) behavior.Validation
Risks
staleBaseBranchcheck relies on the local repo already having an up-to-dateorigin/<branch>ref; it does not fetch, so a caller who never fetches gets no protection (documented in code comment). Low risk since it's opt-in via policy and defaults tooff.Breaking changes
No.
Review focus
strict-worktreedefaultingstaleBaseBranchtoenforceis the right call, or if it should start atadvisoryfor a rollout period.