Skip to content

feat(ci): run coder-eval through the published action, and publish an expiring evalboard link - #2942

Merged
bai-uipath merged 9 commits into
mainfrom
bai/run-coder-eval-evalboard-link
Sep 1, 2026
Merged

feat(ci): run coder-eval through the published action, and publish an expiring evalboard link#2942
bai-uipath merged 9 commits into
mainfrom
bai/run-coder-eval-evalboard-link

Conversation

@bai-uipath

@bai-uipath bai-uipath commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Consumer half of giving run-coder-eval dispatches a shareable evalboard link while moving the Linux job onto the published composite action. Upstream half: UiPath/coder_eval#147.

Blocked on that PR being merged AND released. uses: UiPath/coder_eval@v0 reads its inputs from the action at that tag, and that PR rewrites the input surface: tasks, model, junit-path and step-summary no longer exist, and args / extras / extra-packages / install-flags / working-directory do not exist at today's v0. GitHub silently ignores unknown inputs, so a dispatch before the release would run with none of them rather than fail. Do not dispatch until v0 has moved.

What this does

Dogfoods the action. It had no real consumers: only its own repo's self-test, so a bad release surfaces there instead of in someone's workflow. What blocked adoption was concrete, and is fixed upstream: the suite lives in tests/, needs an agent extra, and for delegate-sdk needs a plugin inside the environment the CLI actually runs from.

Being the first real consumer is also what reshaped the action. It promoted five of coder-eval run's 21 flags to named inputs with no principle behind the choice, and since GitHub silently ignores an unknown input, every forwarding input is a way for a run to measure something else and still exit 0. The upstream PR drops all of them, so the whole command line arrives through one args block — the task globs, --model, --type, -j, -v and the -D overrides, all composed by the prep step that already existed here.

That makes this side simpler, not more complex. The model step output goes away. step-summary: "false" goes away because the action no longer writes a job summary at all: it reports run-md-path and the append happens after redaction, which is the order this workflow needs and previously had to work around with a comment explaining why the input could not simply be turned on. Globs also stop being shell-expanded, so ** works without globstar and a glob matching nothing exits 1 instead of reaching the CLI as a literal path.

Emits an evalboard link per run, into a dedicated runs-gha container that deletes it after 14 days. No Slack, no new dashboard tab, no pollution of nightly history.

Fixes two things that were already broken and would have looked like my doing once I rewrote the neighbouring steps. See below.

The refactor, and why it is safe

The single 150-line run step becomes prep + invoke, because working-directory: is illegal on a uses: step and a job-level defaults.run does not reach inside a composite action. The prep body is the head of the old step moved, not rewritten: each export FOO=bar became a line in an env-block accumulator, and the trailing coder-eval run is gone. Read it as a diff against the old step.

Since a paired dispatch is not yet possible, I extracted the prep script straight out of the YAML and drove it locally through ten scenarios. All ten behaved:

Checked Result
claude / codex / antigravity / delegate-sdk correct model, --type, extras, args
CODEX_MODEL / ANTIGRAVITY_MODEL / CLAUDE_CODE_MODEL unset still hard-fails at prep with the same message
DELEGATE_MODEL unset still defaults to kimi-k2-7-code without failing
parallelism: 20 on delegate ::notice:: fires, -j 6 reaches the action
~/.uipath/.auth missing one key still ::error:: + exit 1, per key
the three *_VAR knobs forwarded only when non-empty, each with its echo
the 12-name bracketed -D override arrives as one intact argument

All twenty non-_VAR env entries are forwarded, including the four model variables only the shell reads. That last part is not defensive padding: coder-eval's docker driver passes CODEX_MODEL and ANTIGRAVITY_MODEL into task containers by default, so dropping them because the shell also reads them would have been a real change.

The bracketed -D override goes through the action's new args input, which appends one argument per line verbatim, preserving exactly what the old array-and-quote protected it from.

The evalboard path

Best-effort by construction. Every step in that block warns and returns 0, and the whole block is skipped until AZURE_STORAGE_KEY is set as a repo secret, so this merges and behaves exactly as before while the credential is still being provisioned. Four failure paths tested locally: no run.json, uploader absent, upload fails, happy path. All exit 0; only the last one prints a link.

