Typed Scala DSL for shell, expressions, steps, and step bundles - #58
Merged
Conversation
Replaces s-interpolated bash with Scala values. Layer 1 of the typed step
and shell DSL; see the new ROADMAP subsection for the other three layers.
Validation is structural via neotype newtypes, so an invalid value is
unconstructible and an invalid literal fails at compile time with the
validator's own message. Ten primitives cover what the shell and YAML
actually require: no single quote inside '...' (it cannot be escaped), no
'}' inside ${...} (it closes the expansion early), no leading tab on a
script line (block scalar indentation must be spaces), exit codes 0 to 255
(the shell truncates modulo 256), single-digit file descriptors only.
Three classes of bug move from render-time throw to unconstructible: a
ParamMod enum replaces mutually exclusive optional fields, Block makes an
empty if or loop body impossible, and a Quotable marker keeps single quotes
out of double quotes. GlobMatch takes a GlobPattern rather than a Word, so
a quoted pattern (which silently becomes literal comparison) cannot be
expressed.
Command is an open trait, not an enum: a consumer needing a construct zipx
does not model implements it in their own build. The accepted cost is that
a match over Command cannot be exhaustive, which is why rendering is a
method rather than a match in Script.
Raw holds List[ScriptLine], so the type guarantees raw content cannot emit
YAML GitHub fails to parse; Script.raw returns Either and names the
offending line. YamlPrinter.writeBlockScalar gains the same check for
hand-written strings, which still reach it.
sh"..." takes Word*, so a bare String splice does not compile.
Tests: 93 in shell (every newtype's accepting cases, rejecting cases and
boundaries, with generators where the rule is a character class, plus
typeCheck for the compile-time half), 36 in workflow. Dogfood regenerates
to a zero diff.
Layer 2 of the typed DSL: `${{ … }}` becomes a Scala value instead of an
interpolated string, and every Actions-syntax name becomes a neotype newtype
that validates a literal at compile time.
`Expr` is closed rather than open, unlike `zipx.shell.Command`: GitHub fixes
the context list, so a new case would be a new GitHub feature, and `Expr.Raw`
covers anything not yet modelled. `Lit` and `Raw` render bare while every other
case wraps itself, which is what lets `Concat` assemble `sbt-${{ runner.os }}`
without an interpolator.
Every rule in the newtypes is one GitHub documents, not a convenient subset:
- ids start with a letter or `_`; a digit-leading job id is a parse error
- secret names reject the reserved `GITHUB_` prefix case-insensitively, since
GitHub stores them uppercase and matches case-insensitively, so checking one
spelling would be bypassable; `GITHUB_TOKEN` itself is admitted because it is
injected rather than created
- env names reject the `GITHUB_` prefix outright and delegate to zipx-shell's
`Patterns.Ident`, which is the mechanical check that the two layers agree on
what a name is: an `env:` key becomes a shell variable in every `run:` step
- output names reject the disabled `set-output` / `save-state` commands
- matrix axes reject the `include` / `exclude` directives
- context paths allow `[n]` and `*` segments but not an empty one
- `uses:` refuses an unpinned `owner/repo`, which is what ActionPinFile exists
to prevent
- `RawExpr` requires balanced `${{ }}`, one line, and a bounded length
`EnvValue.requireName` and `JobCondition`'s `requireIdent` / `requireLiteral` /
`requireRaw` now delegate to those newtypes instead of restating the regexes, so
there is one definition of each rule and both files keep their public throwing
signatures. `EnvValueSpec` and `JobConditionSpec` pass unmodified.
The cross-layer coupling is two one-liners: `JobCondition.expr` lifts a
validated condition into a step field, and `Expr.asWord` embeds an expression in
a script as `Word.Opaque`, the one word kind the shell renderer never escapes.
Verified: 475 tests green across shell, workflow, core, central and docs;
`plugin/scripted zipx/*` passes; `sbt reload` clean; regeneration leaves
`.github/workflows/` byte-identical.
Layer 3 of the typed DSL. `Step` keeps its flat all-optional case class shape, which is fixed by `derives Schema` and the on-disk mapping, so no rendered byte moves. The good end is `StepBuilder`. `Step.run(script)` and `Step.uses(ref)` decide the mutually exclusive run/uses pair before any other field is set, so a builder cannot express a step with both or neither. Its fields are typed the way the layer above wants them: a `Script` for a body, an `Expr` for `if:` and `with:` values, `StepId` / `EnvName` for names, each validated at compile time for a literal. `rawFragments` is carried forward rather than dropped, so `zipxWorkflowGenerate` can later warn about escape-hatch content and name the step it came from. The closing end is `Step.validate`, called from both of `Render`'s encode paths: `encodeStep` for step-sequence fragments, and `encodeJob`, because the derived job codec encodes nested steps itself and would otherwise bypass the check. A hand-built step with both keys, neither key, or a `with:` on a `run:` step fails at generate time with the step's name or id in the message. Validation is not in the constructor because a `Step` is also a decode target: validating there would reject a value a codec is still filling in. `StepBuilderSpec` covers both ends plus the compile-time half via `typeCheck`: an invalid literal id or env name does not compile, a runtime id is pushed to `withStepId`, and `when` refuses a hand-written condition string. Verified: shell 93, workflow 130, core 220, central 17, docs 40 tests pass; `plugin/scripted zipx/*` passes; `sbt reload` plus `zipxWorkflowGenerate` leaves `.github/` with a zero diff.
Layer 4 of the typed DSL. `Steps` extends `(StepContext => List[Step])`,
so every field that took a bare lambda (`Capability.extraSteps` /
`postSteps`, `PlanConfig.cacheRehydrateExtraSteps`, the
`zipxCacheRehydrateExtraSteps` setting) accepts one with no signature
change, and `Planner.stepsFor` needs no edit at all. What the type adds
is what a lambda cannot have: a name that reaches diagnostics, `++` to
concatenate, `when(JobCondition)` to gate a whole bundle, and a stable
identity to publish, so an org's shared bundle is an ordinary published
Scala value.
Also adds `Expr.unwrapped`, because an `if:` is already an expression
context: `${{ a }} && ${{ b }}` is a template string that evaluates to
neither operand, where the bare form is the conjunction the caller
asked for. `StepBuilder.when` and `Steps.gate` render through it, which
is also what keeps the output matching the bare conditions the planner
has always emitted.
Raw escape-hatch use becomes visible: `Steps.built` collects its
builders' `rawFragments`, composition and `when` / `named` / `mapSteps`
preserve them, and `zipxWorkflowGenerate` logs one warning per fragment
naming the bundle. A bare lambda reports nothing, because it has
nowhere to carry the information.
`ZipxCentral`'s three step lambdas are the first consumers, and
`releaseOnce` composes two of them with `++` where it previously
threaded one by hand. The dogfood `git diff` after regeneration is
empty, which is the proof that migration moved no bytes.
The four DSL layers landed with no consumers. This moves the codebase onto
them and proves the generated YAML did not move a byte.
Scripts. ZipxCentral.gpgImportSteps, ActionPinsSyncWorkflow.commitScript,
CacheEpoch.gitTagsResolveScript, Planner.verifyGateScript / affectedScript and
verifyCommandStep are Script values rather than s-interpolated bash. The
doubled-$$ warning comment in ZipxCentral is gone (a Word.Subst renders itself),
and so is affectedScript's .replace("\n\n", "\n") hack, which existed only to
undo interpolation's blank lines.
Expressions. Expr.Call over a FunctionName newtype gives the planner's gates a
constructor: contains(fromJson(...)), !cancelled(), startsWith(github.ref, ...)
and the four copies of the release-tag prefix are built now, not assembled. The
cache-key builders use Expr.concat. Supporting AST work: Continued for backslash
continuations, Exec.of, multi-line Command.render, Word.Subst over render,
Expr.asWord narrowed to Word.Opaque.
Failures are types, not thrown exceptions. Removed at each site in the strongest
way available: unrepresentable where possible (JobCondition.All takes a head and
a tail, so no empty conjunction exists to reject; PlanConfig.verifyCleanLabel is
an ExprLiteral, so a quote that would break out of '...' cannot reach a plan;
InlineCommand is the subtype whose inlineRender is total), else a compile-time
check on a literal via an inline constructor, else Either naming the offending
value (Script.raw, Step.validate, Cron, ActionPinsSyncWorkflow.plan, the *Make
siblings). EnvValue.requireName and JobCondition's requireIdent / requireLiteral
/ requireRaw are deleted rather than delegating, so each rule has one definition
in its newtype. Throwing is confined to ZipxPlugin.orFail, the sbt boundary,
where sbt's own contract is to throw.
Byte parity, three ways: zipxWorkflowGenerate leaves git diff empty across all
three workflows; plugin/scripted zipx/generate-check passes, an independent check
since its assertGraph asserts the literal gate strings Expr.Call now builds; and
PlannerSpec / RenderSpec / the docs pages pass unmodified. Zero-diff only proves
nothing moved, not which script is which, so ScriptRenderSpec pins each migrated
script against the exact string its pre-migration source produced.
examples/monorepo was compiled against a publishLocal build. Its
project/Deploy.scala holds validated EnvValues rather than secret-name Strings,
which is what keeps the compile-time check available to a typed target list.
The four typed layers now have a page that compiles: `Shell and steps` walks the shell AST, `Expr`, step builders and `Steps` bundles, and every example on it is compiled and its rendered output asserted like the rest of the site. That includes the examples that demonstrate a failure, which are the ones a prose doc gets wrong first: a hand-built invalid `Step` is `Left`, a raw fragment produces a warning naming its bundle, and a bare lambda produces none. Registered after `Custom capabilities`, with the `summaryMarkdown` guide chain updated, since that chain enumerates page order as prose and drifts silently otherwise. `autoImport` gains the rest of the DSL, with a `type` alias wherever a build might annotate one: shell structure (`ShTest`, `Block`, `If`, `ForIn`, `While`, `Assign`), the names those constructors take (`VarName`, `GlobPattern`), and the escape hatches (`Raw`, `RawLine`). So a `build.sbt` writes the whole DSL with no imports. `zipx.shell.Command` and `InlineCommand` stay out on purpose: `Command` is sbt's own name in a build file (`commands += Command.command(...)`), and shadowing it there would break an unrelated line. A build implementing its own shell construct imports it explicitly, which is in `project/*.scala` anyway. `Exec` is the same hazard already accepted: it shadows `sbt.Exec`, which a build file does not name. The two docs pages that still described `extraSteps` as a lambda now describe the bundle, and Verify's asserted example uses `Steps.built` so the snippet above it and the YAML below it agree. ROADMAP: the DSL subsection is done. The "Extension language" decision now covers construction down to the shell, and states the escape-hatch policy (typed so it cannot break the YAML, `Either` where the text could, warned and named at generate time). Fifth design guardrail: no stringly-typed construction in the public API. Deviation recorded: `zipx-shell` is a general shell AST rather than a GHA-specific one, with `Word.Opaque` as the only seam between the layers. Generated YAML unchanged: dogfood regenerate leaves `git diff` empty.
Every existing spec tests the shell AST against scripts zipx wrote, which is a weak test of a DSL meant for other people: the same person chose the AST cases and wrote the scripts, so of course they fit. `ConsumerStepsSpec` rebuilds `early-effect/chekhov`'s browser-setup steps instead, the build that motivated issue #46, written long before this module existed. They turned out to be the useful kind of awkward: an `if` whose condition is a glob test on a command's exit status, `$(…)` substitutions nested inside double quotes, a `ls -1 … | wc -l | tr -d ' '` pipeline inside one of them, `sudo` fronting another program, a `${HOME}`-relative path assembled from parts, and a cache key mixing `runner.os` with `hashFiles`. The expected strings are chekhov's current `run:` bodies verbatim, so this asserts two things at once: the DSL can express a real consumer's steps, and it emits the same bytes, which makes adoption a refactor rather than a workflow diff to re-review. The spec also covers what a lambda could not promise: every step validates, the bundle reports no escape-hatch use, and the one bundle assigned to both `extraSteps` and `cacheRehydrateExtraSteps` yields identical steps, which is the requirement #46 actually had. One rough edge found and recorded rather than smoothed over: `ActionPins` fields are `String`, so `ctx.actions.cache` cannot reach `Step.uses`'s compile-time check and a build has to use `usesMake` and handle an `Either`. Typing those fields as `ActionRef` would remove that from every consumer referencing a pin; worth doing when the pin file's decode path is next touched. Two DSL calls also needed the braced form (`"${apt_mirror}"`, not `"$apt_mirror"`) to match, which is `Word.vBraced` earning its place: the two spellings are only accidentally equivalent, and chekhov wrote the explicit one.
Every file in this PR had accumulated prose that restated what the code already said. Delete it, and where a comment was carrying the meaning, restructure so the code carries it instead: named constants, named intermediate vals, better identifiers, more descriptive test names. Tests lose every comment about the test itself; a test name says what a comment above the assertion used to. The two comments that survive in tests both describe something outside the test: chekhov's build.sbt as the source of expected values, and why PrimitivesSpec names its control characters. Main sources keep only what is genuinely external and non-derivable: Scala Steward's repo-config default-path behaviour, that its action reads config from the runner filesystem, the sbt-remote-cache POM re-listing sbt as a compile dependency, and the shell rule that a single quote cannot be escaped inside single quotes. Docs-page snippets are untouched: their comment lines are published content, not code comments. No behaviour change. 587 tests pass across shell, workflow, core, central and docs; zipxWorkflowGenerate leaves .github/ byte-identical; plugin/scripted zipx/generate-check passes.
ActionPins.field took a String and threw on anything outside its seven legal names, while both call sites already iterated a hand-maintained list of those same names. The string parameter was the defect: an ActionPins.Field enum carries the pin-file key and the uses: prefix per case, so field, withField and version are total and the list cannot drift from the case class. ActionPinFile now folds over Field.values instead of FieldPrefixes, which also collapses the three places that spelled out all seven fields (render, pullFromWorkflow, fromMaps). Field declaration order is the committed pin file's line order, and a new test asserts that rather than leaving it as a comment. loadResource returns Option instead of throwing an IllegalStateException that ActionPins.Defaults immediately caught as control flow. .github/ is byte-identical; 270 core tests pass.
Expr.Lit held an unvalidated String and Expr.lit was not inline, so
Expr.lit("a\nb") compiled and produced a Word.Opaque carrying a newline,
which YAML would quote-escape and collapse the whole run: script onto one
line. Lit now holds a ShText and lit gets the inline / litMake pairing
every other constructor in the file already has.
That makes Expr.render provably ShText-valid, so asWord goes through a new
renderShText with one unsafeMake instead of ShText.makeOrThrow. RawExpr
gains the control-character rule it was missing, which is the last case
whose text was otherwise unconstrained.
EnvValue.Plain is deliberately not an Expr.Lit: an env value may be a
multi-line PEM or JSON blob, which GitHub reads as a block scalar. So
asExpr becomes an Option and says which cases are expressions.
Script.Ctx.line took a String, split it on newlines and revalidated each
piece with ScriptLine.makeOrThrow. That throw was reachable from every
Command implementation: any caller interpolating text into ctx.line got a
runtime failure for something the types could have refused.
Introduce ShLines, a non-empty sequence of already-validated ScriptLines,
and make it the currency of the whole module. Word.lines, ShTest.lines and
InlineCommand.inlineLines return one; render and inlineRender become final
derived methods over it, leaving String at exactly one place, the
serialization boundary. Nothing splits text on newlines any more, so
nothing revalidates the pieces.
Non-emptiness is structural rather than validated: ShLines wraps
NonEmptyChunk, whose map, prepend and append return one, so every
operation here is total with no assertion to make. Command.lines stays a
possibly-empty List, because a fully disabled SetOpts legitimately emits
nothing where a word or a test cannot.
Concatenation joins onto the left unit's last physical line, which is
where a pipe or a redirect attaches; Continued(...) | wc -l therefore puts
the pipe after the final continuation rather than after the first line.
Joining two ScriptLines needs no recheck because ScriptLine's rules (no
newline, no carriage return, no control characters, no leading tab) are
closed under concatenation. Making that closure usable required ShText to
carry the leading-tab rule, which it does not need for its own sake but
which makes ShText a subset of ScriptLine and the conversion total.
SquoteText and ParamText stay as they were: they render behind a quote or
a ${, so their first character never begins a line.
ctx.line now takes a literal, checked while the calling file compiles, so
interpolating into it is a compile error. The extensibility test and the
docs page both demonstrate the composition that replaces it.
Byte-identical output, proven three ways: ScriptRenderSpec, the dogfood
zipxWorkflowGenerate zero-diff, and plugin/scripted zipx/generate-check.
`topologicalSort`, `topologicalLayers` and `subsetLayers` each threw `CyclicGraphError` on a cyclic graph, so four public methods carried the same failure and no caller could see it in a type. Validate once instead. `ModuleGraph.make` runs the toposort, reports a cycle as `Left`, and hands the layers it computed to a private constructor, so `topologicalLayers` is a field rather than a fallible computation and every ordering query below it is total. `CyclicGraphError` is deleted; the sbt plugin reports through the `orFail` boundary it already had. Two companions keep the boundary honest. `cycle` returns just the ids involved, for `Planner.validateCapabilities`, which is ordering capabilities rather than modules and has to word the error in those terms. `apply` throws where `make` reports, documented as the fixture constructor: a cycle in a hand-written test graph is a test bug, not user input. A private constructor privatises the synthesized `copy`, which turned out to be the right pressure. All twelve `copy(nodes = …)` sites only flip attribute flags, never edges, so they become `mapNodes`: it takes `id` and `dependsOn` from the original node, which is what makes it total and lets it reuse the layers already computed. Changing edges still requires `make`.
sbt's project-id rule is `Character.isLetter` then `isLetterOrDigit || '-'
|| '_'`, so `café` and `プロジェクト` are legal sbt projects. A GitHub
`jobs.<job_id>` key is ASCII, so such a module produced a workflow GitHub
rejects on push, and zipx only noticed halfway through planning, where it
threw. `ModuleId` moves that check to the boundary.
A `neotype.Subtype[String]`, not a `Newtype`: `ModuleId <: String`, so
reading an id needs no unwrapping. `_.id == "service"` in a `build.sbt`,
`s"${node.id}/test"` in a command, and `Map[String, ModuleNode]` all keep
compiling. Only construction is checked, and the only construction from
user input is `ZipxPlugin.buildGraph`, which reports through `orFail`
before anything is written.
The payoff is two of `Planner.orThrow`'s four callers:
- `affectedContains` took a `String` and validated it per call. It now
takes an `ExprLiteral`, reached via `ModuleId.asExprLiteral`, which is
total because `Names.ActionsId`'s character set is a strict subset of
`Names.ExprLiteral`'s in both the first and subsequent positions.
`ModuleIdSpec` checks that over an alphabet rather than asserting it,
and checks the converse fails, since that asymmetry is why the
conversion goes one way.
- `runtimeEpochCacheSteps` validated `CacheEpoch.Script`'s step id every
time it read `steps.<id>.outputs.epoch`. `CacheEpoch.Script` now holds
a `StepId`, so the read is `Expr.StepOutput` directly.
`ModuleGraph.cycle` now takes `Map[String, List[String]]` instead of
nodes. Its caller is `Planner.validateCapabilities`, which was
fabricating `ModuleNode(c.name, …)` to order capabilities: a capability
name is not a module id, and the new rule is entitled to reject one.
Edges make the helper honest about serving any named graph, and drop the
caller's filtering, since `cycle` already restricts to its own keys.
A second scripted test, `reject-unicode-id`, builds a project sbt loads
happily and asserts both tasks fail and no partial workflow is left
behind. The existing gates hold: 607 tests, `generate-check` green, and
`zipxWorkflowGenerate` leaves `.github/` byte-identical.
`Planner.orThrow` survives for `jobResultOf`, whose job ids are composite
`capability-module-target` strings, and `lit`, fed by `PlanConfig` text.
Both need their own newtypes.
`Planner.sbt` built `sbt '<cmd>'` by interpolating a `String` into a shell word, and threw if the result was not a legal single-quoted word. A capability `command` was a `String` all the way from a `build.sbt`, so a newline in one reached the renderer and turned into an unparseable `run:` scalar, reported halfway through planning. `SbtCommand` moves that to construction. Its text rules are only the ones that would break something: non-empty, one line, no control characters. A single quote is *allowed*, because `'…'` cannot escape one but `render` splits the word into `'a'\''b'` segments, which is the concatenation `SquoteText`'s own docs point at. Rejecting it, as a first cut did, made a legitimate `set v := "a'b"` unrepresentable while catching nothing. So `render` is total and the throw is gone. Structure is a combinator rather than an interpolation: `module`, `crossModule`, `join`, `prefixedBy`, `underScalaVersion`. `VerifyClean.prefixCommand` and `Capability.testJoined` compose those instead of assembling strings, and `ModuleNode.testTask` / `publishTask` are commands rather than task names, since `Compile/test` was always legal there. zipx does not parse sbt, and modelling it would foreclose aliases, cross `+`, config axes and compound `a; b` for no gain. So the escape hatch is a *case*, not just a constructor: `Unchecked` renders identically but `rawFragments` reports it, and provenance survives composition, so a hand-written command joined into a larger session still warns at generate time. That follows `Command.Raw`, and it is why `unchecked` is not simply `make`. Each `unsafeMake` names its subset argument rather than claiming convenience: `AttributeKey` labels are lowercase-camelCased for `scopedLabel`, `Expr` renders `ShText` for `underScalaVersion`, `split` never yields a segment containing its separator for the quote encoding. Generated YAML is unchanged: dogfood `zipxWorkflowGenerate` leaves `git diff .github/` empty, and `generate-check` still asserts the same `sbt '…'` strings.
A capability's name and a target's name both reach a `jobs.<job_id>` key, joined with `-`. They were plain `String`s, so the planner had to validate the id it had just assembled and throw when its own construction turned out to be invalid: a space or a `/` in a name produced a workflow that GitHub rejected on push, reported nowhere near the build file that caused it. `CapabilityName` and `TargetName` are now `Subtype[String]` over `Names.ActionsId`, the same rule `JobId` and `ModuleId` use. Because `-` is in that pattern's trailing character set, joining two of them yields a third, which is what makes job-id construction total rather than fallible. The planner builds ids through `CapabilityName.asJobId` and `.jobId(rest*)`, and `Planner.orThrow` and `jobResultOf` are deleted. A `Subtype` and not a `Newtype` so the names stay usable as strings: `name == "test"` and interpolation keep working, and only construction is checked. A literal in a `build.sbt` is validated where it is written, which is the usual case, so the check costs nothing at generate time. Also renames the `JavaVersion` newtype to `JdkVersion`. sbt 2.0 exports a `sbt.JavaVersion`, and the plugin re-exports zipx's types into `build.sbt`'s scope, so naming the type in a build file was an ambiguous reference rather than a shadow. `JdkVersion` also matches the setting's own wording. Generated YAML is unchanged: dogfood `zipxWorkflowGenerate` leaves `git diff .github/` empty, `plugin/scripted zipx/*` passes (its `assertGraph` asserts the literal job ids), and 624 tests pass across every module.
The interpolator validated its literal parts with `ShText.makeOrThrow`, justified by the parts being source literals: a failure is deterministic, so it fires on the first run rather than in production. True, but it was the last throw left below the sbt boundary, and "deterministic" is an argument for checking it at compile time rather than for throwing. `sh` is now `inline`, expanding through `shMacro`, which validates each part and reports through `report.errorAndAbort` naming the offending text. The generated tree calls `Word.lit`, the ordinary checked constructor, so it names no unsafe entry point and the part is validated again as the expansion inlines. `CapabilityTasks.cmdMacro` is the precedent, including keeping the work in plain Scala that the macro only hands off to. Worth knowing for anyone reading the validator: interpolator parts arrive raw, so `sh"a\nb"` holds the two-character escape and is one line. A part spans lines only from a `sh"""…"""` that really does, or from a hand-built `StringContext`, which is how the compile-time spec provokes it. The runtime test that caught the old throw moves to `CompileTimeSpec` for that reason: there is no longer a way to construct the failure at runtime. That leaves exactly one `throw` in every `src/main` in the repo, `ModuleGraph.apply`, which is documented as fixture-only and reports through `make` for real input.
The fifth design guardrail said no stringly-typed construction, without saying how anyone would know. It now carries its check: the grep over `modules/*/src/main`, the tier order a failure must be removed at, and what a new escape hatch owes (typed, self-reporting, warned by name). Also records what the follow-up pass closed, site by site, and the one remaining `throw`, `ModuleGraph.apply`, which is the fixture constructor. Two smaller notes: `JdkVersion`'s name is a deviation forced by sbt 2.0's own `sbt.JavaVersion` colliding in `build.sbt` scope, and `ExitCode` / `FileDescriptor` could be built on `neotype.common.NonNegativeInt`, which is left open because neither validator can be wrong in a way a test would miss.
`ModuleGraph.apply` unwrapped `make`'s Either by throwing. Its only callers
were fixtures writing a literal node list, so the unwrap moved to where such a
fixture belongs: a `GraphFixture` in test scope, one per module that needs it
(`zipx.core` and `zipx.docs`, which have no test-to-test edge between them).
`RemoteCacheSmoke` was the one main-source caller, and it is itself a planner
fixture used only by `RemoteCacheSmokeSpec`, so it moves to test scope rather
than keeping an unchecked constructor alive in `src/main` for its sake.
With that gone, guardrail 5's grep returns nothing:
grep -rn "throw \|makeOrThrow\|orThrow" modules/*/src/main
No main source in this build raises. Every failure is an Either that
`ZipxPlugin.orFail` reports as an sbt error. Two comments that pointed at
`makeOrThrow` as the runtime route now point at `make` instead, since that
advice was stale as well as grep noise.
Acceptance unchanged: 624 tests, `zipxWorkflowGenerate` leaves `.github/`
byte-identical, and `plugin/scripted zipx/*` passes.
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.
Closes #56. Supersedes and closes #46.
Four layers of the API were stringly typed, and each had already produced a real defect or a comment apologising for one:
run:bodies were s-interpolated bash (so a literal$had to be$$, andZipxCentralcarried a warning comment where a type belonged), expressions outsideenv:andif:were raw${{ … }}strings,Stepwas flat and all-optional (soStep()andStep(run =, uses =)both compiled and both rendered YAML GitHub rejects), and reusable step groups were bareStepContext => List[Step]lambdas with no name, no composition, and no identity to publish.Eighteen commits, each independently green, bottom-up. The first seven build and adopt the DSL; the rest hold it to its own standard.
Building the DSL
f398d9bzipx-shell: a general shell AST (Script/Command/Word/ShTest) over neotype-validated primitives. No zipx, GitHub, or zio-blocks dependency.71ad6e1Expr: the GHA expression AST, withEnvValueandJobConditiondelegating to it.d2fe23eStepBuilder+Step.validate: step validity closed from both ends.3acbbaeSteps: named, composable, publishable step bundles.46c258261c72e6autoImportre-exports.1768955Holding it to its own standard
e962135is a comment pass: wherever a comment restated what the code said, the code carries it instead.The rest close a gap the review found. The design claims failures are removed at the strongest tier available, but the first pass still left seven
throws insrc/mainthat the tiers above should have caught. Each was closed at the tier it belonged in, not one tier down:b6c0ab8ActionPins.Fieldis an enum, so there is no unknown field name to reject andFieldPrefixescannot drift from the case class.loadResourcereturns anOption, deleting a throw its own caller already handled as control flow.b501770Expr.LitholdsShText, so an expression literal cannot carry a newline.4eeb80cShLinescarries validated lines through rendering, so a structural command never revalidates text it built from validated pieces.be47ce3ModuleGraph.makerejects a cyclic node list at construction. That makeslayers/subsetLayerstotal and deletesCyclicGraphError, which nothing caught (a cycle produced an sbt stack trace).a4ed3c0ModuleIdis a newtype over GitHub's id rule, which is stricter than sbt's: a project sbt allows but GitHub cannot name is rejected where the graph is built, not midway through planning.27cda85SbtCommandis an enum validated once where a command enters the planner, withUncheckedas the documented, generate-time-warned escape for sbt syntax zipx does not model.4698c44CapabilityNameandTargetNamejoinModuleIdonNames.ActionsId. Because-is in that pattern's trailing set, joining two ids yields a valid job id, soPlanner.orThrowis gone: the planner builds ids rather than validating ones it assembled.7555e26sh"…"is a macro, so an invalid literal part is a compile error naming the text rather than a runtime failure.199b9ecModuleGraph.makebecomes the only constructor. The throwingapplybeside it existed for fixtures writing a literal node list, so the unwrap moved to a test-scopeGraphFixture.4698c44also renamesJavaVersiontoJdkVersion. sbt 2.0 definessbt.JavaVersion, and the plugin re-exports this type intobuild.sbt's scope, so twoJavaVersions there is an ambiguous reference rather than a shadow. That was a build-load failure, not a style preference.The result is checkable, which is the point. Guardrail 5 in the ROADMAP now carries a grep:
It returns nothing. No main source in this build raises: every failure is an
EitherthatZipxPlugin.orFailreports as an sbt error, because sbt's task contract is where throwing belongs. Test-scope helpers that unwrap anEitherloudly (GraphFixture,DocsRender.yaml) are the deliberate exception, and they live in test scope precisely so the grep stays honest.Design decisions worth reviewing
Commandis an open trait, not anenum. A consumer needing a construct zipx does not model (acasestatement, a function definition) implementsCommandin their own build rather than waiting on a release. The accepted cost is that a match overCommandcannot be exhaustive, which is why rendering is a method.WordandShTeststay closed, since the shell's grammar fixes them.Failures are removed in the strongest way available, in order. Make it unrepresentable with a type (
Blockis a head plus a tail, so an emptyifbranch is unconstructible;InlineCommandis the subtype whose render is total, so a compound command in a pipeline leg does not compile;withInputexists only on theuses:builder, sowith:on arun:step does not compile); failing that, check a literal at compile time with aninlinenewtype constructor; failing that, returnEither[String, A]naming the offending value; failing that, throw at the sbt boundary and nowhere else.sh"…"splices areWord*. A bareStringsplice does not compile: string interpolation is exactly how the untyped hole would come back, and there is deliberately no implicitString => Word.Escape hatches stay, typed and loud.
RawholdsList[ScriptLine], so the type prevents YAML GitHub cannot parse and there is no separate lint pass to forget. What raw can still produce is broken shell, so its text is reported viarawFragmentsandzipxWorkflowGeneratewarns naming the bundle. A bare lambda reports nothing, which is the honest incentive to useSteps.built.Rules cover what the shell and GitHub actually do. No
'inside'…'(it cannot be escaped), no}inside${…}(it closes the expansion early), no leading tab on a script line (YAML block-scalar indentation must be spaces),ExitCode0-255 (the shell truncates modulo 256), secret names rejecting the reservedGITHUB_prefix case-insensitively while still admittingGITHUB_TOKEN, output names rejecting the disabledset-output/save-state,uses:refusing an unpinnedowner/repo.The generated YAML did not move a byte
The migration touches every script in the repo, so this is the claim that matters. Proven three ways:
git diffempty acrossci.yml,zipx-action-pins-sync.yml,zipx-scala-steward.yml.plugin/scripted zipx/generate-checkpasses. Independent, because itsassertGraphasserts the literal gate strings (contains(fromJson(needs.affected.outputs.modules), 'api'),!cancelled(),startsWith(github.ref, 'refs/tags/v')) thatExpr.Callnow builds.PlannerSpec/RenderSpec/ the docs pages pass unmodified. An edit needed there would have meant output moved.Zero-diff only proves nothing moved, not which script is which, so
ScriptRenderSpecpins each migrated script against the exact string its pre-migration source produced. All three still hold after the throw-elimination commits, which is what makes those refactors rather than rewrites.Verified on a real consumer, not just on zipx
Testing a DSL only against the scripts of the person who designed it is a weak test.
ConsumerStepsSpecrebuildsearly-effect/chekhov's browser setup, the build that motivated #46, written long before this module existed. Its steps were the useful kind of awkward: anifwhose condition is a glob test on a command's exit status,$(…)substitutions nested in double quotes with a three-stage pipeline inside one,sudofronting another program, a${HOME}-relative path assembled from parts, a cache key mixingrunner.oswithhashFiles.All of it expressed, byte-identical to chekhov's current YAML, so adoption there is a refactor rather than a workflow diff to re-review. The spec also asserts what the lambda version could not promise: every step validates, the bundle reports no escape-hatch use, and one bundle assigned to both
extraStepsandcacheRehydrateExtraStepsyields identical steps, which was #46's actual requirement.examples/monorepowas separately built against apublishLocalto prove the consumer-facing types work outside this repo.Known rough edges
ActionPinsfields areString, soctx.actions.cachecannot reachStep.uses's compile-time check and a build must callusesMakeand handle anEitherfor a value the pin file already validated. Filed as #57, to land when the pin file's decode path is next touched.ActionPinFile.parsesilently drops a line it cannot match, so a malformed pin file decodes to a partialActionPinsrather than an error. Pre-existing, not a regression from this branch. Filed as #59.ExitCodeandFileDescriptorre-derive bounds thatneotype.common.NonNegativeIntalready provides. Noted in the ROADMAP rather than churned here.Test plan
sbt "shell/testFull; workflow/testFull; core/testFull; central/testFull; docs/testFull": 624 tests, 0 failures.sbt "plugin/scripted zipx/*": passes.sbt zipxWorkflowGeneratethengit diff --exit-code .github/: clean.grep -rn "throw \|makeOrThrow\|orThrow" modules/*/src/main: no output.Script.rawand confirming the warning names the bundle, then reverted.