You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The codegen job is the single place that runs scripts/codegen.sh (and therefore the only job that builds the Rust crate to produce the rustdoc JSON). It uploads the generated TypeScript surface as a codegen-output artifact:
The downstream jobs (ts-client, playground, explorer) download that artifact before npm ci. With the generated files already present, scripts/ensure-generated.sh short-circuits, so npm ci/npm run build compile the client with tsc alone and never invoke codegen.sh.
Because those jobs no longer touch Rust, the dtolnay/rust-toolchain and Swatinem/rust-cache steps are dropped from them, and the explicit ./scripts/codegen.sh invocations in playground and explorer are removed. ts-client now declares needs: codegen so the artifact is available when it runs.
Review: share codegen output across jobs via artifact
Summary. The codegen job becomes the single place that runs scripts/codegen.sh (and thus the only job that builds the Rust crate for rustdoc JSON). It uploads the generated TypeScript surface as the codegen-output artifact; ts-client, playground, explorer, and e2e download it before npm ci, so ensure-generated.sh short-circuits and those jobs compile with tsc alone. The Rust toolchain + cache steps and the explicit ./scripts/codegen.sh calls are dropped from the downstream jobs, and a new TRUAPI_REQUIRE_GENERATED=1 guard makes ensure-generated.sh fail loudly (instead of silently re-running codegen) when the artifact didn't restore the expected files.
Why all three mechanisms are needed (not just needs:): GitHub Actions jobs run on separate ephemeral runners with no shared filesystem, and the generated files are gitignored, so needs: alone transfers nothing — a downstream job would check out a tree with no generated files. The artifact is the actual data transfer; needs: only orders the jobs; TRUAPI_REQUIRE_GENERATED=1 is a fail-fast guard that turns "files missing → silently run codegen → cryptic cargo error (toolchain now removed)" into a clear "add the path to the upload-artifact step" message.
Findings
#
Severity
File
Description
1
Info
.github/workflows/ci.yml
TRUAPI_REQUIRE_GENERATED: 1 is repeated across 4 jobs. Could be hoisted to a workflow-level env:. Harmless to set globally: the rust job never runs ensure-generated.sh, and codegen always produces the files so the guard never trips. DRY nit only.
2
Info
.github/workflows/ci.yml
The graph now serializes codegen → ts-client → {playground, explorer} → e2e. This trades parallelism for not rebuilding Rust 4×; net win, but codegen is now a hard gate on everything downstream.
Verified
Artifact paths cover every required file. Both ensure-generated.sh checklists (truapi: src/generated/{client,types,wire-table}.ts, playground/codegen/services.ts, explorer/codegen/types.ts, explorer/versions.ts, playground/test/generated/examples/*.ts; host: server.ts, types-by-version.ts) are fully covered by the six upload paths.
Path consistency.codegen.sh writes examples to playground/test/generated/examples, matching both the ensure-generated check and the upload path.
upload-artifact@v4 least-common-ancestor behavior preserves repo-root-relative structure (paths span js/... and playground/...), so download restores files to the correct locations.
Dependency chain reaches codegen transitively from every artifact consumer.
if-no-files-found: error and retention-days: 1 are appropriate.
Verdict: Mergeable. Clean PR, satisfies #134 (single Rust build, generated surface shared via artifact). The two items above are Info-only. A fresh CI run is currently in progress on a diff identical to the previously-green run.
The reason will be displayed to describe this comment to others. Learn more.
LGTM — clean artifact-sharing refactor for #134. Only Info-level nits (see review comment). Verified artifact paths cover all required generated files and the dependency chain reaches codegen.
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
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 #134.
The
codegenjob is the single place that runsscripts/codegen.sh(and therefore the only job that builds the Rust crate to produce the rustdoc JSON). It uploads the generated TypeScript surface as acodegen-outputartifact:js/packages/truapi/src/generatedjs/packages/truapi/src/playground/codegenjs/packages/truapi/src/explorer/codegen+src/explorer/versions.tsjs/packages/truapi-host/src/generatedplayground/test/generatedThe downstream jobs (
ts-client,playground,explorer) download that artifact beforenpm ci. With the generated files already present,scripts/ensure-generated.shshort-circuits, sonpm ci/npm run buildcompile the client withtscalone and never invokecodegen.sh.Because those jobs no longer touch Rust, the
dtolnay/rust-toolchainandSwatinem/rust-cachesteps are dropped from them, and the explicit./scripts/codegen.shinvocations inplaygroundandexplorerare removed.ts-clientnow declaresneeds: codegenso the artifact is available when it runs.