Repository navigation
Conversation
stdout now carries only command results: the engine and skill-freshness
banners, all spinners, warnings, and the reporters' live run progress
("Running N checks…", live summary redraws, streamed logs) move to
stderr. This makes today's JSON surfaces trustworthy — a piped
`checkly --output json | jq` now always parses, because nothing prints
to stdout around the JSON document. Interactive terminal output is
unchanged: both streams render in a terminal, so humans see the same
banners, spinner animations, and live summaries as before.
Details:
- CommandStyle: ux.action spinners render on oclif's default stderr
stream (drop `{ stdout: true }`); the blank separator after a finished
spinner follows it to stderr; longWarning/shortWarning go to stderr
(covers the engine + skill-freshness banners in BaseCommand and the
deploy/import/pw-test warnings).
- Reporters: printLnErr for "Running N checks" announcements, live
summary redraws (clear codes now target the stream the summary
renders on, keyed off stderr.isTTY), and streamed check logs. Check
results, retry/failure details, final summaries, and session links
stay on stdout.
- sync-playwright: same spinner stream fix.
- Document the output contract (streams, envelopes, exit-code table)
in docs/output-contract.md, AGENTS.md, and the published skill.
- Tests: stream-split coverage in command-style, abstract-list, and
dot specs; login spinner expectations updated.
Resolve the one conflict in AGENTS.md by keeping both additions: this branch's "Output Streams" section and main's new "Construct Conventions" section.
hlenke
marked this pull request as draft
October 16, 2026 21:16
hlenke
marked this pull request as ready for review
October 16, 2026 21:25
Construct-diagnostic warnings ("Check has no location", deprecated
constructs) now print to stderr, so a successful deploy no longer has
an empty stderr. Replace the six `expect(stderr).toBe('')` assertions
with a guard that fits the output contract: stderr may carry warnings,
but never an error report (no ✖/×-led line, stripped of ANSI codes).
hlenke
marked this pull request as draft
October 16, 2026 21:25
A final summary printed to stdout (onEnd, onCancel, onDetach) still set _clearString whenever stderr was a TTY. With stdout piped and stderr on a terminal, a Ctrl-C run then wrote those erase codes to the TTY against lines that were never drawn there, wiping unrelated terminal content. Erase codes now pair exclusively with live summaries on stderr, the cancel redraw joins the live loop on stderr (the cancelled result summary still arrives on stdout via onEnd), and the brief summary no longer registers codes nothing ever clears. The command-style spec's ux.action spies get mock implementations so no real spinner writes during tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hlenke
marked this pull request as ready for review
October 17, 2026 00:46
This branch has not been deployed
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
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.
What this does, in plain words
A terminal has two output pipes: stdout (the "data" pipe) and stderr (the "chatter" pipe). A person watching a terminal sees both, interleaved, so moving text between them changes nothing for humans. But scripts, CI jobs, and AI agents usually read only stdout and try to parse it — and today the CLI prints banners, spinner text, and live run progress into stdout, right next to the JSON we tell them to parse.
This PR moves all of that chatter to stderr, so stdout now carries only the command's result. The one-line version:
checkly whoami --output json | jq .used to fail; now it always parses.What changed
longWarning/shortWarning(deploy, import, pw-test) now write to stderr. They were the worst offenders: they printed to stdout ahead of the JSON document on nearly every command.CommandStyleexplicitly asked oclif to draw spinners on stdout ({ stdout: true }); oclif's default is stderr, so the fix is dropping that option. The blank separator line after a finished spinner follows it to stderr.sync-playwright's direct spinner call got the same one-line fix.test/trigger/pw-test: the "Running N checks…" announcement, the live summary redraws, and streamed check logs now go to stderr (the redraw/erase codes now key offstderr.isTTY, the stream the summary actually renders on). Results stay on stdout: check results and failure details, the final tally, session IDs and links, report-file paths.docs/output-contract.md(streams, the JSON envelopes, and an exit-code table, including the known exit-2 overload documented as such), an "Output Streams" section inAGENTS.md, and the same section in the published skill, so agents know stdout is safely pipeable.eslintnow ignores.context/(agent scratch space), same as the existing.claude/ignore — without itpnpm lintfails in any workspace carrying scratch files.Why it's safe for humans
Both streams render in the same terminal, interleaved in real time, so the interactive experience is what it always was: the same banners, the same spinner animation, the same live summary at the bottom of
checkly test. The only people who notice a difference are the ones piping — and for them stdout gets cleaner:checkly test > log.txtnow captures a result document instead of mixing in progress noise.Evidence
Before/after, captured by packing the CLI at
mainand with this change, running both against the same demo project (stale skill installed, one passing + one deliberately failing check). Full screenshot set for every scenario (banners, spinners, reporter progress — TTY and piped) is in the linked workspace; the decisive captures:checkly whoami --output json | jq .checkly validate 1>stdout 2>stderrcheckly test 1>stdout 2>stderr(one passing, one failing check)Both runs exit 1 (the failing check), as before.
Tests
command-style.spec.ts(spinner + warnings on stderr, JSON errors stay on stdout),abstract-list.spec.ts(progress vs results split, clear-codes keyed to stderr TTY, streamed logs on stderr),dot.spec.ts(announcement on stderr, dots on stdout).login.spec.tsspinner expectations (no{ stdout: true }; the spinner's blank separator no longer appears on stdout).expect(stderr).toBe('')assertions in thedeploye2e specs. Those fixtures' checks have no locations, so construct-diagnostic warnings ("Check has no location", deprecated constructs) now land on stderr — exactly where the contract puts warnings. The assertions become a guard for the new contract: stderr may carry warnings, but never an error report (no ✖/×-led line after stripping ANSI).Notes / follow-ups
pull_requestworkflows on GitHub, so CI was started by briefly marking the PR ready while pushing (it is a draft again; the runs and their results stay on the PR).deploy -oboolean rename,env lsJSON, and the versioned envelope are deliberately not in this PR.