Authentication reuses the credential the ADO pipelines already use for this storage account (AZURE_STORAGE_KEY / AZURE_STORAGE_ACCOUNT, out of coder-eval-athena-{secrets,config}) rather than standing up a second, GitHub-only Entra app with a federated credential. eval-runner passes the value straight to BlobServiceClient, and the Azure SDK sniffs the string, so the secret can hold either the account key or a container-scoped SAS token with no change to this workflow.

A scoped SAS is the better thing to put there. workflow_dispatch runs the workflow definition from whatever branch the dispatcher picks, and an account key is authority over the entire account, including the runs container that holds months of nightly history. A SAS limited to runs-gha with create/write bounds the blast radius to the container this feature owns; the cost is an expiry to rotate.

Two things narrow the exposure regardless: the credential is set only on the upload step, never job-wide, which matters because the steps before it run an agent with shell access (the ADO pipelines scope it the same way and say so); and the container name is hardcoded with no variable override, so no repo setting can redirect an upload into runs.

permissions: is job-level on run-linux only. A permissions: block makes every permission it omits none for every job it covers, and this file declares none today, so a workflow-level one would have silently stripped the Windows job's token.

Already broken, fixed here

  • No redaction step. smoke-skills.yml has had one since the reports started carrying agent transcripts; this workflow never did, so its artifact has been shipping them unredacted. Ported into both jobs, covering every secret this job forwards rather than the three smoke-skills strips, plus the ROPC token read from disk. Runs after the .venv/node_modules cleanup so it does not walk tens of thousands of files for nothing.
  • Linux artifact globs off by one level. --run-dir /tmp/runs makes that directory itself the run dir, so /tmp/runs/*/experiment.html matched nothing and no experiment- or variant-level report has ever been uploaded. The Windows copies are correct as written, because that job passes no --run-dir.

Deliberately not done

  • minimum-task-score is not adopted. The gate here is "every row is SUCCESS", which is a different predicate from a weighted_score floor, and swapping gate semantics inside a behaviour-preserving refactor would make a regression indistinguishable from the intended change.
  • The job summary is appended here, not by the action. The action reports run-md-path and writes nothing itself, which is what lets the append land after redaction. It reads that output with the literal path as a fallback, because a composite action's outputs may not propagate from a step that exited non-zero and this step runs on failure. Same 1 MB cap: GitHub rejects a summary over 1 MiB outright, losing the verdict with it.
  • The Windows job keeps its per-task retry loop and is untouched apart from redaction and a one-line note that the evalboard link, if any, covers only the Linux half. That note is its own step so the verdict step stays byte-identical: an echo appended after its python | tee pipeline would have become the step's exit status and turned every red Windows run green.

Verification

actionlint (with the two self-hosted labels configured) is clean, and shellcheck reports fewer findings than origin/main for this file: the two unquoted-expansion warnings in the old run step are gone and nothing new was added. Every run: body parses under bash -n, and both embedded Python blocks compile.

Azure side is already in place: container runs-gha created private, and lifecycle rule expire-runs-gha-14d (prefixMatch: runs-gha/, 14 days) appended to the account policy without disturbing expire-runs-live-7d.

End-to-end, on a temporary SHA pin

uses: cannot take an expression, so both dispatches below pinned UiPath/coder_eval@a747ce35 (the upstream branch head) in place of @v0. That pin is reverted before this merges and is marked as such in the file.

Run Agent Result
33559491097 claude 2/2 SUCCESS
33559898478 delegate-sdk 2/2 SUCCESS

Both ran tasks/uipath-automationhub/*.yaml at -j 2, with the Windows job correctly skipped (neither task carries the windows tag). Confirmed from the logs: the tests/.coder-eval-version pin resolved (coder-eval==0.11.5), the litellm extra composed into the install requirement rather than being added afterwards (litellm==1.98.0), /tmp/runs/junit.xml and /tmp/runs/run.md were written and reported as outputs, and the evalboard link resolved against the SAS now held in AZURE_STORAGE_KEY.

The delegate-sdk run is the one that exercises extra-packages: coder-eval-uipath==0.1.0 installed into coder-eval's own tool environment and its coder_eval.plugins entry point registered delegate_sdk_agent. That discovery is only possible when the plugin shares the CLI's virtualenv, which is the entire reason the input exists. Its twelve-name -D sandbox.docker.env_passthrough_extra=[...] override also arrived as one intact argument through args.

Still outstanding before a dispatch produces a link: the AZURE_STORAGE_KEY repo secret on UiPath/skills. Optionally AZURE_STORAGE_ACCOUNT as a repo variable, which defaults to coderevaltests. No Azure-side provisioning is left: the runs-gha container and its expire-runs-gha-14d lifecycle rule already exist.

🤖 Generated with Claude Code

bai-uipath and others added 6 commits August 31, 2026 14:42
…expiring evalboard link

Three things, all in the Linux job of the dispatch workflow.

**Dogfood the action.** `UiPath/coder_eval@v0` had no real consumers: it was exercised only by its own repo's self-test, so a bad release would surface there rather than in someone's workflow. This job now installs and invokes coder-eval through it. What blocked adoption before is fixed upstream: the suite lives in `tests/`, needs an agent extra, and for delegate-sdk needs a plugin inside the environment the CLI actually runs from, and none of that was expressible.

The single 150-line run step becomes prep + invoke. `working-directory:` is illegal on a `uses:` step and a job-level `defaults.run` does not reach inside a composite action, so everything conditional resolves into plain inputs first. The prep body is the head of the old step moved rather than rewritten: each `export FOO=bar` became a line in an env-block accumulator, and the trailing `coder-eval run` is gone. Every guard is preserved and was exercised locally against the extracted script: the three `:?` model hard-failures, delegate's `-j` clamp with its notice, its per-key `~/.uipath/.auth` validation, and the non-empty-only re-export of the three `*_VAR` knobs. All twenty non-`_VAR` env entries are forwarded, including the four model variables the shell also reads, because coder-eval's docker driver passes `CODEX_MODEL` and `ANTIGRAVITY_MODEL` into task containers by default.

The bracketed `-D sandbox.docker.env_passthrough_extra=[...]` override now goes through the action's `args` input, which appends one argument per line verbatim. That is the same protection the old array-and-quote gave it, for the same reason: to bash the list is a character class.

**An evalboard link per run.** The run uploads to a dedicated `runs-gha` container, which expires it after 14 days, and the link lands in the job summary. Entirely best-effort: every step warns and returns 0, because a missing dashboard link must never be mistaken for a failed eval. Inert until `AZURE_EVAL_UPLOAD_CLIENT_ID` exists as a repo variable, so this behaves exactly as before until the Azure identity is provisioned. Credentials are OIDC, not a storage key: `workflow_dispatch` runs the workflow definition from whatever branch the dispatcher picks, so a key in a repo secret would let any branch delete months of nightly history on that account.

**Two fixes to things already broken.** The workflow had no redaction step, unlike smoke-skills.yml, so its artifact has been shipping unredacted transcripts; it is ported into both jobs and covers every secret this job forwards, not just the three smoke-skills strips. And the Linux artifact globs were off by one directory level, uploading no experiment- or variant-level report at all, because `--run-dir /tmp/runs` makes that directory itself the run dir.

`run-name` is one line and independently the highest value per character available here: all ~2,500 historical runs are titled "Run Coder Eval".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rage credential

The evalboard upload minted a GitHub OIDC token and exchanged it for an Azure credential, which needed a new Entra app registration, a federated credential scoped to a GitHub environment, and a container role assignment before it could work at all. The ADO pipelines already authenticate to this same storage account with `AZURE_STORAGE_KEY` / `AZURE_STORAGE_ACCOUNT`, and eval-runner passes that value straight to `BlobServiceClient`, so reusing it removes every one of those provisioning steps: two repo settings and nothing else.

The Azure SDK sniffs the credential string, so the secret holds either the account key or a container-scoped SAS token with no change here. The SAS is preferable, since an account key is authority over the whole account including the nightly-history container, and this workflow runs its definition from whatever branch the dispatcher picks.

Drops `id-token: write`, the `eval-upload` environment and the token-minting step. The gate moves to a job-env boolean because the `secrets` context is unavailable in any `if:`; deriving a boolean rather than exporting the secret keeps the credential in the upload step alone, matching how ADO scopes it. The container name is now hardcoded, since the unlisted evalboard source and the 14-day expiry rule both depend on ad-hoc runs never landing in `runs`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on's args

Follows the action's input simplification: it no longer promotes any of `coder-eval run`'s flags to named inputs, so the task globs and `--model` join the `-e`, `--type`, `-j`, `-v` and `-D` entries in the single `args` block the prep step already composed. The `model` step output goes away with them.

Task globs are word-split when appended (TASK_GLOBS is a space-separated list from the partition job) but never pathname-expanded, which is an improvement on what the action used to do with them: the CLI expands them itself, so `**` works without `globstar` and a glob matching nothing exits 1 rather than reaching the CLI as a literal path.

`step-summary: "false"` and `junit-path` are gone. The action writes no job summary at all now and reports `run-md-path` instead, so the append can happen after redaction, which is the order this workflow needs and previously had to work around. The append reads that output with the literal path as a fallback, because a composite action's outputs may not propagate from a step that exited non-zero and this step runs on failure. The JUnit report moves to /tmp/runs/junit.xml, derived from run-dir; nothing consumed it at the old path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y run

The `v0` tag still serves the ten-input action, so this workflow's `with:` block
would have `args`, `extras`, `extra-packages` and `working-directory` silently
dropped — GitHub ignores an input the referenced tag does not declare. Pinning
the PR head validates the whole path before anything merges and before Release
moves `v0`.

REVERT TO @v0 BEFORE MERGING.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comments only. Fifteen blocks cut to their contract, 118 lines out for 58 in, so
the file sits at its existing 33% comment density instead of above it.

Also drops a stale claim: the two-step-split banner said TASK_GLOBS "is handed
straight to the action's `tasks` input, which word-splits it". There is no
`tasks` input, and `args` entries are passed verbatim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The link was appended after run.md, which is roughly 40 lines of tables, so
it landed below the fold and was easy to miss.

Prepend rather than reorder the steps: the evalboard step moves /tmp/runs,
which both the artifact upload and the verdict step read, so it has to stay
last. $GITHUB_STEP_SUMMARY is an ordinary file, so the block is written to a
temp file with the existing summary appended, then truncate-and-rewritten in
place to keep the inode the runner tracks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bai-uipath
bai-uipath marked this pull request as ready for review September 1, 2026 21:38
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @bai-uipath's task in 3m 3s —— View job


PR Review: feat(ci): run coder-eval through the published action

  • Read review criteria and project conventions
  • Read the full diff (origin/main...HEAD)
  • Read the full changed file for surrounding context
  • Check for secrets, tokens, or personal paths
  • Check for security issues in credential handling
  • Verify shell correctness and quoting
  • Post review findings

Summary

This PR migrates the Linux coder-eval job from an inline coder-eval run invocation to the published UiPath/coder_eval composite action, adds a best-effort evalboard link upload with 14-day expiry, fixes two pre-existing bugs (missing secret redaction and artifact globs off by one directory level), and adds a dynamic run-name for better run-list scannability. The single changed file is .github/workflows/run-coder-eval.yml (+478/−90).

Change-by-Change Review

1. Temporary SHA pin — must revert before merge

Severity: Critical

.github/workflows/run-coder-eval.yml:624 pins the action to a commit SHA on the upstream branch:

uses: UiPath/coder_eval@a747ce353111beadd51a6370b9e8e2dba88f4133  # bai/action-inputs-and-gha-source

The comment on line 622–623 says TEMPORARY — REVERT TO @v0 BEFORE MERGING. This is clearly marked and intentional for e2e testing, but it is the single hard blocker: merging with this pin would break when the upstream branch is deleted or force-pushed, and it bypasses the release process the PR description explicitly depends on.

Fix this →

2. Prep step uses set -uo pipefail without -e

Severity: Low

.github/workflows/run-coder-eval.yml:443 — the prep step uses set -uo pipefail (no -e). All error exits are handled explicitly with exit 1, so this is intentional and consistent with complex scripts where set -e causes surprising control flow. The same pattern is used at line 722 (summary step) and line 819 (evalboard step). Mentioning for visibility only — not a bug.

3. Credential scoping and security model

Severity: OK

Well designed:

  • AZURE_STORAGE_KEY is scoped to only the upload step's env: block (line 807), never the job env — mirrors the ADO pipelines' scoping rationale.
  • EVALBOARD_UPLOAD_ENABLED launders a boolean (not the secret itself) into the job env (line 235) because secrets context isn't available in if: conditions.
  • Container name runs-gha is hardcoded at line 813 with an explicit comment (line 809–812) explaining why there's no variable override — prevents a repo variable from redirecting uploads into the production runs container.
  • The permissions: contents: read block is correctly job-level (lines 225–226), not workflow-level, to avoid stripping the Windows job's default token.
  • AUTH_TOKEN from the delegate auth file is re-masked via ::add-mask:: (lines 546–548) to survive step reordering.

The PR description's recommendation to use a container-scoped SAS token instead of the account key is sound and should be tracked.

4. Redaction step addition (both jobs)

Severity: OK

Linux (lines 651–708) and Windows (lines 1339–1393) both get the missing redaction. The scripts redact 8 named secrets plus the ROPC access token from the auth file, scanning only text-format files (.html, .json, .log, .md, .xml). Runs after .venv/node_modules cleanup to avoid walking tens of thousands of irrelevant files.

Minor observation: the redaction Python is near-duplicated across both jobs (differences: auth file path and scan root). This is somewhat unavoidable in workflow YAML since run: blocks can't be shared, and extracting to a script file is a style choice. Not flagging as an issue.

5. Artifact glob fix (Linux)

Severity: OK

Lines 743–757 fix the paths from */experiment.* to experiment.* (and similarly for variant). The explanation is correct: --run-dir /tmp/runs makes /tmp/runs the run dir itself, so reports land at /tmp/runs/experiment.*, not /tmp/runs/*/experiment.*. The Windows job's paths are left unchanged because it uses no --run-dir and gets a timestamped subdirectory.

6. Evalboard publish block

Severity: OK

Lines 779–881. Every step uses continue-on-error: true and/or explicit exit 0 on failure paths. Four failure modes are handled: missing eval-runner, missing run.json, failed mv, and failed upload. The blob ID includes timestamp + run ID + attempt number for uniqueness (line 838). The ?src=gha query parameter is required by the evalboard's sourceById routing (line 854).

The summary prepend logic (lines 868–880) is a smart workaround: it can't run earlier because it moves /tmp/runs, and appending would put the link below the fold. The truncate-and-rewrite via a temp file preserves the runner's inode tracking.

7. Prep → action split

Severity: OK

The old monolithic step is cleanly split into a prep step (computes all inputs) and the composite action invocation (executes). The env forwarding via env_lines array (lines 455–462) covers all 20 non-_VAR entries. The multi-line heredoc output (lines 602–612) with a randomized delimiter handles the -D override safely — bash's [...] is no longer a glob risk because printf '%s\n' emits each array element as its own line.

The # shellcheck disable=SC2206 at line 594 is correct: args+=($TASK_GLOBS) intentionally word-splits because TASK_GLOBS is a space-separated list from the partition job.

8. run-name addition

Severity: OK

Line 5: run-name: ${{ inputs.task_globs }} · ${{ inputs.agent }} · @${{ github.actor }} — useful UX improvement. These are all controlled inputs (choice type for agent, string for globs, and github.actor is GitHub-controlled), so no injection risk.

9. Windows job — evalboard note step

Severity: OK

Lines 1442–1447. Correctly placed as its own step to avoid changing the exit status of the verdict step above it (an appended echo after python | tee would become the step's exit code). The comment on lines 1435–1441 explains this clearly.

What's Missing

  • The @v0 revert — the PR is explicit about this being outstanding, but it's the merge-blocking item.
  • Tracking issue for SAS token rotation — the PR description recommends a container-scoped SAS over the account key. If this isn't tracked elsewhere, it should be.

Area Ratings

Area Status Notes
Frontmatter N/A No skill files changed
E2E Tests N/A CI workflow change only
Skill Body N/A No skill files changed
References & Assets N/A No skill files changed
Repo Hygiene OK No secrets committed; credential scoping is well-designed

Issues for Manual Review

  • Upstream dependency: This PR is blocked on UiPath/coder_eval#147 being merged AND released to v0. Dispatching before that release would silently run with no inputs (GitHub ignores unknown with: keys on a composite action).
  • AZURE_STORAGE_KEY provisioning: The evalboard feature is inert until this repo secret is set. The PR is safe to merge before that — all evalboard steps gate on EVALBOARD_UPLOAD_ENABLED.
  • eval-runner upload CLI contract: The uploader's --adhoc, --title, and --description flags come from the coder-eval-uipath-eval-runner package. If that package's CLI surface changes, this step breaks silently (continue-on-error: true). Worth confirming the contract is stable.

Conclusion

This is a well-engineered PR with thorough attention to security (credential scoping, redaction, least-privilege permissions), failure resilience (every evalboard step is best-effort), and backward compatibility (evalboard is inert without the secret, artifact paths are fixed not changed). The code is heavily commented with clear rationale for every decision.

One Critical item: revert the temporary SHA pin at line 624 to @v0 before merging. The author has already flagged this. Everything else is clean.


The previous attempt prepended to $GITHUB_STEP_SUMMARY inside the evalboard
step. That cannot work: the variable points at a SEPARATE file per step, and
GitHub builds the job summary by concatenating them in step order, so the step
read its own empty file and its block still rendered last. Verified on run
33564007553, where the link stayed at the bottom.

Move the publish above the step that appends run.md, which is the only way to
control position, and copy the run directory instead of moving it so the
artifact upload and the verdict still find /tmp/runs. Also emit a ::notice::,
which renders above the job list on the run page.

Keep "Run Coder Eval" as the run-name prefix so the run list stays scannable
by workflow as well as by glob, agent and actor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@uipreliga

Copy link
Copy Markdown
Collaborator

Code review

Multi-model review (Gemini 3 + an Opus sub-agent + my own reading). The disputed GitHub Actions semantics are verified against GitHub's docs and against the coder_eval repo itself, not asserted. Line numbers refer to the post-PR file.

Critical

1. run-coder-eval.yml:622-624 — action pinned to a PR head SHA, and the revert instruction is itself wrong.

# TEMPORARY — REVERT TO @v0 BEFORE MERGING. `v0` still serves the old
# ten-input action, which would silently ignore every input below.
uses: UiPath/coder_eval@a747ce353111beadd51a6370b9e8e2dba88f4133

I checked both action versions. coder_eval main's action.yml has inputs tasks / tags / model / extra-args / version / run-dir / junit-path / step-summary / env / minimum-task-score — no working-directory, no extras, no extra-packages, no args, and no run-md-path output. The pinned SHA has the new interface.

So the note as written instructs a change that breaks the workflow: GitHub only warns on unexpected with: keys, so reverting to @v0 today would run coder-eval run --run-dir /tmp/runs --junit-xml ... with no experiment, no tasks, no model and no env passthrough. The real precondition is "merge coder_eval#147, re-tag v0, then pin to the release SHA" — worth saying explicitly, or the next person follows the comment literally.

High

2. run-coder-eval.yml:864-880 — the headline change doesn't work: the evalboard link still lands at the bottom.

$GITHUB_STEP_SUMMARY is per-step, not per-job. From GitHub's docs: "The path to this file is unique to the current step and changes for each step in a job" and "After a step has completed, job summaries are uploaded and subsequent steps cannot modify previously uploaded Markdown content."

So cat "$GITHUB_STEP_SUMMARY" at line 877 reads this step's own empty file, and the truncate-and-rewrite at 879 truncates only this step's file. The Evalboard block renders as this step's summary — i.e. after Append the run report (717) and Plaintext summary + verdict (760), which is exactly where 1b800ef was trying to move it away from. The mktemp/inode machinery wraps a no-op, and its premise ("$GITHUB_STEP_SUMMARY is an ordinary file; truncate-and-rewrite rather than mv so the runner keeps tracking the same inode") is wrong.

The clean fix is to stop moving the directory: compute blob_id in the prep step, pass run-dir: /tmp/<blob_id> to the action, and run the publish step immediately after redaction — before the summary append. Then the link is genuinely first, the mv disappears, and the "safe to move because this is the last step to touch /tmp/runs" comment (834-835) goes with it.

3. run-coder-eval.yml:261-263 / 346-358 / 502-509 — the continue-on-error checkout defeats the delegate-sdk guard twice over.

  • Too weak: actions/checkout creates path: coder_eval_uipath before fetching, so an auth failure leaves an empty directory. [ ! -d ../coder_eval_uipath ] passes, extra_packages is emitted anyway, and you get the uv resolve error the guard exists to prevent. Line 300 already uses the stronger form (-d coder_eval_uipath/eval_runner) — a file test (-f ../coder_eval_uipath/pyproject.toml) would be better still.
  • Too late: Overlay skills image for delegate-sdk (line 346) builds from coder_eval_uipath/eval_runner/scripts/ci/delegate-overlay.Dockerfile at line 357, ~50 lines before the prep step runs. On a delegate-sdk run with a stale PAT you now burn the GHCR pull, the skills image build and the ROPC mint, then fail on an opaque docker build error. Pre-PR the job died at the checkout with a clear message. The comment at 252-255 ("delegate-sdk still fails loudly: the prep step below asserts the checkout is present before the plugin is needed") describes a protection the step ordering doesn't provide.

Fix: an explicit if: inputs.agent == 'delegate-sdk' assert step directly after the checkout.

4. run-coder-eval.yml:667-670 — "Every secret this job forwards into the run" is not true.

CODEX_BASE_URL (422), AWS_REGION (404) and BEDROCK_MODEL (405) are secrets.*, are forwarded via env_lines, and are not in the redaction list. CODEX_BASE_URL in particular is an internal gateway URL deliberately stored as a secret, with no stated reason to exclude it — unlike the org/tenant slugs, whose exclusion at 690-691 is well justified. Either extend the list or narrow the claim.

5. run-coder-eval.yml:698 vs 756 — the redaction suffix allowlist doesn't cover what the artifact actually ships.

Redaction touches only {.html,.json,.log,.md,.xml}, but the upload includes /tmp/runs/**/artifacts — whole agent scratch trees full of .txt, .jsonl, .py, .env and extensionless files. Given the stated motivation ("its artifact has been shipping unredacted transcripts all along"), this is half a fix. Walking every regular file with a size cap and a b"\0" in raw[:8192] binary sniff is barely more code.

