Skip to content

fix(cli): carry oocheol's CLI validation and diagnostics fixes (#6429 #6431 #6432 #6433 #6434 #6435 #6437 #6438) - #6447

Merged
lidge-jun merged 12 commits into
devfrom
codex/carry-oocheol-cli-batch
Oct 2, 2026
Merged

lidge-jun merged 12 commits into
devfrom
codex/carry-oocheol-cli-batch

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Summary

Carries eight small CLI and diagnostics fixes by @oocheol into one PR so they land together on current dev. Each source PR is one commit authored by oocheol; three small follow-up commits sit on top.

Source PR Change
#6429 ocx capabilities rejects unknown arguments, repeated flags and blank --route with exit 64, so a mistyped filter no longer prints the full list and exits 0.
#6431 Shared integer options reject blank, hexadecimal, exponent, fractional and unsafe values before any request is sent.
#6432 ocx config validate and ocx config import accept UTF-8 JSON with a leading BOM (Windows PowerShell exports).
#6433 ocx init validates the setup port (1–65535, decimal) instead of silently truncating 10100oops to 10100.
#6434 ocx alias --json works without an explicit list action.
#6435 Human ocx logs output shows id=<request-id> for use with ocx logs explain.
#6437 ocx route policy show <id> exits 4 (not found) for a missing profile instead of 2 (usage).
#6438 Upstream error parsing uses the first nonblank message, so an empty error.message no longer hides a usable fallback.

The follow-up commits change one thing in #6433 and add coverage for #6438:

  • fix(cli): validate setup ports before saving config #6433 amendment. In the source PR, an invalid port ended the whole wizard without saving, so one typo threw away every earlier answer. Now setup reports the error and asks for the port again. EOF and SIGINT at the repeated prompt still cancel without saving (exit 1 / 130). The wizard tests cover all three paths. The docs (en, ko) and structure/runtime.md describe the new behavior.
  • fix(responses): keep usable fallback upstream error messages #6438 consumer tests. The message helper also feeds encrypted-output resend detection (core-opaque-recovery.ts) and quota alternate-account detection (core-codex-account.ts). Behind a blank primary message, both now see the fallback text. That is the intent of the fix, and the new tests pin it, including that a nonblank primary message still wins. Relay code selection uses its own candidate scan and is unchanged.
  • Docs. The appended reference sections in reference/cli.md and reference/cli/agents.md (en, ko) conflicted at the same insertion point. Every section is kept exactly once, with normal spacing.

#6436 (ocx usage --top) is not included because it needs a product decision first.

Co-authored-by: 정우철 oocheol@naver.com

Verification

  • The user instructed that no tests run locally, so none were run for this PR. Hosted CI on the final head is the only verification.
  • Review: an independent reviewer compared each carried commit with its source PR diff and checked the re-prompt loop and the docs and structure wording.

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. (No auth, credential, workflow or release paths are touched.)

Summary by CodeRabbit

  • Improvements
    • CLI commands now handle integer options and setup ports with clearer validation, and accept UTF-8 JSON configuration files with a leading BOM.
    • Alias listing supports --json in different argument positions. Capability and route-policy commands provide clearer handling of invalid arguments and missing profiles.
    • Human-readable logs show valid request IDs for history lookup; JSON and JSONL output remain unchanged.
    • Error messages use the first available nonblank upstream diagnostic.
  • Documentation
    • Updated CLI and configuration references to describe these behaviors.

oocheol and others added 12 commits October 2, 2026 19:43
Carries #6429 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6431 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6432 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6433 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6434 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6435 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Carries #6437 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
…6438)

Carries #6438 by @oocheol onto current dev as part of the oocheol CLI batch.

Co-authored-by: 정우철 <oocheol@naver.com>
Amends the #6433 carry: an invalid interactive port now reports the error and re-asks, so a typo no longer discards every earlier answer. EOF and SIGINT at the repeated prompt still cancel without saving (exit 1 / 130); the wizard tests cover all three paths.

Co-authored-by: 정우철 <oocheol@naver.com>
Follow-up to the #6438 carry: encrypted-output resend and quota alternate-account detection now see a fallback message hidden behind a blank primary one; a nonblank primary still wins.

Co-authored-by: 정우철 <oocheol@naver.com>
Co-authored-by: 정우철 <oocheol@naver.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 2, 2026 10:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 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-10-02T10:54:28.180452Z 283bc4a PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (2)
src/AGENTS.md — auto-discovered
structure/AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f9d82fca-b26c-45f2-b9a1-7ddabb738d56

📥 Commits

Reviewing files that changed from the base of the PR and between a300b57 and 283bc4a.

