refactor: extract statusline settings panel

[ndycode]

Summary

• extract the summary/statusline settings flow from settings-hub.ts into a dedicated panel module
• keep the current preview, toggle, reordering, and save behavior intact while continuing the settings-hub split in small slices

What Changed

• added lib/codex-manager/statusline-settings-panel.ts containing the statusline panel prompt flow and its local action model
• updated lib/codex-manager/settings-hub.ts so promptStatuslineSettings(...) delegates to the extracted panel module
• preserved the existing preview, move-up, move-down, reset, save, and cancel 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 f63e949 to restore the inline statusline panel implementation in settings-hub.ts

Additional Notes

• this continues the settings-hub split after the dashboard display extraction 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 statusline settings panel from settings-hub.ts into a dedicated lib/codex-manager/statusline-settings-panel.ts module, continuing the incremental settings-hub split. the logic is a faithful one-to-one port — preview, toggle, reorder, reset, save, and cancel behaviour are all preserved — with full dependency injection via StatuslineSettingsPanelDeps so the panel is decoupled from the hub's internal helpers.

key changes:

• StatuslineConfigAction type and StatuslineFieldOption interface are now exported from the new panel module rather than kept private in settings-hub.ts
• promptStatuslineSettings in settings-hub.ts is reduced to a thin delegation call, passing all required helpers and constants through deps
• four new vitest cases cover the tty guard, reorder hotkeys, reset hotkey, and last-field-guard behaviour via the existing hub test api
• no concurrency or windows filesystem safety concerns are introduced by this change

minor gap: the tty-guard test only exercises the both-streams-false path; the single-stream OR branch is not covered. additionally, no dedicated test/statusline-settings-panel.test.ts file exists — the new module's public api is only tested through the hub delegation path, which may reduce isolation if the hub wiring changes later.

Confidence Score: 4/5

• safe to merge; refactoring is a faithful port with no behavioural changes and adequate integration coverage through the hub test api
• the extraction is clean, all nine deps thread through correctly, and four new tests validate the key paths. score stops at 4 rather than 5 because the project's established pattern calls for per-module test files and the 80% coverage threshold means a missing dedicated test file for statusline-settings-panel.ts is a real gap — especially given the injected-deps design makes direct unit tests straightforward to write as a follow-up.
• test/settings-hub-utils.test.ts — tty guard test only covers both-streams-false; lib/codex-manager/statusline-settings-panel.ts — no dedicated test file

Important Files Changed

Filename	Overview
lib/codex-manager/statusline-settings-panel.ts	new module extracted from settings-hub.ts; logic is a faithful port of the original inline implementation with full dep injection; no behavioural changes introduced. only gap is no dedicated test file for direct unit coverage.
lib/codex-manager/settings-hub.ts	promptStatuslineSettings now delegates to the extracted panel; all nine dependencies are threaded through correctly; StatuslineConfigAction type moved to the panel module. clean delegation with no logic changes.
test/settings-hub-utils.test.ts	four new test cases cover tty guard, reorder, reset, and last-field-guard behaviour via the hub's test api; tty test only exercises both-streams-false, leaving single-stream OR branches uncovered.

Sequence Diagram

sequenceDiagram
    participant SH as settings-hub.ts
    participant Panel as statusline-settings-panel.ts
    participant Select as ui/select.ts

    SH->>Panel: promptStatuslineSettingsPanel(initial, deps)
    Panel-->>Panel: TTY guard check
    Panel-->>Panel: cloneDashboardSettings(initial)
    loop while true
        Panel->>Panel: buildAccountListPreview(draft, ui, focusKey)
        Panel->>Panel: build MenuItem list
        Panel->>Select: select(items, options)
        Select-->>Panel: StatuslineConfigAction
        alt cancel / null
            Panel-->>SH: null
        else save
            Panel-->>SH: draft
        else reset
            Panel->>Panel: applyDashboardDefaultsForKeys(draft, STATUSLINE_PANEL_KEYS)
        else move-up / move-down
            Panel->>Panel: reorderField(fields, key, ±1)
        else toggle
            Panel->>Panel: toggle field in menuStatuslineFields
        end
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/settings-hub-utils.test.ts
Line: 645-657

Comment:
**TTY guard test only covers both-streams-false**

the test description says "when stdin **or** stdout is not a tty", but both streams are set to `false` together. the guard is `!input.isTTY || !output.isTTY`, so the function should also return null when only one stream is non-TTY. the current test doesn't exercise either single-stream branch, leaving a gap in the OR coverage.

consider adding a second assertion, e.g. setting only stdout to `false` while stdin stays `true`:

```suggestion
		it("returns null for statusline settings when stdin or stdout is not a tty", async () => {
			const api = await loadSettingsHubTestApi();
			setStreamIsTTY(process.stdin, false);
			setStreamIsTTY(process.stdout, false);
			await expect(
				api.promptStatuslineSettings({
					...DEFAULT_DASHBOARD_DISPLAY_SETTINGS,
				}),
			).resolves.toBeNull();
			setStreamIsTTY(process.stdin, true);
			await expect(
				api.promptStatuslineSettings({
					...DEFAULT_DASHBOARD_DISPLAY_SETTINGS,
				}),
			).resolves.toBeNull();
		});
```

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/statusline-settings-panel.ts
Line: 51-55

