Skip to content

fix(cli): reject ignored capabilities arguments - #6429

Closed
oocheol wants to merge 2 commits into
lidge-jun:devfrom
oocheol:codex/capabilities-reject-unknown-arguments
Closed

oocheol wants to merge 2 commits into
lidge-jun:devfrom
oocheol:codex/capabilities-reject-unknown-arguments

Conversation

@oocheol

@oocheol oocheol commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • A misspelled capabilities filter or repeated flag currently exits successfully and emits an unfiltered index. Reject leftover arguments with exit 64 before rendering; preserve valid filtering and unmatched-route exit 4.
  • Add focused regression coverage and update the source-owner contract plus English/Korean user documentation.
  • Independent branch from dev 8b23fe340a53799aac5ad7654582a75df7ade7c8; this PR contains only this change.

Verification

Validated on Windows with Bun 1.4.0 at current head ed898c241f76b586061dcba54a2ba51158584b10. The layout and file-size guards run from a fresh worktree with the default test budget; no guard, fixture directory, baseline cap, or timeout was changed.

  • bun run scripts/test.ts tests/cli/cli-capabilities-arguments.test.ts tests/cli/cli-capabilities.test.ts: PASS, 26 pass / 0 fail.
  • bun node_modules/typescript/bin/tsc --noEmit: PASS, exit 0.
  • bun scripts/structure-ssot.ts: PASS, exit 0.
  • bun scripts/privacy-scan.ts: PASS, exit 0.
  • bun run scripts/test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: PASS, 27 pass / 0 fail.

The first validation from the original Windows checkout failed on a pre-existing EPERM fixture directory and, in some runs, a five-second subprocess timeout. Those attempts remain historical failures; the fresh-worktree current-head guard result above replaces neither the failure record nor a required CI run. The first cold file-size scan for #6429 also exceeded five seconds, then passed at the same head on the same default budget. No inaccessible fixture was deleted and no test budget was raised.

Integration validation used actual local merges of all ten PR heads onto dev 03ed9a3b1c1535d25f8f47c3d6bf38db8ffc1fc9 (local-only assembled commit dd7d1fe254c3fc86144a2c3cce0ae483332b55bb). Owner-contract edits in runtime/config/usage now merge independently; shared observe.ts consumers merge without a source conflict. Additive EN/KO document-append conflicts and test-map insertion conflicts were reconciled by retaining both sections/registrations, not choosing one PR over another.

  • The combined 24-file focused regression set: 331 pass / 3 platform skips / 0 fail.
  • Combined layout and file-size guards: 27 pass / 0 fail with the default budget.
  • Combined typecheck, structure SSOT, privacy scan, generated operating surface check, and docs-site build: PASS.

Full bun run test and bun run test:changed remain unrun: nine small independently validated revisions plus a combined 24-file run share one Windows machine, so repeated full repository runs on each draft would be disproportionate. The explicit regression files cover the touched behavior and source-oracle consumers; broader platform/integration coverage remains for exact-head CI. Updated-head CI and automated review must be confirmed separately; older-head results, queued jobs, skipped jobs, and approval-pending runs are not passing evidence.

Review response: Added a docstring to the capabilities command entry point. Added function documentation addresses the warning in source; the remote coverage percentage is not claimed as rerun. All descriptions retain only the gate-managed canonical readiness checklist.

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 credential values, authentication policy, dependency installation, or workflow permissions are changed.

Review readiness checklist

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

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • The ocx capabilities command now rejects unknown or repeated flags, extra arguments, and blank --route values with exit code 64.
    • A valid route with no matching capabilities continues to return exit code 4.
  • Documentation
    • Updated the CLI reference to describe argument validation and exit codes.

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

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b0036f32-b13e-4146-949c-6eb9c2dafda7

📥 Commits

Reviewing files that changed from the base of the PR and between 0d6acad and ed898c2.

📒 Files selected for processing (1)
  • src/cli/capabilities-command.ts

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: 5cde6835-d7e0-464a-a508-2c29fc5d3ec7

📥 Commits

Reviewing files that changed from the base of the PR and between 3d77e3d and 0d6acad.

📒 Files selected for processing (7)
  • docs-site/src/content/docs/ko/reference/cli.md
  • docs-site/src/content/docs/reference/cli.md
  • scripts/test-layout/layout.json
  • src/cli/capabilities-command.ts
  • structure/runtime.md
  • tests/cli/cli-capabilities-arguments.test.ts
  • tests/fixtures/test-layout-expected.json

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


📝 Walkthrough

Walkthrough

The ocx capabilities command now rejects leftover arguments and blank --route values with exit code 64. Tests and documentation cover invalid arguments and valid routes with no declared capabilities, which return exit code 4.

Changes

Capabilities argument validation

