Skip to content

fix(cli): keep no-wait reauth JSON parseable - #4603

Open
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/cli-reauth-json-20260914
Open

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/cli-reauth-json-20260914

Conversation

@luvs01

@luvs01 luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • ocx account main reauth --device --no-wait --json printed a JSON status object followed by a human follow up: line, making the complete stdout fail JSON parsing. Emit that hint only for human output; JSON callers retain flowId for polling.
  • Extend the existing native-account regression to parse all captured JSON stdout while retaining the human-output assertions. Document the output contract in its source owners and the eight CLI reference locales.

Verification

  • Bun 1.4.2, Windows, based on upstream dev commit 849f3c9ccfbb9672ff263322c2c56882a5616a0a.
  • Negative control: the new regression against unchanged upstream source fails with SyntaxError: JSON Parse error (10 pass, 1 fail).
  • bun test tests/cli/cli-native-profile.test.ts: 11 pass, 82 assertions after the fix.
  • bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff --check: pass.
  • cd docs-site && bun run build: pass, 441 pages. Reused existing dependencies whose frozen lockfile matches this checkout.
  • bun run test:changed resolved origin/dev at the base above: 381 pass, 5 fail, 1,614 assertions across 10 files. This was not a green run.
  • Rechecked only the five failures with the isolated test wrapper: the dashed-provider logout case passed alone. The two restore/eject timeouts, remote-connect status timeout, and preferred-runtime probe assertion reproduced with the production source restored to unchanged upstream 849f3c9; those test files are byte-identical to that baseline. The patch was restored and the checkout verified clean afterward. These cases do not enter native-main reauth; their root causes are outside this stdout change.
  • Exact-head CI 34818150800 completed successfully on this published head: all applicable ordinary gates and the supplemental macOS control suite passed. The existing macos-control lane includes ordinary gates and the supplemental macOS control suite, while intentionally skipping the supplemental full Windows matrix. A skip is not Windows proof. Author readiness is supported by this exact-head hosted run and the recorded local checks. The earlier local change-impact run remains a failure with the scoped baseline comparison above; it is not relabeled green. CodeRabbit completed its requested review without actionable code findings.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features

    • JSON output for device reauthentication now returns a single structured object, including the flowId, without human-readable follow-up text.
    • Use the returned flowId with the status command to monitor reauthentication progress.
  • Documentation

    • Updated CLI, runtime, configuration, operations, and integration documentation across supported languages to describe the JSON output behavior.
  • Tests

    • Added coverage confirming that device reauthentication JSON output can be parsed and includes status and device-flow details.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4fe88888-c13f-474c-ab79-7f16d783f7c1

📥 Commits

Reviewing files that changed from the base of the PR and between 849f3c9 and 6893dd9.

📒 Files selected for processing (14)
  • docs-site/src/content/docs/fr/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/ja/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/ko/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/ru/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/tr/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/zh-cn/reference/cli/providers-accounts.md
  • docs-site/src/content/docs/zh-tw/reference/cli/providers-accounts.md
  • src/cli/account-main.ts
  • structure/clients/claude-desktop.md
  • structure/config.md
  • structure/ops/docs-and-release.md
  • structure/runtime.md
  • tests/cli/cli-native-profile.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The CLI now emits only the JSON object for device reauthentication in --no-wait --json mode. Tests validate the structured output. Runtime, configuration, operations, client, and localized CLI documentation describe the output contract and status polling.

Changes

Native reauthentication JSON contract

