Skip to content

fix(sdl): preserve service command/args on import instead of forcing sh -c - #3290

Merged
baktun14 merged 1 commit into
mainfrom
fix/sdl-preserve-command-args-on-import
Jun 10, 2026
Merged

fix(sdl): preserve service command/args on import instead of forcing sh -c#3290
baktun14 merged 1 commit into
mainfrom
fix/sdl-preserve-command-args-on-import

Conversation

@baktun14

@baktun14 baktun14 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Why

The simple SDL builder mangles a service's command/args on import → preview → deploy. When re-serializing the Command field it forces a hardcoded sh -c wrapper, so any command that isn't exactly sh -c is corrupted. Reported by a user: importing

command: [bash, -lc]
args: ["./run.sh"]

and clicking Preview rewrites it to command: [sh, -c, "bash\n-lc\n"] — the original bash -lc is shoved into the script slot of an sh -c wrapper.

The round-trip was actually broken for every command shape, not just the reported one:

Imported SDL Old (buggy) export Fixed
command:[bash,-lc] + args:[script] [sh,-c,"bash\n-lc\n"] + [script] preserved
command:[sh,-c] + args:[script] command + args dropped entirely preserved
command:[sh,-c,script] (no args) bare string (invalid SDL) + bogus args:[""] preserved

Since the generated SDL is what gets deployed (form → generateSdl → manifest → chain), this is a correctness bug, not just cosmetic.

What

