refactor: extract behavior settings panel

[ndycode]

Summary

• extract the behavior-settings interaction flow from settings-hub.ts into a dedicated panel module
• keep the current settings behavior intact while continuing the settings-hub split in small reviewable slices

What Changed

• added lib/codex-manager/behavior-settings-panel.ts containing the behavior panel prompt flow and its local action model
• updated lib/codex-manager/settings-hub.ts so promptBehaviorSettings(...) delegates to the extracted panel module
• preserved the existing save, reset, toggle, and TTL-cycling behavior by passing the current helpers and constants into the new module

Validation

• npm run test -- test/codex-manager-cli.test.ts
• npm run lint
• npm run typecheck
• npm run build

Risk and Rollback

• Risk level: low
• Rollback plan: revert 66d07c6 to restore the inline behavior panel implementation in settings-hub.ts

Additional Notes

• this continues the settings-hub split after the manager command train, still in the same isolated worktree and stacked review flow

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr extracts the behavior settings interaction loop from settings-hub.ts into a dedicated behavior-settings-panel.ts module, continuing the ongoing settings-hub split. the extracted panel accepts all formerly-closed-over helpers and constants via an explicit BehaviorSettingsPanelDeps interface — same pattern as theme-settings-panel.ts — which keeps settings-hub.ts's promptBehaviorSettings as a thin one-call delegator.

• the logic extraction is a clean 1:1 lift; no behavioral difference from the original inline implementation
• BehaviorConfigAction discriminated union is re-exported from the new module (previously private to settings-hub.ts), making it available for future testing or consumers
• lib/AGENTS.md and the root AGENTS.md still describe codex-manager/ as containing only settings-hub.ts; both should be updated to list behavior-settings-panel.ts (and theme-settings-panel.ts from the prior split)
• no dedicated vitest unit file (test/behavior-settings-panel.test.ts) was added; the deps-injection design makes this easy to add and would cover branches like the non-TTY early return, set-menu-quota-ttl cycling when currentIndex < 0, and the number-key shortcuts in onInput
• no windows filesystem, token, or concurrency surface introduced — this is pure tty ui logic

Confidence Score: 4/5

• safe to merge — pure refactor with no logic changes and existing integration tests passing
• the extraction is a faithful 1:1 lift verified by passing integration tests in codex-manager-cli.test.ts; no token, fs, or concurrency surface is touched; the small score deduction is for missing dedicated unit coverage on the new module and stale AGENTS.md knowledge bases
• lib/codex-manager/behavior-settings-panel.ts — no unit test file; unused ttlMs field in set-menu-quota-ttl action

Important Files Changed

Filename	Overview
lib/codex-manager/behavior-settings-panel.ts	new 217-line panel module — logic is a faithful 1:1 lift from settings-hub.ts; deps-injection pattern is clean; no windows fs or token handling; missing dedicated vitest unit file and result.ttlMs is unused in the cycling handler
lib/codex-manager/settings-hub.ts	deletion of inline implementation + thin delegation wrapper; BehaviorConfigAction removed, promptBehaviorSettings now calls promptBehaviorSettingsPanel with correct deps; no logic changes, no token or fs surface introduced

Sequence Diagram

sequenceDiagram
    participant SH as settings-hub.ts<br/>promptBehaviorSettings()
    participant BP as behavior-settings-panel.ts<br/>promptBehaviorSettingsPanel()
    participant SEL as ui/select

    SH->>BP: call with (initial, deps{})
    BP->>BP: TTY guard (return null if non-TTY)
    BP->>BP: clone draft from initial

    loop while true
        BP->>SEL: await select(items, options)
        SEL-->>BP: BehaviorConfigAction result
        alt cancel / null
            BP-->>SH: return null
        else save
            BP-->>SH: return draft
        else reset
            BP->>BP: applyDashboardDefaultsForKeys(draft, BEHAVIOR_PANEL_KEYS)
        else toggle-pause / toggle-menu-limit-fetch / toggle-menu-fetch-status
            BP->>BP: spread-update draft field
        else set-menu-quota-ttl
            BP->>BP: cycle to next index in MENU_QUOTA_TTL_OPTIONS_MS
        else set-delay
            BP->>BP: draft.actionAutoReturnMs = result.delayMs
        end
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/codex-manager/behavior-settings-panel.ts
Line: 1

Comment:
**missing vitest unit tests for this module**

no dedicated test file (`test/behavior-settings-panel.test.ts`) was added. the deps-injection pattern introduced here is ideal for direct unit testing — you can stub `select`, supply a controlled `deps` object, and drive every branch (reset, toggle-pause, toggle-menu-limit-fetch, toggle-menu-fetch-status, set-menu-quota-ttl cycling, set-delay, cancel, save, non-TTY early return) without going through the full settings-hub integration.

`codex-manager-cli.test.ts` covers some of these paths via integration, but the new module's own branches (e.g. the `currentIndex < 0` fallback in `set-menu-quota-ttl`, the non-TTY guard, number-key shortcuts in `onInput`) currently have no isolated coverage. given the 80% threshold enforced by vitest, worth adding a unit suite here before the line count grows further.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: lib/codex-manager/behavior-settings-panel.ts
Line: 198-212

Comment:
**`result.ttlMs` field is never actually read**

the `set-menu-quota-ttl` action carries a `ttlMs` field, but the handler ignores it entirely — it re-derives the current index from the draft-local `menuQuotaTtlMs`. `result.ttlMs` is only set so the type compiles; a reader could reasonably expect it to be the *next* TTL or the value being applied, leading to confusion.

options:
- drop `ttlMs` from the action type and the return sites in `onInput` / the menu item value, or
- add a short comment noting the field exists for type-narrowing purposes only

neither is a bug, but the dead field in the discriminated union will confuse whoever next touches the cycling logic.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "refactor: extract be..."}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• skip-review

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 973c1cd4-fe2e-463f-98bf-458886bc4de1

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/pr2-behavior-settings-split

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/pr2-behavior-settings-split

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ndycode]

Closing because this work is now included in main via #318 and #319.