Repository navigation
fix(core): repair default client, packaging, and gateway ranking (2.0.2) - #257
Conversation
Three defects that the monorepo's own test suite could not observe, plus the e2e coverage that would have caught them. - new Wayfinder() could not serve a single request. The gateways provider was injected only when the caller supplied their own strategy, so the default PingRoutingStrategy resolved an empty candidate list and threw "No gateways available" on every call. Measured 0/20 before, 20/20 after. - The published package could not be imported in Node at all. @dha-team/arbundles imports axios without declaring it, and src/index.ts reaches it via the signature strategy. This was masked in-repo because another dependency hoists axios into the root node_modules. - NetworkGatewaysProvider ignored sortBy/sortOrder. The Solana backend of @ar.io/sdk accepts both and applies neither, so limit selected an arbitrary subset rather than the top-ranked gateways. Sorting now happens client-side over the full registry. Also: ping strategies probe the gateway root instead of the sandboxed content URL (~85% vs ~23% success at the default 1s budget), TrustedPeersGatewaysProvider retries and then fails loudly instead of silently returning an empty list, strict-mode verification failures no longer leak an unhandled rejection, and chunk retrieval reports an unsupported endpoint instead of blaming the data. The extension's mainnet Solana RPC was a provider URL with an embedded token that now returns UNAUTHORIZED, silently breaking registry sync and collapsing routing onto a single hardcoded gateway. It now uses the SDK's MAINNET_RPC_URL, with a migration for installs still holding the dead endpoint. Adds src/e2e/ (38 tests over all four extension points against the live AR.IO Solana network) and test:package, which packs and installs the tarball into a clean project — the only check that can catch an undeclared dependency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
|
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe pull request updates Wayfinder core gateway and routing behavior, adds live-network and package validation, migrates extension RPC settings, improves React provider memoization, and refreshes repository and package documentation for the 2.0.2 release. ChangesWayfinder reliability and release
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🔵 Low · up to The PR improves chunk-retrieval error reporting but still leaves the coverage matrix describing chunk retrieval as working despite documented network-wide failures. This bounded documentation risk should be corrected or explicitly accepted before release; the code changes otherwise appear mergeable. Sequence Diagram(s)sequenceDiagram
participant Client
participant Wayfinder
participant GatewayProvider
participant RoutingStrategy
participant Gateway
Client->>Wayfinder: request data
Wayfinder->>GatewayProvider: resolve gateways
GatewayProvider-->>Wayfinder: return gateway pool
Wayfinder->>RoutingStrategy: select gateway
RoutingStrategy->>Gateway: probe configured path
Gateway-->>RoutingStrategy: return health response
RoutingStrategy-->>Wayfinder: return selected gateway
Wayfinder->>Gateway: retrieve and verify data
Gateway-->>Client: return response
🚥 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: 9
🧹 Nitpick comments (11)
packages/wayfinder-core/src/gateways/network.ts (1)
59-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the new
anyvalues with a gateway interface.
sortGatewaysintroducesany[]andanyvalues. Use an SDK gateway type or a local structural interface for sortable gateway records. This keeps invalid gateway shapes and sort access errors detectable before runtime.As per coding guidelines,
packages/*/src/**/*.{ts,tsx}requires: “Code to interfaces, prefer composition over inheritance, prefer type safety.”🤖 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 `@packages/wayfinder-core/src/gateways/network.ts` around lines 59 - 72, Update sortGateways and its sortValue helper to use the SDK gateway type or a local structural interface instead of any and any[]. Type the nested sort-property access safely while preserving the existing numeric conversion and ascending/descending ordering behavior.Source: Coding guidelines
packages/wayfinder-core/src/e2e/verification.e2e.test.ts (2)
96-107: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDrain or cancel the probe response bodies.
gatewayServingTxreads onlyres.ok. It leaves each response body unconsumed.firstGatewayWherecan probe several gateways, and each undrained body holds its socket until garbage collection. Callres.body?.cancel()after the status check.♻️ Proposed change to release each probe socket
async (gateway) => { const res = await fetch(`${gateway}/tx/${txId}`, { signal: AbortSignal.timeout(20_000), }); + await res.body?.cancel(); return res.ok; },🤖 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 `@packages/wayfinder-core/src/e2e/verification.e2e.test.ts` around lines 96 - 107, Update gatewayServingTx so every fetch probe cancels or drains res.body immediately after evaluating res.ok, including unsuccessful responses, before returning the status result. Preserve the existing gateway selection behavior and timeout handling.
184-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the obsolete unhandled-rejection workaround.
tapAndVerifyReadableStreamnow handles the verification rejection, andverifycatches the strict body-read failure. Remove the listener,leaked, and related comments.🤖 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 `@packages/wayfinder-core/src/e2e/verification.e2e.test.ts` around lines 184 - 209, Remove the unhandled-rejection workaround surrounding the verify call: delete the leaked collection, captureLeak handler, process listener registration/removal, and related comments. Keep the existing verify invocation and result assertion in the test unchanged.packages/wayfinder-core/src/e2e/gateways.e2e.test.ts (2)
50-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA rejected snapshot promise stays cached and fails every later test.
registrySnapshot ??=stores the promise, not the result. If the registry read rejects afterwithRpcRetryexhausts its attempts, the rejected promise remains assigned. Every later call tojoinedGateways()then rejects with the same stale error, and no retry can recover. Clear the cache on rejection.♻️ Proposed change to avoid caching a rejection
let registrySnapshot: Promise<any[]> | undefined; const joinedGateways = () => { registrySnapshot ??= withRpcRetry(() => ario.getGateways({ limit: 1000 }), - ).then(({ items }) => - items.filter((gateway: any) => gateway.status === 'joined'), - ); + ) + .then(({ items }) => + items.filter((gateway: any) => gateway.status === 'joined'), + ) + .catch((error) => { + registrySnapshot = undefined; + throw error; + }); return registrySnapshot; };🤖 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 `@packages/wayfinder-core/src/e2e/gateways.e2e.test.ts` around lines 50 - 58, Update joinedGateways so a rejected registrySnapshot promise clears registrySnapshot before propagating the error, allowing subsequent calls to perform a fresh withRpcRetry request while preserving successful promise caching.
50-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the
anyannotations in the end-to-end helpers. The registry record shape and the caught error are both left untyped, which drops the type safety the guidelines ask for. The coding guidelines state: "Code to interfaces, prefer composition over inheritance, prefer type safety."
packages/wayfinder-core/src/e2e/gateways.e2e.test.ts#L50-L58: typeregistrySnapshotwith the SDK gateway type exported by@ar.io/sdkinstead ofPromise<any[]>, and drop the(gateway: any)annotation on thefiltercallback. The same applies to the(gateway: any)callbacks at Lines 118 and 124.packages/wayfinder-core/src/e2e/fixtures.ts#L133-L135: catch asunknownand narrow before readingcontext.statusCodeandmessage.As per coding guidelines: "Code to interfaces, prefer composition over inheritance, prefer type safety."
🤖 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 `@packages/wayfinder-core/src/e2e/gateways.e2e.test.ts` around lines 50 - 58, Replace the untyped gateway records in joinedGateways and the gateway filter callbacks at lines 118 and 124 of packages/wayfinder-core/src/e2e/gateways.e2e.test.ts with the SDK gateway type exported by `@ar.io/sdk`, removing the any annotations. In packages/wayfinder-core/src/e2e/fixtures.ts lines 133-135, catch errors as unknown and narrow them before accessing context.statusCode or message.Source: Coding guidelines
packages/wayfinder-core/src/e2e/fixtures.ts (1)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard against a non-numeric
E2E_TIMEOUT_MS.
Number('fast')returnsNaN. The suite then passesNaNas thetimeoutoption todescribe. Fall back to the default when the parsed value is not a finite number.♻️ Proposed change to validate the parsed timeout
-export const E2E_TIMEOUT_MS = Number(process.env.E2E_TIMEOUT_MS ?? 90_000); +const parsedTimeoutMs = Number(process.env.E2E_TIMEOUT_MS); +export const E2E_TIMEOUT_MS = Number.isFinite(parsedTimeoutMs) + ? parsedTimeoutMs + : 90_000;🤖 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 `@packages/wayfinder-core/src/e2e/fixtures.ts` at line 93, Update the E2E_TIMEOUT_MS initialization in fixtures.ts to validate the parsed environment value with a finite-number check, falling back to 90_000 whenever Number(...) produces NaN or another non-finite value; retain valid numeric timeout values.packages/wayfinder-core/src/e2e/routing.e2e.test.ts (1)
209-225: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winA duplicate host in
POOLmakes this assertion impossible to satisfy.
POOLat Line 51 concatenatesTRUSTED_GATEWAYwithFALLBACK_GATEWAYS. Both come from environment variables. If an operator setsE2E_GATEWAYSto a list that already contains the trusted gateway,POOL.lengthexceeds the number of distinct hosts andnew Set(seen).sizecan never match it. The test then reports a round-robin defect that does not exist. Compare against the distinct host count, or deduplicatePOOL.♻️ Proposed change to deduplicate the pool
-const POOL = [TRUSTED_GATEWAY, ...FALLBACK_GATEWAYS]; +const POOL = [...new Set([TRUSTED_GATEWAY, ...FALLBACK_GATEWAYS])];🤖 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 `@packages/wayfinder-core/src/e2e/routing.e2e.test.ts` around lines 209 - 225, Deduplicate the gateway pool before the RoundRobinRoutingStrategy test uses it, or compare against the pool’s distinct host count instead of POOL.length. Update the POOL setup and the assertion in the “RoundRobinRoutingStrategy cycles through the pool” test so duplicate TRUSTED_GATEWAY and FALLBACK_GATEWAYS entries do not produce an impossible expectation.packages/wayfinder-core/scripts/verify-package.mjs (2)
56-62: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOn Windows,
shell: truecan break paths that contain spaces.
execFileSyncwithshell: truepasses each argument through the shell without quoting. The tarball path andworkdircome fromtmpdir(), which frequently contains a space on Windows, for exampleC:\Users\Some Name\AppData\Local\Temp. Thenpm install <tarball>call at Line 88 then receives two arguments. Quote the path arguments, or restrict the script to a POSIX shell and document that.🤖 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 `@packages/wayfinder-core/scripts/verify-package.mjs` around lines 56 - 62, Update the run helper’s Windows execution path to preserve arguments containing spaces, especially the tarball path and workdir passed to npm install. Quote or otherwise safely escape path arguments when shell mode is enabled, while keeping normal argument boundaries intact for other commands.
88-88: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd a timeout to the install step.
npm installreaches the network. Withouttimeout, a stalled registry connection blocks the check indefinitely.execFileSyncaccepts atimeoutoption, so a single addition inruncovers every call.♻️ Proposed change to bound each child process
-const run = (command, args, cwd) => +const run = (command, args, cwd, timeout = 300_000) => execFileSync(command, args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], + timeout, shell: process.platform === 'win32', });🤖 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 `@packages/wayfinder-core/scripts/verify-package.mjs` at line 88, Update the shared run helper used by verify-package.mjs to pass a finite timeout option to execFileSync, ensuring the npm install invocation and every other child-process call cannot hang indefinitely. Preserve the existing command arguments, working directory, and behavior aside from enforcing the timeout.packages/wayfinder-core/src/retrieval/chunk.ts (1)
214-231: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider treating a missing
chunkTxIdalone as an unsupported endpoint.The guard requires both headers to be absent. If a gateway returns
X-Arweave-Chunk-Read-Offsetbut omits the transaction ID, control reaches the mismatch check at Line 239, which reportsGot: null. That is the misleading attribution the comment above aims to avoid.♻️ Proposed change to classify a missing chunk transaction ID as unsupported
- if (chunkReadOffsetHeader === null && chunkTxId === null) { + if (chunkTxId === null) { throw new Error( `Gateway did not return a chunk response at offset ${currentOffset}: ` + `status ${chunkResponse.status} with no ${arioHeaderNames.chunkTxId} or ` + `${arioHeaderNames.chunkReadOffset} headers. The gateway likely does not ` + 'support the /chunk/<offset>/data endpoint.', ); }🤖 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 `@packages/wayfinder-core/src/retrieval/chunk.ts` around lines 214 - 231, Update the unsupported-endpoint guard in the chunk response handling to trigger whenever chunkTxId is null, even if chunkReadOffsetHeader is present. Keep the existing diagnostic and transaction validation behavior unchanged for responses that include the transaction ID.packages/wayfinder-extension/README.md (1)
92-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep documented RPC values tied to the SDK.
packages/wayfinder-extension/src/constants.tsusesMAINNET_RPC_URLandDEVNET_RPC_URL, but this line duplicates their current values. If the SDK changes either constant, the runtime and README will diverge. Refer to the SDK-provided public endpoints instead of hardcoding both URLs.Proposed wording
-- **Solana RPC URL**: Endpoint for Solana JSON-RPC calls (editable only in Custom mode). Both presets use the public Solana endpoints (`api.mainnet-beta.solana.com` / `api.devnet.solana.com`). The extension reads the gateway registry once every 24 hours from your own IP, which sits well inside those endpoints' per-IP limits — but if you hit rate limiting, or want a dedicated provider, switch to the **Custom** preset and supply your own RPC URL. +- **Solana RPC URL**: Endpoint for Solana JSON-RPC calls (editable only in Custom mode). Both presets use the SDK-provided public Solana endpoints. The extension reads the gateway registry once every 24 hours from your own IP, which sits well inside those endpoints' per-IP limits — but if you hit rate limiting, or want a dedicated provider, switch to the **Custom** preset and supply your own RPC URL.🤖 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 `@packages/wayfinder-extension/README.md` at line 92, Update the Solana RPC URL documentation in the README to reference the SDK-provided public endpoint constants, MAINNET_RPC_URL and DEVNET_RPC_URL, rather than hardcoding their current URL values. Preserve the existing explanation of presets, rate limiting, and Custom mode.
🤖 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 `@packages/wayfinder-core/package.json`:
- Line 49: Update the test:e2e script in package.json so quoted glob arguments
are compatible with supported Node.js versions: either declare an engines.node
requirement of Node.js 21 or newer, or expand the glob before invoking tsx.
Ensure older Node.js versions do not silently run zero tests.
In `@packages/wayfinder-core/src/e2e/gateways.e2e.test.ts`:
- Around line 114-148: Update joinedGateways() to ensure the ground-truth
registry data is complete: either assert that the response hasMore flag is false
or repeatedly fetch using nextCursor until no cursor remains. Build the
stakeByFqdn and ranked values from all fetched gateways so the ranking
assertions remain valid beyond the 1000-entry page limit.
In `@packages/wayfinder-core/src/e2e/retrieval.e2e.test.ts`:
- Around line 43-46: Update fetchWith to accept a single object parameter
containing dataRetrievalStrategy and optional gateway, then adjust all three
call sites to pass those named properties while preserving the existing default
gateway behavior.
In `@packages/wayfinder-core/src/gateways/network.ts`:
- Around line 86-99: Update the pagination loop in the gateway registry
retrieval method to retry the current page when a request fails, rather than
allowing an undefined cursor or partial results to terminate normally. After the
retry limit is exhausted, propagate the error so the fallback provider can run;
only sort and return the complete collected registry after pagination succeeds.
In `@packages/wayfinder-core/src/gateways/trusted-peers.ts`:
- Around line 48-52: Update the constructor’s retries initialization to validate
that retries is a positive safe integer, preventing Infinity or NaN from
reaching the retry loop; normalize invalid values to the established default
while preserving the minimum retry behavior.
In `@packages/wayfinder-core/src/wayfinder.ts`:
- Around line 382-387: Update the class-level TSDoc describing the default
routing strategy to reflect that the constructor creates PingRoutingStrategy
wrapping RandomRoutingStrategy, replacing the stale FastestPingRoutingStrategy
reference. Keep the documentation aligned with the actual default behavior
configured in the constructor.
In `@packages/wayfinder-extension/src/background.ts`:
- Around line 384-386: Update the migration log in the retired RPC endpoint
handling to remove the raw rpcUrl value from its structured metadata. Replace
retired with a fixed migration identifier or safely redacted origin while
preserving the replacement value and existing migration message.
- Around line 368-389: Update migrateRetiredMainnetRpcUrls to read both rpcUrl
and network from chrome.storage.local before replacing the endpoint. Skip
migration when network is 'custom'; otherwise use AR_IO_SOLANA_DEVNET.rpcUrl for
devnet records and EXTENSION_DEFAULTS.rpcUrl for mainnet records, preserving the
existing exact retired-URL match.
- Around line 389-396: Reorder the migration in the startup flow so the cached
registry keys are removed before setting EXTENSION_DEFAULTS.rpcUrl, ensuring a
remove() failure leaves the retired URL in place and allows the migration to
retry on the next startup. Preserve the existing cache keys and initial-sync
scheduling behavior.
---
Nitpick comments:
In `@packages/wayfinder-core/scripts/verify-package.mjs`:
- Around line 56-62: Update the run helper’s Windows execution path to preserve
arguments containing spaces, especially the tarball path and workdir passed to
npm install. Quote or otherwise safely escape path arguments when shell mode is
enabled, while keeping normal argument boundaries intact for other commands.
- Line 88: Update the shared run helper used by verify-package.mjs to pass a
finite timeout option to execFileSync, ensuring the npm install invocation and
every other child-process call cannot hang indefinitely. Preserve the existing
command arguments, working directory, and behavior aside from enforcing the
timeout.
In `@packages/wayfinder-core/src/e2e/fixtures.ts`:
- Line 93: Update the E2E_TIMEOUT_MS initialization in fixtures.ts to validate
the parsed environment value with a finite-number check, falling back to 90_000
whenever Number(...) produces NaN or another non-finite value; retain valid
numeric timeout values.
In `@packages/wayfinder-core/src/e2e/gateways.e2e.test.ts`:
- Around line 50-58: Update joinedGateways so a rejected registrySnapshot
promise clears registrySnapshot before propagating the error, allowing
subsequent calls to perform a fresh withRpcRetry request while preserving
successful promise caching.
- Around line 50-58: Replace the untyped gateway records in joinedGateways and
the gateway filter callbacks at lines 118 and 124 of
packages/wayfinder-core/src/e2e/gateways.e2e.test.ts with the SDK gateway type
exported by `@ar.io/sdk`, removing the any annotations. In
packages/wayfinder-core/src/e2e/fixtures.ts lines 133-135, catch errors as
unknown and narrow them before accessing context.statusCode or message.
In `@packages/wayfinder-core/src/e2e/routing.e2e.test.ts`:
- Around line 209-225: Deduplicate the gateway pool before the
RoundRobinRoutingStrategy test uses it, or compare against the pool’s distinct
host count instead of POOL.length. Update the POOL setup and the assertion in
the “RoundRobinRoutingStrategy cycles through the pool” test so duplicate
TRUSTED_GATEWAY and FALLBACK_GATEWAYS entries do not produce an impossible
expectation.
In `@packages/wayfinder-core/src/e2e/verification.e2e.test.ts`:
- Around line 96-107: Update gatewayServingTx so every fetch probe cancels or
drains res.body immediately after evaluating res.ok, including unsuccessful
responses, before returning the status result. Preserve the existing gateway
selection behavior and timeout handling.
- Around line 184-209: Remove the unhandled-rejection workaround surrounding the
verify call: delete the leaked collection, captureLeak handler, process listener
registration/removal, and related comments. Keep the existing verify invocation
and result assertion in the test unchanged.
In `@packages/wayfinder-core/src/gateways/network.ts`:
- Around line 59-72: Update sortGateways and its sortValue helper to use the SDK
gateway type or a local structural interface instead of any and any[]. Type the
nested sort-property access safely while preserving the existing numeric
conversion and ascending/descending ordering behavior.
In `@packages/wayfinder-core/src/retrieval/chunk.ts`:
- Around line 214-231: Update the unsupported-endpoint guard in the chunk
response handling to trigger whenever chunkTxId is null, even if
chunkReadOffsetHeader is present. Keep the existing diagnostic and transaction
validation behavior unchanged for responses that include the transaction ID.
In `@packages/wayfinder-extension/README.md`:
- Line 92: Update the Solana RPC URL documentation in the README to reference
the SDK-provided public endpoint constants, MAINNET_RPC_URL and DEVNET_RPC_URL,
rather than hardcoding their current URL values. Preserve the existing
explanation of presets, rate limiting, and Custom mode.
🪄 Autofix
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: 52803eab-0c9b-404d-a9b3-306a532cb15b
📒 Files selected for processing (30)
CLAUDE.mdREADME.mdpackages/wayfinder-core/CHANGELOG.mdpackages/wayfinder-core/README.mdpackages/wayfinder-core/package.jsonpackages/wayfinder-core/scripts/verify-package.mjspackages/wayfinder-core/src/e2e/README.mdpackages/wayfinder-core/src/e2e/fixtures.tspackages/wayfinder-core/src/e2e/gateways.e2e.test.tspackages/wayfinder-core/src/e2e/retrieval.e2e.test.tspackages/wayfinder-core/src/e2e/routing.e2e.test.tspackages/wayfinder-core/src/e2e/verification.e2e.test.tspackages/wayfinder-core/src/gateways/network.tspackages/wayfinder-core/src/gateways/trusted-peers.tspackages/wayfinder-core/src/retrieval/chunk.tspackages/wayfinder-core/src/routing/ping.test.tspackages/wayfinder-core/src/routing/ping.tspackages/wayfinder-core/src/routing/preferred-with-fallback.test.tspackages/wayfinder-core/src/utils/verify-stream.tspackages/wayfinder-core/src/wayfinder.test.tspackages/wayfinder-core/src/wayfinder.tspackages/wayfinder-core/tsconfig.jsonpackages/wayfinder-extension/CHANGELOG.mdpackages/wayfinder-extension/CLAUDE.mdpackages/wayfinder-extension/README.mdpackages/wayfinder-extension/package.jsonpackages/wayfinder-extension/src/background.tspackages/wayfinder-extension/src/constants.tspackages/wayfinder-react/CHANGELOG.mdpackages/wayfinder-react/package.json
Review fixes: - NetworkGatewaysProvider no longer returns a partial registry. A failed page left `cursor` undefined, so a single first-page failure exited the loop and returned an empty list with no retry; a later failure returned earlier pages sorted as if complete. Since `limit` is applied after sorting, that returned the wrong gateways rather than merely fewer. Retries the failed page, then throws so a composite provider can fall through. - TrustedPeersGatewaysProvider validates `retries`. `Math.max(1, n)` passed `NaN` (skipping every attempt) and `Infinity` (retrying forever) straight through. - The extension RPC migration reads `network` first: it skips the `custom` preset instead of overwriting a user's setting, and uses the devnet default on devnet rather than writing a mainnet RPC onto the wrong cluster. It clears the cached registry before rewriting `rpcUrl`, so a crash between the two writes re-runs the migration instead of stranding a stale registry. Logs only the retired endpoint's origin, since the URL embeds an access token. - Corrected the Wayfinder TSDoc claiming FastestPingRoutingStrategy is the default, made the e2e ground-truth snapshot page to the end of the registry, and converted `fetchWith` to an object parameter per the coding guidelines. Beyond the review: peer discovery had no fallback, and turbo-gateway.com's /ar-io/peers was returning an empty list on every attempt while this was being written — which left both default clients unable to serve any request. createWayfinderClient() and new Wayfinder() now default to a composite that falls back to the trusted gateway itself, so that outage degrades routing instead of stopping it. Verified against the live failure: 20/20 and 10/10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Adds an explicit matrix of what the live suite proves and what it does not, so the scope is a checked-in statement rather than an assumption. Records the untested surfaces (events end-to-end, telemetry, browser/localStorage, x402) and the packages with no suite at all — wayfinder-react, the extension, and the CLI — which are the largest coverage gaps in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Wayfinder injected its default gateways provider into any strategy lacking one, including wrappers such as PingRoutingStrategy. A wrapper resolves a candidate list and passes it down, and RandomRoutingStrategy prefers a supplied list over its own provider — so the injection silently replaced whichever provider the caller had configured on the inner strategy. The Chrome extension hit this in its 'Balanced (Random)' routing mode: gateways came from the default peer list instead of ChromeStorageGatewayProvider, so the user's blacklist and the joined/failed-epoch filtering were bypassed and a blacklisted gateway could still be selected. Reproduced against the extension's exact wiring (provider consulted 0 times before, 5 after). Injection is now limited to strategies that select gateways themselves; nested strategies are still reached by the existing recursion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Consumer audit: are the extension, CLI and React using core correctly?Traced each package's actual wiring rather than assuming. One real bug, one wasted call, one React smell. 🐞 Extension
|
| mode | gateway chosen | ChromeStorageGatewayProvider calls |
|---|---|---|
topStaked (default) |
extension's list | 1 ✅ |
random |
turbo-gateway peer list | 0 ❌ |
Root cause is in core, not the extension. Wayfinder injects its default provider into any strategy lacking one — including the PingRoutingStrategy wrapper. The wrapper then resolves that list and passes it down, and RandomRoutingStrategy prefers a supplied list over its own provider. So the extension's blacklist, status === 'joined' filter and failed-epoch filter were all silently discarded, and a blacklisted gateway could still be routed to.
Fixed in 6bf210e: injection is now limited to strategies that select gateways themselves; nested strategies are still reached by the recursion. After the fix the provider is consulted on every attempt (0 → 5). Regression test added.
Worth noting the two inner strategies disagree by design: RoundRobinRoutingStrategy.selectGateway() takes no arguments and ignores a supplied list, which is the only reason topStaked — the extension's default — escaped this.
⚠️ Extension topStaked makes a wasted request
The ping wrapper still fetches turbo-gateway's ~78KB peer list on every selection, then discards it because RoundRobin ignores the argument. Harmless but pointless; the extension could pass its provider at the Wayfinder level instead of relying on injection.
⚠️ React recreates the client on every render
wayfinder-provider.tsx memoises on [options], an object prop. Unless the caller memoises it, the identity changes each render and a new Wayfinder is constructed every time — discarding routing state and re-fetching gateways. Not changed here: the React package has zero tests, so I can't verify a fix, and it's outside this PR's scope.
✅ CLI
Uses createWayfinderClient with createRoutingStrategy, which builds leaf strategies directly — no wrapper-over-provider nesting, so it was never affected. It also picks up the new resilient default provider automatically.
Also verified in this round: new Wayfinder() 62/62 integration, 132/132 unit after the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/wayfinder-core/src/gateways/network.ts (1)
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyin the new sorter.Use the SDK gateway item type for
sortGatewaysandsortValue. Type the dotted accessor against the supportedSortBypaths.🤖 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 `@packages/wayfinder-core/src/gateways/network.ts` around lines 62 - 69, Update sortGateways and its nested sortValue in the network gateway class to use the SDK gateway item type instead of any. Type the dotted property accessor using the supported SortBy paths, while preserving the existing numeric conversion and non-finite fallback behavior.Source: Coding guidelines
🤖 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 `@packages/wayfinder-core/src/e2e/README.md`:
- Line 61: Update the “Retrieve via chunks” entry in the e2e README from
successful to unavailable or unverified, matching the documented failure status
for live chunk tests. Keep the existing test reference and failure details
intact.
---
Nitpick comments:
In `@packages/wayfinder-core/src/gateways/network.ts`:
- Around line 62-69: Update sortGateways and its nested sortValue in the network
gateway class to use the SDK gateway item type instead of any. Type the dotted
property accessor using the supported SortBy paths, while preserving the
existing numeric conversion and non-finite fallback behavior.
🪄 Autofix
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: da96bf9e-5c04-4a0d-ae95-715c3102ec2c
📒 Files selected for processing (13)
packages/wayfinder-core/CHANGELOG.mdpackages/wayfinder-core/README.mdpackages/wayfinder-core/src/client.tspackages/wayfinder-core/src/e2e/README.mdpackages/wayfinder-core/src/e2e/gateways.e2e.test.tspackages/wayfinder-core/src/e2e/retrieval.e2e.test.tspackages/wayfinder-core/src/gateways/default.tspackages/wayfinder-core/src/gateways/network.tspackages/wayfinder-core/src/gateways/trusted-peers.tspackages/wayfinder-core/src/index.tspackages/wayfinder-core/src/wayfinder.test.tspackages/wayfinder-core/src/wayfinder.tspackages/wayfinder-extension/src/background.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/wayfinder-core/CHANGELOG.md
- packages/wayfinder-core/src/e2e/gateways.e2e.test.ts
- packages/wayfinder-core/README.md
- packages/wayfinder-core/src/gateways/trusted-peers.ts
- packages/wayfinder-extension/src/background.ts
RoundRobinRoutingStrategy loaded its gateway list once and cycled it for the life of the instance. That made any SimpleCacheGatewaysProvider or LocalStorageGatewaysProvider TTL wrapped around it meaningless, and a list that narrows was never picked up — in the Chrome extension that is why blacklisting a gateway had no effect until the service worker restarted (gateways.ts writes the blacklist without resetting the instance). It now asks the provider per selection, leaving caching to the provider layer where it belongs, and honours a caller-supplied list when provider-backed as the RoutingStrategy interface advertises. A list pinned at construction still wins — that is long-standing behaviour asserted by existing tests, so the change is scoped to the provider-backed case. WayfinderProvider collects props with a rest spread, so `options` was a new object on every render and the useMemo keyed on it never memoised anything: a fresh Wayfinder, gateways provider, emitter and telemetry setup were built on every render. It now depends on the individual options and memoises the context value. The gateways provider is also always wrapped in LocalStorageGatewaysProvider, including the default one — previously an app that passed no provider (or passed routingSettings) got no gateway caching at all in the browser. The extension's wasted peer-list fetch needed no separate fix: limiting provider injection to non-wrapper strategies already removed it (verified, 0 fetches). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Follow-up: all three consumer issues resolved (383f370)Verified each before changing anything — one needed no fix at all. ✅ Extension wasted peer fetch — already fixed, no change neededLimiting provider injection to non-wrapper strategies (6bf210e) had already removed it. Measured by counting outbound 🐞
|
Building a React test harness immediately caught a regression in the previous commit: always wrapping the gateways provider in LocalStorageGatewaysProvider throws outside a browser, so server-side rendering would crash on any app using WayfinderProvider. The browser/node branch that createWayfinderClient kept privately is now exported from core as createCachedGatewaysProvider(), and both callers use it — localStorage in the browser, an in-memory cache elsewhere. wayfinder-react gains a suite (react-test-renderer, no DOM required) covering the memoisation fix, the SSR path, and that a caller-supplied provider survives. Its tsconfig previously had no exclude, so test files would have shipped in dist. RoundRobinRoutingStrategy now resumes from the gateway it last served rather than from a numeric index. Re-reading the provider per selection means the pool can change between calls, and an index into a list that shifted points somewhere arbitrary — measured before the fix, a gateway was skipped and another re-served early the moment one dropped out of the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
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 `@packages/wayfinder-react/src/components/wayfinder-provider.test.tsx`:
- Around line 25-29: In
packages/wayfinder-react/src/components/wayfinder-provider.test.tsx, lines
25-29, sort the imports according to Biome’s ordering rules. Also format the
type assertion at lines 109-111 with Biome’s expected syntax; no other behavior
changes are needed.
🪄 Autofix
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: a948fe77-2ef5-4be5-af38-b6184202acd1
📒 Files selected for processing (13)
README.mdpackages/wayfinder-core/CHANGELOG.mdpackages/wayfinder-core/src/client.tspackages/wayfinder-core/src/e2e/README.mdpackages/wayfinder-core/src/gateways/default.tspackages/wayfinder-core/src/routing/round-robin.test.tspackages/wayfinder-core/src/routing/round-robin.tspackages/wayfinder-react/CHANGELOG.mdpackages/wayfinder-react/LICENSEpackages/wayfinder-react/package.jsonpackages/wayfinder-react/src/components/wayfinder-provider.test.tsxpackages/wayfinder-react/src/components/wayfinder-provider.tsxpackages/wayfinder-react/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- packages/wayfinder-core/src/client.ts
The react lint job failed on import ordering and formatting in the new test file. Reproduced with the exact command CI runs (from the package directory, against ../../biome.json) rather than from the repo root, which is what let it through locally. Also reverts packages/wayfinder-react/LICENSE, which was an uncommitted MIT -> Apache-2.0 change already present in the working tree and got swept into an earlier commit by a directory-wide `git add`. Relicensing is a deliberate decision that shouldn't ride along in a bug-fix PR, and it currently disagrees with package.json, which still declares MIT. The edit is left in the working tree untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Chunk retrieval was marked as covered and passing while the notes below said the live test fails on every gateway tested. Adds a third state so the table cannot contradict itself: covered and passing, covered but failing for reasons outside the library, and not verified at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Evaluation of wayfinder against the live AR.IO Solana network, and fixes for what it turned up. Ships as core/react 2.0.2, extension 2.0.1.
Why these were invisible
Each of the three main defects is something the existing suite structurally could not see:
new Wayfinder()served 0/20 requestsaxiosis hoisted into the rootnode_modulesby another workspace, so it resolves in-repo and nowhere elsesortBy/sortOrdersilently ignoredFixes
new Wayfinder()could not serve a single request. The gateways provider is injected onlyif (routingSettings?.strategy), so the defaultPingRoutingStrategynever got one and threwNo gateways availableunconditionally.createWayfinderClient()was unaffected. 0/20 → 20/20.npm i @ar.io/wayfinder-core+importfailed outright withERR_MODULE_NOT_FOUND: axios.@dha-team/arbundlesimports it without declaring it (still true on 1.0.4, the latest). Declared explicitly — the/websubpath looks like a fix but breaks signature verification with a TypeError.NetworkGatewaysProviderignoredsortBy/sortOrder. The SDK's Solanapaginate()slices without sorting, and the provider also exited pagination early — solimit: 20returned 20 arbitrary gateways, not the top 20 staked. Now sorts client-side over the full registry, verified against ground truth for asc, desc, and dotted paths.probePath.TrustedPeersGatewaysProviderreturned[]silently when a gateway served an emptygatewaysmap, surfacing later as an opaqueNo gateways available. Now retries, then throws naming the endpoint. Both the composite and cache providers treat that as a fall-back signal.--unhandled-rejections=throw.Chunk transaction ID mismatch … Got: null.Extension
The mainnet Solana RPC was a provider URL with an embedded token that now returns
UNAUTHORIZED(verified with and without extension origin/referer headers). Registry sync was failing silently and routing was collapsing onto the single hardcoded fallback gateway — decentralized routing gone, with no user-visible signal.Now uses the SDK's
MAINNET_RPC_URL. It's the only keyless endpoint that servesgetProgramAccounts, which the registry read requires — I tested 15 alternatives; the rest 403/402/429 or need a key. At one sync per 24h from each user's own IP, it sits well inside the per-IP limits. No credential ships in the bundle any more; an extension bundle is public, so a token there was never a secret.rpcUrlis only written when absent, so existing installs would have kept the dead endpoint forever —migrateRetiredMainnetRpcUrls()rewrites it and forces a resync.New coverage
src/e2e/— 38 tests across all four extension points against the live network, including the Solana registry path that nothing previously touched, plus regression guards for the zero-config constructor.test:package— packs, installs into a clean ESM project, imports. Verified it catches the real bug: removingaxiosreproduces the exactERR_MODULE_NOT_FOUND. Worth wiring into the release workflow.Verification
unit 132/132 · integration 60/60 · e2e 36/38 · packaging check passes · biome clean (86 files)
The 2 e2e failures are gateway-side and expected:
/chunk/<offset>/datais unusable network-wide (0/25 top-staked gateways). turbo-gateway returns correct bytes without theX-Arweave-Chunk-*headers; the rest 404. Documented insrc/e2e/README.mdas expected rather than silenced.Notes for review
main:origin/developis 870 commits behind, and stable releases publish frommain.preferred.comis a real site that answers 200), the other asserted the content-URL probing being replaced here.yarn testthat fails on the CLI package; the extensionCLAUDE.mdhad no Solana content at all.@ar.io/wayfinder-core@2.0.1is on thealphadist-tag —latestis still 2.0.0. Verifynpm dist-tag lsafter release.wayfinder-reacttagged correctly from the same run.Not addressed
Chunk retrieval left as-is pending gateway support. Two upstream reports worth filing:
@dha-team/arbundlesshould declareaxios, and the SDK's Solanapaginate()should applysortBy/sortOrder— it affects ~20 paginated reads for every SDK consumer, though wayfinder no longer depends on it.🤖 Generated with Claude Code
https://claude.ai/code/session_0153F31T8zBhNtJgrZPwzK1y
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores