Repository navigation
feat: allow per-PR fix-loop budget via label #1042
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| #!/usr/bin/env bash | ||
|
There was a problem hiding this comment. 6. Protected scripts require human approval The PR modifies multiple files under the protected scripts/ path, so it must receive human review and must not be auto-approved. The feature rationale provides context, but there is no linked issue authorizing these governance/infrastructure changes. Agent Prompt
|
||
| # shellcheck shell=bash | ||
| # fix-budget.lib.sh — parse a per-PR fix-loop budget from PR labels. | ||
| # | ||
| # A label of the form `fullsend-fix-budget/N` (N a positive integer) lets a | ||
| # maintainer cap the review->fix loop for a single PR below the global | ||
| # iteration cap. The label can only TIGHTEN the cap, never raise it: | ||
| # enforcement lives in pre-fix, which applies min(label_budget, cap). | ||
| # | ||
| # Bundled into pre-fix.sh via bundle-sh.sh. | ||
| # | ||
| # Expected env vars (optional): | ||
| # PR_LABELS — PR label names separated by commas and/or newlines. Absent/empty | ||
| # is fine: parse_fix_budget then returns nothing and the cap is | ||
| # unchanged. (The upstream dispatcher comma-joins labels; a | ||
| # newline-joined value is also accepted.) | ||
|
|
||
| [[ -n "${FIX_BUDGET_SH_LOADED:-}" ]] && return 0 | ||
| FIX_BUDGET_SH_LOADED=1 | ||
|
|
||
| FIX_BUDGET_LABEL_PREFIX="fullsend-fix-budget/" | ||
|
|
||
| # parse_fix_budget [labels] | ||
| # Reads label names (arg 1, or PR_LABELS env when omitted) separated by commas | ||
| # and/or newlines. Echoes the smallest valid budget found, or nothing when no | ||
| # valid label is present. A malformed value (non-integer, zero, negative) is | ||
| # ignored, not fatal — a bad label must not silently drop the existing cap. | ||
| parse_fix_budget() { | ||
| local labels="${1-${PR_LABELS:-}}" | ||
| local best="" label n | ||
| # Accept comma-joined labels (the upstream dispatcher format) as well as | ||
| # newline-joined: normalize commas to newlines before splitting. | ||
| labels="${labels//,/$'\n'}" | ||
| while IFS= read -r label; do | ||
|
Comment on lines
+28
to
+34
There was a problem hiding this comment. 5. Feature lacks linked authorization This PR adds a new parser, runtime guard, generated bundle changes, and tests well beyond the rule's 20-line threshold, but the PR metadata contains no linked authorizing issue. The non-trivial feature therefore lacks the required explicit authorization. Agent Prompt
|
||
| # Trim surrounding whitespace so " fullsend-fix-budget/3 " still matches. | ||
| label="${label#"${label%%[![:space:]]*}"}" | ||
| label="${label%"${label##*[![:space:]]}"}" | ||
| [[ "${label}" == "${FIX_BUDGET_LABEL_PREFIX}"* ]] || continue | ||
| n="${label#"${FIX_BUDGET_LABEL_PREFIX}"}" | ||
| # Bound the digit count. An arbitrarily long value would overflow Bash's | ||
| # signed 64-bit arithmetic in the `-lt` comparison (e.g. 2^64 evaluates as | ||
| # 0), which would look "tighter" than any cap and block every fix run. | ||
| # A budget above 99999 is meaningless next to caps of 5/10, so treat an | ||
| # over-long value as malformed and ignore it. | ||
| [[ "${n}" =~ ^[1-9][0-9]{0,4}$ ]] || continue | ||
|
There was a problem hiding this comment. [low] fail-open A malformed Suggested fix: Optional: emit a |
||
| if [[ -z "${best}" || "${n}" -lt "${best}" ]]; then | ||
| best="${n}" | ||
| fi | ||
| done <<< "${labels}" | ||
| [[ -n "${best}" ]] && printf '%s\n' "${best}" | ||
| return 0 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| #!/usr/bin/env bash | ||
|
There was a problem hiding this comment. [low] scope-creep The PR moves Suggested fix: Disclose the review-agent behaviour change and the extraction in the PR body. |
||
| # shellcheck shell=bash | ||
| # review-labels.lib.sh — recognize pipeline-managed "control" labels. | ||
| # | ||
| # Control labels are set by the review pipeline (or a maintainer, for the | ||
| # fix-budget knob), not by the review agent. post-review refuses to add or | ||
| # remove them via agent-recommended label_actions. Kept in a sourceable lib so | ||
| # the same definition is exercised by both production and the unit test — a | ||
| # duplicated copy in the test would pass even if the production branch drifted. | ||
| # | ||
| # Bundled into post-review.sh via bundle-sh.sh. | ||
|
|
||
| [[ -n "${REVIEW_LABELS_SH_LOADED:-}" ]] && return 0 | ||
| REVIEW_LABELS_SH_LOADED=1 | ||
|
|
||
| REVIEW_CONTROL_LABELS=( | ||
| "ready-for-merge" "requires-manual-review" "rejected" | ||
| "ready-for-review" "fullsend-no-fix" "fullsend-fix" | ||
| ) | ||
|
|
||
| # is_control_label LABEL — return 0 if LABEL is pipeline-managed, 1 otherwise. | ||
| is_control_label() { | ||
| local label="$1" | ||
| local cl | ||
| for cl in "${REVIEW_CONTROL_LABELS[@]}"; do | ||
| if [[ "${cl}" == "${label}" ]]; then | ||
| return 0 | ||
| fi | ||
| done | ||
| # Pipeline-managed label prefixes. | ||
| if [[ "${label}" == risk/* ]]; then | ||
| return 0 | ||
| fi | ||
| # Maintainer-set fix-loop budget (fullsend-fix-budget/N); pipeline-managed so | ||
| # the review agent preserves it rather than treating it as a contextual label. | ||
| if [[ "${label}" == fullsend-fix-budget/* ]]; then | ||
|
There was a problem hiding this comment. [medium] auth-bypass is_control_label matches Suggested fix: Compare control labels case-insensitively (e.g. lowercase with |
||
| return 0 | ||
| fi | ||
| return 1 | ||
| } | ||
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.
[low] scope-creep
Two existing lines in the agents/fix.md iteration-cap paragraph ("This ensures humans are / never locked out...") are rewrapped with no text change, which is unrelated churn under AGENTS.md §3. post-fix.src.sh also adds a
HUMAN_CAPlocal only to avoid inlining${ITERATION_CAP_HUMAN:-10}in an echo that otherwise did not change. See also: [code-organization] finding at this location (same reflow).Suggested fix: Revert the whitespace-only rewrap in agents/fix.md and the unneeded HUMAN_CAP variable in post-fix.src.sh (then rebuild scripts/post-fix.sh with
make script-build).