Let reconnect change a connection's access level - #16
Conversation
A read-only connection could not be upgraded to trading from the CLI: `reconnect` took no connection type and `handleConnect` explicitly sent `connectionType: undefined` for every reconnect, so the API always kept the existing level. Users with only read-only connections hit "No valid accounts available" on `trade` and were then told to run `reconnect`, which reported nothing to do because it only listed disabled connections. - `reconnect --connection-type <read|trade>`, forwarded to the login call. Omitting it still preserves the current level. - Without an id, `--connection-type trade` also lists active read-only connections, skipping brokerages that cannot trade. - With an id, validate the brokerage supports trading first; the API error for this has no handler and would print a raw stack. - `selectAccount` now reports the per-account rejection reasons it already computes, and names the matching remedy.
|
@codex review |
![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: 093e180643
ℹ️ 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".
The candidate filter only added healthy connections for a trade upgrade, so `reconnect --connection-type read` without an id considered disabled connections only. An active trade connection was never offered and the command reported nothing to do, even though the requested downgrade was available. Select on whether the requested level differs from the current one, in either direction, and keep the allows_trading check on upgrades only.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Problem
A read-only connection could not be upgraded to trading from the CLI.
reconnectaccepted no connection type, andhandleConnectexplicitly sentconnectionType: undefinedfor every reconnect ("Don't modify connection type if reconnecting"), so the API always kept the existing level.That left users with only read-only connections in a loop (SNAP-9604):
trade … option call→No valid accounts available… fix your disabled connections with snaptrade reconnectreconnect→No disabled connections found, therefore there's no need to reconnect(it only listed disabled connections)error: too many arguments for 'reconnect'The reporter's own session confirms step 3's dead end: his Questrade reconnect completed successfully and the connection stayed
read.The upstream cause — the personal dashboard minting read-only connections — is fixed in passiv#14210. This PR is the remedy for connections that already exist.
Change
reconnect --connection-type <read|trade>, forwarded throughhandleConnect. Omitting it still preserves the current level, so existing behaviour is unchanged.--connection-type tradealso lists active read-only connections, not just disabled ones, filtered to brokerages that can actually trade.connectionType=tradefor a brokerage with no trade auth type and there's no global error handler inmain.ts, so that would otherwise print a raw stack.selectAccountreports the per-account rejection reasons it already computes and names the matching remedy, instead of pointing every failure at disabled-connection repair.Before / after for the reporter's case:
Tests
npm run verifygreen — lint, 28 tests, build. Six new: the upgrade path, explicit-id forwarding, the trade-incapable guard (both interactive and by-id), invalid type rejection, and three onhandleConnectpinning that reconnect-without-type stays undefined, reconnect-with-type forwards, and new connections still default totrade-if-available.Note
Options are still gated by the hardcoded
brokers_with_mleg_optionslist, so Questrade and IBKR can't place option orders through the CLI even with a trade connection. Fixing that means exposingBrokerage.has_option_trading_supporton the brokerage API — separate change, spansapi.yamland an SDK regen.