Skip to content

fix(ci): classify agent authorship with python3 instead of Ruby#155

Open
haasonsaas wants to merge 1 commit into
mainfrom
fix/agent-authorship-ruby
Open

fix(ci): classify agent authorship with python3 instead of Ruby#155
haasonsaas wants to merge 1 commit into
mainfrom
fix/agent-authorship-ruby

Conversation

@haasonsaas

@haasonsaas haasonsaas commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Root cause

agent-authorship-label fails fleet-wide on every pull_request_target event since ~2026-07-26T19:35Z with ruby: command not found (exit 127). Callers pin the reusable workflow by SHA (@45c31c99), whose classify step runs ruby org-defaults/.github/scripts/classify-agent-authorship.rb. The resolved vars.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:

  • Ported 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.
  • Dropped the ruby/setup-ruby step (a live tool install) and added a preflight step asserting python3 exists before classification, matching the pattern in codex-rails-check.yml — a missing interpreter now fails loudly with a named error.
  • Ported the classifier test to Python (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

  • Differential test, old Ruby script vs Python port: 65 cases, 0 mismatches (stdout + exit code), covering complete/incomplete/mixed trailers, case and whitespace variation, unicode, nested commit.message vs top-level message, missing message, and empty input.
  • File-argument invocation and --github-output append 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 @45c31c99 pin cited in the platform issue cannot be retrofixed — callers must bump their uses: ref (and helper_ref if 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).


Open in Devin Review

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

Open in Devin Review


def extract_messages(text):
messages = []
for line in text.splitlines():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Suggested change
for line in text.splitlines():
for line in text.split("\n"):
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +16 to +29
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,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +49 to +50
commit = parsed.get("commit")
message = commit.get("message") if isinstance(commit, dict) else None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +82 to +89
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant