Skip to content

a5b52047 reached main without a task branch, and check-role-discipline cannot be waived #250

Description

@localai-bot

a5b52047 ("perf(SPEC-DSPARK): deep dive -- the drafter is not the gap, the verify step is") is on main and fails scripts/check-role-discipline.py standalone:

$ python3 scripts/check-role-discipline.py --commit a5b52047
ERROR: a5b52047: repository change (src/vllm/v1/worker/gpu/runner.cpp,
src/vllm/v1/worker/gpu/spec_decode/dspark/speculator.cpp) reached main without
arriving on a task branch. Work happens in its own worktree on a `row/<ID>`
branch and lands through a reviewed PR or an authorized local merge naming that
branch; never directly on the shared checkout

git log -1 --format='%P' a5b52047 gives a single parent, c9924566 (the merge of #211), so it sits directly on main's first-parent line rather than arriving through a row/* merge. Two product src/ files, both real feature code.

Why this is worth a row rather than a shrug

It is latent until someone merges main forward. Found while landing #242: that branch forward-merged origin/main twice, and the second merge went RED, attributing these two files to my merge commit. The violation is a5b52047's, but the cost lands on the next unrelated branch that integrates. I worked around it by cherry-picking onto current main instead of merging, which drops the merge commit and reads green. That workaround is not general, and nobody should have to discover it.

The waiver route does not exist here

Worth recording, because it is the obvious first idea: check-role-discipline.py does not consult .agents/waivers.csv. Its imports are argparse, re, subprocess, sys, pathlib and nothing else. Only three checkers load waivers:

$ grep -ln 'waivers' scripts/check-*.py
scripts/check-commit-trailers.py
scripts/check-prompt-contract.py
scripts/check-pr-size.py

So a waiver row for this would parse, sit in the CSV, and change nothing, while making a red gate look like an authorized exception. Not written for that reason.

Options

  1. Re-land a5b52047 through a row/* branch. Correct, but rewrites published main with several sessions active, and main moved four times in the ~1h I was working. Risky.
  2. Make check-role-discipline waiver-aware, consistent with the other three checkers, then waive this commit. This is a checker semantics change, so per AGENTS.md it needs a spec, a red-before test or mutation, and green-after evidence. Additive, not a weakening.
  3. Accept and document that forward-merging main can inherit this red, with the cherry-pick workaround written down.

(2) looks like the honest general fix: the waiver mechanism exists precisely for "visible debt", and this checker is currently the one enforcement path with no way to record a sanctioned exception.

Prevention

The direct-to-main path that produced this is the thing to close. Whatever session pushed a5b52047 had the access to do it; the checker only notices afterwards, and only when someone merges forward.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions