Conversation
github-actions
Bot
added
area:ux
Contributor
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
agatha197
force-pushed
the
fr-3205
branch
10 times, most recently
from
ef922bc to
7050d0f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR redesigns the custom-deployment Start Command input so shell semantics work correctly, backed by the new command/shell GraphQL fields (ModelServiceConfigInput, added in 26.7.0). Instead of tokenizing the command client-side and sending a bare argv array, the WebUI now sends the raw command string and lets the backend either wrap it (shell -c cmd) or shlex.split + exec, depending on whether shell is set. A new collapsible Service Configuration card hosts a Basic/Advanced switch and (in Advanced) a Shell/Exec Execution selector plus a shell picker. Version gating (isManagerVersionCompatibleWith('26.7.0')) and @since fragment fields ensure graceful fallback to the deprecated tokenized startCommand on older managers.
Changes:
- New shared helper
modelServiceCommand.tswithderiveCommandModeState(prefill) andresolveCommandShell(submit), plus shell option/operator constants. - Reworked Start Command UI in
DeploymentAddRevisionModal.tsx(collapsible card, Basic/Advanced, Shell/Exec modes, operator validation) and mirrored Basic/Advanced + Shell controls for admin deployment presets. - Added
command/shellGraphQL selections (version-gated) with regenerated Relay artifacts, and new i18n keys across all 20 locales (en + ko translated, others English fallback).
Reviewed changes
Copilot reviewed 27 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
react/src/helper/modelServiceCommand.ts |
New shared prefill/submit logic; lacks unit tests despite sibling-helper convention. |
react/src/components/DeploymentAddRevisionModal.tsx |
Collapsible card + Basic/Advanced + Shell/Exec UI, version-gated submit (command/shell vs startCommand). |
react/src/components/AdminDeploymentPresetSettingPageContent.tsx |
Prefill via deriveCommandModeState; adds command @since and version gating flag. |
react/src/components/AdminDeploymentPresetModelConfigItem.tsx |
Preset Basic/Advanced + Shell selector (no Exec mode). |
react/src/components/AdminDeploymentPresetFormTypes.ts |
Adds commandAdvanced to ModelServiceFormValue; updates docs on startCommand/shell. |
react/src/pages/AdminDeploymentPresetSettingPage.tsx |
Version-gated submit path using resolveCommandShell / tokenized fallback. |
resources/i18n/*.json (20 files) |
New command/shell/execution keys; CommandExecNewlineError added but unused. |
react/src/__generated__/*.graphql.ts (7 files) |
Regenerated Relay artifacts for the new command/shell selections; consistent with query changes. |
Files not reviewed (7)
- react/src/generated/AdminDeploymentPresetSettingPageContent_preset.graphql.ts: Generated file
- react/src/generated/AdminDeploymentPresetSettingPageCreateMutation.graphql.ts: Generated file
- react/src/generated/AdminDeploymentPresetSettingPagePresetQuery.graphql.ts: Generated file
- react/src/generated/AdminDeploymentPresetSettingPageUpdateMutation.graphql.ts: Generated file
- react/src/generated/DeploymentAddRevisionModal_revisionSource.graphql.ts: Generated file
- react/src/generated/DeploymentDetailPageQuery.graphql.ts: Generated file
- react/src/generated/DeploymentRevisionHistoryTabListQuery.graphql.ts: Generated file
Contributor
Coverage Report for root-coverage
File CoverageNo changed files found. |
…ec modes Wrap the command/config-file region in one collapsible card, move the command|config-file segment into the card header, and add a Basic/Advanced switch that reveals Execution (Shell|Exec) + Shell selector. Switch the write/prefill path to the new 26.7.0 command/shell GraphQL fields (raw string sent to backend; no client-side tokenization), with version-gated fallback to the deprecated startCommand list on older managers. Shell mode: multi-line input, shell operators work. Exec mode: single-line input (Enter blocked), warns on shell operators, shell sent as null so the backend shlex-splits and exec's directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nowgnuesLee
requested changes
Jul 10, 2026
nowgnuesLee
left a comment
nowgnuesLee
left a comment
Contributor
There was a problem hiding this comment.
The other locale files all seem to contain English text — please double-check.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #8035 (FR-3205)
Summary
Redesigns the custom-deployment Start Command input so shell semantics work correctly, backed by the new
command/shellGraphQL fields (ModelServiceConfigInput, "Added in 26.7.0"). The WebUI no longer tokenizes the command client-side — it sends the raw string and lets the backend wrap (shell -c cmd) orshlex.split+ exec.Follow-up to #7957 (FR-3166), which only shipped a temporary helper-text mitigation while the client kept tokenizing.
UI
The command/config-file region is wrapped in one collapsible card:
Command input | Config filesegment moves into the card header (right side) with per-mode tooltips.?tooltip) sits on the Start Command label row./bin/bash), multi-line input, no helper.Execution (Shell | Exec)+Shellselector; multi-line input; helper notes;/&&/|/$VAR/redirection work. Sendscommand+ selectedshell.Shelldisabled; sendscommand+shell: nullso the backend splits into argv.Admin Deployment Presets get the same Basic/Advanced + Shell selector (no Exec mode —
PresetModelServiceConfigInput.shellis non-null).Version gating
isManagerVersionCompatibleWith('26.7.0')selectscommand/shellvs the deprecated tokenizedstartCommand; fragment fields use@since(version:"26.7.0")so prefill falls back to reconstructing from the token list on older managers. Never sends bothcommandandstartCommand.Files
react/src/helper/modelServiceCommand.ts(new) — shared prefill/submit logic (deriveCommandModeState,resolveCommandShell, operator pattern).DeploymentAddRevisionModal.tsx,AdminDeploymentPresetModelConfigItem.tsx,AdminDeploymentPresetSettingPageContent.tsx,AdminDeploymentPresetSettingPage.tsx,AdminDeploymentPresetFormTypes.ts.resources/i18n/*.json(en + ko translated; other locales English fallback).Verification
pnpm relay— compiles clean.tsc— no errors in touched files (the lone remainingFluentEmojiIconerror is a known worktree patched-dep symlink issue, unrelated).TODO
🤖 Generated with Claude Code