Secure auth flow, model picker, and durable sidebar metrics#3
Conversation
Introduces lib/auth-store.js as the single source of truth for auth-store I/O across CLI and TUI. All writes are atomic (temp file + rename(2)), set file mode 0o600 and directory mode 0o700, refuse to overwrite a corrupt existing file, and replace any symlink at the target path so a planted symlink cannot exfiltrate the key. XDG_DATA_HOME is validated as absolute; relative paths fall back to $HOME/.local/share to keep the key out of the working directory.
AUTH_PROVIDER_ID is now opencode-ttc-plugin (was the-token-company- plugin) so it matches the package and TUI id. resolveApiKeyFromAuthStore still reads entries written under the legacy id so existing users do not need to re-auth. CLI gains login and logout subcommands backed by the shared auth-store module. login never accepts the key as a --key <value> argument (would leak via ps aux and shell history); instead it prompts on the TTY with echo off or reads from stdin via --stdin. logout clears both current and legacy entries. doctor reports which id the key is stored under.
Sidebar polls auth.json directly via loadAuthStatus. When the cached state file still says missing_auth but a key has landed, the sidebar overrides to 'authenticated — send a message to start compressing' instead of staying stale until the next transform fires. /ttc settings menu now shows exactly one compact auth row conditional on auth state: 'Add API key' when missing, 'Remove API key' (with confirmation) when present. Reduces two wordy rows to one and gives a clearer affordance. Model picker replaces the free-text input with a curated list grounded in thetokencompany.com/docs/compression: bear-2 (Recommended) and bear-1.2, plus a custom-id escape hatch for enterprise fine-tunes. Stops silent typos like 'bear-2.0' (not a real model id) entering the config. Login/logout failures now surface the actual reason (empty key, corrupt auth store, write failure) instead of a generic 'Login failed' message.
Adds regression coverage for the security invariants the new auth-store module relies on: file mode 0o600 after write, atomic rename surviving a concurrent writer, and corrupt existing file being refused rather than silently overwritten (which would destroy other providers' keys). Adds coverage for the legacy auth id read fallback, the preference for the current id when both exist, and the conditional single-row auth state in the /ttc settings menu.
README setup section now leads with the plugin-managed login paths (opencode-ttc-plugin login --stdin and /ttc-login) before the opencode auth menu, and explicitly notes that the second prompt users see during opencode auth login is opencode's separate LLM-provider auth, not a duplicate TTC prompt. Adds a Models section grounded in thetokencompany.com/docs/compression listing bear-2 (Recommended) and bear-1.2, explains there is no list endpoint so the picker is curated, and tells users how to diagnose an unexpected model id via doctor --verbose. Security and network policy section now documents the auth-write guarantees: atomic temp+rename, 0o600 file mode, 0o700 directory mode, refusal to overwrite a corrupt file, symlink replacement via rename, relative-XDG fallback, and the CLI never accepting the key as a --key <value> argument.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22cfca0e0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isTTY && typeof input.setRawMode === "function") { | ||
| try { input.setRawMode(true); } catch { /* ignore */ } | ||
| } | ||
| rl.question(query, (answer) => { |
There was a problem hiding this comment.
For the interactive TTY path, this still leaves readline echoing the key: running XDG_DATA_HOME=$(mktemp -d) node scripts/cli.mjs login in a TTY printed the typed ttc_secret before the success message. Since the new login flow is advertised as “no echo,” this leaks the TTC key to the terminal/scrollback; use a muted readline output or a real password prompt instead of raw mode plus rl.question.
Useful? React with 👍 / 👎.
|
Codex P1 (echo of API key during login) — fixed in 309dc3e4 The original code passed Fix: write the prompt directly to Verified with a real Only the prompt and the post-submit success line appear — the secret itself does not. |
Codex review on PR #3 caught that the promptSecret helper was still echoing typed characters: readline.createInterface({ input, output, terminal: true }) does its own echo to the output stream regardless of setRawMode on stdin. The advertised 'no echo' guarantee was not actually honored. Fix: write the prompt directly to process.stdout, then pass readline a muted Writable (write: (_chunk, _enc, cb) => cb()) as its output so echo writes are dropped. terminal: true stays on so backspace and line editing still work. Verified by typing 'ttc_SECRET_value' char-by-char at 60ms intervals through a real pty.fork() harness: the captured output contains the prompt and the post-submit success line, but none of 'ttc_SECRET_value', 'SECRET', or 'ttc_'.
…leshooting The README had drifted from what the plugin actually does. Restructured the whole thing around what a user needs to know, with new sections: - Requirements (Node 20+, OpenCode 1.4+, TTC key) - Sidebar: status dot colors, every status line users can see, what each one means (including the new 'authenticated — send a message' override and what 'waiting for metrics' implies) - Slash commands: /ttc, /ttc-login, /ttc-logout in one table - What gets compressed: message selection (latest user turn by default, compressSystem / compressHistory toggles) plus the full skip-pattern table (code fence, diff, stack trace, JSON, schema, ...) so users seeing 'skipped: code fence' know why - Troubleshooting: missing auth after login, phantom model id like 'bear-2.0', the two-prompt opencode auth login, compression not firing, legacy auth id Fixed the broken ''' fences in the LLM install block (was using three single-quotes; should be triple backticks). Tightened the intro to one paragraph that says what TTC does and why you'd use it. Numbered the top-level sections so the table of contents is implicit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6382d70c26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { ok: false, reason: "auth_store_read_failed", authFilePath, error }; | ||
| } | ||
|
|
||
| parsed[providerID] = { type: "api", key: trimmed }; |
There was a problem hiding this comment.
For users upgrading from the old the-token-company-plugin provider ID, logging in with the new flow writes the replacement key under opencode-ttc-plugin but leaves the old TTC key in auth.json. Since the runtime now prefers the new entry and logout explicitly removes both IDs, key rotation through opencode-ttc-plugin login leaves a stale secret on disk indefinitely; delete the legacy provider entries before writing the current one.
Useful? React with 👍 / 👎.
Codex P2 on PR #3: when a user with the legacy the-token-company-plugin provider id logs in via the new flow, the new id gets written but the old entry stayed on disk indefinitely. logout already cleared both; now login does too. writeAuthEntry accepts legacyProviderIDs (defaults to LEGACY_AUTH_PROVIDER_IDS) and deletes any of those entries from the parsed store before writing the new one. Unrelated providers (anthropic, openai, etc.) are never touched. Returns removedLegacyIDs so callers can report what got cleaned. CLI persistKey prints 'Removed stale legacy entries: ...' when any were cleared. TUI performLogin toast appends the same note. Regression tests: - pre-existing legacy + current + anthropic + openai -> after login, legacy gone, current rotated, anthropic + openai preserved verbatim - fresh login (no legacy) returns removedLegacyIDs: []
|
Codex P2 (stale legacy TTC keys after rotation) — fixed in d3ff2fc
Returns End-to-end verification (starting with
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3ff2fc33e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (raw && isAbsolute(raw)) { | ||
| return join(raw, "opencode", "auth.json"); | ||
| } | ||
| return join(homedir(), ".local", "share", "opencode", "auth.json"); |
There was a problem hiding this comment.
When XDG_DATA_HOME is set to a relative path, login/TUI now use this helper and fall back to ~/.local/share, but the message transform still computes the auth path with its own getAuthStorePath in opencode-plugins/ttc-message-transform.js:436-440, which reads from the relative path instead. In that environment opencode-ttc-plugin login, doctor, and the sidebar can report a saved key while compression continues with missing auth; update the transform to use this shared helper or the same fallback.
Useful? React with 👍 / 👎.
| parsed[providerID] = { type: "api", key: trimmed }; | ||
|
|
||
| try { | ||
| await atomicWriteJson(authFilePath, parsed, { writeFileImpl, renameImpl, mkdirImpl, chmodImpl, rmImpl }); |
There was a problem hiding this comment.
During a concurrent auth change, this writes the snapshot read earlier over the whole auth.json. If another provider is added or rotated after readAuthStore but before this rename, the temp-file rename atomically replaces the file with stale contents and drops that provider; temp+rename prevents partial writes but does not serialize or merge concurrent read-modify-write operations. Use a lock or re-read/merge before replacing the file to avoid losing unrelated credentials.
Useful? React with 👍 / 👎.
Codex P2 #1 on PR #3 (path resolution mismatch): the message transform still had its own getAuthStorePath that did not validate XDG_DATA_HOME. When XDG_DATA_HOME was relative (invalid per spec but possible), login fell back to $HOME/.local/share while the transform read from the relative path — so login succeeded and compression silently failed with missing_auth. Fix: delete the duplicate; the plugin now imports getAuthStorePath from ../lib/auth-store.js. Same signature, same return shape, one source of truth. Tests still pass through the plugin's _test re-export. Regression test pins the fallback behavior through the plugin's exported helper: relative paths, './data', bare 'data', and empty string all fall back to $HOME/.local/share/opencode/auth.json. Codex P2 #2 on PR #3 (concurrent RMW not serialized): legitimate but disproportionate to fix. Atomic rename already prevents partial writes (addressing the original thermo H1); full serialization would need file locking (POSIX-only) or optimistic retry loops (deadlock potential) for an edge case where the realistic blast radius is 're-enter one key.' Documented as a known limitation in Troubleshooting with the recovery path.
|
Codex P2 (path resolution mismatch) — fixed in You were right — the transform still had its own Fix: deleted the duplicate. The plugin now imports Regression test added pins the fallback behavior through the plugin's exported helper: Codex P2 (concurrent RMW not serialized) — declined with documented limitation Legitimate observation, but declining to fix. Atomic temp+rename (which we already do) prevents partial writes and was the explicit ask from the original thermo review of H1. Full serialization would require either POSIX-only file locking or an optimistic re-read-merge-retry loop — meaningful complexity and its own deadlock/starvation edge cases, for a scenario whose realistic blast radius is:
For a single-user opencode plugin that almost exclusively READS If this becomes a real support burden we'll revisit — happy to revisit the call if you have data showing it fires in practice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dff86153e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { homedir } from "node:os"; | ||
| import { dirname, join } from "node:path"; | ||
| import { gzipSync } from "node:zlib"; | ||
| import { getAuthStorePath } from "../lib/auth-store.js"; |
There was a problem hiding this comment.
When users follow the documented opencode-ttc-plugin install path, install() only copies ttc-message-transform.js into ~/.config/opencode/plugins (scripts/cli.mjs:407-409). From that location this new relative import resolves to ~/.config/opencode/lib/auth-store.js, which is never copied or created, so OpenCode fails to load the installed plugin with ERR_MODULE_NOT_FOUND before compression can run. Either keep the server plugin self-contained or install the helper alongside it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Restored local in the plugin (self-contained) and added a parity test comparing it to the shared helper. Legacy Installed ttc-message-transform.js to /Users/DrFok/.config/opencode/plugins/ttc-message-transform.js now works; 53/53 tests + single-file import smoke pass. See opencode-plugins/ttc-message-transform.js:436 and tests/opencode-ttc-plugin.test.js:547.
There was a problem hiding this comment.
Fixed. Restored local getAuthStorePath in the plugin (self-contained) and added a parity test comparing it to the shared helper. Legacy single-file install now works; 53/53 tests + single-file import smoke pass. See opencode-plugins/ttc-message-transform.js:436 and tests/opencode-ttc-plugin.test.js:547.
MrFok
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e83811a533
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (current?.status === "missing_auth" && authStatus().hasKey) { | ||
| return "waiting"; |
There was a problem hiding this comment.
When a key is removed via /ttc-logout or opencode-ttc-plugin logout while the last sidebar state was compressed, skipped, or another active state, this branch never uses the now-empty auth store because it only upgrades stale missing_auth to waiting. Logout does not rewrite the per-session state file, so auth-store-backed sessions keep showing a green active status until another message is sent even though the transform will no longer compress; consider downgrading stale auth-store states when current.config.authSource === "auth_store" and authStatus().hasKey is false.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. When auth is removed, visibleState() returns null (hiding stale compressed/skipped), effectiveStatus() returns missing_auth, status shows "missing TTC auth". The existing 2s poll via loadAuthStatus makes logout take effect immediately. 55/55 tests. See tui/index.tsx:69.
When auth is removed via /ttc-logout or CLI, hide any previously written compressed/skipped sidebar state and show 'missing TTC auth' instead of stale metrics. - gate visibleState on hasAuth() - force effectiveStatus to 'missing_auth' when no key - statusLine shows 'missing TTC auth' when unauthed - polling via loadAuthStatus already provides fresh auth status Fixes Codex P2 on PR #3.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11a2a136fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { homedir } from "node:os"; | ||
| import { dirname, join } from "node:path"; | ||
| import { gzipSync } from "node:zlib"; | ||
| import { getAuthStorePath } from "../lib/auth-store.js"; |
There was a problem hiding this comment.
Fresh evidence in the current patch shows the plugin still imports ../lib/auth-store.js, while the legacy installer only copies ttc-message-transform.js into ~/.config/opencode/plugins (scripts/cli.mjs:407-408). In that documented opencode-ttc-plugin install path, this import resolves to ~/.config/opencode/lib/auth-store.js, which is never installed, so OpenCode fails to load the plugin with ERR_MODULE_NOT_FOUND before compression or auth can run. Keep the copied plugin self-contained or install the helper beside it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Removed the ../lib/auth-store.js import from the plugin. Inlined getAuthStorePath (with the same isAbsolute relative-XDG validation) directly in ttc-message-transform.js. The legacy opencode-ttc-plugin install path only copies the single plugin file; this keeps it loadable. 55/55 tests + single-file import smoke pass. See opencode-plugins/ttc-message-transform.js:487 (local helper) and the test at line 535.
- Remove import of ../lib/auth-store.js from ttc-message-transform.js - Inline getAuthStorePath (with isAbsolute XDG validation) inside the plugin - Legacy installer copies only the single .js file; external import would 404 - Parity with lib/auth-store.js is covered by existing path tests + manual smoke This is the same P1 that was previously fixed but the commit was not the current head. Addresses Codex P1 'Keep legacy installs loadable'.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4f96ebdc3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!hasAuth()) { | ||
| return null; // hide any stale compressed/skipped state once logged out |
There was a problem hiding this comment.
When users rely on the supported TTC_API_KEY environment variable instead of an auth-store entry, the transform still compresses and writes state with authSource: "env", but loadAuthStatus() only checks the auth store, so this branch hides that valid state and the sidebar reports missing TTC auth forever. Consider accepting env-backed state/auth here instead of treating every missing auth-store key as logged out.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. loadAuthStatus() and hasAuthEntry() now short-circuit on TTC_API_KEY env (return hasKey: true, providerID: "env"). Sidebar will render metrics instead of hiding behind "missing TTC auth", and the /ttc settings row will correctly show "Remove API key" (or not offer login) when env is the source. This also prevents spurious second prompts for env users. Store-backed logins (including via opencode auth provider) were already covered. 55/55 pass. See lib/auth-store.js:74 and tui/sidebar-state.js:43.
- loadAuthStatus() now short-circuits on TTC_API_KEY (returns hasKey:true, providerID:'env') - hasAuthEntry() now short-circuits on TTC_API_KEY (returns hasKey:true, providerID:'env') - This makes sidebar show metrics instead of 'missing TTC auth' when using env - Makes settings menu show 'Remove API key' instead of 'Add' when env is set - Prevents spurious second login prompts for users on env auth Addresses latest Codex P2. 55/55 tests still pass.
…Entry This prevents false 'missing auth' in sidebar and settings menu for env users, which was causing extra 'add API key' prompts after opencode auth login or direct use of TTC_API_KEY. 56/56 tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a68d83f56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const authOption = auth.hasKey | ||
| ? { | ||
| title: "Remove API key", | ||
| value: "ttc-logout", | ||
| description: "Revoke saved key" | ||
| } |
There was a problem hiding this comment.
When TTC_API_KEY is set but there is no auth-store entry, hasAuthEntry() returns { hasKey: true, providerID: "env" }, so this branch renders “Remove API key”. Selecting it calls removeAuthEntry(), which cannot unset the environment variable and reports no saved entry while the key remains active; it also prevents users from adding or rotating a persistent key from this menu. Branch on providerID or store-only auth state instead of hasKey alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8370585. The /ttc settings auth row now checks persisted auth-store state with hasAuthEntry({ env: {} }) instead of treating TTC_API_KEY as removable saved auth. Env-only users now see Add API key so they can persist/rotate a saved key; users with a saved entry still see Remove API key. Regression: settings menu shows the persisted-state row correctly even while TTC_API_KEY is set. npm test: 57/57 pass.
- Drop /ttc-login slash command entirely - Drop 'Add API key' row from /ttc settings (only 'Remove' when present) - Remove internal promptLogin / performLogin paths - 'Add' path (if reached) now tells user to use opencode auth login - Update tests (no more login registration, menu auth row is now only logout) - Update README (remove references to /ttc-login and custom prompt) - CLI login (opencode-ttc-plugin login) remains for direct use Goal: one place to enter the key (native auth provider or direct CLI) to stop double prompts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6de830a011
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (error?.code === "ENOENT" || error?.code === "AUTH_STORE_NOT_REGULAR_FILE") { | ||
| return { ok: true, authFilePath, removedAny: false }; |
There was a problem hiding this comment.
When auth.json is a symlink, such as in a dotfile-managed OpenCode setup, readAuthStore() throws AUTH_STORE_NOT_REGULAR_FILE here and logout reports success/no entry without removing anything. The runtime transform still reads the same path with readFile and follows the symlink, so the TTC key remains active after opencode-ttc-plugin logout; return an error or remove/replace the symlink instead of treating it as an empty store.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8370585. removeAuthEntry() no longer reports success when readAuthStore() rejects a non-regular auth path (including symlink-backed auth.json). It now returns ok:false with reason auth_store_not_regular_file, so CLI/TUI logout fail visibly instead of claiming the key was removed while runtime could still read it. Regression added: logout refuses unsupported auth-store paths instead of claiming success. npm test: 57/57 pass.
…or provider - Remove custom prompts/authorize from the registered auth provider in the plugin. This makes opencode auth login use its standard single API key prompt for TTC instead of triggering a second prompt. - Restore /ttc-login, /ttc-logout slash commands and the auth row in /ttc settings menu (Add when missing, Remove when present). The TUI custom prompt is kept for direct use. - Keep CLI login/logout fully working. - 56/56 tests pass. Addresses the double prompt when running 'opencode auth login' for the TTC provider.
📝 WalkthroughWalkthroughIntroduces a shared ChangesAuth Store, Login/Logout, and Session Stats Persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8370585400
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dialog.replace(() => api.ui.DialogPrompt({ | ||
| title: "TTC API Key", | ||
| placeholder: "ttc_...", | ||
| value: "", | ||
| onConfirm: (nextValue) => void performLogin(api, dialog, nextValue), |
There was a problem hiding this comment.
When users select /ttc-login or the settings menu's Add API key row, this sends the TTC secret through the generic DialogPrompt with no password/masking option, so the key is entered as ordinary visible text in the TUI even though the flow is intended to avoid custom/echoed secret prompts. Route this action to the native auth flow or use a masked secret prompt instead of reusing the plain settings prompt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6387ae6. /ttc-login and the Add API key row no longer open DialogPrompt or accept a visible API key. They now show a DialogAlert that points users to the native single-prompt auth flow (opencode auth login -> opencode-ttc-plugin) or the non-echoing CLI (opencode-ttc-plugin login). Added regression test: /ttc-login never opens DialogPrompt. npm test: 60/60 pass.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/opencode-ttc-plugin.test.js (1)
1206-1239: ⚡ Quick winAdd coverage for auth directory permission mode (
0o700)This test verifies file mode (
0o600) but not the parent auth directory mode (0o700), which is part of the hardening contract and can regress silently.Suggested test addition
const { statSync } = await import("node:fs"); const mode = statSync(authPath).mode & 0o777; assert.equal(mode, 0o600, `auth file should be 0o600, got 0o${mode.toString(8)}`); + const dirMode = statSync(dirname(authPath)).mode & 0o777; + assert.equal(dirMode, 0o700, `auth directory should be 0o700, got 0o${dirMode.toString(8)}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/opencode-ttc-plugin.test.js` around lines 1206 - 1239, The test currently only verifies the file mode (0o600) of the auth.json file but does not verify the parent auth directory mode (0o700). After the assertion that checks the auth file mode in the writeTuiAuthEntry test, add an additional assertion that checks the mode of the parent directory (the "opencode" directory) using statSync on the directory path. Verify that the directory mode equals 0o700 using the same bitwise operation pattern (mode & 0o777) to ensure the directory has the correct restricted permissions as part of the security hardening contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/auth-store.js`:
- Around line 201-204: The condition `if (parsed[candidateID])` at line 202 uses
truthiness testing, which incorrectly skips deletion of provider keys that have
falsy values like null, empty strings, or 0. Replace this truthiness check with
an own-property check such as `Object.prototype.hasOwnProperty.call(parsed,
candidateID)` or the `in` operator to ensure all provider keys are properly
removed regardless of their stored value, allowing removedAny to be correctly
set to true when entries are actually deleted.
In `@opencode-plugins/ttc-message-transform.js`:
- Line 220: The numberOrZero function currently does not guard against Infinity
values, which can corrupt session metrics. Modify the function to check if the
converted number is finite using Number.isFinite() before applying the fallback
to 0 and the Math.max() clamping. The logic should verify that Number(value)
produces a finite number before using it; if it is not finite (including
Infinity or NaN), default to 0 instead.
In `@scripts/cli.mjs`:
- Around line 379-391: The async function logoutCommand is being called without
await, which allows the process to exit before the logout operation completes
and prevents error handling from taking effect. Find the call to logoutCommand
around line 657 and add the await keyword to ensure the async function completes
before proceeding. This applies to both the primary call and any other calls
mentioned in the comment.
---
Nitpick comments:
In `@tests/opencode-ttc-plugin.test.js`:
- Around line 1206-1239: The test currently only verifies the file mode (0o600)
of the auth.json file but does not verify the parent auth directory mode
(0o700). After the assertion that checks the auth file mode in the
writeTuiAuthEntry test, add an additional assertion that checks the mode of the
parent directory (the "opencode" directory) using statSync on the directory
path. Verify that the directory mode equals 0o700 using the same bitwise
operation pattern (mode & 0o777) to ensure the directory has the correct
restricted permissions as part of the security hardening contract.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e890c58e-a4cc-4b88-b629-f45544649c40
📒 Files selected for processing (10)
README.mdlib/auth-store.jsopencode-plugins/ttc-message-transform-core.jsopencode-plugins/ttc-message-transform.jspackage.jsonscripts/cli.mjstests/opencode-ttc-plugin.test.jstui/index.tsxtui/settings.jstui/sidebar-state.js
|
CodeRabbit nitpick also fixed in 6387ae6: the auth helper test now verifies the parent opencode auth directory is mode 0o700 in addition to auth.json mode 0o600. Full verification: npm test 60/60 pass, syntax checks pass, npm pack --dry-run passed. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
opencode-plugins/ttc-message-transform.js (1)
490-496: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider importing
getAuthStorePathfromlib/auth-store.jsto eliminate duplication.This function is identical to the one in
lib/auth-store.js. While both implementations are now in sync (both validateisAbsolute), maintaining two copies creates a maintenance risk where they could diverge in future updates.♻️ Suggested refactor
+import { getAuthStorePath } from "../lib/auth-store.js"; + // ... at lines 490-496, remove the local function: -function getAuthStorePath(env = process.env) { - const raw = String(env.XDG_DATA_HOME ?? "").trim(); - if (raw && isAbsolute(raw)) { - return join(raw, "opencode", "auth.json"); - } - return join(homedir(), ".local", "share", "opencode", "auth.json"); -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@opencode-plugins/ttc-message-transform.js` around lines 490 - 496, The function getAuthStorePath in ttc-message-transform.js is a duplicate of the one already present in lib/auth-store.js, creating maintenance risk if they diverge. Remove the getAuthStorePath function definition (currently at lines 490-496) from ttc-message-transform.js and instead import it from lib/auth-store.js at the top of the file with the other imports. This ensures a single source of truth for this shared utility function while maintaining the same functionality throughout the codebase.README.md (1)
247-247:⚠️ Potential issue | 🟡 MinorThe sidebar 2-second polling interval claim is verified and accurate.
Line 247 correctly states the sidebar polls every 2 seconds—this is confirmed in
tui/index.tsx:59wheresetInterval(() => setRefreshCount((count) => count + 1), 2000)implements the 2000ms polling mechanism. The plugin controls this directly; it is not OpenCode's default behavior.The apparent conflict with line 70 (immediate refresh) is resolved: the sidebar uses both mechanisms. Beyond polling, it subscribes to OpenCode events (
message.updatedandsession.statusattui/index.tsx:57-58) which trigger immediate refresh viatriggerRefresh. The 2-second polling serves as a fallback.The README statement is accurate but could clarify that the sidebar also refreshes immediately on auth-related events, not solely on the 2-second interval.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 247, Update line 247 in the README to clarify that the sidebar uses two refresh mechanisms, not just the 2-second polling interval. The current statement only mentions polling but should also explain that the sidebar subscribes to auth-related events (message.updated and session.status, as implemented in tui/index.tsx at lines 57-58) which trigger immediate refresh, while the 2-second polling interval (setInterval at tui/index.tsx line 59) serves as a fallback. Modify the text to make it clear the sidebar refreshes both immediately on events and periodically on the 2-second interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tui/settings.js`:
- Around line 266-272: The promptLogin function (and confirmLogout function at
lines 280-303) lacks defensive guards to validate the api and dialog parameters
before using them, while openTtcSettingsMenu has such guards. Since promptLogin
and confirmLogout are both entry points via slash commands at lines 445 and 455,
add defensive guard checks at the start of both functions to validate that api
and dialog are valid objects before calling dialog.replace(), returning early if
either parameter is invalid. Apply the same validation pattern used in
openTtcSettingsMenu to ensure consistency across all entry points.
---
Outside diff comments:
In `@opencode-plugins/ttc-message-transform.js`:
- Around line 490-496: The function getAuthStorePath in ttc-message-transform.js
is a duplicate of the one already present in lib/auth-store.js, creating
maintenance risk if they diverge. Remove the getAuthStorePath function
definition (currently at lines 490-496) from ttc-message-transform.js and
instead import it from lib/auth-store.js at the top of the file with the other
imports. This ensures a single source of truth for this shared utility function
while maintaining the same functionality throughout the codebase.
In `@README.md`:
- Line 247: Update line 247 in the README to clarify that the sidebar uses two
refresh mechanisms, not just the 2-second polling interval. The current
statement only mentions polling but should also explain that the sidebar
subscribes to auth-related events (message.updated and session.status, as
implemented in tui/index.tsx at lines 57-58) which trigger immediate refresh,
while the 2-second polling interval (setInterval at tui/index.tsx line 59)
serves as a fallback. Modify the text to make it clear the sidebar refreshes
both immediately on events and periodically on the 2-second interval.
🪄 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 Plus
Run ID: 3b3fbb7f-4748-4da6-a1da-94c347d47092
📒 Files selected for processing (6)
README.mdlib/auth-store.jsopencode-plugins/ttc-message-transform.jsscripts/cli.mjstests/opencode-ttc-plugin.test.jstui/settings.js
| function promptLogin(api, dialog) { | ||
| dialog.replace(() => api.ui.DialogAlert({ | ||
| title: "Add TTC API Key", | ||
| message: "Use `opencode auth login` and choose opencode-ttc-plugin, or run `opencode-ttc-plugin login` in a terminal. The TUI does not accept API keys because plain dialogs can display secrets.", | ||
| onConfirm: () => openTtcSettingsMenu(api, dialog) | ||
| })); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding defensive guards for consistency with openTtcSettingsMenu.
openTtcSettingsMenu guards against invalid api/dialog at line 323, but promptLogin and confirmLogout are also entry points via slash commands (lines 445, 455) and lack similar guards. If the dialog object is unexpectedly invalid, dialog.replace() would throw.
🛡️ Proposed defensive guard
function promptLogin(api, dialog) {
+ if (!api?.ui || !dialog?.replace) return;
dialog.replace(() => api.ui.DialogAlert({
title: "Add TTC API Key",
message: "Use `opencode auth login` and choose opencode-ttc-plugin, or run `opencode-ttc-plugin login` in a terminal. The TUI does not accept API keys because plain dialogs can display secrets.",
onConfirm: () => openTtcSettingsMenu(api, dialog)
}));
} async function confirmLogout(api, dialog) {
+ if (!api?.ui || !dialog?.replace) return;
dialog.replace(() => api.ui.DialogConfirm({Also applies to: 280-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tui/settings.js` around lines 266 - 272, The promptLogin function (and
confirmLogout function at lines 280-303) lacks defensive guards to validate the
api and dialog parameters before using them, while openTtcSettingsMenu has such
guards. Since promptLogin and confirmLogout are both entry points via slash
commands at lines 445 and 455, add defensive guard checks at the start of both
functions to validate that api and dialog are valid objects before calling
dialog.replace(), returning early if either parameter is invalid. Apply the same
validation pattern used in openTtcSettingsMenu to ensure consistency across all
entry points.
1 participant
Summary
Hardens the TTC plugin auth flow, replaces free-text model selection with a docs-grounded picker, and fixes sidebar metrics so they survive OpenCode session switches and process restarts.
Auth flow
opencode-ttc-pluginwhile preserving legacythe-token-company-pluginreads.opencode-ttc-plugin login/logoutplus/ttc-login//ttc-logout.--key; supports no-echo TTY input and--stdin.0o600file mode,0o700dir mode, corrupt-file refusal, and legacy-entry rotation.Model/settings UX
bear-2/bear-1.2options plus a custom-id escape hatch./ttcto one contextual row.Sidebar metrics durability
cmd+x+lsession switches.Verification
npm test-> 55/55 passgit diff --checknpm run smoke:pluginblocked locally:TTC_API_KEYis not setRegression Coverage
tests/opencode-ttc-plugin.test.js: hydrates persisted sidebar state before writing resume metrics.tests/opencode-ttc-plugin.test.js: concurrent first-use hydration shares one stats object.tests/opencode-ttc-plugin.test.js: session-switch helpers avoid rendering stale state as zeroed metrics.Manual QA Needed
With a real TTC key in OpenCode:
cmd+x+l, then switch back to A.Expected: A's sidebar totals restore and continue accumulating instead of resetting.
Generated with opencode
Summary by CodeRabbit
Release Notes
login/logoutcommands (TTY prompt and--stdin), and improved sidebar session stats that persist/resume across conversations.