internal/util: exit codes and the output.Writer - #51
Merged
Merged
Conversation
All user-facing output now goes through output.Writer — PrettyWriter for a human, JSONWriter for a machine — and log/slog is a debug-only channel on stderr that says nothing at all on a normal run. Settling the boundary here means no later package has to decide it again. PrettyWriter wraps each stream once in a colorprofile.Writer, so a run piped into a CI log gets the same text with the escapes stripped rather than a wall of \x1b[. The environment is a constructor argument, not an ambient read, which is what makes the golden files identical on a terminal and in CI. IsTTY covers the decisions colour degradation cannot make for us: the \r countdown and the prune prompt. JSONWriter emits one complete object per line. An array could only be closed once the last row was known, and a run killed halfway would leave a truncated document behind. Exit codes are constants with the reasoning attached, and exit_test.go spells each number out literally so a renumbering fails there rather than in someone's pipeline.
This was referenced Aug 7, 2026
Four decisions the original boundary left implicit, taken now because all four are free today and expensive later: Info has no callers, the exit numbers have not shipped, and IsTTY has no call sites. Table is now the only Writer method on stdout. Progress is narration, not the product, and it must not land in the file when a user redirects stdout. In JSON mode the cost was concrete: an info object carries none of the keys a data object does, so jq -r .group yielded null for it. Every line on stdout is now a data record, which the tests assert directly rather than by inspection. This diverges from specs-cli, where commands already depend on Info being on stdout; inheriting the defect to stay symmetrical was the wrong trade. The outcome codes are disjoint bits that OR together, so a dry run that finds drift and also cannot reach a repository exits 6 instead of forcing a precedence rule that throws half the answer away. Skipped moves from 3 to 4 to make room. Error stays outside the bit space and stays exclusive: when a run fails the live state is unknown, so "failed and drifted" is not a statement labelsync can honestly make — and with Error in the mask, 3 would have to mean exactly that. The cost is that callers test bits rather than equality, which is written down next to the table. exit.Err carries a code out to main, because RunE returns an error and nothing else. A nil Err field means silent: exit 2 on a drifting dry run must not print an error line, because the drift was the successful result and the diff is already on stdout. Unwrap is load-bearing — a carrier that hid the error it wraps would strip error_kind from exactly the failures that carry a code — and Of maps a carrier holding a failure to Error even when Code was left unset, since a zero exit on a failed run is the one wrong answer a pipeline cannot detect. IsTTY takes any rather than an io.Writer. The prune guard has to ask about stdin, and the old signature let IsTTY(os.Stdin) compile only because *os.File happens to have a Write method. The two gated decisions ask about different streams: the countdown about stderr, where it draws, and the prompt about stdin, because the hang it prevents is a read with nobody to answer it. The architecture page absorbs the reasoning that produced all of this, and gains the Cobra wiring and review checklist that #14 has to follow.
Ilyes512
force-pushed
the
feat/GH-13-exit-codes-output-writer
branch
from
August 8, 2026 12:41
dd9d394 to
233e007
Compare
Table(headers []string, rows [][]string) made two things impossible. A row
could disagree with its headers — ragged, or reordered — and nothing caught it;
JSONWriter.Table had to defend against short rows on every call. And every JSON
value had to be a string, because the pretty renderer needed one, so a count
came out as "12" and no consumer could filter on it numerically.
Commands now call output.Table(w, rows, cols...) with the rows they already
have. The two audiences get different projections of the same value: the table
comes from the columns, the JSON from marshalling the row itself. Keys and
types are the struct's own json tags, so {"repositories":12} is a number, and
those tags are a public contract in the same way error_kind is. It also
decouples formatting from the record — a size can be an int64 in JSON and
"1.2 MiB" in the table, where before it had to be one or the other.
Columns are explicit descriptors rather than struct tags, because a column
routinely needs something a tag cannot express: a formatted value, or one
computed from no field at all. Col() exists so type inference names the row
type once instead of once per column.
Table is a generic function rather than a method because Go does not allow type
parameters on methods. It prepares a TableData and hands that to the interface
method, now WriteTable — and going through the constructor is what makes cells
aligned with headers a property of the type rather than a hope.
JSONKey goes with the old signature. It existed to normalise a heading into a
key because the two audiences shared one set of strings; they no longer do, so
there is nothing left to normalise and one fewer contract to reason about.
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.
Closes #13.
What
internal/util/exitandinternal/util/output. Everything a user is meant to read goes throughoutput.Writer; everything only a maintainer is meant to read goes throughlog/slog, which issilent on a normal run. The boundary is settled here so no later package has to decide it again.
Scope
exit: the codes —OK/Error/Drift/Skippedas named constants, each with a doccomment explaining what it lets a caller infer, plus
exit.Errandexit.Ofto get one outof a command. The outcome codes are disjoint bits that combine; see
Outcomes combine below.
exit_test.gospells thenumbers out literally, so a renumbering fails there rather than in a pipeline.
output.Writerinterface, plusprettyandjson—PrettyWriter(lipgloss) andJSONWriter(NDJSON), both taking their streams as constructor arguments.WriteTableis theonly method on stdout; progress, warnings, and errors all go to stderr, so
--output=json | jqsurvives a mid-run failure and never sees a line it cannot type.
WriteErraddserror_kindfromlabelsync.KindOf. Tables take typed rows — seeTable takes typed rows below.
and the list commands want different shapes:
RenderColumns(aligned, no header, no border —the diff) and
RenderTable(bordered, with a header row). Widths are measured in displaywidth, not bytes. Each stream is wrapped once in a
colorprofile.Writer, so styling degradesto plain text off a terminal instead of dumping escape sequences into a CI log.
IsTTYcoversthe decisions colour degradation can't make for us: the
\rcountdown and the prune prompt.slogwired to stderr, enabled only by--debug—SetupLoggerreturns theLevelVarrather than baking the level in, since Cobra parses persistent flags after the tree is built.
The default is
LevelSilent, above every levelslogdefines, so a normal run emits nothing.--debug --output=jsongives a JSON stderr stream.errcheckexclusion — in the package doc, with the reason:Writerhas noerror channel, and reporting a failed write would itself need a working stream.
.golangci.ymlis unchanged.
Decisions taken during review
Writing the architecture page surfaced five things the original scope had left implicit. Every one
is cheap now and expensive later —
Infohas no callers,Tablehas no non-test callers, the exitnumbers have not shipped, and
IsTTYhas no call sites — so they were taken here rather thandeferred. Each is a ticked box on #13 marked added during review.
The table is the only thing on stdout
Inforeports progress, and progress is not the product: it must not land in the file when a userredirects stdout. In JSON mode the cost was concrete — an info object carries none of the keys a
data object does, so this stream is valid NDJSON on which
jq -r .groupyieldsnull:{"level":"info","message":"resolving 3 groups"} {"group":"websites","repositories":12,"source":"org: specsnl"}Now stdout is uniformly typed and stderr carries every line with a
level.TestJSONWriter_StdoutIsUniformlyTypedasserts that directly: every stdout line has the data keyand no
level.This diverges from specs-cli, where commands already depend on
Infobeing on stdout. Nothing herecalled
Infoyet, so there was no contract to break, and inheriting the defect to stay symmetricalis the wrong trade. If a command ever needs a result line that is not a table, it gets a new
product-level method rather than
Infomoving back.Table takes typed rows
Table(headers []string, rows [][]string)made two things impossible. A row could disagree with itsheaders — ragged, or reordered — with nothing to catch it;
JSONWriter.Tablehad to defend againstshort rows on every call. And every JSON value had to be a string, because the pretty renderer
needed one, so a count came out as
"12"and no consumer could filter on it numerically.The two audiences now get different projections of the same value: the table from the columns, the
JSON from marshalling the row itself. Keys and types are the struct's own, so the stdout golden went
from
"repositories":"12"to"repositories":12, and thosejsontags are a public contract inthe same way
error_kindis. It also decouples formatting from the record —cache info(#42) canhave an
int64of bytes in JSON and1.2 MiBin the table, where before it had to pick one.Columns are explicit descriptors rather than struct tags because a column routinely needs something
a tag cannot express: a formatted value, or one computed from no field at all.
Col()exists sotype inference names the row type once instead of once per column.
Tableis a generic function rather than a method because Go does not allow type parameters onmethods. It prepares a
TableDataand hands that to the interface method, nowWriteTable— andgoing through the constructor is what makes cells-aligned-with-headers a property of the type rather
than a hope.
JSONKeyis deleted along with the old signature. It existed to normalise a heading into a keybecause the two audiences shared one set of strings; they no longer do, so there is nothing left to
normalise and one fewer contract to reason about.
Outcomes combine; failure does not
A single run can be two things at once: a
--dry-runthat finds pending actions and cannot reacha repository is both drift and skipped. Rather than rank them, the outcome codes are disjoint bits
that OR together, so that run exits
6.exit.OK0exit.Error1exit.Drift21exit.Skipped42Skippedrenumbers3→4. Free today — nothing has shipped and no pipeline branches onit — and a breaking change after the first release.
Erroris deliberately outside the bit space and stays exclusive: when a run fails the live stateis unknown, so "failed and drifted" is not a statement labelsync can honestly make. It is also
what keeps every combination meaningful, since with
Errorin the mask3would have to meanexactly that. The cost — callers test bits rather than equality — is named in the docs with a shell
example.
if labelsync sync; thenis unaffected.exit.Errcarries a code tomainRunEreturns anerrorand nothing else, so there was previously no way for a dry run to reportdrift. A nil
Errfield means silent: exit2must not also print an error line, because the driftwas the successful result and the diff is already on stdout.
Two guards that are load-bearing rather than decoration, both tested:
Unwrapkeeps the wrapped sentinel visible, or the carrier would striperror_kindfrom exactlythe failures that carry a code.
Ofmaps a carrier holding a failure toErroreven whenCodewas left unset. A zero exit on afailed run is the one wrong answer a pipeline cannot detect.
This is pure
internal/util/exitwith no Cobra dependency, so it lands here and unblocks #14 ratherthan the reverse.
IsTTYtakesanyThe prune guard in #44 has to ask about stdin. The
io.Writersignature letIsTTY(os.Stdin)compile only because
*os.Filehappens to have aWritemethod, and read as though stdin were outof scope. It now asserts for
Fd() uintptr, and the test covers a stdin-shaped value with noWritemethod at all — which would not have compiled before.The two gated decisions ask about different streams, which is now written down: the countdown about
stderr, where it draws; the prompt about stdin, because the hang it prevents is a read with nobody
to answer it. A job with a terminal on stderr and its stdin closed must still refuse to prompt.
Tests
Golden files for both renderings, under
internal/util/output/testdata/, regenerated withtask dc:run:go-builder -- go test ./internal/util/output/ -update.The pretty rendering has two goldens per stream — one with an empty environment where every
escape is stripped, one with
CLICOLOR_FORCE=1where they are not. The pair is the actual claim:that the plain rendering is colour degrading cleanly, rather than styles never having been applied.
Detection takes the environment as an argument rather than reading it ambiently, so the goldens are
the same on a terminal and in CI.
The JSON tests assert the NDJSON property directly: every line of both streams parses on its own as
a complete object, and
Tableemits one object per row rather than an array. An array could only beclosed once the last row was known, and a run killed halfway would leave a truncated document.
From the review decisions:
TestWriters_OnlyTableReachesStdout— table-driven over both implementations, covering all fournarration methods. It replaces the old warn/error-only assertion.
TestJSONWriter_StdoutIsUniformlyTyped— the claimInfo-on-stderr actually makes.TestJSONWriter_TableKeepsValueTypesandTestTable_CellsAlignWithHeaders— the two defects thetyped API removes.
TestJSONWriter_TableShortRowis gone: a short row is now unrepresentable, sothe test that asserted graceful degradation had nothing left to assert.
TestTable_ComputedColumn— a column with no backing field, and the record passing throughuntouched by the rendering.
TestOutcomeCodes_AreDisjointSingleBits— replaces the distinctness test. Distinct is no longerenough: two outcomes sharing a bit would make
6ambiguous, and an outcome overlappingErrorwould show up in a mask for failure.
TestCodes_Combine,TestOf,TestErr_UnwrapsToTheSentinel,TestErr_Error— the carrier,including the unset-
Code-with-a-failure case.Of the seven goldens, six changed only by the info line moving stream;
json_stdoutalso gainedreal numbers and struct field order in place of alphabetical map order.
Docs
Output & Exit Codes is now the single place this
boundary is written down, linked from the architecture index,
AGENTS.md, and the overview. Beyondthe four decisions above it gained the reasoning that produced them: the one rule (stdout is the
product, stderr is the story of making it), a channel-choosing table, the Cobra wiring #14 has to
follow — command accessors over
os.*,SilenceUsage/SilenceErrors, the singleos.Exitinmain— a review checklist, and the specs-cli defects each rule exists to prevent.docs/design.md§ Exit codes has the new numbering and the combining rule. § Output already pointsat the architecture page as landed.
Consequences recorded on the issues they land in: #14 (the
mainhandler and theSilence*wiring), #41 (the exit-code table, and the first place a run can be both drift and skipped), #37
(the countdown gates on stderr, where it draws — the scope box said stdout), #44 (the prune
guard gates on stdin), #39 and #42 (the two
Tablecall sites, with the typed-row shape each onewants).
Not done
No README change. Nothing in this PR is reachable by a user yet — there is no command tree to
run it from. The exit codes and the
--outputcontract belong in the README next to the commandsthat produce them, which is #14.
Checks
task lint,task test, andtask md:checkall pass.