Layer / File(s) Summary
CLI output behavior and validation
src/cli/account-main.ts, tests/cli/cli-native-profile.test.ts
At src/cli/account-main.ts:248, the human-readable follow up: instruction is suppressed when --json is used. The test at tests/cli/cli-native-profile.test.ts:198-206 parses stdout and checks flowId, status, verificationUrl, and deviceCode.
JSON contract documentation
structure/runtime.md, structure/config.md, structure/ops/docs-and-release.md, structure/clients/claude-desktop.md, docs-site/src/content/docs/reference/cli/providers-accounts.md, docs-site/src/content/docs/*/reference/cli/providers-accounts.md
Documentation describes the single JSON object, the omitted follow up: line, and use of flowId with reauth status --flow <id> --json.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 6893d

The change makes JSON-mode device reauthentication emit only the structured result while preserving human-readable output, with matching regression coverage and documentation. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (12 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: it fixes JSON parseability for the no-wait reauthentication CLI flow. It is concise and directly related to the code, tests, and documentation updates.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (12 skipped: 12 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 14, 2026
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@luvs01

luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

설명

이 PR은 CLI 출력 계약 버그 수정입니다. 핵심 코드는 src/cli/account-main.ts 한 줄이고, 같은 계약을 문서·구조 메모·회귀 테스트에 적습니다. draft 상태입니다.

현재 dev HEAD 는 99678c12e 입니다. ocx account main reauth --device --no-wait 경로는 account-main.ts 대략 246-249 줄에서 printStatus(...) 로 JSON(또는 사람이 읽는 상태)을 찍은 뒤, 바로 다음 줄에 사람용 follow up: ocx account main reauth status --flow ... 를 무조건 console.log 합니다. --json 을 켠 호출자도 같은 줄을 받습니다. 그래서 stdout 전체를 JSON.parse 하면 깨집니다. flowId 는 이미 JSON 객체 안에 있으므로, 기계 호출자는 follow-up 문장 없이도 폴링할 수 있습니다.

이 PR은 그 한 줄을 if (!wantsJson) 로 감쌉니다. --json 이면 stdout 에 JSON 객체 하나만 남고, 사람 모드에서는 예전처럼 follow-up 안내가 남습니다. 회귀는 tests/cli/cli-native-profile.test.ts 에서 --no-wait --json stdout 전체를 파싱해 status/deviceCode/userCode/verificationUrl 을 확인합니다. 기존 사람 모드 단언도 유지합니다.

문서 쪽은 영어 CLI 레퍼런스와 fr/ja/ko/ru/tr/zh-cn/zh-tw 여덟 로케일, 그리고 structure/runtime.md 에 “Native main reauth JSON output” 소유자 메모를 추가하고 clients/config/ops 쪽에서 그 앵커로 링크합니다. 범위는 작고 라우팅·pool·types/config 분할과 무관합니다.

현재 dev 방향과의 관계도 단순합니다. 최근 tip 은 cost-guard / affinity observability (#4602, #4592) 쪽이고, 이 PR은 native main reauth JSON 계약만 고칩니다. 중복 PR로 보이지 않습니다. 작성자 본문도 Exact-head CI 가 아직 pending 이고, test:changed 일부 실패는 upstream 재현이라 이 패치 밖이라고 적어 두었습니다. draft 체크리스트도 아직 비어 있습니다.

라인 248 (src/cli/account-main.ts, 현재 dev) - printStatus 다음에 follow-up 이 무조건 나갑니다. PR의 if (!wantsJson) 감싸기는 맞는 최소 수정입니다. wantsJson 은 같은 함수 앞쪽 flag(args, "--json") 이라 스코프도 맞습니다.

경로 tests/cli/cli-native-profile.test.ts - JSON 전체 파싱 회귀는 이 버그를 직접 잡습니다. 사람 모드에서 follow-up 문자열이 여전히 나오는지까지 같은 파일에서 유지하는 점도 좋습니다. --json 이지만 noWait 가 아닌 blocking 경로에 비슷한 이중 출력이 있는지는 이 PR 범위 밖이고, 현재 diff 도 no-wait 만 건드립니다.

경로 문서 8로케일 + structure/runtime.md - 계약 문장을 여러 곳에 복사합니다. 내용은 일치합니다. 다만 CI/로케일 빌드가 깨지면 문서 동기화 비용이 커지므로, Exact-head CI 초록을 draft 해제 조건으로 두는 게 맞습니다.

경로 draft 체크리스트 - 작성자가 “ready for review” 와 “All CI tests green” 을 아직 체크하지 않았습니다. 코드 자체는 작지만, 리뷰 대기열에 올리기 전에 CI 확인이 필요합니다.

메인테이너의 판단이 필요한 지점

  • draft 를 Exact-head CI 초록 후에만 ready 로 올릴지 (추천: 예)
  • blocking reauth 경로에도 비슷한 stdout 오염이 있는지 한 번만 스캔할지, 이번엔 no-wait 만 닫을지
  • 여덟 로케일 문장 동기화를 이 PR에 유지할지, 영어+structure 만 남기고 번역은 후속으로 쪼갤지 (지금은 이미 들어 있음)
  • types.ts/config.ts 분할과 무관하므로 close-don't-rebase 대상 아님

너의 추천

Exact-head CI 가 초록이면 draft 해제 후 dev 에 병합하세요. 코드 변경은 한 줄이고 JSON 호출자 계약을 올바르게 고칩니다. CI 가 빨강이면 문서/테스트만 손보고 코드 한 줄은 유지한 채 재시도하면 됩니다. 머지 전 blocking 경로에 같은 follow-up 이중 출력이 없는지 account-main.ts 를 한 번만 눈으로 확인하세요.

이 댓글은 grok-bot이 작성했습니다

@luvs01
luvs01 marked this pull request as ready for review September 14, 2026 07:53
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T07:55:21.354235Z 6893dd9 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants