Skip to content

ci: drop CLICOLOR_FORCE=1 from tend-setup action#5819

Merged
max-sixty merged 1 commit intomainfrom
daily/review-runs-24711995590
Apr 21, 2026
Merged

ci: drop CLICOLOR_FORCE=1 from tend-setup action#5819
max-sixty merged 1 commit intomainfrom
daily/review-runs-24711995590

Conversation

@prql-bot
Copy link
Copy Markdown
Collaborator

Summary

Remove CLICOLOR_FORCE=1 from .github/actions/tend-setup. This env var silently breaks the CI-polling pattern used by every tend-review session, causing 15–20 min hangs on each polling attempt.

The bug

CLICOLOR_FORCE=1 tells tools to emit ANSI colour codes even when their stdout is a pipe. gh respects this for --json output: the JSON gets prefixed with escape codes, so piping gh pr view --json statusCheckRollup | jq ... fails with jq: parse error: Invalid numeric literal at line 1, column 2 on every call.

That is exactly the polling pattern the bundled tend-ci-runner:running-in-ci skill documents:

pending() {
  gh pr view <num> --json statusCheckRollup \
    | jq --arg own "/runs/$GITHUB_RUN_ID/" '
      [.statusCheckRollup[] | select(...) | ...] | length'
}
for i in $(seq 1 15); do
  sleep 60
  [ "$(pending)" -gt 0 ] && continue
  ...

When pending() returns an empty string (stderr-only output), [ "" -gt 0 ] is an integer expression expected error, so the first branch never triggers; the grace check [ "$(pending)" -eq 0 ] likewise errors and hits continue. The loop runs to its hardcoded timeout without ever registering that CI turned green.

Session-log evidence — run 24686601620 (review of PR #5816) spent 46 minutes bouncing between two polling attempts:

[iter 8] pending=-1
[iter 9] pending=-1
...
[iter 20] pending=-1
CI still running after 20 minutes

Why this wasn't caught

Past review-runs entries misattributed the 15-min/25-min review wall-clock to "CI waiting for tests to finish" (tracking issue #5787 entries for 2026-04-19 and 2026-04-20). Re-reading the raw session logs confirms the same jq: parse error was present in reviews for PRs #5810, #5811, and #5813 — 5+ occurrences over three days, all structurally the same.

Why this is safe

  • CARGO_TERM_COLOR=always (kept) already forces cargo to emit coloured output — that's what most of the tend sessions actually care about.
  • Other tools like grep / ls will fall back to the unix-convention behaviour (colour when tty, plain when piped), which is what the rest of the repo's CI already does.
  • tests.yaml and the language-binding test workflows still set CLICOLOR_FORCE=1 at their own workflow level — untouched. Only tend's bot sessions lose the forced colour.

After this lands

  • The next tend-review run will use the standard gh pr view --json ... | jq polling loop and actually see pending=0 when checks settle, exiting within the real CI duration instead of padding to the full 15-min cap.
  • Follow-up: the tracking issue's 2026-04-19 / 2026-04-20 "15-min polling" findings can be marked resolved once the next review of a bot PR reports sub-5-min wall clock on CI-green.

Test plan

  • Merge and observe the next tend-review run triggered by a subsequent bot PR; verify the review job exits within ~2 min of CI settling rather than hitting the 15-min timeout.
  • Spot-check the session log from that run — confirm no jq: parse error: Invalid numeric literal in the pending() tool output.

The var forces `gh` to emit ANSI escapes in its --json output, which
breaks every `gh pr view --json X | jq` pipeline used by the bundled
`tend-ci-runner:running-in-ci` polling loop. CARGO_TERM_COLOR=always
already covers the cargo case; the rest of the tools can use the
unix-convention tty detection.

Session evidence: review of PR #5816 spent 46 min bouncing between two
polling attempts that could never exit because `pending()` kept parsing
ANSI-prefixed JSON and returning empty/-1.
@max-sixty max-sixty merged commit 9d209fb into main Apr 21, 2026
39 of 40 checks passed
@max-sixty max-sixty deleted the daily/review-runs-24711995590 branch April 21, 2026 14:44
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