Skip to content

fix(cli): describe the full Codex desktop restart scope - #4610

Open
luvs01 wants to merge 3 commits into
lidge-jun:devfrom
luvs01:agent/restart-scope-wording-20260914
Open

luvs01 wants to merge 3 commits into
lidge-jun:devfrom
luvs01:agent/restart-scope-wording-20260914

Conversation

@luvs01

@luvs01 luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Make the CLI restart message describe the full Codex desktop restart scope so it matches the action requested and performed. Translations retain the same command and behavior.

Current author verification

  • Published head: dabe8f396c4e9e363c25a0b58ff30a3faccfd073.
  • Full ordinary CI run 34959259158 completed successfully on this exact published head; the run head SHA was verified. This is hosted execution, not a claim that the full matrix was repeated locally.
  • The branch remains within the repository's current allowed dev-drift window. No tip-only rebase was performed.
  • All known applicable inline and review-body findings have been addressed. Author implementation, current scoped validation and known review findings are complete. Maintainer approval and merge remain separate decisions.

Review readiness checklist

The validation checkbox refers to the explicit scope above. Historical run IDs and prior local results are not represented as new-head full-suite execution.

  • 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

    • The Codex restart command now fully quits and relaunches the desktop app while restarting its app-servers.
    • Confirmation remains required unless explicitly bypassed, and no restart request is sent without confirmation.
    • JSON output includes the complete server result, while status and failure messages clearly describe the desktop-app relaunch.
    • Added coverage for confirmation, restart requests, and human-readable and JSON results.
  • Documentation

    • Updated CLI, runtime, operations, and Claude Desktop documentation to clarify the restart scope and behavior.

@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: c119dead-3cdf-4b28-b247-127a63b99eeb

📥 Commits

Reviewing files that changed from the base of the PR and between 2141613 and dabe8f3.

📒 Files selected for processing (4)
  • structure/clients/claude-desktop.md
  • structure/config.md
  • structure/ops/docs-and-release.md
  • structure/runtime.md

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


📝 Walkthrough

Walkthrough

The CLI now describes and reports a full Codex desktop-app relaunch with app-server restart. Tests cover confirmation, the restart request, and human-readable and JSON results. Runtime and integration documentation define the same scope.

Changes

Codex restart scope

Layer / File(s) Summary
CLI restart behavior
src/cli/capabilities.ts, src/cli/system-command.ts, tests/cli/cli-headless-parity.test.ts, skills/ocx/references/01_management_surface.md
The CLI text and messages now describe fully quitting and relaunching the Codex desktop app and restarting its app-servers. Tests verify confirmation requirements, the unchanged POST request, and skipped desktop restart results in human-readable and JSON output.
Restart scope documentation
structure/runtime.md, structure/clients/claude-desktop.md, structure/ops/docs-and-release.md, structure/config.md
Runtime documentation defines the confirmation requirement and preservation of the full --json server result. Related documents reference the restart scope contract.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to dabe8

This change aligns restart messaging and documentation while preserving confirmation and server-result behavior, with no unresolved merge-blocking risk identified.

