fix(labels): prune unmanaged blocked/infra-attempt/* and blocked/infra-model/* drift

[itsmiso-ai]

Added guards and pruning for unmanaged blocked/infra labels to prevent filter pollution.

Fixes #916

Opened by foreman on review GO (workload wl-misospace-dispatch-916).

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

Recommendation

Approve. The PR is a tight, well-scoped implementation of issue PR 916's acceptance criteria: prune accumulated blocked/infra-attempt/* and blocked/infra-model/* drift, add a guard so they cannot be re-created, and document the rule. All CI checks (build, lint, typecheck, tests, coverage, DB migrations, DB integration, smoke, docker build, npm audit) report success. The diff is internally consistent, follows repo conventions, and aligns with the linked issue's expected files and acceptance text.

Change-by-change findings

.github/labels.yaml (+9 / -1)

• Adds the managed umbrella blocked/infra (color e11d21) with a description that explicitly excludes per-attempt variants. This makes the umbrella a first-class managed label so the label-sync action will (re)create it consistently and delete-other-labels: false will not strip it on later syncs. The trailing comment clarifying "Unknown labels are not pruned" is replaced by an explanatory comment block — accurate and helpful.
• The yaml structure matches the surrounding entries (name/color/description keys), consistent with the other entries in the file.

.github/workflows/label-sync.yaml (+34 / -1)

• Adds the workflow file itself to paths: so changes here also trigger the workflow (sensible self-trigger).
• Adds a weekly schedule: cron (23 4 * * 1) — different from the existing push trigger, so drift is pruned even when the labels.yaml / workflow files do not change. Cron schedule does not collide with anything visible in the impact scan.
• Adds the labels: write permission to the job (required for gh label delete). The existing issues: write alone does not authorize label deletion on all token scopes, so this is correct.
• The new prune step:
  • Uses --limit 500 to bound the gh label list call. Repo has ~97 affected labels today and is unlikely to exceed 500; mention this as a known scale limit. It does not justify a change.
  • Anchors the grep with ^blocked/infra-(attempt|model)/ so it cannot accidentally match a future blocked/infra umbrella or blocked/infra-foo. The umbrella is correctly preserved.
  • Uses || true so an empty result does not fail under set -uo pipefail (the set -e is intentionally omitted, which is the right call here — deleting one label should not abort the whole loop).
  • Wraps gh label delete failures in ::warning:: so a label currently attached to an issue does not break the run. Acceptable; the next run will retry.
  • Dry-run handling is unchanged (dry-run only suppresses the sync action, not this new step). The prune step will run during a manual workflow_dispatch even when dry_run: true. Minor but worth noting — see findings.

AGENTS.md (+18)

• Adds a new "Label hygiene" subsection under the label conventions block. Mirrors the source-of-truth rule from .github/labels.yaml and explains why the umbrella is kept and the per-attempt variants are pruned. This is exactly what the linked issue's acceptance text asked for ("a brief note in AGENTS.md … explaining why the umbrella is allowed and the per-attempt variants are not").
• Wording is generic ("agent/*", no Saffron-specific names) — satisfies the repo's "No agent-specific names in generic docs" convention.

src/app/api/issues/label/route.ts (+13)

• Adds a guard that returns 400 with a descriptive error when action === "add" and the label matches /^blocked\/infra-(attempt|model)\///. The umbrella is not matched, and action === "remove" is explicitly allowed so cleanup of existing drift remains possible — both match the test expectations below and the issue's intent.
• The guard sits after labelName = label.trim() and before getAuthorizedActor. This is fine: the route does not need auth to reject a malformed/illegal label name. (One could argue auth-first is preferable to avoid a cheap DoS surface, but the current route already returns 400 on missing fields before auth, so this is consistent.)
• Error message names the rejected label and points at the umbrella. Good UX, matches the test assertions.

src/app/api/issues/label/route.test.ts (+60)

• Adds four tests:
  1. Reject add of blocked/infra-attempt/42 → 400, no side-effect calls.
  2. Reject add of blocked/infra-model/llama-3 → 400, no side-effect calls.
  3. Allow add of blocked/infra → 200, success path.
  4. Allow remove of blocked/infra-attempt/7 even when present on the issue → 200, calls removeLabel, never addLabel.
• The four tests cover the three behavioral branches of the guard (add-blocked, add-allowed, remove-allowed), plus a sanity check that audit is not written on rejection. Good coverage.
• Note: there is no test asserting the umbrella blocked/infra is still rejected as blocked/infra-attempt (e.g. via a value like blocked/infra/attempt). The regex is anchored on -, so it is safe, but a small additional test asserting blocked/infra is the only allowed string in that namespace would harden the contract.

Standards Compliance

• Tech stack & commands — N/A (no package.json / schema changes). The PR does not modify dependency or schema files, so no npm install / prisma generate / migration steps are required.
• Code Standards
  • Generic patterns in docs: AGENTS.md text uses "agent/*"-style language, no Saffron specifics. ✅
  • Error handling: errorResponse is used (matches the existing pattern in the file). ✅
  • Validation: Input is validated (label.trim(), regex check) before any DB or auth call. ✅
  • API status codes: 400 is the right code for a syntactically valid but policy-rejected label name. ✅
  • No secrets committed: Diff contains no secrets. ✅
• Label conventions — The PR explicitly documents why only the umbrella is allowed and the per-attempt variants are not, which strengthens rather than violates the convention. ✅
• GitHub Actions CI — Lint and typecheck both passed in CI, consistent with the standards file's requirement that they block CI. ✅
• Prisma — No schema changes; no migration risk. ✅
• Container / Docker — No Dockerfile changes. ✅

Linked Issue Fit

The linked issue (PR 916) explicitly asks for:

1. Prune accumulated blocked/infra-attempt/* and blocked/infra-model/* labels — Covered by the new Prune unmanaged … step in the label-sync workflow. ✅
2. Add a label-sync guard if the source keeps emitting them — Covered by the regex guard in src/app/api/issues/label/route.ts rejecting those shapes at the API layer, and by the workflow running on push + weekly schedule. ✅
3. Expected files: .github/labels.yaml, src/app/api/repos/route.ts or whatever reads labels, AGENTS.md (only if a guard rule is added) — Touched .github/labels.yaml and the label-route guard. src/app/api/repos/route.ts is not modified, but that file reads repo configs, not labels per se; the actual label-application API is src/app/api/issues/label/route.ts, which the PR correctly modifies. The issue's "or whatever reads labels" clause accommodates this. ✅
4. Follow-up gh label list showing 0 or 1 (umbrella only) — The workflow will prune the drift on first run after merge. Acceptance is operationally verifiable post-merge; the PR contains the mechanism. ✅
5. Brief note in AGENTS.md (or docs/) explaining umbrella vs variants — "Label hygiene" subsection added with exactly this rationale. ✅

Acceptance criteria are satisfied.

CI / Tool Harness Findings

CI is green across all 11 reported checks for the head commit. Tool harness issued no tool calls and reported no findings. No evidence-provider output to interpret.

Unknowns / Needs Verification

• The prune step runs even when the workflow is dispatched with dry_run: true. The issue did not call this out, and it is unlikely to cause harm (the only effect is faster label cleanup on a dry-run dispatch), but it is a minor behavioral asymmetry. Optional follow-up: gate the prune step behind if: ${{ !inputs.dry_run }} to match the sync action's behavior.
• The grep --limit 500 is an implicit scale cap. Today the repo has ~97 affected labels, well under the cap. If the emitter is ever fixed (which is outside this PR's scope), no action is needed; if it ever spams beyond 500, the first prune run will still delete the oldest 500 and subsequent runs will clear the rest. Worth a comment in the workflow but not a blocker.

Verification of must_check items

The PR classification reports must_check: [], so there are no mandatory checklist items to enumerate. The escalated review status was driven by the preliminary review being judged insufficient (fast_low_confidence), not by specific must-checks — the thorough review above addresses that directly.