feat: embeddable + deps migration#22
Conversation
📝 WalkthroughWalkthroughLarge migration to Effect v4: new wallet adapter services, regenerated API client, added domain schemas, overhauled atoms/hooks/components, introduced transaction execution flow, updated runtime/providers, examples, CI, and Vite build/output/publishing configs. ChangesEffect v4 refactor, wallet adapters, domain models, and packaging
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/widget/src/components/molecules/sign/sign-content.tsx (1)
35-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid hard redirect on failed transaction state.
Line 35 unconditionally redirects to
/on failure, which drops failure context and removes any in-place recovery path for signing/submission errors. Please keep users on an actionable failure state (e.g., explicit error + retry/close choice) instead of immediate navigation.🤖 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/widget/src/components/molecules/sign/sign-content.tsx` around lines 35 - 40, The current failure branch in sign-content.tsx uses Result.isFailure(state) and immediately renders <Navigate to="/" />, which nukes failure context; remove the Navigate and instead render an actionable error UI inside that same return branch (keep the Skeleton if needed) showing the failure message from state, plus explicit retry and close actions (e.g., call provided retry/submit handlers or emit onClose/onRetry callbacks). Update the block guarded by Result.isFailure(state) to display the error details and wire buttons to the existing sign/submission handlers rather than performing an unconditional redirect.packages/common/src/atoms/tokens-atoms.ts (1)
134-144:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid casting partial network results to full
TokenBalances.Line 134 onward drops failed network requests, then casts a partial record to
TokenBalances. Missing keys can surface as undefined reads later.🔧 Proposed fix (preserve all network keys)
- Effect.map((res) => - pipe( - Record.fromIterableBy(res, (r) => r.network), - (record) => - Record.map(record, (v) => v.balances) as TokenBalances, - ), - ), + Effect.map((res) => { + const defaults = Record.map( + yieldApiNetworkToMoralisChain, + () => [], + ) as TokenBalances; + + const byNetwork = pipe( + Record.fromIterableBy(res, (r) => r.network), + (record) => Record.map(record, (v) => v.balances), + ); + + return { ...defaults, ...byNetwork } as TokenBalances; + }),🤖 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/common/src/atoms/tokens-atoms.ts` around lines 134 - 144, The code drops failed network results and then unsafely casts a partial record to TokenBalances; instead, after Effect.map(_Array.getSomes) build a complete TokenBalances record by iterating the full set of expected network keys and filling each key with the found balances or a defined empty/default balances value before returning—i.e., replace the direct Record.fromIterableBy + cast with logic that uses the complete network list and merges/resolves missing entries to a default so the resulting object truly matches the TokenBalances shape (referencing effects, _Array.getSomes, Record.fromIterableBy and the TokenBalances type).packages/dashboard/src/components/molecules/header/withdraw/state.tsx (1)
29-42:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
useProvidersto read and write to the same atom.
selectedProviderreads from the globalselectedProviderAtombutsetSelectedProviderwrites to the localwithdrawSelectedProviderAtom. This breaks the expected behavior: callingsetSelectedProviderwon't update the returnedselectedProvidervalue. All similar hooks in the codebase (deposit, Account/Withdraw) correctly use a single atom for both operations. UseselectedProviderAtomfor both read and write, or remove the unusedwithdrawSelectedProviderAtom.🤖 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/dashboard/src/components/molecules/header/withdraw/state.tsx` around lines 29 - 42, The hook useProviders reads selectedProvider from selectedProviderAtom but writes to withdrawSelectedProviderAtom, so setSelectedProvider won't update the returned value; fix by making setSelectedProvider call useAtomSet(selectedProviderAtom) (or else remove/replace withdrawSelectedProviderAtom so both read and write use the same atom), ensuring useProviders consistently uses selectedProviderAtom for both reading and writing.
🟠 Major comments (18)
packages/common/src/domain/index.ts-1-20 (1)
1-20: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftThis barrel expansion conflicts with the refactor direction to avoid re-exports.
Please avoid broad re-exporting here and move consumers to direct module imports instead; this keeps boundaries explicit and reduces coupling in ongoing migration work.
Based on learnings, "During refactoring, avoid re-exporting functionality. Update imports in files that are importing this functionality".
🤖 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/common/src/domain/index.ts` around lines 1 - 20, The barrel file packages/common/src/domain/index.ts currently re-exports many modules (e.g., "./action", "./action-transaction", "./api-event", "./chains", "./ids", "./internal-event", "./market", "./model-updates", "./pending-action", "./portfolio", "./provider", "./scalars", "./token", "./tokens", "./transactions", "./types", "./wallet", "./wallet-adapter", "./wallet-errors", "./wallet-source"); remove these broad exports from the barrel (or reduce to only truly shared, stable items) and update every consumer to import directly from the specific module path they need (e.g., import from "./action" or "./tokens" instead of from "./domain"); search for imports that reference the domain barrel (import ... from "packages/common/src/domain" or similar) and change them to the explicit module paths, then run type-checks/build to ensure no broken imports remain.apps/dashboard-react-example/src/main.tsx-6-11 (1)
6-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate required config values before rendering.
Required env-backed fields are used without checks, so empty/missing values can fail later in less obvious places. Fail fast with explicit validation.
Proposed fix
const config: PerpsConfig = { perpsBaseUrl: import.meta.env.VITE_PERPS_BASE_URL, perpsApiKey: import.meta.env.VITE_PERPS_API_KEY, reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID || undefined, moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY, }; + +const requiredConfig: Array<keyof PerpsConfig> = [ + "perpsBaseUrl", + "perpsApiKey", + "moralisApiKey", +]; + +for (const key of requiredConfig) { + if (!config[key]) { + throw new Error(`Missing required config: ${key}`); + } +}Also applies to: 19-19
🤖 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 `@apps/dashboard-react-example/src/main.tsx` around lines 6 - 11, The config object (PerpsConfig) is built from import.meta.env values without validation, so add a validation step (e.g., a validateConfig or assertRequiredEnv function) that checks perpsBaseUrl, perpsApiKey, and moralisApiKey (and any other required env values such as reownProjectId if required) immediately after constructing config in main.tsx; if any required value is missing or empty, log a clear error and fail fast (throw or render an explicit error UI) rather than proceeding to render, so downstream code using perpsBaseUrl/perpsApiKey/moralisApiKey cannot run with invalid configuration.packages/widget/src/vanilla.tsx-24-34 (1)
24-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against remounting into the same container without root cleanup.
The
mountWidgetfunction callscreateRoot(...)on every invocation. For embeddable usage, repeated calls tomountWidgeton the same target will trigger React's "container already has a root" warning and cause lifecycle leaks. React 19 requires either reusing an existing root withroot.render()or unmounting the previous root before creating a new one.Store mounted roots and unmount the previous one before creating a new root to ensure clean lifecycle management and prevent conflicts.
Proposed fix
import ReactDOM from "react-dom/client"; import { Widget, type WidgetProps } from "./app"; export type WidgetMountTarget = Element | DocumentFragment | string; @@ export type MountedWidget = { readonly unmount: () => void; }; + +const mountedRoots = new WeakMap< + Element | DocumentFragment, + ReturnType<typeof ReactDOM.createRoot> +>(); @@ export const mountWidget = ( target: WidgetMountTarget, props: WidgetProps, ): MountedWidget => { - const root = ReactDOM.createRoot(getMountTarget(target)); + const container = getMountTarget(target); + mountedRoots.get(container)?.unmount(); + + const root = ReactDOM.createRoot(container); + mountedRoots.set(container, root); root.render(<Widget {...props} />); return { - unmount: () => root.unmount(), + unmount: () => { + root.unmount(); + mountedRoots.delete(container); + }, }; };🤖 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/widget/src/vanilla.tsx` around lines 24 - 34, mountWidget currently calls ReactDOM.createRoot(getMountTarget(target)) every time which can trigger "container already has a root" warnings and lifecycle leaks; modify mountWidget to track roots per mount container (keyed by getMountTarget(target)) and if a root already exists unmount it (or reuse it) before calling createRoot or calling render on the existing root, then store the new root so subsequent mountWidget calls reuse or clean up the previous root; update the returned unmount to remove the stored root entry after unmounting.apps/dashboard-vanilla-example/src/main.js-5-12 (1)
5-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate required env config before mounting.
Lines 6, 7, and 9 are required config inputs but are not validated. Fail fast with a clear error to avoid opaque runtime failures.
Suggested guard
+const required = [ + ["VITE_PERPS_BASE_URL", import.meta.env.VITE_PERPS_BASE_URL], + ["VITE_PERPS_API_KEY", import.meta.env.VITE_PERPS_API_KEY], + ["VITE_MORALIS_API_KEY", import.meta.env.VITE_MORALIS_API_KEY], +]; + +const missing = required.filter(([, value]) => !value).map(([key]) => key); +if (missing.length > 0) { + throw new Error(`Missing required env vars: ${missing.join(", ")}`); +} + const config = { perpsBaseUrl: import.meta.env.VITE_PERPS_BASE_URL, perpsApiKey: import.meta.env.VITE_PERPS_API_KEY, reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID || undefined, moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY, };🤖 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 `@apps/dashboard-vanilla-example/src/main.js` around lines 5 - 12, The config object is using environment vars but doesn't validate required fields (perpsBaseUrl, perpsApiKey, moralisApiKey) before calling mountDashboard; add a guard that checks these keys on the config (perpsBaseUrl, perpsApiKey, moralisApiKey) are present and non-empty and, if any are missing, log/throw a clear error and stop execution (e.g., throw new Error or process.exit(1)) before invoking mountDashboard("#app", { config }); leave reownProjectId as optional (it can remain undefined).packages/widget/src/components/molecules/sign/index.tsx-16-18 (1)
16-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid rendering a blank route when
actionis missing.Lines 16–18 return
null, which can leave users on an empty protected screen after refresh/deep-link. Render a fallback with recovery navigation (or redirect) instead.Suggested fallback UI
- if (!action) { - return null; - } + if (!action) { + return ( + <div className="flex h-full w-full items-center justify-center"> + <Button variant="secondary" className="w-full" onClick={() => navigate({ to: "/" })}> + Back to Home + </Button> + </div> + ); + }🤖 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/widget/src/components/molecules/sign/index.tsx` around lines 16 - 18, The Sign component currently returns null when the action prop is missing which leaves users on a blank protected screen; instead update the Sign component (where the action variable is checked) to render a fallback UI or trigger a recovery navigation/redirect (e.g. render a small fallback view with a descriptive message and a link/button to the safe route, or call your router redirect/navigation in an effect such as useEffect to push to a login/home route). Replace the `if (!action) { return null; }` branch with this fallback behavior and ensure the fallback gives users a clear recovery path and uses your app's navigation utilities (e.g., useNavigate or Link) so refreshes/deep-links don't surface an empty screen.packages/common/src/context/index.tsx-16-17 (1)
16-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse an atom for
QueryClientto prevent cache sharing across instances.Module-level
QueryClientsingleton shares cache between separate mounts, risking cross-instance state bleed in embedded usage. Per coding guidelines, all state handling must use@effect-atom/atom-react—create aqueryClientAtomand inject viaRegistryProvider(following theperpsConfigAtompattern) instead ofuseState.🤖 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/common/src/context/index.tsx` around lines 16 - 17, The module-level QueryClient singleton (const queryClient = new QueryClient()) causes shared cache across mounts; replace it with an atom-based provider: create and export a queryClientAtom using `@effect-atom/atom-react` that initializes new QueryClient instances, mirror the pattern used by perpsConfigAtom, inject that atom via RegistryProvider, and update consumers to read the queryClient from the RegistryProvider instead of relying on the module-level queryClient or local useState so each mount gets its own QueryClient instance.packages/widget/src/components/molecules/provider-select.tsx-71-73 (1)
71-73:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMove provider selection state from React hooks to atom-based state management.
Lines 71-73 use
useStateforinternalProvider, which violates the project guideline requiring all state to use@effect-atom/atom-react. Additionally, line 70 usesuseStatefor theopenstate. Both should be refactored to use atoms as the single source of truth for state in this component.🤖 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/widget/src/components/molecules/provider-select.tsx` around lines 71 - 73, Replace the local useState hooks for internalProvider and open with atom-based state from `@effect-atom/atom-react`: create atoms that hold the provider value (initialized to defaultProvider ?? providers[0] ?? null) and the open boolean, then swap occurrences of internalProvider and setInternalProvider and the open state setter to read/write via the atom hooks (e.g., the atom hook provided by `@effect-atom/atom-react`). Ensure all components and handlers in this file reference the new provider atom and open atom as the single source of truth instead of the previous useState variables (internalProvider, setInternalProvider, and the open state)..github/workflows/ci.yml-22-28 (1)
22-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid piping remote install scripts directly in CI.
Line 24 executes
curl ... | sh, which weakens supply-chain guarantees for your CI runner. Prefer a pinned action for mise installation.Suggested safer change
- - name: Install mise - run: | - curl https://mise.run | MISE_VERSION=v2026.5.6 sh - echo "$HOME/.local/bin" >> "$GITHUB_PATH" - echo "$HOME/.local/share/mise/shims" >> "$GITHUB_PATH" - export PATH="$HOME/.local/bin:$HOME/.local/share/mise/shims:$PATH" - mise install + - name: Install mise + uses: jdx/mise-action@v2 + + - name: Install toolchain + run: mise install🤖 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 @.github/workflows/ci.yml around lines 22 - 28, Replace the unsafe piped installer ("curl https://mise.run | MISE_VERSION=... sh") with a pinned installation method: use a pinned GitHub Action for mise or fetch a specific mise release artifact and verify its checksum/signature before installing; update the CI step that currently contains the "curl ... | sh", the PATH modifications ("echo \"$HOME/.local/bin\" >> \"$GITHUB_PATH\"" and "echo \"$HOME/.local/share/mise/shims\" >> \"$GITHUB_PATH\"") and the "mise install" invocation to instead call the verified installer provided by the pinned action (or a verified downloaded binary) with the specific MISE_VERSION; ensure the workflow references the action tag/commit hash to lock the supply chain.packages/common/src/atoms/transaction-execution-atoms.ts-93-95 (1)
93-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid
Effect.diefor recoverable operational errors in state machines with catch handlers.
wallet disconnectedandtransaction not foundare runtime failures that should emit through the catch handler to trigger state updates and lifecycle events. UsingEffect.diecreates defects that bypass the handler at line 254, preventing proper error state transitions.Use
Effect.fail(new TransactionFailedError())instead, consistent with the error handling pattern already used elsewhere in this file.Suggested changes
- if (walletState.status === "disconnected") { - return yield* Effect.die(new Error("Wallet is disconnected")); - } + if (walletState.status === "disconnected") { + return yield* Effect.fail(new TransactionFailedError()); + } ... - Effect.catch(() => - Effect.die( - new Error("Transaction not found in response"), - ), - ), + Effect.catch(() => + Effect.fail(new TransactionFailedError()), + ),Also applies to: 203-206
🤖 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/common/src/atoms/transaction-execution-atoms.ts` around lines 93 - 95, The code uses Effect.die when checking walletState.status === "disconnected" (and similarly for the "transaction not found" case) which creates defects that bypass the module's catch handler; replace Effect.die(...) with Effect.fail(new TransactionFailedError(...)) so these runtime failures flow through the existing catch handler and state-machine lifecycle logic — update the occurrences that call Effect.die to use Effect.fail and construct/throw a TransactionFailedError consistent with other uses in this file (keep the same error message/context).packages/common/src/services/wallet/external-wallet-adapter.ts-25-27 (1)
25-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSynchronous decode in subscription callback may throw.
The subscription callback (line 26) uses
decodeState(state)which is a synchronous decoder that can throw. If the external source publishes invalid state, this will throw an exception that may not be properly caught by the FiberHandle runtime.Consider using
Effect.tryto wrap the decode:🛡️ Proposed fix to handle decode errors in subscription
const unsubscribe = source.subscribe((state) => { - run(SubscriptionRef.set(stateRef, decodeState(state))); + run( + Effect.try(() => decodeState(state)).pipe( + Effect.andThen((decoded) => SubscriptionRef.set(stateRef, decoded)), + Effect.catchAll((error) => Effect.logError(error)) + ) + ); });🤖 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/common/src/services/wallet/external-wallet-adapter.ts` around lines 25 - 27, The subscription callback calls decodeState(state) synchronously and can throw, so wrap the decode in an effect to catch errors instead of letting exceptions escape; change the subscribe handler to use Effect.try (or the project's error-wrapping utility) around decodeState(state) and then run the resulting effect which on success calls SubscriptionRef.set(stateRef, decoded) and on failure logs or handles the error appropriately—update the handler that currently calls run(SubscriptionRef.set(stateRef, decodeState(state))) inside source.subscribe to first create an Effect.try(() => decodeState(state)).flatMap(decoded => SubscriptionRef.set(stateRef, decoded)) (or equivalent) and run that effect.packages/common/scripts/generate-tradingview-symbols/index.ts-143-151 (1)
143-151:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t silently skip failed market pages in symbol generation.
Effect.option+getSomeshides page failures and can generate incompletetradingview-symbols.jsonwithout signaling failure.🔧 Proposed fix (fail fast on page errors)
- const restPages = yield* Effect.all( + const restPages = yield* Effect.all( Array.from({ length: totalPages - 1 }).map((_, index) => getMarketsPage({ providerId, offset: (index + 1) * DEFAULT_LIMIT, limit: DEFAULT_LIMIT, - }).pipe(Effect.option), + }), ), - ).pipe(Effect.map(_Array.getSomes)); + );🤖 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/common/scripts/generate-tradingview-symbols/index.ts` around lines 143 - 151, The current code swallows page errors by wrapping getMarketsPage calls with Effect.option and then using _Array.getSomes, producing silently incomplete results; instead remove Effect.option so failures propagate (fail-fast) and collect the page Effects directly (e.g., use Effect.all / Effect.collectAll over the array of getMarketsPage(...) calls) and then flatten/map the resulting array of page results into restPages; update references around restPages, getMarketsPage, DEFAULT_LIMIT, and remove the use of Effect.option and _Array.getSomes so any page error surfaces as a failure.packages/common/src/domain/ids.ts-22-29 (1)
22-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten address schemas to prevent invalid runtime identifiers.
WalletAddresscurrently accepts0x+ any string, andTokenAddressonly enforces lowercase. This allows malformed addresses through domain validation.🛡️ Proposed fix
-export const WalletAddress = Schema.TemplateLiteral([ - Schema.Literal("0x"), - Schema.String, -]).pipe(Schema.brand("WalletAddress")); +export const WalletAddress = Schema.String.pipe( + Schema.pattern(/^0x[a-fA-F0-9]{40}$/), + Schema.brand("WalletAddress"), +); -export const TokenAddress = LowercaseString.pipe(Schema.brand("TokenAddress")); +export const TokenAddress = LowercaseString.pipe( + Schema.pattern(/^0x[a-f0-9]{40}$/), + Schema.brand("TokenAddress"), +);🤖 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/common/src/domain/ids.ts` around lines 22 - 29, WalletAddress and TokenAddress allow malformed values; tighten both to require the 0x-prefixed 40-hex-character format. Replace the current WalletAddress TemplateLiteral + Schema.String and TokenAddress LowercaseString usages with a regex-branded schema that validates /^0x[0-9a-f]{40}$/ (i.e., 0x + exactly 40 lowercase hex chars) and keep the Schema.brand("WalletAddress") / Schema.brand("TokenAddress") and associated types the same so runtime IDs are strictly enforced.packages/common/src/services/wallet/browser-wallet-adapter.ts-41-53 (1)
41-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse HyperEVM's actual chain metadata here.
This custom chain is defined as
1337, but HyperEVM mainnet is999, and the documented mainnet JSON-RPC endpoint is different as well. As written, AppKit/wagmi will point switching and signing at the wrong network for HyperEVM. (hyperliquid.gitbook.io)🛠️ Suggested fix
const hyperLiquidL1 = defineChain({ - id: 1337, - caipNetworkId: "eip155:1337", + id: 999, + caipNetworkId: "eip155:999", chainNamespace: "eip155", name: "Hyperliquid L1", nativeCurrency: { name: "HYPE", symbol: "HYPE", decimals: 18 }, rpcUrls: { - default: { http: ["https://api.hyperliquid.xyz/evm"] }, + default: { http: ["https://rpc.hyperliquid.xyz/evm"] }, }, blockExplorers: { default: { name: "Hyperliquid", url: "https://app.hyperliquid.xyz" }, }, });🤖 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/common/src/services/wallet/browser-wallet-adapter.ts` around lines 41 - 53, The custom chain definition hyperLiquidL1 created via defineChain is using incorrect metadata (id 1337 and wrong RPC/explorer); update hyperLiquidL1 to use HyperEVM mainnet values: set id to 999, caipNetworkId to "eip155:999", update rpcUrls.default.http to HyperEVM's documented mainnet JSON-RPC endpoint, and correct blockExplorers.default.url/name to HyperEVM/Hyperliquid mainnet explorer info so wagmi/appkit will target the proper network.packages/common/src/services/wallet/browser-wallet-adapter.ts-213-225 (1)
213-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
switchAccountis indexing the connections map with the wrong key.Earlier in this file,
state.connectionsis read bynextState.current, so the map is keyed by connection id. Looking it up withaddresshere means this branch falls intoNonefor normal multi-account wallets, and the requested account never becomes active.🛠️ Suggested fix
const switchAccount = Effect.fn( function* (address: typeof WalletAddress.Type) { - const connection = yield* Option.fromNullishOr( - wagmiAdapter.wagmiConfig.state.connections.get(address), - ); + const connection = yield* Option.fromNullishOr( + Array.from(wagmiAdapter.wagmiConfig.state.connections.values()).find( + (connection) => connection.accounts.includes(address), + ), + ); yield* Effect.tryPromise(() => switchConnection(wagmiAdapter.wagmiConfig, { connector: connection.connector, }),🤖 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/common/src/services/wallet/browser-wallet-adapter.ts` around lines 213 - 225, The function switchAccount is incorrectly looking up wagmiAdapter.wagmiConfig.state.connections by the wallet address (address) but the connections Map is keyed by connection id (read elsewhere via nextState.current); change the lookup to use the connection id (e.g., obtain connectionId from wagmiAdapter.wagmiConfig.state.current or nextState.current) before calling connections.get(connectionId), then pass that connection into switchConnection; alternatively, if you must resolve by address, iterate the connections Map to find the entry whose accounts include the given address and use its id/connector instead of indexing by address.patches/@lucas-barake__effect-form@0.23.0.patch-18-19 (1)
18-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
unknownerror types withSchema.SchemaErrorin the form atoms declarations.In Effect v4,
ParseResult.ParseErrorhas been replaced bySchema.SchemaError. The patch currently widens error channels tounknown(lines 18-19, 27-28, 32-33, 35-36), which causesE | unknownto collapse tounknownin TypeScript, erasing all downstream type discrimination for form validation and submission failures. UseSchema.SchemaErrorinstead—the codebase already references this type inpackages/common/src/services/api-client/api-schemas.tsas the standard v4 schema error representation.Affected lines in patch
- readonly validationAtom: Atom.AtomResultFn<unknown, void, ParseResult.ParseError>; + readonly validationAtom: Atom.AtomResultFn<unknown, void, unknown>;Also applies to:
submitAtom(line 27-28),validationAtomsRegistry(line 32-33),getOrCreateValidationAtom(line 35-36).🤖 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 `@patches/`@lucas-barake__effect-form@0.23.0.patch around lines 18 - 19, The types for form error channels were widened to unknown; update the type parameters to use the v4 schema error type Schema.SchemaError instead of unknown for the Atom result types: change validationAtom and submitAtom declarations from Atom.AtomResultFn<unknown, void, unknown> back to Atom.AtomResultFn<unknown, void, Schema.SchemaError>, and likewise update validationAtomsRegistry and getOrCreateValidationAtom signatures/returns to use Schema.SchemaError so the error union E | Schema.SchemaError preserves discriminated typing.packages/common/src/services/api-client/client-factory.ts-30-31 (1)
30-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore literal discriminants for activity items.
ActivityActionItemDto["type"]andActivityEventItemDto["type"]both allow"event" | "action", soActivityControllerGetActivity200.itemsis no longer a discriminated union. Consumers can’t safely narrow onitem.type, and impossible shapes like{ type: "action", event: ... }become type-valid. Please emit"action"forActivityActionItemDtoand"event"forActivityEventItemDtoin the generator output as well, otherwise this public client surface stays unsound.Also applies to: 74-74
🤖 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/common/src/services/api-client/client-factory.ts` around lines 30 - 31, The generated DTOs lose discriminant literal types: change ActivityActionItemDto's "type" to the literal "action" and ActivityEventItemDto's "type" to the literal "event" so ActivityControllerGetActivity200.items becomes a proper discriminated union; update the generator/template that emits these types (also update the other occurrence of the same pattern) to emit the specific literal string for each variant instead of `"event" | "action"`.packages/common/src/atoms/portfolio-atoms.ts-90-103 (1)
90-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t drop provider balance failures silently.
Wrapping each fetch/decode in
Effect.optionturns auth, rate-limit, transport, and schema failures intoNone, so a broken provider just vanishes from the balances list. That underreports balances and makes outages indistinguishable from “no balance”. Let this atom fail, or carry per-provider errors alongside successful balances instead of erasing them.🤖 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/common/src/atoms/portfolio-atoms.ts` around lines 90 - 103, The current pipeline turns all provider fetch/decode failures into None via Effect.option, hiding errors; replace the terminal .pipe(Effect.option) with .pipe(Effect.either) (or remove it entirely if you want the atom to fail) and then collect per-provider results so you retain errors alongside successes (e.g. map each provider result to { providerId: provider.id, result } where result is Either<Error, Balance>), keeping the existing PortfolioControllerGetBalances call and Schema.decodeEffect(Balance) in place so you no longer silently drop failures.patches/vite-plugin-node-polyfills@0.26.0.patch-8-8 (1)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpand the dev transform filter to plain
.jsmodules.The new
/\.(m?ts|[jt]sx)$/filter skips.js,.mjs, and.cjs, so the banner no longer gets prepended for vanilla entrypoints. This codebase hasapps/dashboard-vanilla-example/src/main.jsandapps/widget-vanilla-example/src/main.js, which meansglobal,process, andBufferpolyfills will be unavailable in dev mode. Change the pattern to/\.[cm]?[jt]sx?$/to include plain JavaScript files.🤖 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 `@patches/vite-plugin-node-polyfills`@0.26.0.patch at line 8, The dev-transform filter assigned to F only matches .ts/.mts and .jsx/.tsx but skips plain JavaScript files like .js/.mjs/.cjs; update the predicate in the F arrow function (the function that tests file extensions) to use a regex that also accepts an optional leading "c" or "m" prefix and optional "s" suffix so it matches .js, .mjs, .cjs as well as .ts/.mts/.cts and their JSX/TSX variants; replace the existing regexp in F with one that allows optional "c" or "m" before the j/t and optional "s" after to ensure banner injection runs for vanilla .js entrypoints.
🟡 Minor comments (3)
packages/dashboard/src/main.tsx-7-12 (1)
7-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd required config validation before rendering to fail fast on missing environment variables.
perpsBaseUrl,perpsApiKey, andmoralisApiKeyare required fields consumed directly inApiClientServicewithout any guard checks. If environment variables are missing, the app boots successfully but fails downstream when attempting HTTP requests.Suggested fix
const config: PerpsConfig = { perpsBaseUrl: import.meta.env.VITE_PERPS_BASE_URL, perpsApiKey: import.meta.env.VITE_PERPS_API_KEY, reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID || undefined, moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY, }; +if (!config.perpsBaseUrl || !config.perpsApiKey || !config.moralisApiKey) { + throw new Error( + "Missing required env vars: VITE_PERPS_BASE_URL, VITE_PERPS_API_KEY, VITE_MORALIS_API_KEY", + ); +}🤖 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/dashboard/src/main.tsx` around lines 7 - 12, Validate required environment variables in main.tsx before creating the config and rendering the app: check that config.perpsBaseUrl, config.perpsApiKey, and config.moralisApiKey are non-empty (reownProjectId can remain optional), and if any are missing, fail fast by throwing a descriptive Error or rendering a clear error UI instead of proceeding; update the PerpsConfig construction/usage near the config object and the code path that instantiates ApiClientService so downstream callers never receive undefined config values.apps/widget-vanilla-example/src/main.js-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove redundant
|| undefinedlogic.The string literal
"29e1b718ad16983a0705cf24d5b5b747"is always truthy, so|| undefinedhas no effect. If you intended to make this optional based on an environment variable, use:- reownProjectId: "29e1b718ad16983a0705cf24d5b5b747" || undefined, + reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID,If the hardcoded value is intentional for this example, remove the
|| undefined:- reownProjectId: "29e1b718ad16983a0705cf24d5b5b747" || undefined, + reownProjectId: "29e1b718ad16983a0705cf24d5b5b747",🤖 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 `@apps/widget-vanilla-example/src/main.js` at line 8, The assignment to reownProjectId uses a literal string followed by "|| undefined", which is redundant because the string is always truthy; edit the reownProjectId initialization in main.js (the reownProjectId variable) to either remove the "|| undefined" when keeping the hardcoded value, or replace the literal with an environment-based expression (e.g., use process.env.REOWN_PROJECT_ID) if you intended it to be optional—ensure only one of these approaches is used and delete the trailing "|| undefined".packages/dashboard/src/components/modules/trade/order-form/sign-dialog.tsx-69-75 (1)
69-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFailure state renders a skeleton instead of an error UI.
When
Result.isFailure(state)is true, the component renders aSkeleton, which gives users a loading impression rather than indicating an error occurred. Consider rendering an error message or retry option instead.Suggested approach
if (Result.isFailure(state)) { return ( <div className="flex flex-col gap-4"> - <Skeleton className="w-full h-[200px]" /> + <div className="flex flex-col items-center justify-center h-[200px] gap-2"> + <Text variant="bodySmWhiteNeg">Something went wrong</Text> + <Button variant="secondary" onClick={onClose}>Close</Button> + </div> </div> ); }🤖 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/dashboard/src/components/modules/trade/order-form/sign-dialog.tsx` around lines 69 - 75, The failure branch currently returns a loading Skeleton when Result.isFailure(state) is true; replace that with an explicit error UI so users know something went wrong. Inside the conditional that checks Result.isFailure(state) in sign-dialog.tsx, render an error message (e.g., "Failed to load signature data") and a retry/action button that calls the component's existing refetch/retry handler (or expose a new onRetry prop) and include any error details from state for debugging; ensure you remove the Skeleton from this branch and keep the success/loading branches unchanged.
🧹 Nitpick comments (12)
packages/widget/src/routes/position-details/$marketId/index.tsx (1)
5-12: ⚡ Quick winRegenerate route artifacts for this route-file change.
This file lives under
routes, so please regenerate routes to keep generated route metadata/types in sync.As per coding guidelines "
**/routes/**: Runpnpm turbo generate-routes --filter=@yieldxyz/perps-{package}when files are added, removed, or changed in theroutesfolder".🤖 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/widget/src/routes/position-details/`$marketId/index.tsx around lines 5 - 12, The route file modified (symbols: Route, createFileRoute, PositionDetails, validateSearch, Schema) requires regenerating the generated route artifacts so metadata/types stay in sync; run the project generator command (pnpm turbo generate-routes --filter=@yieldxyz/perps-{package}) to regenerate route artifacts for the routes folder and then add/commit the updated generated files to the PR.packages/common/src/hooks/use-order-actions.ts (1)
12-30: ⚡ Quick winUse
_Array.reduce()instead of native.reduce()for consistency with project conventions.This hook uses native
.reduce()while the codebase pattern uses_Array(imported from "effect") for array operations. Replace with_Array.reduce()to align with the Effect-first approach and coding guideline that requireseffect/Arrayfor array manipulation.🤖 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/common/src/hooks/use-order-actions.ts` around lines 12 - 30, The hook useOrderActions currently calls the native array .reduce on order.pendingActions; replace that with effect's _Array.reduce to follow project conventions—import and use _Array.reduce from "effect/Array" (or the existing _Array binding) and pass order.pendingActions as the first arg and the same reducer function and initial accumulator ({ cancelOrderAction: null } typed as typeof CancelOrderPendingActionSchema.Type | null) as the second and third args; keep the inner logic (Schema.decodeUnknownOption(CancelOrderPendingActionSchema)(pa).pipe(Option.getOrNull) and setting acc.cancelOrderAction) unchanged.packages/widget/src/main.tsx (1)
7-12: ⚡ Quick winConsider validating required configuration values.
The configuration object is created directly from environment variables without validation. If required values like
perpsBaseUrl,perpsApiKey, ormoralisApiKeyare missing, the Widget will receiveundefinedvalues, which could cause runtime errors downstream.🛡️ Add runtime validation for required config
Consider using Effect Schema to validate the config at startup:
import { Schema } from "effect"; const PerpsConfigSchema = Schema.Struct({ perpsBaseUrl: Schema.String, perpsApiKey: Schema.String, moralisApiKey: Schema.String, reownProjectId: Schema.optional(Schema.String), }); const configResult = Schema.decodeUnknownEither(PerpsConfigSchema)({ perpsBaseUrl: import.meta.env.VITE_PERPS_BASE_URL, perpsApiKey: import.meta.env.VITE_PERPS_API_KEY, reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID || undefined, moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY, }); if (configResult._tag === "Left") { throw new Error(`Invalid configuration: ${configResult.left.message}`); } const config = configResult.right;As per coding guidelines, use Effect Schema for validation.
🤖 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/widget/src/main.tsx` around lines 7 - 12, The config object built from import.meta.env (PerpsConfig / config) lacks runtime validation; add an Effect Schema (e.g., PerpsConfigSchema using Schema.Struct with perpsBaseUrl, perpsApiKey, moralisApiKey as required strings and reownProjectId as optional) and validate the raw values with Schema.decodeUnknownEither (or equivalent), then throw a clear Error if validation returns Left so downstream code only sees the validated config (replace the direct config assignment with the decoded right value).packages/common/src/hooks/use-tp-sl-orders.ts (1)
19-26: 💤 Low valueConsider calling Match.exhaustive to ensure type safety.
The Match chain handles "take_profit" and "stop_loss" cases but doesn't call
Match.exhaustive, which means if the schema validation passes an unexpected type, it will silently do nothing. While the schema validation makes this unlikely, adding.exhaustivewould provide compile-time exhaustiveness checking.♻️ Add exhaustive matching
- Match.value(decoded).pipe( + Match.value(decoded.type).pipe( - Match.when({ type: "take_profit" }, (v) => { + Match.when("take_profit", () => { acc.takeProfit = v; }), - Match.when({ type: "stop_loss" }, (v) => { + Match.when("stop_loss", () => { acc.stopLoss = v; }), + Match.exhaustive );🤖 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/common/src/hooks/use-tp-sl-orders.ts` around lines 19 - 26, The Match chain using Match.value(decoded).pipe with two Match.when handlers (type "take_profit" and "stop_loss") lacks exhaustiveness checking; update the chain to call Match.exhaustive (i.e., append a final Match.exhaustive() after the Match.when(...) handlers) so the compiler enforces that all possible variants are handled and will surface unhandled types at compile time.packages/common/src/domain/action.ts (1)
8-10: ⚡ Quick winUse shape-based member access instead of positional indexing for auto-generated union.
Line 9 relies on positional access to extract the struct member from
ActionDto.summary, which is aSchema.Unionauto-generated by@effect/openapi-generator. Sinceapi-schemas.tsis regenerated, the union member order could change if the OpenAPI spec or generator logic shifts. Access the struct member by shape (e.g., filtering for the non-null member) rather than hardcoding index[0].🤖 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/common/src/domain/action.ts` around lines 8 - 10, ActionSummary currently uses positional indexing on ApiSchemas.ActionDto.fields.summary.members[0] which is brittle; change it to select the struct member by shape instead (e.g., find the non-null/struct member from ApiSchemas.ActionDto.fields.summary.members) and spread its .fields into ActionSummary, then keep size: Schema.optionalKey(NumberFromString); update references in the ActionSummary declaration and any related code that assumed members[0] so the schema derives from the shape-filtered member rather than a hardcoded index.apps/widget-vanilla-example/src/main.js (1)
7-7: 💤 Low valueHardcoded API keys in example code.
While hardcoded secrets are acceptable for example applications, consider documenting in comments or README that these are example values and should be replaced with user-specific keys in production use.
Also applies to: 10-10
🤖 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 `@apps/widget-vanilla-example/src/main.js` at line 7, Replace the hardcoded API key value assigned to perpsApiKey in main.js with a clearly documented placeholder and add a comment (or update the README) stating that the shown key is an example only and must be replaced with a user-specific secret for production; search for other occurrences (the reviewer noted "Also applies to: 10-10") and apply the same change to any other files using hardcoded keys so all instances use placeholders and explanatory comments or README guidance.packages/common/src/services/events.ts (1)
35-51: 💤 Low valueVerify cleanup semantics in subscribe method.
The
subscribemethod returns a synchronous cleanup function (line 48) that callsEffect.runFork(Scope.close(...))(line 49). This means the cleanup happens asynchronously, and the caller won't know when cleanup actually completes.If callers expect cleanup to be synchronous or need to await completion, consider either:
- Returning a cleanup function that returns a Promise
- Documenting that cleanup is fire-and-forget
The current implementation is likely fine if cleanup timing isn't critical, but worth confirming this matches the intended behavior.
🤖 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/common/src/services/events.ts` around lines 35 - 51, The subscribe function currently returns a fire-and-forget cleanup that calls Effect.runFork(Scope.close(scope, Exit.void)); change this so the returned cleanup is awaitable: have subscribe return a function that returns a Promise (e.g., Promise<void>) by running the scope close Effect and awaiting it (use Effect.runPromise or Effect.runPromiseExit on Scope.close(scope, Exit.void) and map to void) instead of using Effect.runFork; update the return type accordingly so callers can await cleanup from subscribe.packages/widget/vite.vanilla.config.ts (1)
15-18: 💤 Low valueVerify
emptyOutDir: falseis intentional.Setting
emptyOutDir: false(line 17) means Vite won't clean thedistdirectory before building. This is typically done when multiple build configs output to the same directory (e.g., separate React and vanilla builds).However, this can lead to stale build artifacts accumulating if entry point names change or files are removed. Ensure this is intentional and consider documenting the build orchestration strategy, or add a manual clean step before running all builds.
🤖 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/widget/vite.vanilla.config.ts` around lines 15 - 18, The build config sets emptyOutDir: false which prevents Vite from cleaning the dist directory and can leave stale artifacts; either confirm this is intentional for multi-target builds or change it to emptyOutDir: true to clean before each build, and if keeping false add a documented build orchestration note or a prebuild cleanup step (e.g., npm script or CI job) to remove/verify dist; look for the build object in vite.vanilla.config.ts and update the emptyOutDir setting or add the corresponding cleanup/documentation.packages/dashboard/src/components/molecules/positions/positions-tab.tsx (1)
80-86: 💤 Low valueConsider using
Option.matchinstead of manual_tagcheck.Directly checking
market._tag === "None"bypasses the type-safe Option API. UsingOption.matchorOption.flatMapwould be more idiomatic and maintain type safety.♻️ Suggested refactor using Option combinators
- Record.get(marketsMap, positionRef.value.marketId).pipe((market) => - market._tag === "None" - ? _Result.failVoid - : _Result.succeed({ - positionRef, - marketRef: market.value, - }), + Record.get(marketsMap, positionRef.value.marketId).pipe( + Option.match({ + onNone: () => _Result.failVoid, + onSome: (marketRef) => + _Result.succeed({ + positionRef, + marketRef, + }), + }), ),🤖 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/dashboard/src/components/molecules/positions/positions-tab.tsx` around lines 80 - 86, The code is manually inspecting the Option discriminant (market._tag === "None") when handling Record.get(marketsMap, positionRef.value.marketId); replace that pattern with the Option API (e.g., Option.match or Option.flatMap) to preserve type-safety: call the Option combinator on the result of Record.get(...) and return _Result.failVoid on the None branch and _Result.succeed({ positionRef, marketRef: market }) on the Some branch, ensuring you reference the same symbols (Record.get, marketsMap, positionRef, _Result.failVoid, _Result.succeed) so the logic and return shape remain identical.packages/common/src/components/molecules/sign-transactions/index.tsx (1)
31-33: 💤 Low valueConsider removing or conditionalizing debug logging.
The
console.log(Cause.pretty(...))on line 32 will output to the console in production. Consider either:
- Removing this debug log
- Wrapping it in a development-only check
- Using a proper logging service
🤖 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/common/src/components/molecules/sign-transactions/index.tsx` around lines 31 - 33, The console.debug line using Cause.pretty(Cause.fail(getErrorReason(error))) should not unconditionally log in production; remove the console.log or wrap it behind a development-only check (e.g., process.env.NODE_ENV === 'development') or replace it with the app's logger (e.g., logger.debug/logger.error) so only appropriate environments record this output—update the block where error is handled around the variables/functions Cause.pretty, Cause.fail and getErrorReason to implement one of these changes.packages/widget/src/components/modules/Home/Positions/index.tsx (2)
120-124: 💤 Low valueSame
Option.matchsuggestion applies here.🤖 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/widget/src/components/modules/Home/Positions/index.tsx` around lines 120 - 124, Replace the manual Option tag check on the Record.get result with an Option.match call: where you currently call Record.get(marketsMap, o.marketId).pipe((market) => market._tag === "None" ? _Result.failVoid : _Result.succeed({ marketRef: market.value, order: o })), use Option.match (or the equivalent match/fold helper) to handle the None and Some branches and return _Result.failVoid for the None case and _Result.succeed({ marketRef: value, order: o }) for the Some case; reference Record.get, marketsMap, o.marketId, _Result.failVoid, _Result.succeed and market.value when making the change.
101-110: 💤 Low valueConsider using
Option.matchinstead of manual_tagcheck.Same pattern as in
positions-tab.tsx- directly checkingmarket._tag === "None"bypasses the type-safe Option API.♻️ Suggested refactor
- Record.get(marketsMap, positionRef.value.marketId).pipe((market) => - market._tag === "None" - ? _Result.failVoid - : _Result.succeed({ - marketRef: market.value, - positionRef, - orders: Record.get(ordersMap, positionRef.value.marketId).pipe( - Option.getOrElse(() => [] as Order[]), - ), - }), + Record.get(marketsMap, positionRef.value.marketId).pipe( + Option.match({ + onNone: () => _Result.failVoid, + onSome: (marketRef) => + _Result.succeed({ + marketRef, + positionRef, + orders: Record.get(ordersMap, positionRef.value.marketId).pipe( + Option.getOrElse(() => [] as Order[]), + ), + }), + }), ),🤖 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/widget/src/components/modules/Home/Positions/index.tsx` around lines 101 - 110, Replace the manual discriminant check on the Option returned by Record.get with the Option API: use Option.match (or Option.fold depending on library) on the value from Record.get(marketsMap, positionRef.value.marketId) to return _Result.failVoid when None and _Result.succeed({...}) when Some; update the inner expression that builds the result (market.value -> marketRef, positionRef, and orders via Record.get(ordersMap, ...).pipe(Option.getOrElse(...))) so it uses the matched Some value rather than checking market._tag, mirroring the pattern used in positions-tab.tsx.
| const config: PerpsConfig = { | ||
| perpsBaseUrl: "https://perps.yield.xyz", | ||
| perpsApiKey: "e2d627cf-2ae3-4775-9fbc-76819c7cae38", | ||
| reownProjectId: "29e1b718ad16983a0705cf24d5b5b747", | ||
| moralisApiKey: | ||
| "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJub25jZSI6IjhlNTk0NDE0LTg3NGMtNDZlMC1iMWNlLWU5ZjYzMjY1YWExMiIsIm9yZ0lkIjoiNDI2Nzk1IiwidXNlcklkIjoiNDM4OTk2IiwidHlwZSI6IlBST0pFQ1QiLCJ0eXBlSWQiOiI2YzMwMDY3Yi1kNDEyLTQwYjYtYTQ4OS0zYjEwM2ExNThjOGMiLCJpYXQiOjE3Mzc0NTA4NzcsImV4cCI6NDg5MzIxMDg3N30.pd8sKdRHdtlqoZVS7wb8Jyy2GLhhr95X8yW64W_gSC0", | ||
| }; |
There was a problem hiding this comment.
Remove hardcoded credentials from client code.
Line 8 and Line 11 embed live-looking secrets directly in the frontend bundle. That is a credential leak risk; move them to import.meta.env and rotate the exposed keys/tokens.
Suggested fix
const config: PerpsConfig = {
perpsBaseUrl: "https://perps.yield.xyz",
- perpsApiKey: "e2d627cf-2ae3-4775-9fbc-76819c7cae38",
- reownProjectId: "29e1b718ad16983a0705cf24d5b5b747",
- moralisApiKey:
- "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJub25jZSI6IjhlNTk0NDE0LTg3NGMtNDZlMC1iMWNlLWU5ZjYzMjY1YWExMiIsIm9yZ0lkIjoiNDI2Nzk1IiwidXNlcklkIjoiNDM4OTk2IiwidHlwZSI6IlBST0pFQ1QiLCJ0eXBlSWQiOiI2YzMwMDY3Yi1kNDEyLTQwYjYtYTQ4OS0zYjEwM2ExNThjOGMiLCJpYXQiOjE3Mzc0NTA4NzcsImV4cCI6NDg5MzIxMDg3N30.pd8sKdRHdtlqoZVS7wb8Jyy2GLhhr95X8yW64W_gSC0",
+ perpsApiKey: import.meta.env.VITE_PERPS_API_KEY,
+ reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID,
+ moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config: PerpsConfig = { | |
| perpsBaseUrl: "https://perps.yield.xyz", | |
| perpsApiKey: "e2d627cf-2ae3-4775-9fbc-76819c7cae38", | |
| reownProjectId: "29e1b718ad16983a0705cf24d5b5b747", | |
| moralisApiKey: | |
| "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJub25jZSI6IjhlNTk0NDE0LTg3NGMtNDZlMC1iMWNlLWU5ZjYzMjY1YWExMiIsIm9yZ0lkIjoiNDI2Nzk1IiwidXNlcklkIjoiNDM4OTk2IiwidHlwZSI6IlBST0pFQ1QiLCJ0eXBlSWQiOiI2YzMwMDY3Yi1kNDEyLTQwYjYtYTQ4OS0zYjEwM2ExNThjOGMiLCJpYXQiOjE3Mzc0NTA4NzcsImV4cCI6NDg5MzIxMDg3N30.pd8sKdRHdtlqoZVS7wb8Jyy2GLhhr95X8yW64W_gSC0", | |
| }; | |
| const config: PerpsConfig = { | |
| perpsBaseUrl: "https://perps.yield.xyz", | |
| perpsApiKey: import.meta.env.VITE_PERPS_API_KEY, | |
| reownProjectId: import.meta.env.VITE_REOWN_PROJECT_ID, | |
| moralisApiKey: import.meta.env.VITE_MORALIS_API_KEY, | |
| }; |
🧰 Tools
🪛 Betterleaks (1.2.0)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 11-11: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 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 `@apps/widget-react-example/src/main.tsx` around lines 6 - 12, The PerpsConfig
object in main.tsx currently contains hardcoded secrets (perpsApiKey,
moralisApiKey, reownProjectId); remove these literals and load them from
environment variables via import.meta.env (e.g.
import.meta.env.VITE_PERPS_API_KEY, VITE_MORALIS_API_KEY, VITE_REOWN_PROJECT_ID)
when constructing the config used by PerpsConfig, validate presence and throw or
warn if missing, and rotate the exposed keys/tokens after replacement.
| import type { ChainId, WalletAddress } from "./ids"; | ||
| import type { SignedPayload, TransactionHash } from "./transactions"; | ||
| import type { | ||
| EvmTransactionRequest, | ||
| TypedDataRequest, | ||
| WalletAdapterState, | ||
| } from "./wallet-adapter"; | ||
|
|
||
| export type WalletAdapterStateEncoded = typeof WalletAdapterState.Encoded; | ||
| export type TransactionHashEncoded = typeof TransactionHash.Encoded; | ||
| export type SignedPayloadEncoded = typeof SignedPayload.Encoded; | ||
| export type ChainIdEncoded = typeof ChainId.Encoded; | ||
| export type WalletAddressEncoded = typeof WalletAddress.Encoded; | ||
| export type TypedDataRequestEncoded = typeof TypedDataRequest.Encoded; | ||
| export type EvmTransactionRequestEncoded = typeof EvmTransactionRequest.Encoded; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify value/type mismatch pattern in this file.
rg -n 'import type|typeof .*\.Encoded' packages/common/src/domain/wallet-source.tsRepository: stakekit/perps-widget
Length of output: 707
🏁 Script executed:
# Check what's actually exported in the source files
cat -n packages/common/src/domain/ids.ts | head -50Repository: stakekit/perps-widget
Length of output: 1571
🏁 Script executed:
# Also check transactions.ts and wallet-adapter.ts
cat -n packages/common/src/domain/transactions.ts | head -50Repository: stakekit/perps-widget
Length of output: 1380
🏁 Script executed:
# Check wallet-adapter.ts
cat -n packages/common/src/domain/wallet-adapter.ts | head -80Repository: stakekit/perps-widget
Length of output: 3059
🏁 Script executed:
# Try to understand the structure - search for .Encoded definitions
rg -n '\.Encoded' packages/common/src/domain/ -A 2Repository: stakekit/perps-widget
Length of output: 1004
🏁 Script executed:
# Check for TypeScript errors in the file
cd packages/common && npx tsc --noEmit packages/common/src/domain/wallet-source.ts 2>&1 | head -30Repository: stakekit/perps-widget
Length of output: 207
🏁 Script executed:
# Also check if there's a tsconfig and what compiler options are set
cat -n packages/common/tsconfig.json 2>/dev/null || echo "tsconfig not found in packages/common"Repository: stakekit/perps-widget
Length of output: 355
🏁 Script executed:
# Search for usage of these Encoded types to see if they're referenced elsewhere
rg 'WalletAdapterStateEncoded|TransactionHashEncoded|SignedPayloadEncoded|ChainIdEncoded|WalletAddressEncoded|TypedDataRequestEncoded|EvmTransactionRequestEncoded' packages/Repository: stakekit/perps-widget
Length of output: 2381
🏁 Script executed:
# Run tsc from packages/common directory with proper config
cd packages/common && npx tsc --noEmit 2>&1 | grep -A 2 "wallet-source.ts"Repository: stakekit/perps-widget
Length of output: 47
🏁 Script executed:
# Or check using pnpm turbo lint
cd /repo && pnpm turbo lint --filter=@yieldxyz/perps-common 2>&1 | grep -A 5 "wallet-source"Repository: stakekit/perps-widget
Length of output: 121
🏁 Script executed:
# Run tsc with proper working directory and capture full output
cd packages/common && npx tsc --noEmit 2>&1Repository: stakekit/perps-widget
Length of output: 244
🏁 Script executed:
# Also check what the root directory is
pwd && ls -la | head -20Repository: stakekit/perps-widget
Length of output: 1295
🏁 Script executed:
# Verify the downstream usage doesn't conflict with value imports
grep -r "import.*from.*wallet-source" packages/dashboard/src/index.ts packages/widget/src/index.ts | head -10Repository: stakekit/perps-widget
Length of output: 47
Use value imports for .Encoded typeof queries.
Lines 1–7 use import type, but Lines 9–15 access typeof X.Encoded, which requires value imports. The imported symbols are Effect Schema objects with .Encoded as a runtime property, not a type-level property. import type erases the runtime binding, causing TypeScript to fail with "Cannot find name 'X'" errors.
🐛 Proposed fix
-import type { ChainId, WalletAddress } from "./ids";
-import type { SignedPayload, TransactionHash } from "./transactions";
-import type {
+import { ChainId, WalletAddress } from "./ids";
+import { SignedPayload, TransactionHash } from "./transactions";
+import {
EvmTransactionRequest,
TypedDataRequest,
WalletAdapterState,
} from "./wallet-adapter";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { ChainId, WalletAddress } from "./ids"; | |
| import type { SignedPayload, TransactionHash } from "./transactions"; | |
| import type { | |
| EvmTransactionRequest, | |
| TypedDataRequest, | |
| WalletAdapterState, | |
| } from "./wallet-adapter"; | |
| export type WalletAdapterStateEncoded = typeof WalletAdapterState.Encoded; | |
| export type TransactionHashEncoded = typeof TransactionHash.Encoded; | |
| export type SignedPayloadEncoded = typeof SignedPayload.Encoded; | |
| export type ChainIdEncoded = typeof ChainId.Encoded; | |
| export type WalletAddressEncoded = typeof WalletAddress.Encoded; | |
| export type TypedDataRequestEncoded = typeof TypedDataRequest.Encoded; | |
| export type EvmTransactionRequestEncoded = typeof EvmTransactionRequest.Encoded; | |
| import { ChainId, WalletAddress } from "./ids"; | |
| import { SignedPayload, TransactionHash } from "./transactions"; | |
| import { | |
| EvmTransactionRequest, | |
| TypedDataRequest, | |
| WalletAdapterState, | |
| } from "./wallet-adapter"; | |
| export type WalletAdapterStateEncoded = typeof WalletAdapterState.Encoded; | |
| export type TransactionHashEncoded = typeof TransactionHash.Encoded; | |
| export type SignedPayloadEncoded = typeof SignedPayload.Encoded; | |
| export type ChainIdEncoded = typeof ChainId.Encoded; | |
| export type WalletAddressEncoded = typeof WalletAddress.Encoded; | |
| export type TypedDataRequestEncoded = typeof TypedDataRequest.Encoded; | |
| export type EvmTransactionRequestEncoded = typeof EvmTransactionRequest.Encoded; |
🤖 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/common/src/domain/wallet-source.ts` around lines 1 - 15, The imports
are declared with "import type" but you later use runtime properties (e.g.
WalletAdapterState.Encoded, TransactionHash.Encoded, SignedPayload.Encoded,
ChainId.Encoded, WalletAddress.Encoded, TypedDataRequest.Encoded,
EvmTransactionRequest.Encoded), which requires value imports; change the imports
so these symbols are imported as runtime values (remove "type" for the Effect
Schema objects or import them as normal/value imports) while keeping type-only
imports for pure types like ChainId, WalletAddress if needed—update the import
statement to import the actual schema objects (WalletAdapterState,
TransactionHash, SignedPayload, TypedDataRequest, EvmTransactionRequest) as
values so typeof X.Encoded resolves at runtime.
| const decodeState = Schema.decodeUnknownSync(WalletAdapterState); | ||
| const stateRef = yield* SubscriptionRef.make( | ||
| decodeState(source.getState()), | ||
| ); |
There was a problem hiding this comment.
Potential synchronous error in layer construction.
Schema.decodeUnknownSync can throw exceptions synchronously if the initial state doesn't match the schema. If source.getState() returns invalid data during layer construction (line 20), the Layer effect will fail with an unhandled exception rather than a proper Effect error.
Consider wrapping the initial decode in Effect.try or using Schema.decodeUnknownEffect to ensure all errors flow through the Effect type:
🛡️ Proposed fix to handle decode errors in Effect
- const decodeState = Schema.decodeUnknownSync(WalletAdapterState);
- const stateRef = yield* SubscriptionRef.make(
- decodeState(source.getState()),
- );
+ const initialState = yield* Schema.decodeUnknownEffect(WalletAdapterState)(
+ source.getState()
+ );
+ const stateRef = yield* SubscriptionRef.make(initialState);
+ const decodeState = Schema.decodeUnknownSync(WalletAdapterState);🤖 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/common/src/services/wallet/external-wallet-adapter.ts` around lines
18 - 21, The initial state decode uses
Schema.decodeUnknownSync(WalletAdapterState) which can throw synchronously when
source.getState() is invalid; wrap that decoding in an Effect so errors surface
as Effect failures instead of uncaught exceptions—replace the sync decode with
Schema.decodeUnknownEffect(WalletAdapterState, source.getState()) or perform
Effect.try around Schema.decodeUnknownSync and then pass the resulting Effect to
SubscriptionRef.make (or construct the ref inside an Effect.flatMap) so
SubscriptionRef.make receives a safe Effect-wrapped initial value and all decode
errors are handled through the Effect channel.
| export type WidgetProps = { | ||
| readonly config: Omit<PerpsConfig, "reownProjectId">; | ||
| readonly externalWalletSource?: ExternalWalletSource; | ||
| readonly onLifecycleEvent?: (event: LifecycleEvent) => void; | ||
| }; | ||
|
|
||
| export const Widget = ({ | ||
| config, | ||
| externalWalletSource, | ||
| onLifecycleEvent, | ||
| }: WidgetProps) => { | ||
| preload(TRADING_VIEW_WIDGET_SCRIPT_URL, { as: "script" }); | ||
|
|
||
| return ( | ||
| <Providers> | ||
| <Providers | ||
| config={config} | ||
| externalWalletSource={externalWalletSource} | ||
| onLifecycleEvent={onLifecycleEvent} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting Providers/config plumbing:"
sed -n '1,240p' packages/common/src/context/index.tsx
echo
sed -n '1,240p' packages/common/src/services/config.ts
echo
echo "Searching for Reown project id sources and BrowserWalletAdapterLayer wiring:"
rg -n -C3 'reownProjectId|BrowserWalletAdapterLayer|Providers\(|PerpsConfig' packages/common/src packages/widget/srcRepository: stakekit/perps-widget
Length of output: 12355
Fix type and runtime incompatibility: WidgetProps.config cannot be passed to Providers and browser wallet will crash.
WidgetProps.config is Omit<PerpsConfig, "reownProjectId">, but Providers expects the full PerpsConfig (which includes optional reownProjectId). This creates a type mismatch and prevents embedders from providing the Reown project ID that BrowserWalletAdapterLayer requires.
When externalWalletSource is not provided, the widget defaults to the browser wallet flow, which immediately dies with "Reown project ID has not been provided" if config.reownProjectId is absent. Either restore reownProjectId to WidgetProps, provide a fallback/injection mechanism in the config chain, or ensure the browser wallet path is not used when reownProjectId is unavailable.
🤖 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/widget/src/app.tsx` around lines 37 - 55, WidgetProps currently
omits reownProjectId but Providers (and BrowserWalletAdapterLayer) expect a full
PerpsConfig including reownProjectId, causing runtime crashes when
externalWalletSource is absent; fix by restoring reownProjectId to WidgetProps
(or add an explicit optional reownProjectId prop) and thread it into the
Providers call so Providers receives a PerpsConfig with reownProjectId, or
alternatively add a guard in Widget before rendering Providers that injects a
sensible fallback or disables the browser wallet path when config.reownProjectId
is missing; update the types for WidgetProps, the Widget component props
destructuring, and the Providers invocation (and any BrowserWalletAdapterLayer
initialization) to ensure reownProjectId is present when the browser wallet flow
is possible.
1 participant
Summary by CodeRabbit
New Features
Improvements