Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return None; | ||
| } | ||
| }; | ||
| *credentials.write().await = next; |
There was a problem hiding this comment.
When a stream rotates after an authentication or quota failure, this replaces the shared runtime credentials but leaves the selected account label unchanged. Nothing restores the original credentials when the stream ends, so later requests and prewarms can use the alternate account while the UI and usage reporting still identify the original account.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-openai-runtime/src/openai_provider_impl.rs
Line: 58
Comment:
**Restore rotated credentials**
When a stream rotates after an authentication or quota failure, this replaces the shared runtime credentials but leaves the selected account label unchanged. Nothing restores the original credentials when the stream ends, so later requests and prewarms can use the alternate account while the UI and usage reporting still identify the original account.
**Knowledge Base Used:**
- [Authentication and account management](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/authentication-and-account-management.md)
- [Provider selection and runtime adapters](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-selection-and-runtime-adapters.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| #[cfg(unix)] | ||
| { | ||
| let operation = if shared { libc::LOCK_SH } else { libc::LOCK_EX }; | ||
| if unsafe { libc::flock(file.as_raw_fd(), operation) } != 0 { | ||
| return None; | ||
| } | ||
| } |
There was a problem hiding this comment.
On Windows, these account and health locks open their lock files but never acquire an operating-system lock. Two jcode processes can therefore read the same pool state and publish conflicting updates, causing imported accounts, active selections, cooldowns, or quota history to be lost. Add Windows inter-process locking around the full read-modify-write cycle.
Knowledge Base Used: Authentication and account management
Artifacts
- Ran the existing account-lease cross-process test on Linux; it passed by blocking the first child and allowing the second after release, showing only the Unix flock path works.
- Executed the focused source-level platform proof; it shows both account and health lock acquire paths return an unlocked handle when Unix-only code is excluded, confirming the Windows defect.
- Authored and executed a Python proof that extracts both acquire implementations and validates their Windows-effective code paths contain no lock operation, confirming the missing Windows synchronization.
- Checked installed Rust targets; only the Linux target is present, so a native Windows runtime reproduction was unavailable.
- Checked the reviewed product file for a diff after validation; no product-code edit was made.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/auth/provider_pool.rs
Line: 452-458
Comment:
**Lock account files on Windows**
On Windows, these account and health locks open their lock files but never acquire an operating-system lock. Two jcode processes can therefore read the same pool state and publish conflicting updates, causing imported accounts, active selections, cooldowns, or quota history to be lost. Add Windows inter-process locking around the full read-modify-write cycle.
**Knowledge Base Used:** [Authentication and account management](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/authentication-and-account-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| } | ||
|
|
||
| fn write_unlocked(provider: &str, file: &AccountFile) -> Result<()> { | ||
| let persisted = PersistedAccountFile::from_managed(provider, file)?; |
There was a problem hiding this comment.
Credential values are written to native storage before the metadata file is published, with no rollback if metadata publication fails. For example, a disk-full or permission error while updating an existing account can leave the old metadata pointing at newly replaced credentials; a new account can instead leave orphaned secrets. Stage or reconcile both stores so a failed save cannot partially update an account.
Knowledge Base Used: Authentication and account management
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/auth/provider_pool.rs
Line: 521
Comment:
**Make credential writes atomic**
Credential values are written to native storage before the metadata file is published, with no rollback if metadata publication fails. For example, a disk-full or permission error while updating an existing account can leave the old metadata pointing at newly replaced credentials; a new account can instead leave orphaned secrets. Stage or reconcile both stores so a failed save cannot partially update an account.
**Knowledge Base Used:** [Authentication and account management](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/authentication-and-account-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let item = self.inner.as_mut().poll_next(context); | ||
| if matches!(item, Poll::Ready(None)) { | ||
| self.account_lease = None; | ||
| } | ||
| item |
There was a problem hiding this comment.
The wrapper forwards Some(Err(_)) without invoking account or provider failover, while Cursor starts its network operation only after returning an event stream. Cursor transport, authentication, and quota failures therefore bypass the new same-provider failover path, and the user receives the original error without trying another pooled account.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/provider/dispatch.rs
Line: 44-48
Comment:
**Handle streamed failures**
The wrapper forwards `Some(Err(_))` without invoking account or provider failover, while Cursor starts its network operation only after returning an event stream. Cursor transport, authentication, and quota failures therefore bypass the new same-provider failover path, and the user receives the original error without trying another pooled account.
**Knowledge Base Used:**
- [Model provider integration](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-integration.md)
- [Provider selection and runtime adapters](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-selection-and-runtime-adapters.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| /// List pooled provider accounts or persistently select one account | ||
| Accounts { | ||
| /// Provider to inspect: openai, cursor, antigravity, or all | ||
| #[arg(id = "pool_provider", default_value = "all", value_name = "PROVIDER")] | ||
| pool_provider: String, | ||
|
|
||
| /// Persistently select an account by its displayed label | ||
| #[arg(long, value_name = "LABEL")] | ||
| switch: Option<String>, | ||
|
|
||
| /// Emit JSON instead of plain text | ||
| #[arg(long)] | ||
| json: bool, |
There was a problem hiding this comment.
This is non-blocking, but managed Cursor and Antigravity accounts can be listed and switched without any removal action in the CLI, TUI, picker, or provider_pool. Users cannot remove revoked or unwanted imported credentials and metadata through the account-management flow, creating unnecessary manual cleanup work.
Knowledge Base Used: Authentication and account management
Artifacts
- A non-mutating authored script inspected the CLI, TUI, and provider-pool removal paths; it provides the repeatable evidence command.
Managed account removal audit output
- The executed audit prints the relevant CLI fields and dispatch plus TUI dispatch and picker code, showing no managed removal implementation; the takeaway is that the removal path is absent.
- A non-mutating authored CLI script requests account help and then attempts the unsupported `--remove` option; it is ready to reproduce the parser behavior once the workspace build is available.
CLI removal invocation attempt
- The real CLI invocation with `auth accounts cursor --remove candidate-account --json` was attempted under a 15-second timeout; concurrent Cargo builds prevented completion, so it provides no contrary runtime result.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/args.rs
Line: 1067-1079
Comment:
**Support removing pooled accounts**
This is non-blocking, but managed Cursor and Antigravity accounts can be listed and switched without any removal action in the CLI, TUI, picker, or `provider_pool`. Users cannot remove revoked or unwanted imported credentials and metadata through the account-management flow, creating unnecessary manual cleanup work.
**Knowledge Base Used:** [Authentication and account management](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/authentication-and-account-management.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
0656752 to
c84657c
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
1 participant
Summary
Adds a provider-scoped account pool for authenticated providers, with durable metadata, safe account selection, quota-aware ranking, cooldowns, leases, failover, and account-management UI/CLI flows.
Closes #1238
Closes #1239
Refs #1130
Categorized changes
Validation
upstream/masterand zero commits behind it.scripts/dev_cargo.sh check -p jcode --bin jcodepassed from the final rebased branch.Scope notes
#1130 describes a broader Anthropic streaming failover report. This PR shares the account-first cooldown, lease, retry-after, and request-local failover foundation, but references rather than closes that issue because not every Anthropic-specific streaming behavior is claimed here.