feat: use CcxtOcx exchange-scoping macro (Task 6b)#10
Conversation
📝 WalkthroughWalkthroughTask 6b delivers the public Changesuse CcxtOcx macro entrypoint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 6d7c263c30
ℹ️ 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".
| test "fetch_or_build/1 returns shaped map and materializes cache file" do | ||
| id = :deribit | ||
| caps = ExchangeCaps.fetch_or_build(id) |
There was a problem hiding this comment.
This test is described as an integration probe and calls ExchangeCaps.fetch_or_build/1, which boots QuickBEAM, loads the CCXT browser bundle, and writes priv/exchange_caps; however test/test_helper.exs excludes only tests tagged :integration/:network, so this will still run during a plain mix test and can fail or slow normal unit-test runs in environments without the JS bundle/runtime. Please tag this test or the surrounding describe with :integration like the other bundle/QuickBEAM tests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces the v0.1 public entrypoint use CcxtOcx to scope which per-exchange modules are generated/compiled, and adds a compile-time capability cache helper (ExchangeCaps) that will be consumed by the upcoming defexchange layer.
Changes:
- Added
CcxtOcx.__using__/1+CcxtOcx.Macros.Useto validate/normalize scoping options (exchanges: [...]/tier: .../:all) and emit per-exchangeCcxtOcx.<Exchange>modules. - Introduced
CcxtOcx.Macros.ExchangeCapsto probe CCXT via QuickBEAM and cache per-exchange metadata underpriv/exchange_caps/<id>.exs. - Updated Declarations to expose a pure
known_exchange_ids/0list (and fixed.d.tsfilename parsing), plus added tests and roadmap/changelog updates.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
lib/ccxt_ocx.ex |
Adds the use CcxtOcx macro and per-exchange module emission. |
lib/ccxt_ocx/macros/use.ex |
Implements NimbleOptions schema + resolver + error messaging for scoping. |
lib/ccxt_ocx/macros/exchange_caps.ex |
Adds QuickBEAM-based capability probe + on-disk cache contract. |
lib/ccxt_ocx/declarations/compile.ex |
Fixes .d.ts id parsing, returns atoms, and exposes known exchange ids. |
lib/ccxt_ocx/declarations.ex |
Exposes known_exchange_ids/0 from Declarations. |
test/ccxt_ocx/macros/use_test.exs |
Adds resolver tests + macro emission integration test + ExchangeCaps probe test. |
.sobelow-skips |
Adds skip entries for newly introduced Sobelow findings. |
.gitignore |
Ignores priv/exchange_caps/ cache artifacts. |
CHANGELOG.md |
Documents Task 6b completion and the .d.ts parsing fix. |
roadmap/tasks.toml |
Marks Task 6b done and records implementation notes. |
roadmap/data.json |
Reflects Task 6b completion in rendered roadmap data. |
ROADMAP.md |
Marks Task 6b complete in the task table. |
roadmap/plans/6b-use-ccxtocx-entrypoint.md |
Adds the plan document for Task 6b. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,136 @@ | |||
| defmodule CcxtOcx.Macros.UseTest do | |||
| use ExUnit.Case, async: true | |||
| @spec fetch_or_build(atom()) :: map() | ||
| def fetch_or_build(id) when is_atom(id) do | ||
| path = caps_path(id) | ||
|
|
||
| if File.exists?(path) do | ||
| {term, _binding} = Code.eval_file(path) | ||
| term | ||
| else | ||
| build_and_cache!(id, path) | ||
| end |
| if File.exists?(path) do | ||
| {term, _binding} = Code.eval_file(path) | ||
| term | ||
| else | ||
| build_and_cache!(id, path) | ||
| end | ||
| end | ||
|
|
| Traversal.FileModule: Directory Traversal in `File.mkdir_p!`,lib/ccxt_ocx/macros/exchange_caps.ex:59,2CAF41D | ||
| DOS.StringToAtom: Unsafe `String.to_atom`,lib/ccxt_ocx/declarations/compile.ex:364,4BC0873 | ||
| Traversal.FileModule: Directory Traversal in `File.write!`,lib/ccxt_ocx/macros/exchange_caps.ex:113,762AF1D | ||
| RCE.CodeModule: Code Execution in `Code.eval_file`,lib/ccxt_ocx/macros/exchange_caps.ex:48,771EB7E |
| @doc """ | ||
| Returns the exchange id (as atom) derived from a per-exchange .d.ts path. | ||
|
|
||
| Public so Task 6b's `use CcxtOcx` validator (and later Task 9) can build a | ||
| cheap, pure list of known CCXT exchange ids without spinning a QuickBEAM | ||
| runtime. | ||
| """ | ||
| @spec exchange_id_from_path(String.t()) :: atom() | ||
| def exchange_id_from_path(path) do | ||
| # Path.rootname("foo.d.ts") == "foo.d" — we must strip the .d.ts suffix explicitly. | ||
| path | ||
| |> Path.basename() | ||
| |> Path.rootname() | ||
| |> Path.basename(".d.ts") | ||
| |> String.to_atom() | ||
| end |
| **Status**: Ready for implementation (dependency Task 6 "Declarations" is done; `nimble_options` already in `mix.exs`). | ||
|
|
||
| **Goal (from tasks.toml)**: The single user-facing macro that gates all per-exchange code generation so consumers only pay the compile-time and memory cost for the exchanges they actually use. | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ccxt_ocx/declarations/compile.ex (1)
227-237:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpec mismatch:
exchange_idparameter type is inconsistent.
collect_method/4spec (line 211) declaresexchange_idasatom() | nil, butbuild_method_term/5spec (line 227) still declares it asString.t() | nil. Sinceexchange_id_from_path/1now returnsatom(), this spec should be updated.Proposed fix
- `@spec` build_method_term(String.t(), map(), surface(), String.t() | nil, String.t()) :: map() + `@spec` build_method_term(String.t(), map(), surface(), atom() | nil, String.t()) :: map()🤖 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 `@lib/ccxt_ocx/declarations/compile.ex` around lines 227 - 237, The `@spec` for build_method_term/5 is wrong: change its exchange_id type from String.t() | nil to atom() | nil to match collect_method/4 and the fact that exchange_id_from_path/1 now returns an atom; update the `@spec` line for build_method_term/5 accordingly so the spec and function signature are consistent with collect_method/4 and exchange_id_from_path/1.
🤖 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 @.sobelow-skips:
- Around line 12-17: Update .sobelow-skips with per-entry comments that state
the exact trust boundary for each skip (e.g., "String.to_atom: only used on
Path.wildcard() basenames from hardcoded node_modules/ccxt/js/src/*.d.ts"), and
in the corresponding functions add explicit inline guards/assertions documenting
and enforcing those boundaries: ensure the String.to_atom call is applied only
to Path.basename results from Path.wildcard (reference Path.wildcard and
String.to_atom in lib/ccxt_ocx/declarations/compile.ex around the atom-creation
site), add/verify runtime guards like when is_atom(id) and that id originates
from known_exchange_ids() before calling File.mkdir_p! and File.write!
(reference priv/exchange_caps/#{id}.exs usage in
lib/ccxt_ocx/macros/exchange_caps.ex), and document that Code.eval_file is
invoked only on internally generated files (reference inspect(...) usage that
produces the file content) so the .sobelow-skips entry for Code.eval_file
explicitly notes the internal generation guarantee.
In `@lib/ccxt_ocx/macros/exchange_caps.ex`:
- Around line 41-43: The file has a formatting issue caused by an extra blank
line after the `@moduledoc`; remove the stray blank line (near the `@moduledoc` and
alias CcxtOcx.Declarations area) or simply run `mix format` to reformat the
module so the blank line is gone and CI passes.
In `@roadmap/plans/6b-use-ccxtocx-entrypoint.md`:
- Line 3: Update the status line in the roadmap markdown by replacing the
"**Status**: Ready for implementation (dependency Task 6 "Declarations" is done;
`nimble_options` already in `mix.exs`)." string with a completed status (e.g.,
"**Status**: Implemented" or "**Status**: Completed") so the plan for Task 6b
reflects that it is done; edit the line containing the "**Status**" header in
roadmap/plans/6b-use-ccxtocx-entrypoint.md (the "**Status**:" string) to the
appropriate completed wording.
- Around line 33-40: The fenced code block in
roadmap/plans/6b-use-ccxtocx-entrypoint.md is missing a language identifier
(violates markdownlint MD040); update the opening fence to include a language
(e.g., add "text") so the block starts with ```text and keep the existing
contents (the snippet listing lib/ccxt_ocx.ex, lib/ccxt_ocx/, macros/use.ex,
macros/exchange_caps.ex, generated/); ensure any other similar blocks in that
file use an explicit language identifier as well.
In `@test/ccxt_ocx/macros/use_test.exs`:
- Line 2: Tests in use_test.exs are marked async: true which can cause race
conditions once cache materialization is moved into the use CcxtOcx macro;
update the ExUnit.Case invocation to run serially by changing the option on the
use line (replace async: true with async: false) in the test module that calls
ExchangeCaps.fetch_or_build/1 so the tests do not concurrently emit files under
priv/exchange_caps.
---
Outside diff comments:
In `@lib/ccxt_ocx/declarations/compile.ex`:
- Around line 227-237: The `@spec` for build_method_term/5 is wrong: change its
exchange_id type from String.t() | nil to atom() | nil to match collect_method/4
and the fact that exchange_id_from_path/1 now returns an atom; update the `@spec`
line for build_method_term/5 accordingly so the spec and function signature are
consistent with collect_method/4 and exchange_id_from_path/1.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76cae02e-d1c1-452d-a1b4-e6acccb41237
📒 Files selected for processing (13)
.gitignore.sobelow-skipsCHANGELOG.mdROADMAP.mdlib/ccxt_ocx.exlib/ccxt_ocx/declarations.exlib/ccxt_ocx/declarations/compile.exlib/ccxt_ocx/macros/exchange_caps.exlib/ccxt_ocx/macros/use.exroadmap/data.jsonroadmap/plans/6b-use-ccxtocx-entrypoint.mdroadmap/tasks.tomltest/ccxt_ocx/macros/use_test.exs
| Traversal.FileModule: Directory Traversal in `File.read!`,lib/ccxt_ocx/declarations/compile.ex:180,2BFD6B | ||
| Traversal.FileModule: Directory Traversal in `File.mkdir_p!`,lib/ccxt_ocx/macros/exchange_caps.ex:59,2CAF41D | ||
| DOS.StringToAtom: Unsafe `String.to_atom`,lib/ccxt_ocx/declarations/compile.ex:364,4BC0873 | ||
| Traversal.FileModule: Directory Traversal in `File.write!`,lib/ccxt_ocx/macros/exchange_caps.ex:113,762AF1D | ||
| RCE.CodeModule: Code Execution in `Code.eval_file`,lib/ccxt_ocx/macros/exchange_caps.ex:48,771EB7E | ||
| Traversal.FileModule: Directory Traversal in `File.read!`,lib/ccxt_ocx/macros/exchange_caps.ex:82,C3AF66 No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
targets=(
"lib/ccxt_ocx/declarations/compile.ex:180"
"lib/ccxt_ocx/macros/exchange_caps.ex:48"
"lib/ccxt_ocx/macros/exchange_caps.ex:59"
"lib/ccxt_ocx/macros/exchange_caps.ex:82"
"lib/ccxt_ocx/macros/exchange_caps.ex:113"
"lib/ccxt_ocx/declarations/compile.ex:364"
)
for t in "${targets[@]}"; do
file="${t%%:*}"
line="${t##*:}"
start=$(( line > 10 ? line - 10 : 1 ))
end=$(( line + 15 ))
echo ""
echo "=== Context: ${file}:${line} ==="
nl -ba "$file" | sed -n "${start},${end}p"
done
echo ""
echo "=== Related guard checks ==="
rg -n -C3 'caps_path|known_exchange_ids|File\.(read!|write!|mkdir_p!)|Code\.eval_file|String\.to_atom|String\.to_existing_atom|Path\.expand' \
lib/ccxt_ocx/macros/exchange_caps.ex \
lib/ccxt_ocx/declarations/compile.exRepository: ZenHive/ccxt_ocx
Length of output: 181
🏁 Script executed:
# First, check the .sobelow-skips file to see the entries and any comments
cat -n .sobelow-skips | head -20Repository: ZenHive/ccxt_ocx
Length of output: 1855
🏁 Script executed:
# Get file sizes to avoid reading overly large files
wc -l lib/ccxt_ocx/declarations/compile.ex lib/ccxt_ocx/macros/exchange_caps.exRepository: ZenHive/ccxt_ocx
Length of output: 158
🏁 Script executed:
# Read the targeted code sections using sed (since nl is not available)
echo "=== compile.ex:180 context ===" && \
sed -n '170,195p' lib/ccxt_ocx/declarations/compile.ex | cat -n
echo ""
echo "=== compile.ex:364 context ===" && \
sed -n '354,379p' lib/ccxt_ocx/declarations/compile.ex | cat -n
echo ""
echo "=== exchange_caps.ex:48 context ===" && \
sed -n '38,63p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -n
echo ""
echo "=== exchange_caps.ex:59 context ===" && \
sed -n '49,74p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -n
echo ""
echo "=== exchange_caps.ex:82 context ===" && \
sed -n '72,97p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -n
echo ""
echo "=== exchange_caps.ex:113 context ===" && \
sed -n '103,128p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -nRepository: ZenHive/ccxt_ocx
Length of output: 6313
🏁 Script executed:
# Search for the actual File.write!, File.mkdir_p!, and String.to_atom calls
echo "=== File.write! occurrences ==="
rg -n 'File\.write!' lib/ccxt_ocx/macros/exchange_caps.ex lib/ccxt_ocx/declarations/compile.ex
echo ""
echo "=== File.mkdir_p! occurrences ==="
rg -n 'File\.mkdir_p!' lib/ccxt_ocx/macros/exchange_caps.ex lib/ccxt_ocx/declarations/compile.ex
echo ""
echo "=== String.to_atom occurrences ==="
rg -n 'String\.to_atom' lib/ccxt_ocx/macros/exchange_caps.ex lib/ccxt_ocx/declarations/compile.ex
echo ""
echo "=== exchange_id_from_path implementation ==="
rg -n -A10 'exchange_id_from_path' lib/ccxt_ocx/declarations/compile.exRepository: ZenHive/ccxt_ocx
Length of output: 1882
🏁 Script executed:
# Look at the full build_and_cache! function to see where writes happen
rg -n -A50 'defp build_and_cache!' lib/ccxt_ocx/macros/exchange_caps.ex | head -80Repository: ZenHive/ccxt_ocx
Length of output: 1699
🏁 Script executed:
# Find the exchange_dts_paths implementation
rg -n -A10 'exchange_dts_paths' lib/ccxt_ocx/declarations/compile.ex | head -30Repository: ZenHive/ccxt_ocx
Length of output: 991
🏁 Script executed:
# Also check pro_dts_paths to understand the discovery mechanism
rg -n -A10 'pro_dts_paths|def exchange_dts_paths|def pro_dts_paths' lib/ccxt_ocx/declarations/compile.exRepository: ZenHive/ccxt_ocx
Length of output: 1222
🏁 Script executed:
# Look at the actual guard context for fetch_or_build to confirm id parameter validation
sed -n '10,30p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -nRepository: ZenHive/ccxt_ocx
Length of output: 1257
🏁 Script executed:
# Check for comments near the flagged lines that might explain the rationale
echo "=== Comments around File.mkdir_p! (line 100) ==="
sed -n '95,105p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -n
echo ""
echo "=== Comments around File.write! (line 133) ==="
sed -n '128,138p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -n
echo ""
echo "=== Check what `@cache_dir` and `@exchange_glob` are ==="
rg -n '`@cache_dir`|`@exchange_glob`|`@pro_glob`' lib/ccxt_ocx/macros/exchange_caps.ex lib/ccxt_ocx/declarations/compile.ex | head -20
echo ""
echo "=== Full caps_path context ==="
sed -n '72,87p' lib/ccxt_ocx/macros/exchange_caps.ex | cat -nRepository: ZenHive/ccxt_ocx
Length of output: 2290
🏁 Script executed:
# Check if there are any security-related comments or documentation
echo "=== Security or safety documentation in files ==="
rg -n -i 'security|safe|trusted|valid|sanitiz|guard|constraint' lib/ccxt_ocx/macros/exchange_caps.ex lib/ccxt_ocx/declarations/compile.ex | head -20Repository: ZenHive/ccxt_ocx
Length of output: 1182
Add per-entry comments to .sobelow-skips and inline guards documenting trust boundaries.
The flagged operations are actually constrained to safe inputs, but lack explicit documentation. Each skip suppresses real findings that need rationale:
-
String.to_atom (compile.ex:364): Input safely derived from
Path.wildcard()on hardcodednode_modules/ccxt/js/src/*.d.tspatterns → atom creation from filename only. -
File.read! (compile.ex:180, macros/exchange_caps.ex:82): Paths sourced from controlled glob patterns in
node_modules/ccxt/js/src/→ no user input injection. -
File.mkdir_p! + File.write! (exchange_caps.ex:100, 113): Path constrained to
priv/exchange_caps/#{id}.exswhereidis an atom validated viawhen is_atom(id)and derived exclusively fromknown_exchange_ids()(= exchange_dts_paths filenames). -
Code.eval_file (exchange_caps.ex:48): File at constrained path generated internally via
inspect(term, ...)from probed CCXT data; no external code execution.
Add comments in .sobelow-skips explaining the per-entry trust boundary (e.g., # String.to_atom: only from node_modules/*.d.ts basenames) and reinforce with inline guards in the flagged functions. Without explicit documentation, future changes risk inadvertently breaking these constraints.
🤖 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 @.sobelow-skips around lines 12 - 17, Update .sobelow-skips with per-entry
comments that state the exact trust boundary for each skip (e.g.,
"String.to_atom: only used on Path.wildcard() basenames from hardcoded
node_modules/ccxt/js/src/*.d.ts"), and in the corresponding functions add
explicit inline guards/assertions documenting and enforcing those boundaries:
ensure the String.to_atom call is applied only to Path.basename results from
Path.wildcard (reference Path.wildcard and String.to_atom in
lib/ccxt_ocx/declarations/compile.ex around the atom-creation site), add/verify
runtime guards like when is_atom(id) and that id originates from
known_exchange_ids() before calling File.mkdir_p! and File.write! (reference
priv/exchange_caps/#{id}.exs usage in lib/ccxt_ocx/macros/exchange_caps.ex), and
document that Code.eval_file is invoked only on internally generated files
(reference inspect(...) usage that produces the file content) so the
.sobelow-skips entry for Code.eval_file explicitly notes the internal generation
guarantee.
| @@ -0,0 +1,206 @@ | |||
| # Plan: Task 6b — `use CcxtOcx` Exchange-Scope Entrypoint | |||
|
|
|||
| **Status**: Ready for implementation (dependency Task 6 "Declarations" is done; `nimble_options` already in `mix.exs`). | |||
There was a problem hiding this comment.
Update plan status to reflect completion.
This line still says “Ready for implementation,” but Task 6b is marked done in roadmap metadata in this PR. Please switch to an implemented/completed status to avoid handoff confusion.
🤖 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 `@roadmap/plans/6b-use-ccxtocx-entrypoint.md` at line 3, Update the status line
in the roadmap markdown by replacing the "**Status**: Ready for implementation
(dependency Task 6 "Declarations" is done; `nimble_options` already in
`mix.exs`)." string with a completed status (e.g., "**Status**: Implemented" or
"**Status**: Completed") so the plan for Task 6b reflects that it is done; edit
the line containing the "**Status**" header in
roadmap/plans/6b-use-ccxtocx-entrypoint.md (the "**Status**:" string) to the
appropriate completed wording.
| ``` | ||
| lib/ccxt_ocx.ex # becomes the real home of `defmacro __using__/1` | ||
| lib/ccxt_ocx/ | ||
| macros/ | ||
| use.ex # (new) NimbleOptions schema + resolution + expansion logic | ||
| exchange_caps.ex # (new) small helper: probe + cache "has/urls/timeframes" for one exchange | ||
| generated/ # (new, gitignored) — optional future home for on-disk generated modules if we ever move away from pure in-macro defmodule | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
This block currently violates markdownlint MD040.
Suggested fix
-```
+```text
lib/ccxt_ocx.ex # becomes the real home of `defmacro __using__/1`
lib/ccxt_ocx/
macros/
use.ex # (new) NimbleOptions schema + resolution + expansion logic
exchange_caps.ex # (new) small helper: probe + cache "has/urls/timeframes" for one exchange
generated/ # (new, gitignored) — optional future home for on-disk generated modules if we ever move away from pure in-macro defmodule</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @roadmap/plans/6b-use-ccxtocx-entrypoint.md around lines 33 - 40, The fenced
code block in roadmap/plans/6b-use-ccxtocx-entrypoint.md is missing a language
identifier (violates markdownlint MD040); update the opening fence to include a
language (e.g., add "text") so the block starts with ```text and keep the
existing contents (the snippet listing lib/ccxt_ocx.ex, lib/ccxt_ocx/,
macros/use.ex, macros/exchange_caps.ex, generated/); ensure any other similar
blocks in that file use an explicit language identifier as well.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
- fetch_or_build/1 now validates the atom against known_exchange_ids/0 before path interpolation. Closes the theoretical Code.eval_file/1 RCE via traversal-style atoms (e.g. :"../../tmp/pwn") flagged by Copilot. - build_and_cache!/2 writes via temp-file + rename. Prevents a concurrent reader from observing a partial .exs once Task 9's defexchange wires parallel compile units. - exchange_id_from_path/1 marked @doc false. Compile-time helper that maps discovered .d.ts paths to ids; not a runtime API. Docstring steers external callers to known_exchange_ids/0 (the bounded set).
The previous setup ran a QuickBEAM-booting, filesystem-writing test inside an async: true module that was untagged, so it ran in the default suite and could race with peer tests. Split: - New exchange_caps_integration_test.exs — async: false, @moduletag :integration. Excluded from default suite (per test_helper.exs); run with `mix test.json --include integration`. - use_test.exs keeps the pure resolver tests + adds two ArgumentError rejection tests covering the new fetch_or_build/1 trust boundary (unknown id, traversal-style atom).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/ccxt_ocx/macros/use_test.exs`:
- Line 41: Replace the brittle hardcoded size checks (e.g., the assertion
"assert length(scope.exchanges) >= 5") with non-flaky assertions: assert the
collection is non-empty (e.g., assert scope.exchanges != [] or assert
length(scope.exchanges) > 0) and rely on the existing explicit membership checks
for specific exchanges; update the three occurrences (the assertion at "assert
length(scope.exchanges) >= 5" and the similar assertions at the other two
locations) accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b71fe48-a45b-4337-b973-388d808872cc
📒 Files selected for processing (5)
.sobelow-skipslib/ccxt_ocx/declarations/compile.exlib/ccxt_ocx/macros/exchange_caps.extest/ccxt_ocx/macros/exchange_caps_integration_test.exstest/ccxt_ocx/macros/use_test.exs
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ccxt_ocx/declarations/compile.ex
- lib/ccxt_ocx/macros/exchange_caps.ex
| assert scope.mode == :tier | ||
| assert :binance in scope.exchanges | ||
| assert :deribit in scope.exchanges | ||
| assert length(scope.exchanges) >= 5 |
There was a problem hiding this comment.
Avoid hardcoded exchange-count thresholds in tests.
These size assertions are likely to become flaky as upstream exchange catalogs evolve. Prefer non-empty + explicit membership checks (which you already have).
Suggested test hardening
- assert length(scope.exchanges) >= 5
+ assert scope.exchanges != []
@@
- assert length(scope.exchanges) > 50
+ assert scope.exchanges != []
@@
- assert length(ids) > 50
+ assert ids != []Also applies to: 51-51, 71-71
🤖 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 `@test/ccxt_ocx/macros/use_test.exs` at line 41, Replace the brittle hardcoded
size checks (e.g., the assertion "assert length(scope.exchanges) >= 5") with
non-flaky assertions: assert the collection is non-empty (e.g., assert
scope.exchanges != [] or assert length(scope.exchanges) > 0) and rely on the
existing explicit membership checks for specific exchanges; update the three
occurrences (the assertion at "assert length(scope.exchanges) >= 5" and the
similar assertions at the other two locations) accordingly.
Summary
use CcxtOcx, exchanges: [...]/tier: :tier1/exchanges: :allmacro — the single v0.1 entrypoint that gates which per-exchange modules compile.CcxtOcx.Binance,CcxtOcx.Deribit, … modules with__exchange_id__/0and clear handoff docs for the Task 9defexchangelayer.CcxtOcx.Macros.ExchangeCaps— probe helper +priv/exchange_caps/<id>.exscache contract introduced now, exercised by Task 9. Purecaps_path/1; QuickBEAM probe only runs on the write path.:allmodes, mutex guard between:exchangesand:tier, did-you-mean for unknown ids, eager:allexpansion to keep downstream contract single-shape[atom()].exchange_id_from_path/1now returnsatom()(wasString.t()with caller-side conversion) — single source of truth inDeclarations.Compile.Test plan
harness.yml: format / compile --warnings-as-errors / credo --strict / sobelow --exit Low / doctor / test / dialyzer / coverage)mix test.json— 196 offline tests pass (was 187 pre-6b); 18 integration/network tagged excluded; new tests cover resolver pure cases, mutex guard, did-you-mean suggestions, liveuseemission, andExchangeCaps.fetch_or_build/1probe + cache materializationTODO(Task 9):markers (intentional, non-blocking per project policy)Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation / Chores