ci: the job schema can express ordering - #1762
Conversation
A job that consumes an artifact must not start before the job that uploads it. The generator could not say so: jobSchema named only runs-on and steps, and it is deliberately closed, so a `needs:` key emitted past it would fail the round-trip that parseGitHubAction performs in fjs/ci/proof.f.mjs. Without this, the packed-artifact check in fjs/ci/todo/f-mjs-package-support.md could only fail at download-artifact — a required check red for a reason unrelated to what it tests, which is the one failure mode that trains people to re-run instead of read. Adds `needs: or(option, array(string))`, which widens Job through Ts<typeof jobSchema>. The generated ci.yml is unchanged: nothing orders itself yet, so the field stays absent. The proof covers what the field is for rather than that it exists: an ordered pair round-trips with its dependency intact; independent jobs still parse with it absent; a bare scalar `needs: pack` is rejected, since GitHub accepts that spelling but this generator emits the list form only, so a scalar is drift; and no generated job carries the field today, so the first one to do so is a deliberate change. Prerequisite for both fjs/ci/todo/ci-integration-tests.md's two-stage split and the Stage 2 packed-artifact check, which is why it is owned there rather than by either consumer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | f9cab0e | Commit Preview URL Branch Preview URL |
Aug 28 2026, 07:40 PM |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbca4511bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fjs/AGENTS.md requires a const with a literal initializer to pin its type; the fixture array relied on tsc's default widening, which drops readonly and the literal types — exactly what a schema-oriented proof depends on. The other two consts added here are exempt by the same rule: an arrow function and a call are not literal initializers and already carry non-widening types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved.
needs: or(option, array(string)) is genuinely optional — a job without it validates, one with it validates, and tsc catches the mutant that makes it required, before the runtime proofs even run. jobNeeds also pins optionality independently by unwrapping the needs-less parse, so the guarantee is not only a type-level accident.
Rejecting the bare scalar spelling is a real narrowing against GitHub's string | string[], and the right one here: the generator only ever emits the list form, so a scalar would be drift, not an alternative. Worth having the proof say so, which it does.
ci.yml correctly does not move — the capability is dormant, and the new proof asserts that on the actually-generated workflow rather than assuming it. Regeneration leaves git diff empty. Gates: tsc 0, npm test 3528/3528, zero leaves vanished and exactly one added. No changelog needed, matching five prior fjs/ci-only PRs.
First step of Stage 2's packed-artifact check, recorded in #1757. It is the prerequisite the design names, and nothing else in the stage can start without it.
Why this is a prerequisite, not a detail
A job that consumes an artifact must not start before the job that uploads it. The generator could not say so:
jobSchemanamed onlyruns-onandsteps.Emitting a bare
needs:key past the schema is not an option, and the reason is specific to this repository:jobSchemais deliberately closed, and it is the same schemaparseGitHubActionreads the generated workflow back through infjs/ci/proof.f.mjs. An unmodelled key is generator drift by construction, so the round-trip proof would reject it.Without this, the packed-artifact check could only fail at
download-artifact— a required check red for a reason unrelated to what it tests, which is the one failure mode that trains people to hit re-run instead of reading the log.The change
needs: or(option, array(string))onjobSchema, matching the optional-field idiom already used bystepSchema.Jobwidens automatically throughTs<typeof jobSchema>, sofjs/ci/common/types.tsneeded no edit.The generated workflow is unchanged. Nothing orders itself yet, so the field stays absent and
npm run ci-updatereproduces.github/workflows/ci.ymlbyte-identically. This lands dormant.What the proof covers
Not that the field exists — what it is for:
needs: packis rejected. GitHub accepts that spelling, but this generator emits the list form only, so a scalar is drift rather than an alternative — the same reasoning that makes these schemas closed;Ownership
Owned by
fjs/ci/todo/ci-integration-tests.mdrather than by either consumer. That issue's own two-stage build/integration split needs the same edge, so putting it at the artifact hand-off stops whichever consumer lands first from implementing another issue's task.Checks
npx tscclean; full suite 3528/3528; coverage 100%;npm run ci-updateproduces no diff.No changelog entry: behavior is unchanged and
fjs/ciis this repository's own generator, so this is internal — the entry rules scope release notes to users of the package.🤖 Generated with Claude Code
https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
Generated by Claude Code