Skip to content

feat(codex): initialize account picker selectors - #1152

Open
chrisae9 wants to merge 2 commits into
lidge-jun:devfrom
chrisae9:split/1019-02-selector-initialization
Open

feat(codex): initialize account picker selectors#1152
chrisae9 wants to merge 2 commits into
lidge-jun:devfrom
chrisae9:split/1019-02-selector-initialization

Conversation

@chrisae9

@chrisae9 chrisae9 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a pure selector initializer that runs only after an explicit account-picker opt-in and only when the selector map is absent or empty
  • build the complete selector map before one assignment, preserving non-empty user maps by identity and insertion order
  • reserve the canonical policy namespace and existing slash-qualified routing-profile prefixes during default and append allocation
  • preserve exact-case account/profile prefix matching while keeping provider and reserved namespace collision checks normalized
  • reject routing-profile/account-selector collisions before mutation, persistence, catalog convergence, or reference migration, without exposing private account identifiers
  • document the selector namespace ownership contract in the internal SOT and provider references

This is the second slice requested in #1019, following #1096. It intentionally contains only selector initialization and collision foundations. Catalog convergence and management API/dashboard activation remain in later slices.

Part of #425.

Verification

  • based on current dev@b39eecfc
  • focused feature suite: 189 passed across namespace, config, routing-profile, and management regression coverage
  • final changed-area smoke: 35 passed
  • bun run typecheck
  • bun run privacy:scan
  • bun test tests/repo-hygiene.test.ts
  • node --test .github/scripts/pr-sponsored-surface.test.cjs
  • git diff --check
  • full repository suite: 9,464 passed, 10 skipped, with one unrelated timing failure in native-profile-drain-server.test.ts; that complete file then passed 3/3 in isolation
  • independent design, security/privacy, and repository-conventions reviews found no remaining blockers

Review readiness checklist

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

  • 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

    • Improved automatic account-selector namespace generation to avoid conflicts with providers, combos, routing policies, and routing-profile aliases.
    • Added opt-in initialization for default account selectors while preserving existing configuration.
  • Bug Fixes

    • Prevented invalid namespace collisions, including policy and case-insensitive variants.
    • Routing-profile aliases that conflict with account-selector namespaces are now rejected without changing configuration.
  • Documentation

    • Updated provider configuration guidance across supported languages to clarify namespace collision rules.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c427ff1a-074d-4d0c-bc1c-87aedf4a212e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Codex account selector validation now reserves the policy namespace and routing-profile alias prefixes. Default selector generation and initialization avoid these namespaces. Documentation and tests cover validation, preservation, atomic failure, and routing-profile management operations.

Changes

Namespace validation and allocation

Layer / File(s) Summary
Routing namespace contracts and validation
src/routing/profile-namespace.ts, src/routing/profile.ts, src/config.ts, tests/config.test.ts, docs-site/src/content/docs/.../configuration/providers.md
The policy namespace and routing-profile alias prefixes now participate in selector collision checks. Documentation and tests cover case-insensitive policy collisions.
Account selector generation and initialization
src/codex/account-namespaces.ts, tests/codex-account-namespaces.test.ts, structure/03_catalog-and-subagents.md
Generated selectors avoid routing-profile prefixes and reserved namespaces. Initialization is opt-in, preserves existing non-empty maps, and fails atomically when allocation is unavailable.
Routing-profile management validation coverage
tests/routing-profile-management-editor.test.ts
Create and update operations reject conflicting aliases before mutation, persistence, refresh, or reference migration. Tests verify that private account data and derived log labels are not exposed.

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

Possibly related PRs

Suggested reviewers: ingwannu, wibias, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. 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 clearly and concisely describes the main change: initializing Codex account-picker selectors.
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.
✨ 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.

@Wibias

Wibias commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Looks good from my side overall. I didn’t find a code-level blocker.

Before I approve, please:

  • rebase feat(codex): initialize account picker selectors #1152 onto the latest dev and rerun CI on the rebased head
  • clear the CodeRabbit docstring coverage warning as well (66.67% vs 80%). It looks like a small documentation-only cleanup, so we might as well leave the PR fully clean.

If the rebased diff stays clean and CI passes, I’m good to approve.

@chrisae9
chrisae9 marked this pull request as draft August 7, 2026 07:27
@chrisae9
chrisae9 force-pushed the split/1019-02-selector-initialization branch from 813bd7f to e73ed2b Compare August 7, 2026 07:54
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

✅ 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.
CodeRabbit/Codex review was requested via the review-ready label. If no review appears, comment @coderabbitai review to request one.
Maintainers notified: @lidge-jun @Ingwannu @Wibias

@chrisae9
chrisae9 marked this pull request as ready for review August 7, 2026 07:55
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@github-actions
github-actions Bot marked this pull request as draft August 7, 2026 07:55
@github-actions
github-actions Bot marked this pull request as ready for review August 7, 2026 08:07

chrisae9 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Rebased and pushed. The current head is 440432b5 on dev 44dce33.

I also added JSDoc for the changed namespace helper and the new public policy namespace constant to clear the 66.67% coverage warning.

Local checks on this head are green: all four isolated Linux shards (9,636 pass / 11 skip), GUI tests (646 pass), focused selector tests (170 pass), typecheck, privacy scan, GUI build, release-script build, CLI smoke, hygiene, and sponsorship checks.

The PR is back Ready with the 4/4 checklist complete. GitHub's fork workflows still need maintainer approval before they can run:

CodeRabbit is also re-reviewing the final head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants