feat(sdl): preserve TEE param through builder round-trip - #3345
Conversation
📝 WalkthroughWalkthroughAdds ChangesTEE param round-trip through SDL builder
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
ac3e9c8 to
92d0d0d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3345 +/- ##
==========================================
- Coverage 69.63% 68.24% -1.39%
==========================================
Files 1088 998 -90
Lines 26660 24337 -2323
Branches 6414 5952 -462
==========================================
- Hits 18564 16610 -1954
+ Misses 7106 6769 -337
+ Partials 990 958 -32
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
The SDL builder models only a subset of service params, so switching from the YAML editor to the builder and back dropped the params.tee field. Capture tee on import and re-emit it on generate so it survives the round-trip, even though the builder UI cannot edit it yet. Refs CON-463
92d0d0d to
58464dc
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 `@apps/deploy-web/src/utils/sdl/sdlImport.spec.ts`:
- Line 216: The roundtrip test contains multiple declarations of the variable
`const parsed` in the same scope, causing TypeScript redeclaration errors.
Locate all instances of `const parsed` declarations in the test block around
line 216 and rename the duplicate declarations to have unique variable names
that reflect what each one is parsing (for example, use different names like
`const parsedRegenerated` or `const parsedOriginal` to distinguish between
different yaml.load calls). This will resolve the compilation error while
maintaining code clarity.
🪄 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: Pro
Run ID: 0771b2f6-7255-4a14-9c0e-8636561b297c
📒 Files selected for processing (5)
apps/deploy-web/src/types/sdlBuilder/sdlBuilder.tsapps/deploy-web/src/utils/sdl/sdlGenerator.spec.tsapps/deploy-web/src/utils/sdl/sdlGenerator.tsapps/deploy-web/src/utils/sdl/sdlImport.spec.tsapps/deploy-web/src/utils/sdl/sdlImport.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/utils/sdl/sdlGenerator.ts
- apps/deploy-web/src/types/sdlBuilder/sdlBuilder.ts
- apps/deploy-web/src/utils/sdl/sdlGenerator.spec.ts
- apps/deploy-web/src/utils/sdl/sdlImport.ts
Refs CON-463
Problem
The SDL builder's form model only mirrors a subset of service
params(justpermissions). Switching from the YAML editor to the SDL builder and back droppedparams.tee— the TEE / confidential-compute field. Users authoringteein YAML lost it the moment they touched the visual builder.Fix
Round-trip
params.teethrough the builder without exposing it in the form UI (which doesn't support editing it yet):sdlBuilder.ts— addtee: "cpu" | "cpu-gpu"to theparamszod schema, and makepermissionsoptional so atee-only service still validates.sdlImport.ts— capturesvc.params.teeonto the service model on import.sdlGenerator.ts— re-emitparams.teeon generate, merged with any existingparams(e.g. log-collectorpermissions).The builder UI is untouched —
teeis preserved invisibly.Tests
Added test-first (TDD); new specs:
tee(standalone, and merged alongside log-collector permissions)teeonto the service modelteebaseline leavesparamsundefinedteeAll sdl/builder specs green (63 passing). eslint clean;
tscintroduces no new errors (the repo's pre-existing 87 unrelated errors in auth/api-keys specs are unchanged).Note
This mirrors the same fix landed on the
console-airfork.Summary by CodeRabbit
params) across round-trips, including when permissions are present.params.tee.