ci(studio): add SDK gen check for OpenAPI spec changes (ASTD-218) - #245
Conversation
d867dfa to
571be49
Compare
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCI change detection now treats ChangesSDK Generation CI and Documentation Tooling
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/studio-ci.yaml:
- Around line 118-137: For the web-sdk-gen job, harden the GitHub Actions usage
by pinning external actions and tightening permissions: replace uses:
actions/checkout@v6 and uses: actions/setup-node@v4 with fixed commit SHAs (pin
to specific tags/SHAs), add persist-credentials: false to the actions/checkout
step to avoid leaking GITHUB_TOKEN, and add a minimal job-level permissions
block (e.g., permissions: contents: read) to enforce least privilege for this
job; update the Install pnpm and other steps only if they reference action
versions so they are pinned as well. Ensure the referenced steps are the ones
named in the job (web-sdk-gen -> actions/checkout, actions/setup-node, Install
pnpm via corepack) so reviewers can locate the changes.
In `@web/packages/sdk/package.json`:
- Around line 28-29: Add a new CI-safe npm script named "test:ci" in
package.json so the SDK is run by the top-level web-test task; mirror the
existing "test" script but run Vitest in non-watch/CI mode (for example use the
same command as "test" with CI flags like --run, --coverage or --silent as your
CI prefers). Edit the package.json scripts block that currently contains "test"
and "typecheck" and add "test:ci" alongside them (referencing the "test" script
for the command shape) so web-test's --if-present invocation will include
`@nemo/sdk` in CI. Ensure the new script does not use watch mode and produces exit
codes/coverage compatible with CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c49e3d21-655f-4a22-88e4-f48af6c1e475
📒 Files selected for processing (3)
.github/workflows/studio-ci.yamlweb/packages/sdk/package.jsonweb/packages/sdk/tsconfig.json
Add web-sdk-gen CI job that force-regenerates the TypeScript SDK from all OpenAPI specs and typechecks nemo-studio-ui against the result. Expand studio-ci.yaml trigger paths to include openapi/** and plugins/*/openapi/** so PRs that touch only OpenAPI specs (Python service PRs, plugin PRs) run Studio CI. Root cause: PR #66 broke main because new add_job_routes() calls produced duplicate operationIds, generating duplicate TS function names. studio-ci.yaml only triggered on web/** changes so the failure wasn't caught before merge. Signed-off-by: mschwab <mschwab@nvidia.com>
Add tsconfig.json + typecheck script to @nemo/sdk so CI typechecks the generated TypeScript files directly after gen:all-force, rather than relying on nemo-studio-ui's transitive dependency resolution (which has skipLibCheck:true and no guaranteed path through @nemo/sdk source). This makes the web-sdk-gen check definitive: duplicate operationIds that produce duplicate TS function names now fail at the SDK typecheck step. Signed-off-by: mschwab <mschwab@nvidia.com>
nemo-studio-ui typecheck has pre-existing failures from evaluator/safe- synthesizer SDK API renames unrelated to this change. @nemo/sdk typecheck alone is sufficient to catch the duplicate operationId issue. Signed-off-by: mschwab <mschwab@nvidia.com>
Signed-off-by: mschwab <mschwab@nvidia.com>
1c2bd61 to
8ac4cb1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 507-534: The ci job graph is missing the web-sdk-gen dependency so
failures in the Web SDK generation won't block merges; update the ci-status
job's needs array to include the web-sdk-gen job name (referencing the existing
job identifier "web-sdk-gen") ensuring ci-status depends on it, so CI
status/branch protection reflects SDK generation errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 37e2918e-28bf-4f10-a7e3-7122c7524233
📒 Files selected for processing (3)
.github/actions/changes/action.yaml.github/workflows/ci.yaml.pre-commit-config.yaml
Signed-off-by: mschwab <mschwab@nvidia.com>
Summary
web-sdk-genCI job: force-regenerates the TypeScript SDK from all OpenAPI specs, then typechecksnemo-studio-uiagainst the resultstudio-ci.yamltrigger paths to includeopenapi/**andplugins/*/openapi/**so PRs that change OpenAPI specs outsideweb/run Studio CIRoot cause
PR #66 broke
mainbecause 4 newadd_job_routes()calls produced duplicate OpenAPI operationIds, generating duplicate TypeScript function names.studio-ci.yamlonly triggered onweb/**changes, so the Studio SDK failure was invisible until after merge. PR #219 was the 2-hour hotfix.How this prevents recurrence
Any PR that changes
plugins/*/openapi/**oropenapi/**now triggersweb-sdk-gen. The job fails ifgen:all-forceproduces TypeScript that doesn't compile — catching duplicate operationIds, schema regressions, or any other OpenAPI change that breaks the Studio SDK.Test plan
web-sdk-genjob appears in CI on this PRweb-sdk-genfailsFixes ASTD-218
Summary by CodeRabbit