Comment:
**no dedicated vitest file for this new module**

every other panel in `lib/codex-manager/` has a matching test file (e.g. `dashboard-display-panel.ts` → tested directly). the new module's exported function `promptStatuslineSettingsPanel` is only exercised through the `settings-hub.ts` delegation path in `settings-hub-utils.test.ts`. because the deps are fully injectable, a lightweight direct test file (`test/statusline-settings-panel.test.ts`) could stub the deps and cover the control-flow branches (reset/move-up/move-down/last-field-guard/cancel) in isolation — decoupled from the hub wiring. given the project's 80 % coverage threshold this is worth tracking.

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

_{Reviews (3): Last reviewed commit: "test: cover statusline panel hotkeys" | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

Walkthrough

refactored promptStatuslineSettings in settings-hub.ts to delegate complex interactive cli logic to a new extracted promptStatuslineSettingsPanel function in a dedicated module, moving ~200 lines of menu handling, input parsing, and state mutation into a reusable component.

Changes

Cohort / File(s)	Summary
statusline settings extraction lib/codex-manager/settings-hub.ts	removed inline StatuslineConfigAction type and ~195 lines of tty gating, menu building, cursor/input handling (q/s/r/[ ] hotkeys, numeric toggles), and draft mutation logic. delegated to promptStatuslineSettingsPanel call with callback and constant params.
statusline settings panel module lib/codex-manager/statusline-settings-panel.ts	new file implementing promptStatuslineSettingsPanel function and exporting StatuslineConfigAction, StatuslineFieldOption, StatuslineSettingsPanelDeps types. contains full interactive cli loop: menu building, cursor/preview updates, input parsing (q/s/r/[ ]/numeric), draft mutations, and promise resolution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

---

some flags to check:

• missing regression tests: no test additions visible for the new promptStatuslineSettingsPanel export or its edge cases. verify existing tests in settings-hub.ts still cover the refactored path end-to-end.

• tty edge case: settings-hub.ts returns null when not tty. confirm this behavior is preserved in the delegation path and that promptStatuslineSettingsPanel handles non-tty stdin/stdout consistently across both process.stdin.isTTY and process.stdout.isTTY checks.

• dependency injection completeness: verify all 7+ helper functions passed to promptStatuslineSettingsPanel (e.g., cloneDashboardSettings, buildAccountListPreview, normalizeStatuslineFields, etc.) are correctly bound or imported in the new module and don't create stale closure issues if called from async contexts.

• windows path concerns: if numeric hotkey parsing relies on stdin encoding or terminal control sequences, test on windows to ensure [ and ] reorder commands still map correctly.

🚥 Pre-merge checks | ✅ 2 | ❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	title follows conventional commits format (refactor) and clearly summarizes the main change (extract statusline settings panel), well under 72 chars.
Description check	✅ Passed	the pr description is mostly complete with summary, what changed, validation checklist, risk/rollback plan, and additional notes, though some sections are lightly filled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

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

✨ Simplify code

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

---

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

[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]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/codex-manager/statusline-settings-panel.ts`:
- Around line 51-55: Add focused vitest unit tests for
promptStatuslineSettingsPanel to cover the non-tty short-circuit, the hotkey
parsing branch, and the reset/reorder/toggle/save branches including the
"last-field" guard; specifically, write tests that (1) simulate input/output not
being TTY to assert null return, (2) feed the hotkey sequences exercised in the
existing parsing logic to verify expected state transitions, and (3) drive the
interactive control flow for reset, reorder, toggle, and save actions (including
attempting to remove the final visible field to trigger the last-field guard) to
assert final DashboardDisplaySettings outcomes and no regressions in
promptStatuslineSettingsPanel behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f53e81f6-d1f0-4081-8f86-62b5eabe0681

📥 Commits

Reviewing files that changed from the base of the PR and between 5f0405e and fd2b34b.

📒 Files selected for processing (2)

• lib/codex-manager/settings-hub.ts
• lib/codex-manager/statusline-settings-panel.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Greptile Review

🧰 Additional context used

📓 Path-based instructions (1)

lib/**

⚙️ CodeRabbit configuration file

focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.

Files:

• lib/codex-manager/statusline-settings-panel.ts
• lib/codex-manager/settings-hub.ts

🔇 Additional comments (1)

lib/codex-manager/settings-hub.ts (1)

1352-1362: clean extraction boundary.

lib/codex-manager/settings-hub.ts:1352-1362 stays as a thin adapter, and the windows write retry / queue paths remain untouched in lib/codex-manager/settings-hub.ts:639-791. that keeps this refactor out of the ebusy/429 and concurrency-sensitive code. As per coding guidelines, "focus on auth rotation, windows filesystem IO, and concurrency".

[ndycode]

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