docs: apply spec corrections for identity, decimal parse, mocks

Callers set MCP/OAuth identity on Config before Login/Connect.
Unparseable money errors instead of silent zero. Tests hit an
httptest Robinhood MCP mock, not only injected Callers.
This commit is contained in:
2026-09-01 10:49:37 -05:00
parent 58b9b48e15
commit 6600aa0eff
@@ -26,12 +26,28 @@ A standalone Go module at `/fast/projects/golang/robinhood-agentic-mcp` that:
## Operator contract ## Operator contract
```go
type Config struct {
URL string // empty → DefaultURL
TokenFile string
Name string // MCP Implementation.Name and OAuth ClientName; empty → "robinhood-agentic-mcp"
Version string // MCP Implementation.Version; empty → "0.1.0"
}
func Login(ctx context.Context, cfg Config) (accountID string, err error)
func Connect(ctx context.Context, cfg Config) (*API, error)
```
`Config` is defined in `auth` (Login owns identity and the token file). `rh` re-exports it as `type Config = auth.Config` so callers can import once. This avoids an import cycle (`rh``auth` → not `rh`).
| Call | Behavior | | Call | Behavior |
|---|---| |---|---|
| `auth.Login(ctx, mcpURL, tokenFile)` | If `ROBINHOOD_ACCESS_TOKEN` is set, write it (and optional `ROBINHOOD_REFRESH_TOKEN`) to `tokenFile` mode `0600`. Otherwise run the browser OAuth dance and persist the full token set. | | `auth.Login` / `rh.Login` | If `ROBINHOOD_ACCESS_TOKEN` is set, write it (and optional `ROBINHOOD_REFRESH_TOKEN`) to `cfg.TokenFile` mode `0600`. Otherwise run the browser OAuth dance using `cfg.Name` / `cfg.Version` and persist the full token set. |
| `rh.Connect(ctx, mcpURL, tokenFile)` | Read `tokenFile`, open a session, return an `rh.API` with all subclients wired. Fail closed if tokens are missing or the session cannot be used. | | `rh.Connect` | Read `cfg.TokenFile`, open a session identified as `cfg.Name`/`cfg.Version`, return an `rh.API` with all subclients wired. Fail closed if tokens are missing or the session cannot be used. |
| `client.Call(ctx, name, args)` | Escape hatch: invoke any tool by MCP name and return JSON. | | `client.Call(ctx, name, args)` | Escape hatch: invoke any tool by MCP name and return JSON. |
Identity must be set on `Config` **before** `Login` or `Connect`. After a session exists it is not changeable. Apps that need a distinct Robinhood OAuth client (tradey, another bot) pass their own `Name`. Empty `Name`/`Version` keep the library defaults.
Daemon/headless callers must not call `Login` (no browser). They call `Connect` with an existing token file. Daemon/headless callers must not call `Login` (no browser). They call `Connect` with an existing token file.
## Architecture ## Architecture
@@ -44,10 +60,11 @@ MCP SDK: `github.com/modelcontextprotocol/go-sdk` (same major as tradey, current
One shared session. Asset-class packages wrap it. The root package is a facade so a caller can import once. One shared session. Asset-class packages wrap it. The root package is a facade so a caller can import once.
``` ```
s1d3sw1ped/robinhood-agentic-mcp // rh: DefaultURL, Connect, API, shared enums s1d3sw1ped/robinhood-agentic-mcp // rh: DefaultURL, Config, Connect, API, shared enums
client/ // session, Call, ToolError, transport client/ // session, Call, ToolError, transport
auth/ // Login, TokenSet, Read/Write tokens auth/ // Login, TokenSet, Read/Write tokens
internal/wire // unwrap data envelopes, decimal JSON internal/wire // unwrap data envelopes, decimal JSON
internal/rhntest // httptest Robinhood MCP mock
accounts/ accounts/
equity/ equity/
options/ options/
@@ -73,20 +90,21 @@ type API struct {
Scanner *scanner.Client Scanner *scanner.Client
} }
func Connect(ctx context.Context, mcpURL, tokenFile string) (*API, error) func Connect(ctx context.Context, cfg Config) (*API, error)
func Login(ctx context.Context, mcpURL, tokenFile string) (accountID string, err error) // re-exports auth.Login func Login(ctx context.Context, cfg Config) (accountID string, err error) // re-exports auth.Login
``` ```
Empty `mcpURL` means `DefaultURL`. Subpackages also export `New(c client.Caller) *Client` so callers can wire a single package without the facade. Empty `cfg.URL` means `DefaultURL`. Subpackages also export `New(c client.Caller) *Client` so callers can wire a single package without the facade.
Each subclient depends only on `client.Caller`. That keeps tests stub-only and lets a future tradey adapter inject the same caller. Each subclient depends only on `client.Caller`. Unit tests inject a stub; transport tests hit `rhntest`. A future tradey adapter can inject the same caller.
## Auth and transport ## Auth and transport
Copied from tradeys working path (`internal/broker/oauth.go`, `login.go`, `mcp.go`), with identity strings changed so this library is a distinct OAuth client from tradey and from the Grok-chat MCP. Copied from tradeys working path (`internal/broker/oauth.go`, `login.go`, `mcp.go`). Identity is caller-owned via `Config.Name` / `Config.Version` so each app is a distinct Robinhood OAuth client (tradey, this librarys default, Grok-chat MCP, and so on).
- MCP implementation name: `robinhood-agentic-mcp`, version `0.1.0`. - MCP `Implementation.Name` / `Implementation.Version`: `cfg.Name`, `cfg.Version` (defaults `robinhood-agentic-mcp` / `0.1.0`).
- OAuth dynamic client registration `ClientName`: `robinhood-agentic-mcp` (not `tradey`). - OAuth dynamic client registration `ClientName`: the same `cfg.Name`.
- OAuth callback page text uses `cfg.Name` (not a hardcoded “Tradey is signed in”).
- `StreamableClientTransport` with `DisableStandaloneSSE: true`. - `StreamableClientTransport` with `DisableStandaloneSSE: true`.
- Prefer the SDK session `CallTool`. If session connect fails, fall back to HTTP POST JSON-RPC `tools/call` with `Authorization: Bearer` and `Accept: application/json, text/event-stream`. - Prefer the SDK session `CallTool`. If session connect fails, fall back to HTTP POST JSON-RPC `tools/call` with `Authorization: Bearer` and `Accept: application/json, text/event-stream`.
- Token file JSON matches tradeys `TokenSet` (access, refresh, type, expiry, client_id/secret, auth/token/redirect URLs, account_id). Mode `0600`. - Token file JSON matches tradeys `TokenSet` (access, refresh, type, expiry, client_id/secret, auth/token/redirect URLs, account_id). Mode `0600`.
@@ -119,7 +137,19 @@ Public money, size, and price fields use `github.com/alpacahq/alpacadecimal` imp
- Optional amounts: `*decimal.Decimal` (Alpaca `PlaceOrderRequest.Qty` / `LimitPrice`). - Optional amounts: `*decimal.Decimal` (Alpaca `PlaceOrderRequest.Qty` / `LimitPrice`).
- Never `float64` for money, size, prices, buying power, volume, or bar OHLC. - Never `float64` for money, size, prices, buying power, volume, or bar OHLC.
On the way out, encode with `Decimal.String()` onto Robinhoods string fields (`quantity`, `limit_price`, `stop_price`, `dollar_amount`, option `price`). On the way in, accept JSON string **or** number (tradeys envelope) and parse into `decimal.Decimal`. Missing/empty becomes the zero decimal or a nil pointer, matching whether the field is required. On the way out, encode with `Decimal.String()` onto Robinhoods string fields (`quantity`, `limit_price`, `stop_price`, `dollar_amount`, option `price`). On the way in, accept JSON string **or** number (tradeys envelope) and parse into `decimal.Decimal`.
Parse rules for money/size/price:
| JSON | Optional `*decimal.Decimal` | Required `decimal.Decimal` |
|---|---|---|
| field omitted or JSON `null` | `nil`, not an error | zero value, not an error |
| `0` / `"0"` / `"0.0"` | pointer to zero | zero |
| `""` | `nil`, not an error | zero, not an error |
| valid number or numeric string | parsed value | parsed value |
| present but unparseable (`"n/a"`, object, bool, array) | **error** (`ToolError` parse), never silent zero/nil | **error**, never silent zero |
Failed parsing is never coerced into zero or nil. Absent/intentional empty is never treated as a parse failure.
### Enums (Alpaca-shaped names, Robinhood wire values) ### Enums (Alpaca-shaped names, Robinhood wire values)
@@ -300,8 +330,8 @@ If the captured fixture omits a table row (tool removed upstream), drop that met
## Data flow ## Data flow
``` ```
Login → browser OAuth or env token → tokens.json (0600) Login(Config) → browser OAuth or env token → tokens.json (0600), OAuth client = cfg.Name
Connect → ReadTokens → session (or RPC fallback) → rh.API Connect(Config) → ReadTokens → session (or RPC fallback) as cfg.Name/cfg.Version → rh.API
API.Equity.PlaceOrder(ctx, req) API.Equity.PlaceOrder(ctx, req)
→ map req to MCP args (decimal.String, rh.Limit → "limit") → map req to MCP args (decimal.String, rh.Limit → "limit")
→ client.Call("place_equity_order", args) → client.Call("place_equity_order", args)
@@ -313,32 +343,44 @@ Pagination: request structs take `Cursor string`; results expose `NextCursor` /
## Testing ## Testing
CI must not call the live broker or open a browser. CI must not call the live broker or open a browser. Tests otherwise go as far as they can: they hit an in-process Robinhood MCP mock over HTTP, not only an injected `Caller`.
- Stub `client.Caller` per package (same idea as tradeys `MCP.Call` func field). `internal/rhntest` is an `httptest.Server` that speaks Robinhoods MCP surface the way tradey talks to it:
- Every typed method has a test that asserts the MCP tool name and the JSON args (including decimal→string).
- Parser tests cover tradeys known envelopes: `accounts` with `data` wrapper, quotes as `{quotes:[{quote:{…}, close:{…}}]}`, historicals `{historicals:[{symbol, data_points}]}`, watchlists `title` vs `name`, string-or-number amounts. - Required protocol: JSON-RPC `tools/call` (tradeys RPC fallback). Optional extra: streamable-HTTP `CallTool` if the SDK client can talk to the same httptest without extra machinery.
- Requires `Authorization: Bearer` when the test sets a token.
- Dispatches on tool name and returns checked-in envelope fixtures (`testdata/*.json`) shaped like live Robinhood (`data` wrappers, string-or-number amounts, quote/close objects).
- Can return HTTP 4xx, JSON-RPC errors, and malformed bodies so `ToolError` and parse-error paths are exercised.
Required tests:
- **Transport:** `Connect` against `rhntest` (session if the mock can serve it, otherwise RPC fallback), bearer header, HTTP error → `ToolError`, identity `Name`/`Version` sent on the MCP initialize/OAuth client metadata.
- **Every typed method:** (1) stub `Caller` asserting MCP tool name + JSON args including `decimal.String()`; (2) round-trip through `rhntest` with a realistic success fixture.
- **Wire/decimal:** omitted/null/`""`/`0` vs unparseable (`"n/a"`, object) — the latter errors, the former does not.
- Parser fixtures from tradeys known envelopes: `accounts` with `data` wrapper, quotes as `{quotes:[{quote:{…}, close:{…}}]}`, historicals `{historicals:[{symbol, data_points}]}`, watchlists `title` vs `name`.
- `WriteTokens` mode `0600`. - `WriteTokens` mode `0600`.
- `Login` from `ROBINHOOD_ACCESS_TOKEN` (no OAuth). - `Login` from `ROBINHOOD_ACCESS_TOKEN` (no OAuth, no browser).
- `ToolError` is `errors.As`-able from a typed method failure. - `ToolError` is `errors.As`-able.
- Coverage test: a frozen `tools/list` fixture (captured once, checked in) must have a method for every name. When the live MCP grows, update the fixture and add the method. - Coverage: frozen `testdata/tools.json` from live `tools/list` must have a method for every name.
No live-MCP test in default `go test`. An optional `//go:build live` smoke is out of v1. No live-MCP or browser-OAuth in default `go test`. An optional `//go:build live` smoke is out of v1.
## Package map ## Package map
| Package | Does | Depends on | | Package | Does | Depends on |
|---|---|---| |---|---|---|
| `rh` | `Connect`, `API`, enums | all subpackages | | `rh` | `Config`, `Connect`, `Login`, `API`, enums | all subpackages |
| `client` | session, `Call`, `ToolError`, RPC fallback | MCP SDK, oauth2 | | `client` | session, `Call`, `ToolError`, RPC fallback | MCP SDK, oauth2 |
| `auth` | `Login`, token file | `client` (session during OAuth), MCP auth/oauthex | | `auth` | `Login`, token file | `client` (session during OAuth), MCP auth/oauthex |
| `internal/wire` | unwrap `data`, decimal JSON | alpacadecimal | | `internal/wire` | unwrap `data`, decimal JSON | alpacadecimal |
| `internal/rhntest` | httptest Robinhood MCP mock | `client` protocol |
| `accounts``scanner` | typed methods | `client.Caller`, `rh` enums, `internal/wire` | | `accounts``scanner` | typed methods | `client.Caller`, `rh` enums, `internal/wire` |
## Success criteria ## Success criteria
- `go test ./...` passes with no network. - `go test ./...` passes with no live network (httptest mock only).
- `Connect` against a stub caller is unnecessary; typed methods work with an injected `Caller`. - Typed methods work with an injected `Caller` **and** round-trip through `rhntest`.
- `Connect`/`Login` accept `Config.Name` / `Config.Version`; empty uses library defaults.
- `Login` with `ROBINHOOD_ACCESS_TOKEN` writes `0600` tokens. - `Login` with `ROBINHOOD_ACCESS_TOKEN` writes `0600` tokens.
- `equity.PlaceOrder` sends `quantity`/`limit_price` as decimal strings, `time_in_force` `gfd`, `type` `limit` — matching tradeys `placeArgs` for the same inputs. - `equity.PlaceOrder` sends `quantity`/`limit_price` as decimal strings, `time_in_force` `gfd`, `type` `limit` — matching tradeys `placeArgs` for the same inputs.
- Every name in the frozen `tools/list` fixture has a method. - Every name in the frozen `tools/list` fixture has a method.