fix: promptContinue prevents TUI from overwriting quota/verify output#583
fix: promptContinue prevents TUI from overwriting quota/verify output#583amartidandqdq wants to merge 1 commit into
Conversation
Greptile SummaryThis PR fixes the TUI overwrite bug by introducing a
Confidence Score: 3/5The The src/plugin.ts — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[promptLoginMode returns menuResult] --> B{menuResult.mode}
B -->|check| C[Print quota output]
B -->|manage| D[Toggle account, print status]
B -->|verify-all| E{accounts.length == 0?}
E -->|yes| F[Print No accounts available]
E -->|no| G[Verify each account, print summary]
B -->|verify| H{verifyAccountIndex?}
H -->|undefined| I[promptAccountIndexForVerification]
I -->|cancelled| J[Print Verification cancelled]
I -->|index found| K[Verify single account]
H -->|defined| K
K -->|ok| L[Print ready status]
K -->|blocked| M[Print blocked + URL]
K -->|error| N[Print error]
C --> PC1[promptContinue]
D --> PC2[promptContinue]
F --> PC3[promptContinue]
G --> PC4[promptContinue]
J --> PC5[promptContinue]
L --> PC6[promptContinue]
M --> PC7[promptContinue]
N --> PC8[promptContinue]
PC1 & PC2 & PC3 & PC4 & PC5 & PC6 & PC7 & PC8 --> CONT[continue — back to promptLoginMode]
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
src/plugin.ts:1453
**Undocumented `apiKey` change may inject a literal string as an auth header**
The PR description only mentions `promptContinue`, but `apiKey` has quietly changed from `""` to `"antigravity-oauth"`. Depending on how the opencode plugin framework consumes `LoaderResult.apiKey`, a non-empty value could cause the framework to populate an `Authorization: Bearer antigravity-oauth` header on outgoing requests. While the custom `fetch` override handles real OAuth token injection, if the framework adds API-key-based headers independently before or after that path, the literal string could leak into requests or override the proper OAuth header.
### Issue 2 of 3
src/plugin.ts:2
**Undocumented import path change is not explained in the PR**
The import was changed from `@opencode-ai/plugin` to `@opencode-ai/plugin/tool`. This sub-path change is unrelated to the stated `promptContinue` fix and is not mentioned in the PR description. If `@opencode-ai/plugin` previously resolved to a different entry point than `@opencode-ai/plugin/tool`, the API surface available to this module may differ in subtle ways. A brief note on why this correction was needed would be helpful.
### Issue 3 of 3
src/plugin/cli.ts:22-30
**`promptContinue` silently no-ops in non-TTY but other prompt helpers do not guard the same way**
`promptContinue` guards with `if (!isTTY()) return`, which is correct. However, `promptProjectId` and `promptAddAnotherAccount` do not have this guard — they will try to call `rl.question` on a non-TTY stdin without protection. While this inconsistency pre-exists this PR, the new function establishes the right pattern that the existing helpers don't follow, which may cause confusion about whether the guard is actually required.
Reviews (1): Last reviewed commit: "fix: promptContinue prevents TUI from ov..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| return { | ||
| apiKey: "", | ||
| apiKey: "antigravity-oauth", |
There was a problem hiding this comment.
apiKey change may inject a literal string as an auth header
The PR description only mentions promptContinue, but apiKey has quietly changed from "" to "antigravity-oauth". Depending on how the opencode plugin framework consumes LoaderResult.apiKey, a non-empty value could cause the framework to populate an Authorization: Bearer antigravity-oauth header on outgoing requests. While the custom fetch override handles real OAuth token injection, if the framework adds API-key-based headers independently before or after that path, the literal string could leak into requests or override the proper OAuth header. Is this change intentional? What does the opencode framework do with a non-empty apiKey when a custom fetch is also provided? Was "" causing the provider to be treated as unauthenticated?
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 1453
Comment:
**Undocumented `apiKey` change may inject a literal string as an auth header**
The PR description only mentions `promptContinue`, but `apiKey` has quietly changed from `""` to `"antigravity-oauth"`. Depending on how the opencode plugin framework consumes `LoaderResult.apiKey`, a non-empty value could cause the framework to populate an `Authorization: Bearer antigravity-oauth` header on outgoing requests. While the custom `fetch` override handles real OAuth token injection, if the framework adds API-key-based headers independently before or after that path, the literal string could leak into requests or override the proper OAuth header. Is this change intentional? What does the opencode framework do with a non-empty `apiKey` when a custom `fetch` is also provided? Was `""` causing the provider to be treated as unauthenticated?
How can I resolve this? If you propose a fix, please make it concise.| @@ -1,5 +1,5 @@ | |||
| import { exec } from "node:child_process"; | |||
| import { tool } from "@opencode-ai/plugin"; | |||
| import { tool } from "@opencode-ai/plugin/tool"; | |||
There was a problem hiding this comment.
The import was changed from @opencode-ai/plugin to @opencode-ai/plugin/tool. This sub-path change is unrelated to the stated promptContinue fix and is not mentioned in the PR description. If @opencode-ai/plugin previously resolved to a different entry point than @opencode-ai/plugin/tool, the API surface available to this module may differ in subtle ways. A brief note on why this correction was needed would be helpful.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 2
Comment:
**Undocumented import path change is not explained in the PR**
The import was changed from `@opencode-ai/plugin` to `@opencode-ai/plugin/tool`. This sub-path change is unrelated to the stated `promptContinue` fix and is not mentioned in the PR description. If `@opencode-ai/plugin` previously resolved to a different entry point than `@opencode-ai/plugin/tool`, the API surface available to this module may differ in subtle ways. A brief note on why this correction was needed would be helpful.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export async function promptContinue(): Promise<void> { | ||
| if (!isTTY()) return; | ||
| const rl = createInterface({ input, output }); | ||
| try { | ||
| await rl.question("Press Enter to return to menu..."); | ||
| } finally { | ||
| rl.close(); | ||
| } | ||
| } |
There was a problem hiding this comment.
promptContinue silently no-ops in non-TTY but other prompt helpers do not guard the same way
promptContinue guards with if (!isTTY()) return, which is correct. However, promptProjectId and promptAddAnotherAccount do not have this guard — they will try to call rl.question on a non-TTY stdin without protection. While this inconsistency pre-exists this PR, the new function establishes the right pattern that the existing helpers don't follow, which may cause confusion about whether the guard is actually required.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/cli.ts
Line: 22-30
Comment:
**`promptContinue` silently no-ops in non-TTY but other prompt helpers do not guard the same way**
`promptContinue` guards with `if (!isTTY()) return`, which is correct. However, `promptProjectId` and `promptAddAnotherAccount` do not have this guard — they will try to call `rl.question` on a non-TTY stdin without protection. While this inconsistency pre-exists this PR, the new function establishes the right pattern that the existing helpers don't follow, which may cause confusion about whether the guard is actually required.
How can I resolve this? If you propose a fix, please make it concise.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details🔇 Additional comments (13)
WalkthroughThis PR adds interactive pause points to CLI flows to prevent output from being immediately overwritten by the menu. A new Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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. 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. Comment |
1 participant
Fixes #537. Based on PR #562 by @lawRathod.
Problem: When executing opencode auth login and selecting Check quotas, Verify one account, or Verify all accounts, the output is printed but immediately overwritten by the TUI menu.
Fix: Added promptContinue() helper that waits for Enter before returning to main menu. Added calls after all informational outputs: quota checks, account toggles, verification statuses, model configuration.