Skip to content

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

Closed
oocheol wants to merge 2 commits into
lidge-jun:devfrom
oocheol:codex/route-policy-not-found-exit
Closed

oocheol wants to merge 2 commits into
lidge-jun:devfrom
oocheol:codex/route-policy-not-found-exit

Conversation

@oocheol

@oocheol oocheol commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • A syntactically valid ocx route policy show for a missing profile currently returns usage exit 2. Return the documented resource-not-found exit 4, keeping malformed input at exit 2 and upstream failures intact.
  • 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 f5a94dea180cfe4ffd8e566b2a4f1c81b7097f8f. 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-route-policy-not-found.test.ts tests/cli/route-explainability.test.ts: PASS, 18 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: Documented profile lookup and moved its exit-code contract into the CLI shell-contract section, avoiding the shared readiness-diagnostics edit. 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
    • Looking up a missing routing profile now returns exit code 4, distinguishing it from invalid or missing arguments, which return code 2. Upstream failures continue to return code 1.
  • Documentation
    • Updated the CLI reference in English and Korean with routing profile lookup status details.
  • Tests
    • Added coverage for successful lookups, missing profiles, invalid arguments, and upstream failures.

@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 8 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: 42715945-8ea6-4bd2-b5ba-a66d854eab5e

📥 Commits

Reviewing files that changed from the base of the PR and between f7ebbe4 and f5a94de.

📒 Files selected for processing (2)
  • src/cli/route-policy.ts
  • structure/runtime.md

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: 3629880d-a55f-445f-b0e0-fb2e2cee385d

📥 Commits

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

📒 Files selected for processing (7)
  • docs-site/src/content/docs/ko/reference/cli/agents.md
  • docs-site/src/content/docs/reference/cli/agents.md
  • scripts/test-layout/layout.json
  • src/cli/route-policy.ts
  • structure/runtime.md
  • tests/cli/cli-route-policy-not-found.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; 2 remain after this review.


📝 Walkthrough

Walkthrough

The route-policy show command now reports a missing profile as a runtime API error. Tests and CLI documentation cover exit statuses for missing profiles, invalid arguments, successful lookups, and upstream failures.

Changes

Route policy lookup

Layer / File(s) Summary
Missing-profile error handling and exit-status coverage
src/cli/route-policy.ts, tests/cli/cli-route-policy-not-found.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/reference/cli/agents.md, docs-site/src/content/docs/ko/reference/cli/agents.md, structure/runtime.md
When a profile is absent, show throws a 404 RuntimeApiError with a null payload. Tests cover missing and present profiles, missing input, and an upstream 503 response. The test-layout mappings and documentation include the new test and command exit statuses.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to f7ebb

Missing profiles now return exit code 4, while malformed input and upstream errors retain their existing mappings. No change-specific regression was established; the reported layout-test failures remain unattributed.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to f7ebb

The change corrects a missing-resource exit status without expanding access, privileges, or request scope. Invalid arguments and upstream failures retain their existing handling.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated change is confined to the failure classification visible to callers of route policy show. The inspected base-to-head behavior does not expand resource access, request authority, or attacker-controlled reachability.

Trust Boundaries and Controls

  • observed — The command retains its input validation and existing CLI-to-management-API request boundary. Only the local outcome for an absent profile changes; the inspected path adds no identity transition or privilege.

Resilience and Maintainability Implications

  • observed — Request failures remain separate from local profile absence: fetch failures become status 503, non-OK responses preserve their upstream status, and the shared mapper retains its existing exit-code rules.
🚥 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: returning a not-found result for missing routing profiles in the CLI.
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 f7ebbe40184bc2cd637bb231553a8a69fdd5f1d0. 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.

@oocheol

oocheol commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Please review the current head f5a94dea180cfe4ffd8e566b2a4f1c81b7097f8f. 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.

2 participants