fix(cli): surface verbose error details#416
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: 2d71d726ec
ℹ️ 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".
| options, | ||
| projectId, | ||
| parsed.logLevel, | ||
| parsed.logLevel ?? (options.verbose ? "verbose" : undefined), |
There was a problem hiding this comment.
In the daemon path, gh-symphony -v repo start --daemon converts the parent verbose flag only into --log-level verbose for the detached child. The child is another CLI process whose top-level catch enables stack/cause output only when hasVerboseFlag(process.argv.slice(2)) sees -v/--verbose, so uncaught startup failures in the daemon log still get the old single-line message even though the user requested verbose diagnostics. Pass the verbose flag through separately, or make the child diagnostic check recognize this propagated state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5049ad9: startDaemon now passes --verbose to the child CLI when the parent global verbose option is set, alongside --log-level verbose. Added start.test.ts coverage asserting the daemon child argv includes both flags.
There was a problem hiding this comment.
Pull request overview
This PR improves CLI/orchestrator failure diagnostics by introducing a shared terminal error formatter in @gh-symphony/core and wiring the existing -v/--verbose flag to show stack traces and cause chains, while preserving the current one-line default output. This primarily affects the Observability layer (error formatting/output) and its wiring at CLI entrypoints.
Changes:
- Add a shared
formatErrorForTerminal(error, { verbose })utility (plushasVerboseFlag) in core observability, with tests. - Update CLI and orchestrator top-level
catchhandlers to use the shared formatter and respect verbose mode. - Bridge CLI global
verboseintorepo startby mapping it to orchestratorlogLevel=verboseand documenting the flag in help output.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/observability/error-format.ts | New shared terminal error formatter and verbose flag detection. |
| packages/core/src/observability/error-format.test.ts | Unit tests for verbose/non-verbose terminal formatting behavior. |
| packages/core/src/observability/index.ts | Re-export new error formatting utilities from observability barrel. |
| packages/cli/src/index.ts | Use shared formatter in top-level CLI crash handler; improve verbose option help text. |
| packages/cli/src/commands/help.ts | Update --verbose/-v help description to mention stack traces. |
| packages/cli/src/commands/snapshots/help.test.ts.snap | Snapshot updates for the revised verbose help text. |
| packages/cli/src/commands/start.ts | Map global CLI verbose to orchestrator verbose log level for repo start. |
| packages/cli/src/commands/start.test.ts | Add coverage ensuring global verbose maps to orchestrator verbose logs. |
| packages/orchestrator/src/index.ts | Use shared formatter in top-level catch; accept -v/--verbose as logLevel=verbose. |
| packages/orchestrator/src/index.test.ts | Add coverage ensuring -v is treated as verbose log level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const lines = [formatSingleError(error)]; | ||
| const seenCauses = new Set<object>(); | ||
| let cause = resolveCause(error); | ||
|
|
||
| while (cause !== undefined) { | ||
| if (typeof cause === "object" && cause !== null) { | ||
| if (seenCauses.has(cause)) { | ||
| lines.push("Caused by: [Circular cause]"); | ||
| break; | ||
| } | ||
| seenCauses.add(cause); | ||
| } | ||
|
|
||
| lines.push(`Caused by: ${formatSingleError(cause)}`); | ||
| cause = resolveCause(cause); | ||
| } |
There was a problem hiding this comment.
Fixed in 5049ad9: formatErrorForTerminal now seeds the seen-cause set with the top-level error before walking cause, so an A -> B -> A cycle emits [Circular cause] instead of printing the top-level error twice.
| it("detects both verbose flags", () => { | ||
| expect(hasVerboseFlag(["repo", "start", "--verbose"])).toBe(true); | ||
| expect(hasVerboseFlag(["-v", "repo", "start"])).toBe(true); | ||
| expect(hasVerboseFlag(["repo", "start"])).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Added in 5049ad9: error-format.test.ts now covers a circular cause chain returning to the top-level error and asserts the formatter emits [Circular cause] without repeating the top-level stack.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
✅ 완성도 검증 결과 — Approve
원본 이슈 #396의 수용 조건과 PR 변경 내용을 검증했고, 빌드/테스트/스모크 테스트를 직접 수행했습니다.
수용 조건 충족 여부 (#396)
- ✅
-v/--verbose가 더 이상 no-op이 아님 — top-level catch에서 stack 및cause체인을 출력 - ✅ CLI top-level catch가 argv를 스캔해 verbose 적용 (
packages/cli/src/index.ts) - ✅ Orchestrator top-level catch가
-v/--verbose/--log-level verbose/SYMPHONY_LOG_LEVEL모두 인식 - ✅
-v→ orchestrator verbose 로깅 브리지 (foreground + daemon) - ✅ help 텍스트에 "stack traces" 문서화 + 스냅샷 갱신
직접 수행한 검증
pnpm build— 성공- 대상 테스트 53개 통과 (
error-format/orchestrator index/start/help) pnpm typecheck— 통과pnpm lint— 통과- Smoke test (빌드된
@gh-symphony/core직접 실행):- non-verbose →
template_render_error한 줄 ✔ - verbose → 전체 stack +
Caused by:로 cause 체인 추적 ✔ (이슈가 요구한 동작 그대로)
- non-verbose →
- 순환 cause 보호 로직(
[Circular cause]) 및 non-Error 입력 처리 확인 ✔ - CI: Test ✅ / Container Smoke ✅
코드 품질 메모
- (비차단) 데몬 경로 verbose 전달 누락 — 인라인 코멘트 참조. 좁은 엣지 케이스(데몬 top-level uncaught 에러 한정, 서비스 레벨 verbose 로깅은 정상)라 머지를 막지 않습니다. 후속 한 줄 수정으로 마무리하면 좋습니다.
- orchestrator
parseArgs의startsWith("--")→startsWith("-")변경으로 알 수 없는 single-dash 옵션이 이제 throw 되지만, 내부 전용 바이너리라 영향 없음.
핵심 기능이 이슈 요구사항대로 정확히 동작하고 모든 검사가 통과하므로 Approve 합니다. 위 비차단 항목은 선택적 후속 작업입니다.
Generated by Claude Code
| options, | ||
| projectId, | ||
| parsed.logLevel, | ||
| parsed.logLevel ?? (options.verbose ? "verbose" : undefined), |
There was a problem hiding this comment.
검증 결과: 데몬 경로의 verbose가 자식 프로세스 top-level catch까지 전달되지 않습니다 (Codex P2 확인).
startDaemon은 자식을 gh-symphony repo start --log-level verbose 로 띄웁니다(line ~1087). 그런데 자식 CLI의 top-level catch(packages/cli/src/index.ts)는 hasVerboseFlag(process.argv.slice(2)) 로만 verbose를 켭니다. hasVerboseFlag는 -v/--verbose만 인식하고 --log-level verbose는 인식하지 않습니다.
로컬에서 확인:
hasVerboseFlag(["repo","start","--log-level","verbose"]) === false
따라서 gh-symphony -v repo start --daemon 으로 띄운 데몬이 startup 중 top-level 에러로 죽으면, 사용자가 -v를 줬는데도 데몬 로그에는 여전히 한 줄 메시지만 남습니다 — #396이 정확히 노리는 시나리오입니다.
범위는 좁습니다(서비스 레벨 verbose 로깅은 정상 전달되며, 영향은 데몬의 top-level uncaught 에러에 한정). 선택적/비차단 수정 제안: verbose 요청 시 자식 args에 --verbose도 함께 넘겨 자식의 top-level catch가 인식하도록 하면 됩니다.
Generated by Claude Code
There was a problem hiding this comment.
Fixed in 5049ad9: startDaemon now passes --verbose to the child CLI when the parent global verbose option is set, alongside --log-level verbose. Added start.test.ts coverage asserting the daemon child argv includes both flags.
moncher-dev
force-pushed
the
feat/396-verbose-errors
branch
from
5049ad9 to
e1ae786
Compare
TL;DR
-v/--verboseproduce stack traces andcausechains for top-level CLI and orchestrator failures.repo startorchestrator verbose logging, including daemon child startup diagnostics.변경 지점 다이어그램
여기부터 보세요
packages/core/src/observability/error-format.ts— shared terminal error formatter, verbose flag detection, and circular cause protection.packages/cli/src/index.ts— CLI binary catch now uses verbose formatting.packages/orchestrator/src/index.ts— orchestrator binary catch uses verbose formatting and accepts-v/--verbose.packages/cli/src/commands/start.ts— global CLI verbose flag maps to orchestrator verbose logs and daemon child diagnostics.위험 & 롤백
변경 파일
.changeset/verbose-error-causes.mdpackages/core/src/observability/error-format.tspackages/core/src/observability/error-format.test.tspackages/core/src/observability/index.tspackages/cli/src/index.tspackages/cli/src/commands/help.tspackages/cli/src/commands/__snapshots__/help.test.ts.snappackages/cli/src/commands/start.tspackages/cli/src/commands/start.test.tspackages/orchestrator/src/index.tspackages/orchestrator/src/index.test.tsEvidence
pnpm --filter @gh-symphony/core test -- error-format— passpnpm --filter @gh-symphony/cli test -- start— passpnpm exec vitest run packages/core/src/observability/error-format.test.ts packages/orchestrator/src/index.test.ts packages/cli/src/commands/start.test.ts packages/cli/src/commands/help.test.ts— passpnpm lint— passpnpm test— passpnpm typecheck— passpnpm build— pass.changeset/verbose-error-causes.mdIssues — Closed #396
Closes #396
머지 후/사람 확인
gh-symphony -v ...shows stack/cause details for an uncaught CLI failure.