🚥 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 3 files. (4 skipped: 4 … 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 and concisely describes the main change: updating CLI messaging to document the full Codex desktop-app and app-server restart scope. This matches the changes in the CLI implementatio…
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 3 files. (4 skipped: 4 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 github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • missing_regression_test — Behavior changed under src/ or gui/src/ without a test change. Add focused coverage or obtain test-exception-approved.

@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 has been marked 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

@codex review

@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.

@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-15T01:19:20.751722Z 2141613 New commits
ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00f4330dd7

ℹ️ 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".

Comment thread src/cli/system-command.ts
@github-actions github-actions Bot removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 14, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

이 PR은 동작이 아니라 말이 동작을 따라가게 만드는 수정이다. ocx system codex-restart는 이미 Codex 데스크톱 앱을 완전히 종료·재실행하고 app-server도 다시 띄운다. 그런데 capability 요약, --yes 설명, 확인 문구, 사람용 결과, 관리 레퍼런스(skills/ocx/references/01_management_surface.md)는 오랫동안 “app-server만 재시작”처럼 적혀 있었다. 운영자나 에이전트가 짧은 표면만 읽으면 중단 범위를 과소평가한다. 특히 에이전트가 서브커맨드를 추측해 --yes 없이/있이 호출할 때, 확인 문장이 실제 충격을 말해야 한다.

지금 dev(053cfda11) 기준 cost-guard 열차(#4546/#4609)와는 겹치지 않는 CLI/문서 정합 작업이다. 요청 라우팅, 확인 강제, 실제 restart 동작은 안 건드린다고 본문에 명시돼 있다. 바꾸는 파일은 src/cli/capabilities.ts, src/cli/system-command.ts, 관리 레퍼런스, structure/runtime.md 등 계약 문서 링크, 그리고 tests/cli/cli-headless-parity.test.ts에 확인 문구 테스트다. Windows 데스크톱 재시작 계열(#2292/#4564 쪽)과 같은 주제의 “표면이 계약을 숨기지 않기” 슬라이스다.

왜 우선순위가 중간인가. 틀린 요약은 운영 사고를 낳을 수 있어서 방치하면 안 되지만, 전송 증폭·자격 증명·풀 라우팅 같은 비용/안정성 열차보다는 급하지 않다. 다만 메시지만 고치고 테스트로 “확인 전에 요청이 나가지 않는다 / 문구가 데스크톱 중단을 말한다”를 고정한 점은 좋다. JSON 결과는 서버 결과(스킵·거절된 데스크톱 결과 포함)를 유지하고, 사람용은 여전히 requested라고 말하는 구분도 본문에 남아 있어 과장하지 않는다.

라인/경로 기준으로 보면 큰 논리 버그는 안 보인다. 다만 문서가 여러 structure 파일에 같은 한 줄 링크를 반복 추가한다. 계약의 단일 출처는 structure/runtime.md의 CLI restart scope 섹션이어야 하고, 다른 파일은 링크만 두는 지금 모양이 맞다. capabilities 요약과 system-command 확인 문구·레퍼런스 테이블이 서로 같은 동사(“fully quits and relaunches” / “desktop app and app-servers”)를 쓰는지 한 번만 맞춰 보면 된다.

경로 src/cli/capabilities.ts / system-command.ts - 요약·--yes·확인 에러 문구가 모두 “desktop + app-servers”로 정렬됐는지, 예전 “app-server only” 문자열이 테스트/다른 도움말에 남아 있지 않은지 검색으로 확인하면 좋다.
경로 tests/cli/cli-headless-parity.test.ts - 확인 전에 requests가 비는지, 에러 문구에 데스크톱 중단이 들어가는지만 본다. JSON/--yes 성공 경로의 human “requested” 문구와 management reference 테이블까지 한 테스트에 묶을지는 취향이다. 없어도 머지를 막을 정도는 아니다.
경로 structure/*.md 반복 링크 - 내용은 얇다. 링크 타깃 섹션 제목이 바뀌면 여러 파일이 한꺼번에 깨질 수 있으니, 섹션 anchor 이름만 안정적으로 유지하면 된다.
경로 실제 restart 구현 - 이 PR 범위 밖(본문도 그렇게 말함). 메시지만 맞고 구현이 여전히 부분 재시작이면 오히려 더 위험하니, 리뷰어는 “문구 = 기존 풀 데스크톱 계약”인지만 보면 된다.

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

너의 추천
CI가 통과하면 독립 머지 후보. 동작 변경이 없고 테스트가 확인 표면을 잠근다. 머지 전에 리포 전체에서 “Restart the Codex app-server” 같은 구문 잔여만 rg로 지워 두면 충분하다. cost-guard(#4611)나 types/config 분할과 충돌하지 않으니 닫을 이유도 없다. 우선순위 열차 밖이므로 #4611 hygiene 수리보다 먼저 급하지는 않다.

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

@luvs01
luvs01 marked this pull request as ready for review September 14, 2026 09:54
@github-actions
github-actions Bot marked this pull request as draft September 15, 2026 01:17
@github-actions
github-actions Bot marked this pull request as ready for review September 15, 2026 02:02

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on exact head 2141613744c7a86adc7fcb0ba09973d58ad70836. The change corrects the CLI and management reference to disclose the real desktop interruption, retains the --yes gate, keeps human output at the honest “requested” boundary, and preserves the complete server result in JSON. The focused tests prove no request is sent without confirmation and distinguish human/JSON outcomes. The review thread is resolved and exact-head CI is green. This is text/CLI-surface work, so I am leaving the merge decision to the owner rather than merging it myself.

…start-scope-wording-20260914

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun
lidge-jun force-pushed the agent/restart-scope-wording-20260914 branch from 2141613 to dabe8f3 Compare September 15, 2026 10:41
@github-actions
github-actions Bot marked this pull request as draft September 15, 2026 10:41
@github-actions
github-actions Bot marked this pull request as ready for review September 15, 2026 16:27
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.

3 participants