feat(ibkr): Client Portal API v1 read-path wire layer + paper gateway harness#127
Merged
Conversation
- secdef_info(conid, sec_type) now carries required query params - Endpoint doc clarifies method+path/query only, bodies deferred - tests/endpoint.rs asserts all seven endpoint constructors - fix sso_expires unit doc (milliseconds, not seconds) - harden live.rs curl (-fksS) + document IBKR_GATEWAY override - Dockerfile: drop unused curl, document unpinned-download tradeoff - soften "lossless" wording in cpapi/mod.rs to faithful-subset framing - docker-compose.yml: restart: "no" for the auth-session harness - capture.sh: note it must run from the repo root
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds the ChangesIBKR CPAPI adapter
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docker/cpapi/capture.sh (1)
14-18: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
-k(insecure TLS) is intentional but worth documenting inline.The IBKR Client Portal Gateway ships with a self-signed certificate, so
-kis necessary for local dev. The static analysis flags this as CWE-295, which is correct in general but not actionable here. Consider adding a brief inline comment noting why-kis used (self-signed gateway cert) so future readers don't "fix" it by removing the flag. If the gateway ever supports a custom CA, switching to--cacertwould be preferable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/cpapi/capture.sh` around lines 14 - 18, Document the intentional insecure TLS behavior in the fetch function: add a brief inline comment next to curl’s -k flag explaining that the local IBKR Client Portal Gateway uses a self-signed certificate, while retaining -k for now and noting that a custom CA should use --cacert if supported later.Source: Linters/SAST tools
docs/superpowers/plans/2026-07-10-ibkr-cpapi-readpath-wire.md (1)
283-288: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlan's
secdef_info()signature is stale vs. the implementation.The plan shows
secdef_info()with no parameters, but the actual implementation inendpoint.rstakesconid: i64, sec_type: &strand renders them as query params. The PR objectives confirm this was a deliberate final adjustment ("requiredsecdef_infoquery parameters"). Consider updating the plan's Task 2 Step 3 code block and the test in Step 1 to match the shipped implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-07-10-ibkr-cpapi-readpath-wire.md` around lines 283 - 288, Update the plan’s Task 2 Step 3 `secdef_info` example and the Step 1 test to match the implemented `secdef_info(conid: i64, sec_type: &str)` signature, including both required query parameters and their expected rendered values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/adapter/ibkr/tests/live.rs`:
- Around line 20-23: Add a curl timeout to the Command invocation in the live
test by including the --max-time argument with a suitable limit such as 30
seconds in the arguments passed to curl, ensuring the request fails instead of
hanging indefinitely.
In `@docker/cpapi/Dockerfile`:
- Around line 1-19: Add a dedicated unprivileged user and group in the
Dockerfile, change ownership of /opt/clientportal and the gateway files to that
account, then add a USER instruction before CMD so bin/run.sh runs without root
privileges while retaining port 5000.
---
Nitpick comments:
In `@docker/cpapi/capture.sh`:
- Around line 14-18: Document the intentional insecure TLS behavior in the fetch
function: add a brief inline comment next to curl’s -k flag explaining that the
local IBKR Client Portal Gateway uses a self-signed certificate, while retaining
-k for now and noting that a custom CA should use --cacert if supported later.
In `@docs/superpowers/plans/2026-07-10-ibkr-cpapi-readpath-wire.md`:
- Around line 283-288: Update the plan’s Task 2 Step 3 `secdef_info` example and
the Step 1 test to match the implemented `secdef_info(conid: i64, sec_type:
&str)` signature, including both required query parameters and their expected
rendered values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: db0f1ca2-2365-4258-93e8-9afc92d925e3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (32)
CHANGELOG.mdCargo.tomlJustfileREADME.md_typos.tomlcrates/adapter/ibkr/Cargo.tomlcrates/adapter/ibkr/src/cpapi/auth.rscrates/adapter/ibkr/src/cpapi/endpoint.rscrates/adapter/ibkr/src/cpapi/error.rscrates/adapter/ibkr/src/cpapi/mod.rscrates/adapter/ibkr/src/cpapi/portfolio.rscrates/adapter/ibkr/src/cpapi/secdef.rscrates/adapter/ibkr/src/lib.rscrates/adapter/ibkr/tests/auth.rscrates/adapter/ibkr/tests/endpoint.rscrates/adapter/ibkr/tests/error.rscrates/adapter/ibkr/tests/fixtures/cpapi/auth_status.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/iserver_accounts.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/portfolio_accounts.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/positions.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/secdef_info.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/secdef_search.jsoncrates/adapter/ibkr/tests/fixtures/cpapi/tickle.jsoncrates/adapter/ibkr/tests/live.rscrates/adapter/ibkr/tests/portfolio.rscrates/adapter/ibkr/tests/secdef.rsdocker/cpapi/Dockerfiledocker/cpapi/README.mddocker/cpapi/capture.shdocker/cpapi/docker-compose.ymldocs/superpowers/plans/2026-07-10-ibkr-cpapi-readpath-wire.mddocs/superpowers/specs/2026-07-10-ibkr-cpapi-readpath-wire-design.md
Address CodeRabbit's four findings on PR #127: - tests/live.rs, docker/cpapi/capture.sh: add `curl --max-time 30` so a stalled/partially-reachable gateway fails fast instead of hanging. - docker/cpapi/Dockerfile: run the Java gateway as a non-root `ibkr` user (Trivy DS-0002). Validated by build+run — starts clean as uid 999, no permission errors. - docker/cpapi/capture.sh, tests/live.rs: document why `-k` (the gateway's self-signed cert) so it isn't "fixed" away. - docs/superpowers/plans/…: sync the stale `secdef_info()` example with the shipped `secdef_info(conid, sec_type)` signature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 10, 2026
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First slice of the IBKR venue adapter: a transport-agnostic Client Portal API v1 (CPAPI) read-path wire layer plus a hand-rolled paper gateway harness. Venue-side half of the ADR-0003 anti-corruption boundary; no OATH-domain translation yet (deferred until
InstrumentId/Orderland).Closes #126.
What's here (12 commits)
oath-adapter-ibkrcrate: acpapimodule with read-path DTOs (AuthStatus,TickleResponse,IServerAccounts,PortfolioAccount,Position,SecdefSearchEntry,SecdefInfo), anEndpoint/Methoddescriptor, aCpapiErrorenvelope, and adecode<T>entry point. Depends only onserde/serde_json/thiserror.String; ids/counts arei64; money/quantity areserde_json::Number(no prematuref64); and the deliberateconidsplit —Stringonsecdef/search,i64onpositions/secdef/info— modeled as each endpoint actually sends it.docker/cpapi/) + ajust ibkr-capturerecipe.#[ignore]live integration test (stays out of CI even under--all-features).[Unreleased]entry.Not in this PR (deferred)
crates/adapter/ibkr/tests/fixtures/cpapi/*.jsonare representative (documented shapes). Real sanitized captures via the gateway harness are a pre-merge follow-up — seedocker/cpapi/README.md.webapi(beta OAuth 2.0) /tws(socket) sibling surfaces.Verification
just ciis green — fmt, fmt-toml, typos, lint (-D warnings), check, test (239 tests, live test ignored), deny, doc, machete, gitleaks, actionlint, shellcheck.Process
Built test-first from the spec → plan. Each task passed a spec + quality review; the whole branch passed a final review with fixes applied (notably
secdef_infonow carries its requiredconid/secTypequery params per spec §3.1).🤖 Generated with Claude Code
Summary by CodeRabbit