fix(cli): select runtime during setup#420
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ab0ab2b96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| workflowFile: workflowPath, | ||
| }); | ||
|
|
||
| await warnIfRuntimeMissing(selectedRuntime); |
There was a problem hiding this comment.
When gh-symphony setup --non-interactive --json runs on a host where the selected runtime binary is not on PATH, this call logs the install warning via p.log.warn before writing the JSON result. @clack/prompts log helpers write to stdout, so the command emits a warning block followed by JSON on stdout, which breaks callers that parse --json output; route this warning to stderr or suppress it in JSON mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in fdc4aa2: missing-runtime warnings now go to stderr when setup runs with --json, so stdout remains a single parseable JSON object. Added a regression test that parses the captured JSON output while asserting the warning is emitted on stderr.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
✅ Approve — #390 fully addressed
요구사항 대비 검증 (issue #390 수용 조건):
| 수용 조건 | 상태 |
|---|---|
Interactive setup에 런타임 선택 단계 추가 (Codex / Claude Code) |
✅ Step 1/5 prompt 추가, 나머지 단계 /5로 재번호 |
비대화형 --runtime 플래그 |
✅ 파싱·검증·completion·help 모두 반영 |
선택 런타임 PATH 체크 + 설치 힌트 (doctor commandExistsOnPath 재사용) |
✅ warnIfRuntimeMissing 로 비대화형/대화형 양쪽에서 경고 |
하드코딩된 runtime: "codex" 4곳 제거 |
✅ 4곳 모두 selectedRuntime 로 교체 |
Smoke Test (built CLI, node dist/index.js):
setup --help→--runtime <kind>노출 ✅setup --non-interactive --runtime claud-print→Error: Unsupported runtime 'claud-print'. Choose one of: codex-app-server, claude-print.(exit 1, auth 이전에 거부) ✅setup --non-interactive --runtime(값 누락) →option '--runtime <kind>' argument missing✅completion bash에--runtime포함 ✅--runtime claude-code별칭 →claude-print정규화 + WORKFLOW.md 에kind: claude-print/command: claude작성 (단위 테스트로 확인) ✅
Build / Test: pnpm --filter @gh-symphony/cli build 성공, setup.test.ts 15 tests 통과.
코드 품질도 양호합니다 — doctor 헬퍼 재사용, 별칭 정규화, 누락 인자 가드 등 기존 컨벤션을 잘 따랐습니다. 인라인의 prompt 순서 nit은 비차단 사항입니다. LGTM 🎉
Generated by Claude Code
| ): Promise<void> { | ||
| p.intro("gh-symphony — One-command Setup"); | ||
|
|
||
| const selectedRuntime = await promptRuntimeSelection(); |
There was a problem hiding this comment.
Nit (non-blocking, UX ordering): the runtime prompt runs before the gh auth check below. If auth fails, the user has already answered the runtime step and re-runs from scratch. workflow init resolves auth first; consider moving this prompt after the auth spinner succeeds for symmetry. Minor — not worth blocking the merge.
Generated by Claude Code
There was a problem hiding this comment.
Addressed in fdc4aa2: interactive setup now completes the gh auth check before prompting for runtime selection, matching the workflow init ordering. The auth-remediation test now asserts no runtime/project prompt is shown when auth fails.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
✅ Completeness verification — Approve
PR #420 fully satisfies issue #390. Verified end-to-end below.
Acceptance criteria (issue #390)
| Requirement | Status |
|---|---|
| Runtime-selection prompt in interactive setup (Codex / Claude Code) | ✅ promptRuntimeSelection() added as the first interactive step |
--runtime flag in non-interactive setup |
✅ parsed in parseSetupFlags, validated + normalized, wired through index.ts and completion.ts |
PATH check reusing doctor's commandExistsOnPath with install hint |
✅ commandExistsOnPath exported; warnIfRuntimeMissing warns in both interactive and non-interactive paths with runtime-specific hints |
Smoke test (black-box, built CLI)
setup --helpexposes--runtime <kind>✅setup --non-interactive --runtime bogus→ exits1withUnsupported runtime 'bogus'. Choose one of: codex-app-server, claude-print.✅--runtime claude-printand the alias--runtime claude-codeboth pass validation and proceed (normalization works) ✅- Non-interactive summary now reports
Agent runtime✅
Quality gates (run locally)
vitest run setup.test.ts→ 15/15 pass (incl. new Claude non-interactive, unsupported-runtime rejection, interactive prompt-ordering tests)pnpm --filter @gh-symphony/cli typecheck→ passpnpm --filter @gh-symphony/cli lint→ passpnpm build→ pass
Findings
1 non-blocking nit (inline): the interactive step counter wasn't fully renumbered after inserting the runtime step — the flow shows two "Step 1" prompts and never reaches "Step 5/5". Cosmetic; workflow init already has the correct pattern to mirror. Recommend a quick follow-up but it does not block merge.
Verdict: Approve. Functionality is complete and correct against the issue; the only outstanding item is a cosmetic step-label fix.
Generated by Claude Code
| ); | ||
| const mappings = await promptStateMappings(statusField, { | ||
| stepLabel: "Step 2/4", | ||
| stepLabel: "Step 2/5", |
There was a problem hiding this comment.
Step counter regression (non-blocking nit). Adding the runtime prompt as Step 1/5 shifted every later step, but the project-board prompt at selectProjectSummary (line 225) was left at Step 1/4, and the labels here weren't fully renumbered. The interactive flow now reads:
- runtime →
Step 1/5 - project board (line 225) →
Step 1/4← should beStep 2/5 - state mappings (here) →
Step 2/5← should beStep 3/5 - blocker check →
Step 3/5← should beStep 4/5 - priority →
Step 4/5← should beStep 5/5
So the wizard shows two consecutive "Step 1" prompts and never reaches "Step 5/5". The sibling workflow init flow gets this right (workflow-init.ts:304 runtime = Step 1/5, :1878 project = Step 2/5) — worth mirroring. Onboarding clarity is the point of #390, so a quick renumber is warranted, but it doesn't block merge.
Generated by Claude Code
There was a problem hiding this comment.
Addressed in fdc4aa2: the setup wizard now uses Step 1/5 through Step 5/5 consistently: runtime, project board, state mappings, blocker check, then priority. Added regression assertions over the prompt labels.
There was a problem hiding this comment.
Pull request overview
Updates the CLI onboarding flow (Configuration layer) so gh-symphony setup can configure a repo for a user-selected agent runtime rather than always wiring Codex, aligning setup behavior with the runtime presets already supported by the workflow runtime model.
Changes:
- Add
--runtime <kind>support togh-symphony setup, propagate it through CLI invocation, and include it in shell completion. - Extend interactive setup with a runtime selection prompt and add a runtime binary PATH preflight warning.
- Export
commandExistsOnPathfromdoctorfor reuse and add setup tests covering runtime selection/validation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.changeset/setup-runtime-selection.md |
Adds a patch changeset documenting runtime selection during setup. |
packages/cli/src/commands/setup.ts |
Implements runtime selection/flag parsing and runtime PATH warning during setup. |
packages/cli/src/commands/setup.test.ts |
Adds/updates tests for runtime selection and non-interactive --runtime behavior. |
packages/cli/src/commands/doctor.ts |
Exports commandExistsOnPath so setup can reuse the PATH-check logic. |
packages/cli/src/index.ts |
Wires the new --runtime option through the top-level CLI command definition and argument forwarding. |
packages/cli/src/completion.ts |
Adds --runtime to completion options for the setup command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const mappings = await promptStateMappings(statusField, { | ||
| stepLabel: "Step 2/4", | ||
| stepLabel: "Step 2/5", | ||
| }); |
There was a problem hiding this comment.
Addressed in fdc4aa2: state mapping now receives Step 3/5, with project selection as Step 2/5 and the later prompts shifted accordingly. Covered by the interactive setup prompt-label regression test.
| const lifecycleBase = toWorkflowLifecycleConfig(statusField.name, mappings); | ||
| const blockerCheckStates = await promptBlockerCheck(lifecycleBase, { | ||
| stepLabel: "Step 3/4", | ||
| stepLabel: "Step 3/5", | ||
| }); |
There was a problem hiding this comment.
Addressed in fdc4aa2: blocker check now receives Step 4/5. The interactive setup test asserts the blocker-check confirmation message includes that label.
| const { priority, priorityField } = await promptPriorityConfig({ | ||
| priorityResolution, | ||
| labelNames: priorityLabelNames, | ||
| stepLabel: "Step 4/4", | ||
| stepLabel: "Step 4/5", | ||
| }); |
There was a problem hiding this comment.
Addressed in fdc4aa2: priority configuration now receives Step 5/5, and the test asserts the priority selection prompt uses that label.
| const gitPath = execFileSync("which", ["git"], { | ||
| encoding: "utf8", | ||
| }).trim(); | ||
| const binDir = await mkdtemp(join(tmpdir(), "setup-runtime-bin-")); | ||
| await symlink(gitPath, join(binDir, "git")); |
There was a problem hiding this comment.
Addressed in fdc4aa2: the setup test no longer shells out to which or creates symlinks. It mocks the shared commandExistsOnPath utility directly, so the missing-runtime path is covered without platform-specific PATH setup.
| promptBlockerCheck, | ||
| promptLegacyGhSymphonyCleanup, | ||
| warnDeprecatedSkipContext, | ||
| } from "./workflow-init.js"; | ||
| import { commandExistsOnPath } from "./doctor.js"; | ||
| import { |
There was a problem hiding this comment.
Addressed in fdc4aa2: commandExistsOnPath now lives in packages/cli/src/utils/command-exists-on-path.ts, and both setup.ts and doctor.ts import that lightweight utility instead of setup importing the full doctor command module.
Issues
gh-symphony setuphardcodes the Codex runtime — no prompt, no--runtimeflag, no install check #390TL;DR
gh-symphony setupnow uses the selected runtime throughout onboarding instead of hardcoding Codex, and the review rework keeps that flow clean for real CLI users: auth runs before runtime selection, interactive step labels are consistent, missing-runtime warnings do not corrupt--jsonstdout, and setup no longer imports the large doctor command module for a small PATH check.변경 지점 다이어그램
여기부터 보세요
packages/cli/src/commands/setup.ts— runtime flag parsing, auth-before-runtime prompt ordering, artifact wiring, JSON-safe install warning, 5-step labels.packages/cli/src/utils/command-exists-on-path.ts— lightweight shared PATH lookup used by setup and doctor.packages/cli/src/commands/setup.test.ts— non-interactive Claude runtime generation, unsupported runtime rejection, JSON parseability, auth-before-prompt, and step-label regression coverage.packages/cli/src/commands/doctor.ts— reuses the shared PATH lookup utility.위험 & 롤백
변경 파일
.changeset/setup-runtime-selection.mdpackages/cli/src/commands/setup.tspackages/cli/src/commands/setup.test.tspackages/cli/src/commands/doctor.tspackages/cli/src/utils/command-exists-on-path.tspackages/cli/src/index.tspackages/cli/src/completion.tsEvidence
pnpm --filter @gh-symphony/cli test -- setup.test.ts— passpnpm lint— passpnpm test— passpnpm typecheck— passpnpm build— pass머지 후/사람 확인
gh-symphony setupmanually and confirm the runtime prompt and missing-runtime warning read well in a real terminal.