Medium

6. run-coder-eval.yml:764 — the verdict step still has the exact off-by-one this PR fixed for uploads.

Confirmed in the coder_eval repo (tests/test_integration.py:170: "when --run-dir is provided, it's used directly") and against a real run tree: the run dir holds run.json, run.md, experiment.* and one directory per variant. So ls -td /tmp/runs/*/ | head -n 1 selects a variant directory. It works today only because tests/experiments/nightly.yaml:62 declares a single variant; a second variant would silently grade one arm and could turn a red run green. run_dir=/tmp/runs is the fix. (The Windows copy at 1416 is correct — no --run-dir there.)

Worth noting smoke-skills.yml:500 and its upload globs have the same latent bug, so either the diagnosis generalizes or it deserves a second look.

7. run-coder-eval.yml:588-595 — "NOT pathname-expanded" is wrong bash.

args+=($TASK_GLOBS) with no set -f undergoes word splitting and pathname expansion. It's harmless only because the partition job resolved these to metacharacter-free file paths — which also makes the rest of the comment wrong ("the CLI expands them itself and handles ** without globstar": there is nothing left to expand). The comment it replaced said this correctly, and this one contradicts lines 464-466 four lines above.

8. run-coder-eval.yml:599-612 — nothing enforces the single-line invariant the env_block design rests on.

The comment at 454 says "Values must be single-line," but nothing checks. A secret rotated to a multi-line value splits into two entries and the action parses the second as another NAME=VALUE — env injection into the eval process from a config change nobody would trace back here. Mitigating: the action's parser hard-errors on a line with no =, so it fails closed rather than leaking. Still worth a guard:

case "${!name}" in *$'\n'*)
    echo "::error::$name contains a newline; the env block is single-line only"; exit 1 ;;
esac

Separately, the delimiter-collision risk motivating the randomized delimiter (602) is essentially unreachable — that comment does more work than the risk warrants.

9. run-coder-eval.yml:651-708 and 1339-1394 — the ~45-line redaction heredoc is duplicated verbatim.

Plus a third near-copy at smoke-skills.yml:534. Findings 4 and 5 now have to be fixed three times. .github/scripts/redact-run-report.py --run-dir X --auth-file Y, next to the existing refresh-auth.sh, is the obvious move and the highest-leverage cleanup in the PR.

10. run-coder-eval.yml:618-619 — the JUnit claim is false.

"JUnit lands at /tmp/runs/junit.xml and travels with the run into the artifact and the blob." None of the upload patterns at 743-756 match /tmp/runs/junit.xml. It reaches the blob only. Probably an omission in the path list rather than a comment error.

11. run-coder-eval.yml:711-712 — the 1 MiB rationale is wrong.

The limit is per step, and per the docs upload failures for job summaries don't affect step or job status. An oversized run.md would drop that step's summary, not "lose the verdict too". Keep the head -c cap; fix the sentence.

12. run-coder-eval.yml:222-226 — the job-level permissions: rationale doesn't hold.

The premise is right (an omitted permission becomes none), but all three jobs need exactly contents: read and nothing in the file uses GITHUB_TOKEN for anything else — GHCR and npm both use GH_PACKAGES_TOKEN/GH_PAT. Workflow-level would harden all three jobs and can't be forgotten on the next one added.

Low

  • :680-684UIPATH_CLI_AUTH_TOKEN in the names tuple is dead: nothing in this workflow or refresh-auth.sh ever sets it. The file-based extraction at 687-693 is what actually covers it, and the comment sitting inside the tuple half-admits this. Copy-paste from smoke-skills.yml.
  • :277-282 — orphaned comment block describing the deleted install step, now floating above an unrelated PATH step. Belongs on Run coder-eval at 620.
  • :287-288$HOME/.local/bin hardcoded; uv tool dir --bin reports it authoritatively. If it's ever wrong, command -v eval-runner silently disables the feature rather than failing.
  • :585-586 — task paths moved from first to last in the argv. Fine for click, but it's an untested reordering riding on an untested action; worth one smoke dispatch on the claude agent (the branch with no --type, so the globs directly follow --model <value>).
  • :1435-1447 — 7 lines of comment for 2 lines of echo, and with AZURE_STORAGE_KEY unset (the merge-day state) every Windows run advertises a link that categorically does not exist.
  • Comment density. Roughly half the added lines are prose, and this review found six comments that are outright wrong (the inode reasoning, "the prep step below asserts", "every secret", "NOT pathname-expanded", the JUnit claim, the 1 MiB claim). Wrong comments are worse than none — a reader will trust them over the code. The load-bearing ones are genuinely excellent and should stay: the -j 6 clamp rationale (510-513), the _VAR suffix explanation (435-437), ?src=gha (853-854), the secrets-in-if: workaround (232-234). The banner-style ones (779-797, 735-742, 829-837) could each be a third as long.

What's good here

  • The secrets-in-if: workaround (232-235) is exactly right and matches the documented context table; laundering a boolean rather than the credential is the correct call.
  • AZURE_STORAGE_KEY scoped to one step with the threat model stated, plus the SAS-over-account-key warning at 792-796 — useful to whoever provisions the secret.
  • Best-effort degradation is done properly throughout: command -v guard, run.json existence check, mv || { warn; exit 0; }, if ! eval-runner upload. A missing link genuinely cannot redden a run.
  • The artifact-path fix (743-756) is a real bug fix — the old patterns uploaded no run-, experiment- or variant-level report at all.
  • Resolving image_tag in partition removes a cross-step $GITHUB_ENV dependency rather than adding one.
  • Moving -j into args after the delegate clamp, so the logged command matches the executed one.
  • The Windows note's exit-status reasoning (1439-1441) is correct: an appended echo after python | tee would indeed become the step's exit status.
  • run-name: is a cheap, high-value usability win.

Overall

Three real fixes here — the artifact-path off-by-one, the missing redaction, and a better-structured invocation — but the headline feature doesn't work: $GITHUB_STEP_SUMMARY is per-step, so the prepend puts the link at the bottom, which is where it already was. Not mergeable as-is (the action pin alone blocks it). Since several comments assert protections the code doesn't deliver, the prose needs auditing as carefully as the code.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KGw7DXxbJaSZSkNaBGVxPA

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix what you agree with and 🚢

bai-uipath and others added 2 commits September 1, 2026 15:41
`--run-dir /tmp/runs` makes /tmp/runs the run dir itself, so its subdirectories
are VARIANTS. The verdict step selected one with `ls -td /tmp/runs/*/ | head -n
1`, which is correct only while nightly.yaml declares a single variant: a second
one would silently grade a single arm and could report a red run green. This is
the same off-by-one the artifact globs in this branch already fix, one step
lower.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The temporary SHA pin pointed at the PR branch of coder_eval#147, which is
now merged and released as 0.11.6 with v0 promoted to it, so the eight-input
surface this workflow passes is what v0 serves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bai-uipath
bai-uipath merged commit f9592f9 into main Sep 1, 2026
18 checks passed
@bai-uipath
bai-uipath deleted the bai/run-coder-eval-evalboard-link branch September 1, 2026 23:12
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.

2 participants