fix(ci): classify agent authorship with python3 instead of Ruby#155
fix(ci): classify agent authorship with python3 instead of Ruby#155haasonsaas wants to merge 1 commit into
Conversation
The reusable agent-authorship-label workflow fails fleet-wide with `ruby: command not found` (exit 127) on every pull_request_target event: callers pin it by SHA at a revision whose classify step runs `ruby org-defaults/.github/scripts/classify-agent-authorship.rb`, and the resolved PUBLIC_PR_VALIDATION_RUNNER image no longer ships Ruby. Provisioning Ruby at runtime (ruby/setup-ruby) is a live tool install, which the repo rail reserves against and which #153 already removed from the other org reusable workflows. Follow that established pattern instead: port the classifier to python3 (standard library only), which is proven to exist on every runner image we operate, and assert the toolchain up front with a preflight so a missing interpreter fails loudly instead of after a silent skip. The Python port matches the Ruby original on 65 differential cases (plus identical file-arg and --github-output behavior) covering complete, incomplete, mixed, case-varied, whitespace-varied, unicode, nested commit.message, and empty inputs. Consumers pin this repo by SHA and must bump their `uses:` ref to pick up this fix; the SHA pinned in evalops/platform#5205 cannot be retrofixed. Refs evalops/platform#5205
|
|
||
| def extract_messages(text): | ||
| messages = [] | ||
| for line in text.splitlines(): |
There was a problem hiding this comment.
🟡 Classifier can crash on commit messages containing unusual line-break characters
Each line of the commits data file is split apart (text.splitlines() at .github/scripts/classify_agent_authorship.py:44) using a rule that treats many Unicode/control characters as line breaks, so a single commit record that contains one of those characters gets torn into pieces that no longer parse, aborting the whole labeling job.
Impact: If any pull request commit message contains a rare line-separator character, the authorship-label workflow errors out and fails instead of labeling the PR.
Divergence from the Ruby original in JSONL record splitting
The deleted Ruby classifier read records with input.each_line, which splits only on \n. The Python port uses text.splitlines(), which additionally splits on \r, \v, \f, \x1c-\x1e, \x85, \u2028, and \u2029. The commits file is produced by gh api ... --jq '.[] | {sha, message}' (.github/workflows/agent-authorship-label.yml:78-79), and jq emits characters like U+2028/U+2029 literally inside JSON strings rather than escaping them. If a commit message contains such a character, text.splitlines() breaks one JSON object into two fragments, and json.loads(line) at .github/scripts/classify_agent_authorship.py:47 raises JSONDecodeError, exiting non-zero and failing the classify step (set -euo pipefail). The Ruby version handled this input fine. The inner message.splitlines(keepends=True) at line 75 has the same over-splitting behavior but only affects trailer matching rather than crashing.
| for line in text.splitlines(): | |
| for line in text.split("\n"): |
Was this helpful? React with 👍 or 👎 to provide feedback.
| FLAGS = re.IGNORECASE | re.ASCII | ||
|
|
||
| REQUIRED_PATTERNS = { | ||
| "co_author": re.compile(r"^Co-Authored-By:\s*Maestro\s+<maestro@evalops\.dev>\s*$", FLAGS), | ||
| "version": re.compile(r"^Maestro-Version:\s*\S.*$", FLAGS), | ||
| "prompt_id": re.compile(r"^Maestro-Prompt-Id:\s*\S.*$", FLAGS), | ||
| "approvals_id": re.compile(r"^Maestro-Approvals-Id:\s*\S.*$", FLAGS), | ||
| } | ||
|
|
||
| MARKER_PATTERN = re.compile( | ||
| r"^Co-Authored-By:\s*Maestro\s+<maestro@evalops\.dev>\s*$" | ||
| r"|^Maestro-(?:Version|Prompt-Id|Approvals-Id):", | ||
| FLAGS, | ||
| ) |
There was a problem hiding this comment.
📝 Info: Regex port preserves Ruby semantics via re.ASCII and per-line matching
The Python port at .github/scripts/classify_agent_authorship.py:16-29 faithfully reproduces the Ruby regexes: re.ASCII matches Ruby's ASCII-only \s/\S, the concatenated marker alternatives correctly place $ before | matching the Ruby /x extended pattern, and iterating per-line with .search() reproduces Ruby's default multiline ^/$ anchoring. The empty-string message handling (if message is None) also matches Ruby's || parsed["message"] for non-nil strings. No functional divergence here beyond the line-splitting issue reported separately.
Was this helpful? React with 👍 or 👎 to provide feedback.
| commit = parsed.get("commit") | ||
| message = commit.get("message") if isinstance(commit, dict) else None |
There was a problem hiding this comment.
📝 Info: Python port is more robust than Ruby when commit field is non-dict
At .github/scripts/classify_agent_authorship.py:49-50 the port guards commit with isinstance(commit, dict). The Ruby original used parsed.dig("commit", "message"), which would raise a TypeError if commit were a non-Hash (e.g. a string). This is a behavioral improvement, not a regression, and is unlikely to matter given the fixed jq output shape.
Was this helpful? React with 👍 or 👎 to provide feedback.
| with tempfile.NamedTemporaryFile(mode="r", suffix="github-output") as handle: | ||
| outputs = classify( | ||
| [{"sha": "abc", "message": "fix: regular change"}], | ||
| github_output=handle.name, | ||
| ) | ||
| file_outputs = parse_outputs(handle.read()) | ||
|
|
||
| self.assertEqual(outputs, file_outputs) |
There was a problem hiding this comment.
📝 Info: Test relies on Linux-only NamedTemporaryFile reopen semantics
test/classify_agent_authorship_test.py:82-87 opens a NamedTemporaryFile and lets the classifier reopen it by name to append. This works on Linux runners (where this CI runs) but would fail on Windows due to the file being locked. Not a production concern given the workflow only runs on ubuntu runners, but worth noting if the test suite is ever run cross-platform.
Was this helpful? React with 👍 or 👎 to provide feedback.
Root cause
agent-authorship-labelfails fleet-wide on everypull_request_targetevent since ~2026-07-26T19:35Z withruby: command not found(exit 127). Callers pin the reusable workflow by SHA (@45c31c99), whose classify step runsruby org-defaults/.github/scripts/classify-agent-authorship.rb. The resolvedvars.PUBLIC_PR_VALIDATION_RUNNER || 'ubuntu-latest'image no longer ships Ruby — run history pins the regression to the runner image/routing change, not any PR diff.Fix
Per the repo rail (required CI jobs must not depend on live tool installs) and the precedent set in #153 (
ci: remove the ruby dependency from the org reusable workflows), the classifier no longer uses Ruby at all:classify-agent-authorship.rb→.github/scripts/classify_agent_authorship.py(standard library only). python3 is proven to exist on every runner image we operate, so there is nothing to install.ruby/setup-rubystep (a live tool install) and added a preflight step assertingpython3exists before classification, matching the pattern incodex-rails-check.yml— a missing interpreter now fails loudly with a named error.test/classify_agent_authorship_test.py, stdlib unittest) and updated the workflow guard test to enforce the python3-no-Ruby contract instead of the setup-ruby contract.Validation
commit.messagevs top-levelmessage, missing message, and empty input.--github-outputappend behavior verified byte-identical to the Ruby original.python3 -m unittest discover -s test -p '*_test.py': 4 tests OK.ruby -Itest test/workflow_pr_ref_guard_test.rb: 4 runs, 22 assertions, 0 failures.Note for consumers
Consumers pin this repo by SHA, so the
@45c31c99pin cited in the platform issue cannot be retrofixed — callers must bump theiruses:ref (andhelper_refif overridden) to a revision containing this change.Refs evalops/platform#5205 (cross-repo: closing keyword won't auto-close from this repo, so this is a reference — please close evalops/platform#5205 once caller pins are bumped).