Stop forcing a shell wrapper — the Command field is now treated as the literal SDL command array, one token per line, and preserved verbatim.

  • parseSvcCommand (sdlImport.ts): no longer strips a leading sh -c; all command elements join by newline.
  • buildCommand (sdlGenerator.ts): always returns string[] by splitting on newline (trim + drop empties); no sh -c forcing.
  • Export call site: no longer trims the arg (a multi-line script's whitespace can be meaningful) and only emits args: when one is present (fixes the bogus args: [""]).
  • CommandFormModal.tsx: Command field placeholder updated to teach the one-token-per-line convention.
  • Tests: rewrote buildCommand/parseSvcCommand specs for the new behavior and added round-trip coverage proving all shapes survive (including the two that were silently broken).

Example SDL to test

Import the following, click Preview, and confirm both services' command/args are unchanged:

version: "2.0"
services:
  web:
    image: ubuntu:24.04
    command: [bash, "-lc"]
    args:
      - |
        echo "web started"; while true; do echo hi; sleep 10; done
    expose:
      - port: 8080
        as: 80
        to: [{ global: true }]
  worker:
    image: alpine:3.20
    command: [sh, "-c"]
    args: ["echo up; sleep infinity"]
profiles:
  compute:
    web: { resources: { cpu: { units: 0.5 }, memory: { size: 512Mi }, storage: [{ size: 512Mi }] } }
    worker: { resources: { cpu: { units: 0.5 }, memory: { size: 256Mi }, storage: [{ size: 256Mi }] } }
  placement:
    dcloud:
      pricing:
        web: { denom: uakt, amount: 10000 }
        worker: { denom: uakt, amount: 10000 }
deployment:
  web: { dcloud: { profile: web, count: 1 } }
  worker: { dcloud: { profile: worker, count: 1 } }

Summary by CodeRabbit

  • Improvements

    • Clarified command input placeholder to show "one token per line" with examples.
  • Bug Fixes

    • Command parsing now treats each newline-separated token as a separate command element and trims/drops empty lines.
    • SDL import/export roundtrip preserved original command arrays and no longer injects automatic shell-wrapping or unnecessary args.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f6c115af-e023-48ed-8212-e8f186a52985

📥 Commits

Reviewing files that changed from the base of the PR and between 63310f0 and 80959ee.

📒 Files selected for processing (5)
  • apps/deploy-web/src/components/sdl/CommandFormModal.tsx
  • apps/deploy-web/src/utils/sdl/sdlGenerator.spec.ts
  • apps/deploy-web/src/utils/sdl/sdlGenerator.ts
  • apps/deploy-web/src/utils/sdl/sdlImport.spec.ts
  • apps/deploy-web/src/utils/sdl/sdlImport.ts
💤 Files with no reviewable changes (1)
  • apps/deploy-web/src/utils/sdl/sdlImport.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/deploy-web/src/components/sdl/CommandFormModal.tsx
  • apps/deploy-web/src/utils/sdl/sdlImport.spec.ts
  • apps/deploy-web/src/utils/sdl/sdlGenerator.spec.ts
  • apps/deploy-web/src/utils/sdl/sdlGenerator.ts

📝 Walkthrough

Walkthrough

Refactors SDL command representation from automatic sh -c wrapping to explicit newline-delimited token arrays. buildCommand now returns an array of trimmed tokens, parseSvcCommand joins command arrays with newlines without special-casing sh -c, and SDL generation wiring and tests were updated accordingly. UI placeholder now instructs one token per line.

Changes

SDL Command Tokenization

Layer / File(s) Summary
buildCommand array tokenization
apps/deploy-web/src/utils/sdl/sdlGenerator.ts, apps/deploy-web/src/utils/sdl/sdlGenerator.spec.ts
buildCommand() signature changed to return string[] by splitting input on newlines, trimming each token, and filtering empty lines; removed prior sh -c normalization. Tests updated to assert array output with proper whitespace and empty-line handling and no implicit shell wrapping.
parseSvcCommand simplification and import test updates
apps/deploy-web/src/utils/sdl/sdlImport.ts, apps/deploy-web/src/utils/sdl/sdlImport.spec.ts
parseSvcCommand no longer strips sh -c prefixes; it now joins the entire (filtered) command array with newlines. Tests revised to preserve leading sh -c sequences and validate newline-joined output. Test imports updated to include buildCommand.
SDL generation wiring and roundtrip verification
apps/deploy-web/src/utils/sdl/sdlGenerator.ts, apps/deploy-web/src/utils/sdl/sdlImport.spec.ts
generateSdl sets command from buildCommand() and conditionally emits args only when provided (without trimming). Roundtrip tests verify parseSvcCommand + buildCommand preserve command and optional args and ensure regenerated SDL omits args when a service has command but no args.
Command input UI guidance
apps/deploy-web/src/components/sdl/CommandFormModal.tsx
Updated the "Command" Textarea placeholder to multiline text instructing "One token per line" with example sh\n-c to align user input expectations with token-per-line parsing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size: M

Suggested reviewers

  • stalniy
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sdl-preserve-command-args-on-import

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.69%. Comparing base (9e9a1ee) to head (80959ee).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...deploy-web/src/components/sdl/CommandFormModal.tsx 0.00% 2 Missing ⚠️
apps/deploy-web/src/utils/sdl/sdlGenerator.ts 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3290      +/-   ##
==========================================
- Coverage   68.11%   66.69%   -1.43%     
==========================================
  Files        1073      985      -88     
  Lines       26272    24063    -2209     
  Branches     6283     5848     -435     
==========================================
- Hits        17896    16048    -1848     
+ Misses       7334     7004     -330     
+ Partials     1042     1011      -31     
Flag Coverage Δ *Carryforward flag
api 84.91% <ø> (ø) Carriedforward from 9e9a1ee
deploy-web 52.56% <62.50%> (-0.02%) ⬇️
log-collector ?
notifications 91.06% <ø> (ø) Carriedforward from 9e9a1ee
provider-console 81.38% <ø> (ø) Carriedforward from 9e9a1ee
provider-inventory ?
provider-proxy 86.37% <ø> (ø) Carriedforward from 9e9a1ee
tx-signer ?

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
apps/deploy-web/src/utils/sdl/sdlImport.ts 81.90% <ø> (-0.34%) ⬇️
apps/deploy-web/src/utils/sdl/sdlGenerator.ts 54.45% <83.33%> (-0.79%) ⬇️
...deploy-web/src/components/sdl/CommandFormModal.tsx 33.33% <0.00%> (-6.67%) ⬇️

... and 90 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@baktun14
baktun14 force-pushed the fix/sdl-preserve-command-args-on-import branch from 63310f0 to 80959ee Compare June 10, 2026 02:11
@baktun14
baktun14 merged commit ab10853 into main Jun 10, 2026
57 checks passed
@baktun14
baktun14 deleted the fix/sdl-preserve-command-args-on-import branch June 10, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant