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 2141613744c7a86adc7fcb0ba09973d58ad70836 includes dev snapshot aa91958e3b050084e1edc07dcd66b05ef6eac604. The actual branch connected to this PR was read back after publication. This section replaces older head and validation claims; earlier CI results are historical evidence only.

  • CLI capability/parity run: 90 pass, with one unrelated remote-connect case exceeding its default five-second timeout. That exact case passed separately with a 60-second test budget (1 test, 3 assertions, about 28 seconds). The initial timeout remains recorded; it is not a claim that the original command was entirely green.
  • Typecheck and structure ownership checks passed on the integrated source. Tests were scoped to the changed contracts; this does not claim that every platform matrix was repeated locally. New file-size ratchet conflicts, where present, were fixed by extracting coherent source or test modules, without raising the baseline or removing assertions.
  • Full hosted CI was not multiplied across unchanged implementation merges. The focused integration evidence above and applicable repository checks define this author-validation scope; skipped or unexecuted matrices are not passing results.

Review readiness checklist

The local-validation box refers to the explicit scope and exceptions above. Author readiness remains separate from approval, merge permission and future review findings.

  • 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.
  • 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: 9094f415-6420-4537-8160-e82ec66e5d7b

📥 Commits

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

📒 Files selected for processing (2)
  • structure/config.md
  • structure/runtime.md

Included review availability: Your plan provides up to 10 included reviews per hour; 7 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. Config documentation adds spacing before the “Config surface” heading.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 21416

The PR aligns restart messaging, documentation, and focused confirmation/output tests without changing request routing or restart behavior.

🚥 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. (2 skipped: 2 … 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 the CLI messaging to document the full Codex desktop restart scope.
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. (2 skipped: 2 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
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