fix(trading): use OAuth connection accounts for broker access#125
Conversation
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
fix(license): align attribution and Helm metadata
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
…h accounts and streamline portfolio identity discovery
… and holdings functions
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
fix(integrations): resolve trading accounts from OAuth connections
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
| Filename | Overview |
|---|---|
| apps/tradinggoose/lib/trading/order-detail.ts | Switched to readOrderTokenAccountId from readOrderCredentialId; orders placed before this deploy have credentialId in request JSON, so provider-detail lookups on historical records will return 404. |
| apps/tradinggoose/lib/trading/context.ts | Replaced workspace-credential auth with user-connection auth; removed NextRequest dependency; renamed fields throughout. Logic is internally consistent. |
| apps/tradinggoose/lib/credentials/oauth.ts | Renamed listOAuthConnectionsForUser to listOAuthConnectionAccountsForUser, replaced listOAuthCredentialAccountsForUser with resolveOAuthConnectionAccountForUser (user-scoped single-row lookup with isSignInOAuthProvider guard). |
| apps/tradinggoose/socket-server/trading/portfolio-manager.ts | resolveTradingPortfolioContext now directly calls resolveOAuthConnectionAccountForUser; stop() correctly clears all intervals, subscribers and cache. |
| apps/tradinggoose/app/api/providers/trading/portfolio-identities/route.ts | Dropped workspaceId/workflowId parameters and access checks; route is now authenticated-user-only, returning that user's own OAuth connections. |
| apps/tradinggoose/lib/trading/orders.ts | Order submission drops NextRequest param, uses authorizeTradingConnectionRequest; orderHistoryRequest now stores tokenAccountId instead of credentialId. |
| apps/tradinggoose/lib/trading/holdings.ts | Holdings dropped workspaceId/workflowId/NextRequest; authorization is now user-owned connection only. |
| apps/tradinggoose/hooks/queries/trading-portfolio.ts | Removed workspaceId from all socket subscription payloads and guards; subscriptions now fire as soon as provider is set. |
| apps/tradinggoose/lib/trading/order-records.ts | readOrderCredentialId replaced by readOrderTokenAccountId; credentialid removed from SECRET_KEY_EXACT_KEYS but still matched by SECRET_KEY_PATTERN so no redaction regression. |
| apps/tradinggoose/socket-server/index.ts | Adds tradingPortfolioStreamManager.stop() in the SIGINT/SIGTERM shutdown handler; duplicate connection_error listener appears pre-existing. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Client([Client]) -->|tokenAccountId in portfolioIdentity| OrderRoute[Order Route POST]
Client -->|provider only| IdentitiesRoute[Portfolio Identities Route GET]
Client -->|tokenAccountId in portfolioIdentity| HoldingsRoute[Holdings Route POST]
OrderRoute --> Auth1{Session or Internal Auth}
Auth1 -->|Fail| E401A[401 Unauthorized]
Auth1 -->|OK| checkWS[checkWorkspaceAccess]
checkWS -->|Fail| E404A[404 Not Found]
checkWS -->|OK| authConn1[authorizeTradingConnectionRequest]
authConn1 -->|null| E404B[404 Connection Not Found]
authConn1 -->|OK| resolveCtx1[resolveTradingProviderContext]
resolveCtx1 --> submitOrder[submitTradingOrder stores tokenAccountId]
IdentitiesRoute --> Auth2{Session?}
Auth2 -->|Fail| E401B[401]
Auth2 -->|OK| listConns[listOAuthConnectionAccountsForUser]
listConns --> identOptions[Return PortfolioIdentity list with tokenAccountId]
HoldingsRoute --> Auth3{Session or Internal Auth}
Auth3 -->|Fail| E401C[401]
Auth3 -->|OK| authConn2[authorizeTradingConnectionRequest]
authConn2 --> resolveCtx2[resolveTradingProviderContext]
resolveCtx2 --> getHoldings[getPortfolioDetail]
SocketServer([Socket Server]) --> resolvePortfolioCtx[resolveTradingPortfolioContext]
resolvePortfolioCtx --> pollBroker[Poll broker API per channel]
Reviews (2): Last reviewed commit: "Merge pull request #126 from BWJ2310-bac..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| throw new TradingServiceError('Tradier order history record is missing accountId') | ||
| } | ||
|
|
||
| const credentialId = readOrderCredentialId(order) | ||
| const tokenAccountId = readOrderTokenAccountId(order) | ||
| const serviceId = readOrderServiceId(order) | ||
| if (!credentialId || !serviceId) { | ||
| throw new TradingServiceError('Order history record is missing trading credential context') | ||
| if (!tokenAccountId || !serviceId) { | ||
| throw new TradingServiceError('Order history record is missing trading connection context') |
There was a problem hiding this comment.
readOrderTokenAccountId reads request.tokenAccountId from the stored JSON, but every order placed before this deployment has request.credentialId there instead. Those rows will return null, hit the guard, and throw 'Order history record is missing trading connection context' — so any user trying to view provider detail on a historical order will get an error immediately after rollout.
The PR description calls out that new requests with old payloads "should fail rather than be silently backfilled," but this affects already-persisted rows whose data was written correctly under the old contract. A read-time coalesce (readOrderTokenAccountId(order) ?? readOrderCredentialId(order)) would preserve backwards-read compatibility while the field name migrates.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/tradinggoose/lib/trading/order-detail.ts
Line: 61-67
Comment:
**Existing order records will fail provider-detail lookup after deploy**
`readOrderTokenAccountId` reads `request.tokenAccountId` from the stored JSON, but every order placed before this deployment has `request.credentialId` there instead. Those rows will return `null`, hit the guard, and throw `'Order history record is missing trading connection context'` — so any user trying to view provider detail on a historical order will get an error immediately after rollout.
The PR description calls out that new requests with old payloads "should fail rather than be silently backfilled," but this affects already-persisted rows whose data was written correctly under the old contract. A read-time coalesce (`readOrderTokenAccountId(order) ?? readOrderCredentialId(order)`) would preserve backwards-read compatibility while the field name migrates.
How can I resolve this? If you propose a fix, please make it concise.
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
refactor(trading): remove workspaceId from portfolio flow
2 participants
Summary
Promotes the current
fork/stagingchanges toupstream/main.This PR aligns trading broker access around user-owned OAuth connection accounts across portfolio identity discovery, order submission/detail, holdings, widgets, and socket portfolio streams. It also adds explicit socket portfolio polling shutdown and updates license/chart metadata plus the staging changelog.
Why
Trading flows were mixing workspace-scoped credential IDs with user-owned OAuth account IDs. Using OAuth connection accounts as the runtime broker identity keeps portfolio discovery, widget account selection, order execution, holdings, and realtime portfolio polling on the same contract.
Affected Areas
apps/tradinggooseapps/docspackages/*Issue Links( if any )
None.
Validation
Result: 10 test files passed, 90 tests passed.
Also verified the branch diff touches no
*/migration/*files.Risk / Rollout Notes
Deploy the app and socket server together because trading payloads now use OAuth token account IDs for broker selection.
Risk is concentrated in trading flows that still send old workspace credential IDs; those should fail under the new contract rather than being silently backfilled. Backout is to revert this PR and redeploy the previous app/socket server pair.
Config / Data Changes
Screenshots / Video
N/A. No visual layout changes; license page copy changes are text-only.
Checklist