Layer / File(s) Summary
Argument validation and tests
src/cli/capabilities-command.ts, tests/cli/cli-capabilities-arguments.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
The command rejects leftover arguments and routes that are empty or whitespace-only. Tests check invalid-usage responses and the exit code and output for a valid route with no matches. The test-layout mappings include the new test.
Validation documentation
docs-site/src/content/docs/reference/cli.md, docs-site/src/content/docs/ko/reference/cli.md, structure/runtime.md
The CLI references and runtime documentation describe argument validation, exit code 64 for invalid usage, and exit code 4 for a valid route with no declared capabilities.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 0d6ac

The capabilities command distinguishes invalid usage from a valid query with no matches. No actionable merge-blocking issue is established; complete the planned CI validation before merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 0d6ac

The change affects 5 systems.

Changed systems: src, docs-site, tests, scripts, structure

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — docs-site (service) was modified; 2 changed files map to changed impact.
  • observed — tests (service) was modified; 2 changed files map to changed impact.
  • observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in docs-site/src/content/docs/ko/reference/cli.md: Capability argument validation 절을 추가해 ocx capabilities가 알 수 없는 인수, 반복된 플래그, 빈 --route 값을 종료 코드 64로 거부하고, 유효한 경로에 등록된 기능이 없을 때 종료 코드 4를 반환한다고 문서화했습니다.
  • observed — Modified behavior in docs-site/src/content/docs/reference/cli.md: Adds a capability-validation section documenting exit 64 for unknown arguments, repeated flags, or blank --route, and exit 4 when a valid route has no declared capability.
  • observed — Modified behavior in scripts/test-layout/layout.json: Added the cli-capabilities-arguments.test.ts entry under the cli category.
  • observed — Modified behavior in src/cli/capabilities-command.ts: runCapabilities now rejects leftover arguments and routes whose trimmed value is empty, and prints the general capabilities usage message. Previously, it rejected only an exactly empty route value and printed route-specific usage.
🚥 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. (5 skipped: 5 … 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: rejecting previously ignored arguments for the capabilities CLI command.
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. (5 skipped: 5 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@oocheol

oocheol commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Please review the current head 0d6acad40a8f76bf4d2fb37eead8a4e5e7256a7c. The initial automatic review was skipped while the PR is in draft.

@coderabbitai review

@codex review

@coderabbitai

coderabbitai Bot commented Oct 2, 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.

@Ingwannu

Ingwannu commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Batch intake for the ten independent drafts #6429 and #6431–#6439: I read their complete changed-file patches, including both test-layout registrations and EN/KO contract additions, and authorized the existing read-only GitHub-hosted CI at each immutable current head. No workflow/dependency/install changes are included in these diffs. That authorizes execution only, not PR approval or a readiness waiver. The shared Windows layout-guard failures in Verification remain recorded as failures; their attribution to EPERM/checkout contamination needs a clean exact-head result, not deletion of the guard or an increased budget. Please keep the drafts until the corrected/current validation is established, and keep just one canonical readiness checklist per description (currently duplicated). Before integration, reconcile the overlapping single-line edits in runtime/config/usage structure docs and the shared observe.ts consumers; green standalone heads do not establish their combined tree. I have not run ten full local suites or a security scan, and do not claim the author-local red/green results as independently rerun tests.

@oocheol

oocheol commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Please review the current head ed898c241f76b586061dcba54a2ba51158584b10. The initial automatic review was skipped while the PR is in draft.

@coderabbitai review

@codex review

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

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 added a commit that referenced this pull request Oct 2, 2026
…6431 #6432 #6433 #6434 #6435 #6437 #6438) (#6447)

* fix(cli): reject ignored capabilities arguments (carry #6429)

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

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

* fix(cli): reject ambiguous and unsafe integer options (carry #6431)

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

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

* fix(cli): accept UTF-8 BOM in imported config files (carry #6432)

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

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

* fix(cli): validate setup ports before saving config (carry #6433)

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

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

* fix(cli): support JSON output on the default alias command (carry #6434)

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

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

* feat(cli): show request IDs in human log output (carry #6435)

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

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

* fix(cli): return not-found for missing routing profiles (carry #6437)

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

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

* fix(responses): keep usable fallback upstream error messages (carry #6438)

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

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

* fix(cli): ask for the setup port again instead of abandoning ocx init

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>

* test(responses): pin recovery consumers of the upstream message fallback

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>

* docs(cli): separate the carried reference sections

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

* chore: keep the lane plan record out of the carry

---------

Co-authored-by: 정우철 <oocheol@naver.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks @oocheol — this landed on dev through the batch carry #6447 (one commit per source PR, Co-authored-by credit). Closing as superseded.

@lidge-jun lidge-jun closed this Oct 2, 2026
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.

3 participants