-
Notifications
You must be signed in to change notification settings - Fork 13
feat(tui): migrate auth dashboard and settings flows to ink #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| #!/bin/sh | ||
| . "$(dirname "$0")/_/husky.sh" | ||
|
|
||
| npx lint-staged | ||
| npx lint-staged --no-stash |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| preload = ["@opentui/solid/preload"] | ||
|
|
||
| [test] | ||
| root = "test/tui" | ||
|
|
||
| [build] | ||
| target = "bun" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ export default [ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ignores: ["dist/**", "coverage/**", "node_modules/**", "winston/**", ".tmp*/**", "vendor/**", "*.cjs", "*.mjs"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files: ["index.ts", "lib/**/*.ts"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files: ["index.ts", "lib/**/*.ts", "runtime/**/*.ts"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| languageOptions: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parser: tsparser, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parserOptions: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -39,6 +39,35 @@ export default [ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "no-duplicate-imports": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files: ["runtime/**/*.ts"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| languageOptions: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parser: tsparser, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parserOptions: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ecmaVersion: "latest", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sourceType: "module", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project: "./tsconfig.runtime-opentui.json", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| plugins: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint": tseslint, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rules: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/no-explicit-any": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/no-unused-vars": ["error", { argsIgnorePattern: "^_" }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/explicit-function-return-type": "off", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/no-non-null-assertion": "warn", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/no-floating-promises": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/no-misused-promises": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/await-thenable": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "@typescript-eslint/require-await": "warn", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "no-console": "off", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "prefer-const": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "no-var": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "eqeqeq": ["error", "always"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "no-duplicate-imports": "error", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+42
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This block duplicates the rules and plugins defined in the base configuration above. Since Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files: ["scripts/**/*.js"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| languageOptions: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ import { copyTextToClipboard, openBrowserUrl } from "./auth/browser.js"; | |||||||||||||||
| import { | ||||||||||||||||
| isNonInteractiveMode, | ||||||||||||||||
| promptAddAnotherAccount, | ||||||||||||||||
| promptLoginMode, | ||||||||||||||||
| type LoginMenuResult, | ||||||||||||||||
| } from "./cli.js"; | ||||||||||||||||
| import { | ||||||||||||||||
| extractAccountEmail, | ||||||||||||||||
|
|
@@ -81,20 +81,21 @@ import { paintUiText, quotaToneFromLeftPercent } from "./ui/format.js"; | |||||||||||||||
| import { getUiRuntimeOptions } from "./ui/runtime.js"; | ||||||||||||||||
| import { select, type MenuItem } from "./ui/select.js"; | ||||||||||||||||
| import { | ||||||||||||||||
| buildAuthDashboardViewModel, | ||||||||||||||||
| buildAuthDashboardScreenState, | ||||||||||||||||
| formatCompactQuotaSnapshot, | ||||||||||||||||
| formatRateLimitEntry, | ||||||||||||||||
| getQuotaCacheEntryForAccount, | ||||||||||||||||
| resolveActiveIndex, | ||||||||||||||||
| resolveAuthDashboardCommand, | ||||||||||||||||
| } from "./codex-manager/auth-ui-controller.js"; | ||||||||||||||||
| import { applyUiThemeFromDashboardSettings, configureUnifiedSettings } from "./codex-manager/settings-hub.js"; | ||||||||||||||||
| import { applyUiThemeFromDashboardSettings, configureUnifiedSettings, persistOpenTuiSettingsSave } from "./codex-manager/settings-hub.js"; | ||||||||||||||||
| import { | ||||||||||||||||
| configureInkUnifiedSettings, | ||||||||||||||||
| promptInkAuthDashboard, | ||||||||||||||||
| promptInkRestoreForLogin, | ||||||||||||||||
| type InkShellTone, | ||||||||||||||||
| } from "./ui-ink/index.js"; | ||||||||||||||||
| import { promptOpenTuiAuthDashboard } from "../runtime/opentui/prompt.js"; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
consider exposing a Prompt To Fix With AIThis is a comment left during a code review.
Path: lib/codex-manager.ts
Line: 98
Comment:
**`lib/` importing from `../runtime/opentui/`** — unconventional dependency direction
`import { promptOpenTuiAuthDashboard } from "../runtime/opentui/prompt.js"` (line 98) crosses the `lib/` → `runtime/` boundary in the wrong direction. `runtime/` is supposed to depend on `lib/`, not vice versa. this creates a circular potential and breaks build isolation — tests that mock `lib/codex-manager.ts` now transitively pull in the entire `@opentui/solid` and solid-js dependency chain from `runtime/`.
consider exposing a `promptOpenTuiAuthDashboard` adapter inside `lib/ui-ink/index.ts` (which is already the ui-layer boundary that `codex-manager.ts` imports from) and keeping the opentui import inside that layer.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||
|
|
||||||||||||||||
| type TokenSuccess = Extract<TokenResult, { type: "success" }>; | ||||||||||||||||
| type TokenSuccessWithAccount = TokenSuccess & { | ||||||||||||||||
|
|
@@ -3557,7 +3558,7 @@ async function clearAccountsAndReset(): Promise<void> { | |||||||||||||||
|
|
||||||||||||||||
| async function handleManageAction( | ||||||||||||||||
| storage: AccountStorageV3, | ||||||||||||||||
| menuResult: Awaited<ReturnType<typeof promptLoginMode>>, | ||||||||||||||||
| menuResult: LoginMenuResult, | ||||||||||||||||
| ): Promise<void> { | ||||||||||||||||
| if (typeof menuResult.switchAccountIndex === "number") { | ||||||||||||||||
| const index = menuResult.switchAccountIndex; | ||||||||||||||||
|
|
@@ -3636,6 +3637,9 @@ async function runAuthLogin(): Promise<number> { | |||||||||||||||
| let recoveryStatusText = loginStorage.statusText; | ||||||||||||||||
| let recoveryStatusTone = loginStorage.statusTone; | ||||||||||||||||
| if (existingStorage && existingStorage.accounts.length > 0) { | ||||||||||||||||
| if (isNonInteractiveMode()) { | ||||||||||||||||
| existingStorage = null; | ||||||||||||||||
| } else { | ||||||||||||||||
|
Comment on lines
+3640
to
+3642
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-interactive guard placement may cause unintended behavior.
if the intent is to skip all interactive prompts in non-interactive mode, the guard should be at the top of the flow or the comment should clarify this is intentional: if (existingStorage && existingStorage.accounts.length > 0) {
if (isNonInteractiveMode()) {
+ // skip dashboard menu in non-interactive mode; proceed directly to OAuth
existingStorage = null;
} else {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| while (true) { | ||||||||||||||||
| existingStorage = await loadAccounts(); | ||||||||||||||||
| if (!existingStorage || existingStorage.accounts.length === 0) { | ||||||||||||||||
|
|
@@ -3672,22 +3676,40 @@ async function runAuthLogin(): Promise<number> { | |||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| const flaggedStorage = await loadFlaggedAccounts(); | ||||||||||||||||
| const dashboardViewModel = buildAuthDashboardViewModel({ | ||||||||||||||||
| const dashboardState = buildAuthDashboardScreenState({ | ||||||||||||||||
| storage: currentStorage, | ||||||||||||||||
| quotaCache, | ||||||||||||||||
| displaySettings, | ||||||||||||||||
| flaggedCount: flaggedStorage.accounts.length, | ||||||||||||||||
| statusMessage: showFetchStatus ? () => menuQuotaRefreshStatus : undefined, | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| const menuResult = await promptInkAuthDashboard({ | ||||||||||||||||
| const dashboardViewModel = dashboardState.dashboard; | ||||||||||||||||
| const openTuiDashboardViewModel = recoveryStatusText | ||||||||||||||||
| ? { | ||||||||||||||||
| ...dashboardViewModel, | ||||||||||||||||
| menuOptions: { | ||||||||||||||||
| ...dashboardViewModel.menuOptions, | ||||||||||||||||
| statusMessage: recoveryStatusText, | ||||||||||||||||
| }, | ||||||||||||||||
| } | ||||||||||||||||
| : dashboardViewModel; | ||||||||||||||||
|
|
||||||||||||||||
| const menuResult = await promptOpenTuiAuthDashboard({ | ||||||||||||||||
| dashboard: openTuiDashboardViewModel, | ||||||||||||||||
| onSettingsSave: (event) => { | ||||||||||||||||
| void persistOpenTuiSettingsSave(event).catch((error: unknown) => { | ||||||||||||||||
| console.warn( | ||||||||||||||||
| `OpenTUI settings save failed: ${error instanceof Error ? error.message : String(error)}`, | ||||||||||||||||
| ); | ||||||||||||||||
| }); | ||||||||||||||||
|
Comment on lines
+3700
to
+3704
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial swallowed promise rejection in settings save callback.
consider surfacing save failures to the user via the statusMessage field so they know their settings weren't persisted: onSettingsSave: (event) => {
- void persistOpenTuiSettingsSave(event).catch((error: unknown) => {
+ persistOpenTuiSettingsSave(event).catch((error: unknown) => {
console.warn(
`OpenTUI settings save failed: ${error instanceof Error ? error.message : String(error)}`,
);
+ // optionally set a status message visible to user
});
},🤖 Prompt for AI Agents |
||||||||||||||||
| }, | ||||||||||||||||
| }) ?? await promptInkAuthDashboard({ | ||||||||||||||||
| dashboard: dashboardViewModel, | ||||||||||||||||
| statusTextOverride: recoveryStatusText, | ||||||||||||||||
| statusToneOverride: recoveryStatusTone, | ||||||||||||||||
| }) ?? await promptLoginMode( | ||||||||||||||||
| dashboardViewModel.accounts, | ||||||||||||||||
| dashboardViewModel.menuOptions, | ||||||||||||||||
| ); | ||||||||||||||||
| }) ?? (isNonInteractiveMode() | ||||||||||||||||
| ? { mode: "add" as const } | ||||||||||||||||
| : { mode: "cancel" as const }); | ||||||||||||||||
| recoveryStatusText = undefined; | ||||||||||||||||
| recoveryStatusTone = undefined; | ||||||||||||||||
| const command = resolveAuthDashboardCommand(menuResult); | ||||||||||||||||
|
|
@@ -3756,6 +3778,7 @@ async function runAuthLogin(): Promise<number> { | |||||||||||||||
| break; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| const refreshedStorage = await loadAccounts(); | ||||||||||||||||
|
|
@@ -3792,6 +3815,9 @@ async function runAuthLogin(): Promise<number> { | |||||||||||||||
| if (!addAnother) break; | ||||||||||||||||
| forceNewLogin = true; | ||||||||||||||||
| } | ||||||||||||||||
| if (isNonInteractiveMode()) { | ||||||||||||||||
| return 0; | ||||||||||||||||
| } | ||||||||||||||||
| continue loginFlow; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: ndycode/codex-multi-auth
Length of output: 336
add
runtime/**/*.tsxto the eslint override.the eslint config at lines 42–69 matches only
runtime/**/*.ts, buttsconfig.runtime-opentui.jsonincludesruntime/**/*.tsx. no .tsx files exist in runtime/ today, but this config mismatch means any future .tsx entrypoints will skip the async/promise safety rules (no-floating-promises,await-thenable, etc.). keep the config aligned.suggested fix
{ - files: ["runtime/**/*.ts"], + files: ["runtime/**/*.ts", "runtime/**/*.tsx"], languageOptions: { parser: tsparser, parserOptions: { ecmaVersion: "latest", sourceType: "module",📝 Committable suggestion
🤖 Prompt for AI Agents