fix(io): emit valid GitHub log groups, and only on GitHub - #113
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Three problems with CI log grouping, all of which put output where a reader cannot see it. Workflow commands were gated on isCI, which is true for any provider. ::group:: is GitHub syntax, so every other CI printed literal "::group::name" lines instead of a header. They are now gated on GITHUB_ACTIONS, and everything else gets the styled header that local runs already got. isCI had no other callers, so it is removed rather than left dead. Nested groups are not supported by GitHub: a second ::group:: before the first closes swallows the rest of the log. Depth is now tracked so only the outermost pair is emitted, and a nested caller degrades to no group rather than a broken one. An unmatched EndGroup emitted a stray ::endgroup::, which closes whatever GitHub had open around it. It is now a no-op, so a caller can skip BeginGroup conditionally without mirroring that condition on the way out — which is what lets flow suppress the group for single-task runs, where wrapping the only output in a collapsed group hides it for no benefit. Group names are escaped per GitHub's spec; an unescaped newline previously ended the command early and leaked the remainder as ordinary log lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R328pa3FUUfga4gYah1iQi
jahvon
force-pushed
the
fix/ci-log-grouping
branch
from
August 27, 2026 04:57
ac80913 to
2bcef05
Compare
jahvon
added a commit
to flowexec/action
that referenced
this pull request
Aug 27, 2026
Companion to flowexec/tuikit#113 and flowexec/flow#448, which fix the log grouping this works around. # Summary When a flow executable failed, the reason was hard or impossible to find from the run page. Three changes: **1. stderr was thrown away.** `execute.sh` captured it to a temp file, mined it for a JSON error code, and `rm -f`'d it without ever printing it. Anything flow wrote to stderr was lost outright — most importantly the structured error envelope under `--output json`, whose message never reached the log. It is now echoed through. **2. The annotation reported only an exit code.** It now carries the error code and message when flow emits a structured envelope: ``` ::error::flow test unit failed with exit code 1 (EXECUTION_FAILED): unable to parse file - 14:1 ``` Newlines are folded to spaces, since a literal newline ends a workflow command early. **3. A step summary is written for both outcomes.** > ### ❌ `flow test unit` > > Exit code `1` · `EXECUTION_FAILED` > ``` > unable to parse file - 14:1 > ``` # Why this matters flow wraps a serial/parallel run's output in a GitHub log group, and groups are **always collapsed** — GitHub offers no way to default one open. So the actual output sits one click away, and on failure there was nothing outside the group saying what went wrong. Annotations and the step summary render on the run page itself, where a collapsed group does not. flowexec/tuikit#113 and flowexec/flow#448 stop burying single-task output in a group in the first place. These changes are complementary: they make failures visible regardless of how the output is grouped. # Testing The action has no CI workflows to add cases to, so I exercised `execute.sh` directly against a stand-in `flow` binary, checking `GITHUB_OUTPUT` and `GITHUB_STEP_SUMMARY`: | Case | Result | |---|---| | Success | Clean ✅ summary, no annotation, exit 0 | | Failure with JSON envelope | stderr printed; annotation carries code + message; summary shows both; `error-code` output still set | | Failure with plain multi-line stderr | stderr printed (**previously lost entirely**); annotation stays single-line; summary shows exit code | `bash -n scripts/execute.sh` is clean. The `capture`/`upload` path and the `continue-on-error` path are unchanged. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
jahvon
added a commit
to flowexec/flow
that referenced
this pull request
Aug 27, 2026
Companion to flowexec/tuikit#113 and flowexec/action#3. # Summary A serial or parallel executable wrapped its whole run in a GitHub log group named after the ref. GitHub log groups are **always collapsed**, and there is no way to default one open — so a single-task run hid the only output worth reading behind a click, gaining no structure in exchange. From a real CI run of `test unit`: ``` ##[endgroup] Executing: flow test unit --param CI=true ##[group]test flow/unit ← everything below is collapsed ok github.com/flowexec/flow/v2/cmd/internal 1.271s ... ``` `test unit` is a serial executable with one step, so the group had exactly one child. # Change Emit the group only when there is more than one task: ```go groupOutput := parentTask == nil && len(execs) > 1 ``` `EndGroup` is guarded by the same condition rather than relying on the logger to ignore an unmatched call, so this is correct against the currently pinned tuikit (v0.4.1) as well as the version that hardens it. # Verified Built and run with `CI=true GITHUB_ACTIONS=true`: | Case | Before | After | |---|---|---| | `flow test unit` (1 step) | `::group::test flow/unit` — all output collapsed | no group, output visible | | serial, 2 steps | grouped | `::group::run flow/many` … `::endgroup::` — unchanged | | parallel, 1 step | grouped | no group | | parallel, 2+ steps | grouped | unchanged | `flow validate` passes. # Related - flowexec/tuikit#113 — stops emitting GitHub-only `::group::` syntax on other CI providers, prevents invalid nested groups, and makes an unmatched `EndGroup` a no-op. Independent of this PR, but it is what makes the guard above belt-and-braces rather than load-bearing. - flowexec/action#3 — prints the stderr the action was discarding, and adds a step summary plus a real error message in the failure annotation, so a failure is visible on the run page without expanding any log at all. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01R328pa3FUUfga4gYah1iQi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of making flow's CI logs readable — see flowexec/flow#448 and flowexec/action#3.
Summary
Three problems with CI log grouping, all of which put output where a reader cannot see it.
1. Workflow commands were emitted on every CI, not just GitHub.
isCI()is true whenCI=true, so GitLab, CircleCI, Buildkite and friends printed literal::group::build applines instead of a header.::group::is GitHub syntax, not a CI convention — it is now gated onGITHUB_ACTIONS, and every other provider gets the styled header that local runs already got.isCI()had no other callers, so it is removed rather than left dead.2. Nested groups produced invalid output. GitHub does not support nesting: a second
::group::before the first closes swallows the rest of the log. Depth is now tracked so only the outermost pair is emitted, and a nested caller degrades to no group rather than a broken one.3. An unmatched
EndGroupemitted a stray::endgroup::, which closes whatever GitHub had open around it. It is now a no-op.Group names are also escaped per GitHub's spec — an unescaped newline previously ended the command early and leaked the remainder as ordinary log lines.
Why now
flow wraps every serial/parallel executable's output in a group named after the ref. For a single-task run that means the entire output is collapsed behind one click for no structural benefit —
flow test unitshows nothing but a group header until you expand it.The fix on flow's side is to skip the group when there is only one child task. Point 3 is what makes that safe: the caller can drop
BeginGroupwithout having to mirror the condition aroundEndGroup.Notable Changes
isCI()is replaced byisGitHubActions(); it had no remaining callers.groupDepthis guarded by its own mutex, consistent with the existingmodeMu/writeMusplit.Testing
Five cases in
io/ci_group_test.go, covering each behavior above: the GitHub path, the non-GitHub CI path, nesting, the unmatchedEndGroup, and escaping of%and newlines.go test -race ./...andgolangci-lint run ./...both pass locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01R328pa3FUUfga4gYah1iQi