📒 Files selected for processing (28)
  • docs-site/src/content/docs/ko/reference/cli.md
  • docs-site/src/content/docs/ko/reference/cli/agents.md
  • docs-site/src/content/docs/ko/reference/cli/lifecycle.md
  • docs-site/src/content/docs/reference/cli.md
  • docs-site/src/content/docs/reference/cli/agents.md
  • docs-site/src/content/docs/reference/cli/lifecycle.md
  • scripts/test-layout/layout.json
  • src/cli/alias.ts
  • src/cli/capabilities-command.ts
  • src/cli/config-command.ts
  • src/cli/init.ts
  • src/cli/observe.ts
  • src/cli/route-policy.ts
  • src/cli/runtime-api.ts
  • src/lib/errors.ts
  • structure/config.md
  • structure/dashboard-and-usage.md
  • structure/runtime.md
  • structure/transports/responses-wire-shapes.md
  • tests/cli/cli-alias-json.test.ts
  • tests/cli/cli-capabilities-arguments.test.ts
  • tests/cli/cli-config-bom.test.ts
  • tests/cli/cli-init-port.test.ts
  • tests/cli/cli-integer-options.test.ts
  • tests/cli/cli-logs-request-id.test.ts
  • tests/cli/cli-route-policy-not-found.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/lib/upstream-error-message-fallback.test.ts

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


📝 Walkthrough

Walkthrough

The CLI now validates arguments, config input, integer options, and initialization ports more strictly. Human-readable logs include safe request IDs. Missing route-policy profiles and upstream error messages use updated diagnostic handling. Tests and reference documentation cover these changes.

Changes

CLI contracts

Layer / File(s) Summary
Argument handling
src/cli/alias.ts, src/cli/capabilities-command.ts, tests/cli/cli-alias-json.test.ts, tests/cli/cli-capabilities-arguments.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/cli.md, docs-site/src/content/docs/ko/reference/cli.md, structure/runtime.md
The alias command accepts --json before or after an explicit action. The capabilities command rejects unknown or repeated arguments, leftover arguments, and blank routes with exit code 64; a valid route without capabilities returns 4.
Config and integer input validation
src/cli/config-command.ts, src/cli/runtime-api.ts, tests/cli/cli-config-bom.test.ts, tests/cli/cli-integer-options.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/cli.md, docs-site/src/content/docs/ko/reference/cli.md, structure/config.md
Config validation and import remove one leading UTF-8 BOM from file or standard-input JSON. Integer options accept safe decimal integers with _ or , separators and enforce their configured minimum.
Initialization port validation
src/cli/init.ts, tests/cli/cli-init-port.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/cli/lifecycle.md, docs-site/src/content/docs/ko/reference/cli/lifecycle.md, structure/runtime.md
Empty port input selects 10100. Other input must be a decimal integer from 1 through 65535. Invalid input displays an error and repeats the prompt; EOF and SIGINT cancel without saving.
Log and error diagnostics
src/cli/observe.ts, src/cli/route-policy.ts, src/lib/errors.ts, tests/cli/cli-logs-request-id.test.ts, tests/cli/cli-route-policy-not-found.test.ts, tests/lib/upstream-error-message-fallback.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout.json, docs-site/src/content/docs/reference/cli/agents.md, docs-site/src/content/docs/ko/reference/cli/agents.md, structure/dashboard-and-usage.md, structure/runtime.md, structure/transports/responses-wire-shapes.md
Human-readable log rows include nonempty request IDs without control characters; JSON and JSONL output remains unchanged. A missing route-policy profile returns status 4. Upstream error extraction skips blank or malformed candidates and selects the first nonblank string in priority order.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 283bc

The setup prompt’s whitespace behavior matches its tested behavior, and no issue identified here prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 283bc

The reviewed changes tighten input handling, and the inspected configuration and recovery paths retain important ownership and replay controls. No new security weakness was established, but incomplete historical and recovery-path verification prevents a minimal-risk assessment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected security-relevant scope includes local configuration creation and provider responses that influence recovery, account cooldown, and alternate-account selection. The CLI changes continue to invoke the existing management-request mechanism; the fallback change does not itself supply a destination or credential.

Trust Boundaries and Controls

  • observed — Opaque recovery arms its request-local guard before changing the parsed payload, strips rejected encrypted content and foreign reasoning-item identity, cancels the prior response body, and records rejection scope only after a successful rebuild response. The adapter caller invalidates its same-target request cache before rebuilding.
  • observed — Alternate-account recovery refuses fixed-account or same-account-only moves and cross-account requests carrying uploaded files. Resolution receives the existing admission and authentication policy. Before rebuilding for another account, the path rebinds replay scope and scrubs account-bound conversation state; quota recovery makes one send per helper invocation.

Resilience and Maintainability Implications

  • observed — When an execution budget is supplied, alternate-account recovery reserves a single-use permit and releases it when no usable alternate is resolved. The permitted-send path also checks pool recovery admission, releases unused probe leases on refusal, and releases request-body observation in a finally block. This establishes concrete failure-containment controls, not proof that every caller initializes the budget.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 16 files. (12 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the pull request as a set of CLI validation and diagnostics fixes. It is specific, concise, and includes the source issue references.
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.
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 2, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration decision (dev, admin bypass per MAINTAINERS.md): merging at exact head 283bc4a.

@lidge-jun lidge-jun mentioned this pull request Oct 2, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants