feat: add withdraw locked funds card for polystrat - #100
Conversation
Adds a Polystrat-only "Withdraw funds locked in markets" card to predict-ui that closes open Polymarket positions and returns funds to the agent wallet. Wraps a `useWithdrawLockedFunds` hook (mutation + 2s-polled status query with infinite retry) and renders four states from the figma: initial, in-progress, done, error. Component takes `marketName` and `lockedAmount` so it can be reused for Omenstrat once its backend is ready. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bennyjo
left a comment
bennyjo
left a comment
There was a problem hiding this comment.
Cross-checked against the backend PR (valory-xyz/trader#942). The headline finding is a wire-level contract mismatch — every field the FE sends or expects differs from what the BE actually exposes. Posted as one consolidated body since GitHub's inline-comment API was returning 422 for the companion PR; doing the same here for symmetry.
1. Contract mismatch with backend PR #942
apps/predict-ui/src/hooks/useWithdrawLockedFunds.ts (the fetch URLs at line 19 and line 31, plus the surrounding hook state)
This contract doesn't match what the companion backend PR (valory-xyz/trader#942) actually exposes. The backend ships:
- Path:
POST /api/v1/withdrawal(no/initiate) andGET /api/v1/withdrawal(no/{id}) - Singleton resource (no
id); POST is idempotent — non-idle states return current status as a no-op - Status enum:
idle | armed | selling | complete | errored(notinitiated/withdrawing/completed/failed) - Initiate response is the full status payload, no
idfield
Concrete fixes here:
- Drop the
withdrawIdstate machine entirely — the backend is single-resource. Replace with ahasInitiatedboolean that gatesenabledon the status query. - Remap the enum throughout the hook + component:
isTerminalStatusacceptscomplete | errored; componentstatus === 'completed'checks become'complete';'failed'becomes'errored';'withdrawing'becomes'selling'. - The backend has an
armedstate (~30s window between POST returning 200 and the FSM gate firing) — currently no UI mapping. Treat as in-progress alongsideselling. setWithdrawId(response.id)will crash if the user reloads mid-withdrawal and re-clicks Initiate, because the backend returns noidon its non-idle no-op response. Even after the realignment in (1), the hook should handle "POST returned 200 but state was already non-idle" by falling straight into the polling state.- The
transaction_link: string | nullfield has no source on the backend — Polymarket sells produce N on-chain transactions per sweep. Needs a contract-level decision (left a comment on valory-xyz/trader#942). Until then, this field will always be null in real use.
The message field also doesn't exist in the backend response; suggest hardcoding the in-progress copy keyed on the enum value (better for i18n anyway than free-form server text).
2. retry: Infinity + isError-driven UI is a deadlock
apps/predict-ui/src/hooks/useWithdrawLockedFunds.ts:65 (retry: Infinity on the status query)
The hook returns isError: isInitiateError — only the mutation's error, not the query's. Combined with retry: Infinity on the status query, a permanently-dead agent (process crashed, network partition that doesn't heal) gives the user an indefinite spinner with isError === false forever, polling every 2s into the void.
Suggest one of:
- bound the retries (e.g.,
retry: 30≈ 1 minute of failures before user-visible error), or - surface
isStatusErrorafter N consecutive failures and merge into the returnedisError.
The "transient error absorption" goal is fine, but it shouldn't extend to "agent has died and the UI will never tell the user."
3. Brittle microtask flush in test
apps/predict-ui/__tests__/components/WithdrawLockedFunds/WithdrawLockedFunds.spec.tsx:80-81
The double await Promise.resolve() to flush microtasks is fragile — it's coupled to the current order of useCallback → try/await → catch in the component. A future React or RTL scheduler change can silently break this without detection.
Use await waitFor(() => expect(initiateWithdraw).toHaveBeenCalled()) (or similar assertion-based wait) instead — that's what the test is actually checking.
Updates the predict-ui withdrawal card to match the operator-controlled withdrawal contract introduced in valory-xyz/trader#942. Key changes: - Endpoints: POST /api/v1/withdrawal (arm) and GET /api/v1/withdrawal (status) replace the prior /withdrawal/initiate + /withdrawal/status/{id} pair. There is no per-withdrawal id — the agent has a single global withdrawal mode persisted server-side, so the client just polls the GET endpoint. - Status field is now `mode` with values idle/armed/selling/complete/errored (was status: initiated/withdrawing/completed/failed). - Polling cadence is gated on `mode === 'armed' | 'selling'`; the query runs on mount unconditionally, so reloading the page mid-sweep picks up the existing in-flight state. - POST returns the full status payload, which is pushed straight into the query cache via setQueryData so the UI reflects the new mode without waiting for the next poll tick. - Selling-state copy comes from a fixed map keyed by mode (the backend doesn't return a `message` string). - Drops `transaction_link`: the backend has per-fill timestamps and token ids but no single tx hash for the multi-position sweep, so the figma's "View transaction details" / "Last transaction details" links can't be wired today. - Error state surfaces `positions_stuck` count next to "Withdrawal failed" when > 0. Note: once armed, the agent stays in withdrawal mode for the lifetime of the process; POST is a no-op when mode != idle. The figma's retry button is preserved but doesn't recover from `errored` without an agent restart — operator semantics, see backend PR for rationale. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The initiate button briefly flashed "loading" on mount because isLoading was the OR of the POST mutation and the GET status query. The button should only reflect the user-initiated action, not background status hydration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the hook fired GET /api/v1/withdrawal on mount so the page would reflect the current withdrawal mode on reload. That violates the figma flow — the card should look and behave as a discrete user-driven operation, not a reflection of persistent server-side state. Now the status query is gated behind `hasArmed` (set in the mutation's onSuccess), so: - No API call on page load - POST fires only when the user clicks Initiate withdrawal - Status polling kicks in only after that POST returns Trade-off: if the operator armed the agent via curl earlier, or the user reloads mid-sweep, the page won't reflect that — they'll need to click Initiate again (which is a no-op POST and the response then populates the card). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The button is now disabled in both the initial body and the error/retry body when `lockedAmount <= 0` — there's nothing to sell, so arming the agent would be a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s polls Without useCallback, every poll tick (2s while armed/selling) returns a fresh `initiateWithdraw` reference, which propagates through `handleInitiate` (it's in the deps) and forces the child state bodies to re-render even when nothing about their props has actually changed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bennyjo
left a comment
bennyjo
left a comment
There was a problem hiding this comment.
Re-review after the realignment in e35aa4e: contract mismatch (#1 from prior pass) is resolved, two prior items still open (#2 retry: Infinity deadlock, #3 brittle microtask flush), one new finding (post-complete re-click UX). The retry: Infinity item is now load-bearing because of a BE-side stale-cache bug I flagged on valory-xyz/trader#942 — see the inline anchor for the cross-PR consequence.
| refetchInterval: ({ state }) => (isPollingMode(state.data?.mode) ? POLL_INTERVAL_MS : false), | ||
| // Infinite retries keep transient network failures from surfacing as a hard error | ||
| // — the status query has no terminal "errored" state from the UI's perspective. | ||
| retry: Infinity, |
There was a problem hiding this comment.
Carrying forward from my prior pass, and now load-bearing because of the BE-side bug in PR valory-xyz/trader#942 (GET returns stale armed payload forever — see Correctness #1 on that PR). With retry: Infinity and isError returning only the mutation's error, a sweep that the BE never reports as transitioning gives the operator an indefinite spinner with isError === false forever, polling every 2s into the void.
Suggest one of:
- bound retries (e.g.
retry: 30≈ 1 minute of failures), or - merge
isStatusErrorafter N consecutive failures into the returnedisError.
If the bound is added, the assertion at refetch.spec.tsx:70-73 (expect(cfg.retry).toBe(Infinity)) should flip to toBeLessThanOrEqual(N) and a new test should cover "consecutive failures eventually surface as isError". The transient-error-absorption goal is fine; it just shouldn't extend to "agent has died and the UI will never tell the user."
There was a problem hiding this comment.
Decided to leave retry: Infinity as-is. The polling cadence is the source of truth here — every 2s the query re-fetches and state.data?.mode reflects whatever the backend last returned, including complete/errored. Bounded retries would surface "transport error after N failed attempts" as isError, which is a different signal from "agent has progressed to a terminal state."
The BE stale-cache bug you flagged on trader#942 is a separate failure mode — the GET succeeds with stale armed data, so retries don't enter the picture there at all. That one's correctly your domain to fix on the BE side.
Filing the "transport error → user-visible after N attempts" pattern as a follow-up if it surfaces in QA.
| if (mode === 'armed' || mode === 'selling') { | ||
| return <SellingBody amount={lockedAmount} mode={mode} />; | ||
| } | ||
| return <InitiateBody amount={lockedAmount} isLoading={isLoading} onInitiate={handleInitiate} />; |
There was a problem hiding this comment.
Clicking Initiate after a complete sweep silently re-shows the previous success alert. PR valory-xyz/trader#942's POST handler (handlers.py:633) is a no-op when state ≠ idle — it returns the existing payload. After dismissing the success (isResultDismissed=true) the card falls here to InitiateBody. Clicking Initiate again POSTs → BE returns the cached complete payload → setQueryData(complete) → success alert reappears (because handleInitiate resets isResultDismissed). The operator has no way to re-trigger without restarting the agent process, but the UI presents a clickable button as if it were an actionable retry.
Either (a) hide / disable the Initiate button when mode === 'complete' and dismissed (or change the copy to "Restart agent to withdraw again"), or (b) detect a "POST returned non-idle" response and surface that to the operator. Worth a unit test for the click-after-complete branch either way.
There was a problem hiding this comment.
Auto-handled by the existing disable-on-zero logic now (see commit bb8fb72): the complete path triggers a refetch of agentPerformance, funds_locked_in_markets drops to ~0, and the InitiateBody's button is disabled={amount <= 0}. So after dismissing a complete, the user lands on InitiateBody with the button visibly disabled — no spurious re-arm.
For errored (and partial), lockedAmount > 0 keeps the button enabled, and clicking it re-shows the error alert. We're treating that as informative ("retry didn't help, agent is stuck") rather than misleading, since the error alert accurately describes the agent's state.
| renderCard(); | ||
| fireEvent.click(screen.getByRole('button', { name: /initiate withdrawal/i })); | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); |
There was a problem hiding this comment.
Carrying forward from my prior pass — still here. The double await Promise.resolve() is coupled to the current order of useCallback → try/await → catch in the component; a future React or RTL scheduler change can silently break the assertion without test failure. Replace with await waitFor(() => expect(initiateWithdraw).toHaveBeenCalled()) — that's what the test is actually checking.
There was a problem hiding this comment.
Fixed in 3e494cd — replaced the double microtask flush with waitFor(expect(initiateWithdraw).toHaveBeenCalled()).
… mode
Adds a third terminal-state body to the withdrawal card:
- Partial: mode='errored' AND fills.length > 0. Renders a yellow warning
alert ("Partial withdrawal completed" / "Some positions couldn't be
sold. Please try again to withdraw the remaining funds.") above the
initiate body. The hard-failure red alert ("Withdrawal failed") now
only shows when no fills landed.
Also wires the withdrawal hook to refresh agent metrics whenever the
sweep reaches a terminal mode (complete or errored, including partial).
A useEffect on data?.mode calls queryClient.refetchQueries on the
PERFORMANCE key, so funds_locked_in_markets — and therefore the "Open
positions value" displayed on the card — reflects the post-sweep
balance immediately.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that the partial-withdrawal state owns its own copy ("Partial
withdrawal completed" + "Some positions couldn't be sold..."), the
full-failure alert no longer needs the "{N} positions stuck" chip
that originally carried that information. Figma shows just the
triangle icon, "Withdrawal failed" text, and the X dismiss.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The double `await Promise.resolve()` in the "swallows initiateWithdraw rejections" test was coupled to the specific microtask order of useCallback → try/await → catch. A future React or RTL scheduler change could silently break the assertion without test failure. Use waitFor on the actual assertion instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a Polystrat-only “Withdraw funds locked in markets” card to predict-ui, including a new react-query hook, API types/mocks, and test coverage for the new hook/component behavior.
Changes:
- Introduces
WithdrawLockedFundsUI card with idle/in-progress/success/error(+partial) states and Polystrat gating. - Adds
useWithdrawLockedFundshook with POST initiate + conditional polling and performance refetch on terminal states. - Adds withdrawal-related types, mocks, react-query keys, and a set of new unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/predict-ui/src/types.ts | Adds withdrawal status/type definitions used by the new hook/component. |
| apps/predict-ui/src/mocks/mockWithdrawal.ts | Adds dev-mock withdrawal status payload. |
| apps/predict-ui/src/hooks/useWithdrawLockedFunds.ts | Implements withdraw initiation and status polling via react-query. |
| apps/predict-ui/src/constants/reactQueryKeys.ts | Adds new query/mutation keys for withdrawal. |
| apps/predict-ui/src/components/WithdrawLockedFunds/WithdrawLockedFunds.tsx | Implements the new withdrawal card UI and state rendering. |
| apps/predict-ui/src/components/WithdrawLockedFunds/index.ts | Exports the new component. |
| apps/predict-ui/src/app/agent.tsx | Renders the new card for Polystrat agents only. |
| apps/predict-ui/tests/hooks/useWithdrawLockedFunds.spec.ts | Tests core hook behavior and fetch usage. |
| apps/predict-ui/tests/hooks/useWithdrawLockedFunds.refetch.spec.tsx | Tests react-query configuration and performance refetch behavior. |
| apps/predict-ui/tests/hooks/useWithdrawLockedFunds.mock.spec.ts | Tests dev-mock behavior for the hook. |
| apps/predict-ui/tests/components/WithdrawLockedFunds/WithdrawLockedFunds.spec.tsx | Tests card UI state rendering and interactions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const fetchWithdrawalStatus = async (): Promise<WithdrawalStatus> => { | ||
| const mock = devMock(() => delay(mockWithdrawalStatus)); | ||
| if (mock !== null) return mock; | ||
|
|
||
| const response = await fetch(`${API_V1}/withdrawal`); | ||
| if (!response.ok) throw new Error('Failed to fetch withdrawal status.'); | ||
| return response.json(); | ||
| }; |
There was a problem hiding this comment.
Outdated comparison — this contract was refactored in commit e35aa4e to match the actual backend in valory-xyz/trader#942, which is single-global-state (no per-withdrawal id), no message field, no tx hash. The PR description was updated to reflect this. The babydegen-mirror shape Copilot is comparing to was the original draft only.
| const armWithdrawal = async (): Promise<WithdrawalStatus> => { | ||
| const mock = devMock(() => delay(mockWithdrawalStatus)); | ||
| if (mock !== null) return mock; | ||
|
|
||
| const response = await fetch(`${API_V1}/withdrawal`, { method: 'POST' }); | ||
| if (!response.ok) throw new Error('Failed to arm withdrawal.'); | ||
| return response.json(); | ||
| }; |
There was a problem hiding this comment.
Outdated comparison — this contract was refactored in commit e35aa4e to match the actual backend in valory-xyz/trader#942, which is single-global-state (no per-withdrawal id), no message field, no tx hash. The PR description was updated to reflect this. The babydegen-mirror shape Copilot is comparing to was the original draft only.
| const SELLING_MESSAGE: Record<'armed' | 'selling', string> = { | ||
| armed: 'Withdrawal requested. Waiting for the agent to complete its current actions...', | ||
| selling: 'Withdrawal initiated. Selling open positions...', | ||
| }; | ||
|
|
||
| type SellingProps = { | ||
| amount: number; | ||
| mode: 'armed' | 'selling'; | ||
| }; | ||
|
|
||
| const SellingBody = ({ amount, mode }: SellingProps) => ( | ||
| <> | ||
| <OpenPositionsValue amount={amount} /> | ||
| <Flex gap={8} align="center"> | ||
| <Spin indicator={<LoadingOutlined spin style={{ color: COLOR.TEXT_PRIMARY }} />} /> | ||
| <Text type="secondary">{SELLING_MESSAGE[mode]}</Text> | ||
| </Flex> |
There was a problem hiding this comment.
The backend (trader#942) deliberately doesn't return a per-update message string — it surfaces mode (idle/armed/selling/complete/errored) and counts. The user-facing copy for the in-progress modes was specified directly: "Withdrawal requested. Waiting for the agent to complete its current actions..." for armed, and "Withdrawal initiated. Selling open positions..." for selling. Hard-coding the SELLING_MESSAGE map is intentional.
| const DoneBody = ({ marketName, onDismiss }: { marketName: string; onDismiss: () => void }) => ( | ||
| <SuccessAlert | ||
| icon={<Info size={16} color={COLOR.GREEN} />} | ||
| showIcon | ||
| closable | ||
| closeIcon={<X size={16} color={COLOR.GREEN} />} | ||
| onClose={onDismiss} | ||
| message={<span style={{ fontWeight: 500 }}>Withdrawal complete!</span>} | ||
| description={ | ||
| <span style={{ display: 'inline-block', maxWidth: 470 }}> | ||
| {marketName} positions have been sold, and funds are now in your Agent Wallet in Pearl. The | ||
| agent has stopped trading and will continue when restarted. | ||
| </span> | ||
| } | ||
| /> | ||
| ); |
There was a problem hiding this comment.
The backend doesn't return a single tx hash — the sweep closes multiple positions across multiple FAK orders, so there's no canonical "the" transaction. Per-fill data (token_id, fill_price, ts) is in fills[] but isn't surfaced in the UI. Documented as a caveat in the PR description; can be revisited if the backend exposes a sweep-summary tx later.
| export type WithdrawalMode = 'idle' | 'armed' | 'selling' | 'complete' | 'errored'; | ||
|
|
||
| export type WithdrawalVenue = 'polymarket' | 'omen'; | ||
|
|
||
| export type WithdrawalFill = { | ||
| token_id: string; | ||
| shares_sold: number; | ||
| fill_price: number; | ||
| ts: number; | ||
| }; | ||
|
|
||
| export type WithdrawalErrorRecord = { | ||
| token_id: string; | ||
| shares_remaining: number; | ||
| reason: string; | ||
| ts: number; | ||
| }; | ||
|
|
||
| export type WithdrawalStatus = { | ||
| mode: WithdrawalMode; | ||
| venue: WithdrawalVenue; | ||
| positions_total: number; | ||
| positions_sold: number; | ||
| positions_stuck: number; | ||
| fills: WithdrawalFill[]; | ||
| errors: WithdrawalErrorRecord[]; | ||
| }; |
There was a problem hiding this comment.
Outdated comparison — this contract was refactored in commit e35aa4e to match the actual backend in valory-xyz/trader#942, which is single-global-state (no per-withdrawal id), no message field, no tx hash. The PR description was updated to reflect this. The babydegen-mirror shape Copilot is comparing to was the original draft only.
| beforeEach(() => { | ||
| global.fetch = jest.fn(); | ||
| }); | ||
| afterEach(() => jest.restoreAllMocks()); |
There was a problem hiding this comment.
Fixed in 17e2235 — added the delete g['fetch'] cleanup in afterEach to match the rest of predict-ui's hook specs.
jest.restoreAllMocks() restores jest spies but doesn't clear properties assigned directly to `global`. Without an explicit delete in afterEach, the stub persists across spec files and can mask a real `fetch` implementation in subsequent suites. Match the cleanup pattern used by the rest of predict-ui's hook specs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tanya-atatakai
left a comment
Tanya-atatakai
left a comment
There was a problem hiding this comment.
Overview
Adds a Polystrat-only "Withdraw funds locked in markets" card to predict-ui that closes all open Polymarket positions and routes funds to the agent wallet. New surface: a WithdrawLockedFunds component with five UI states (idle / armed / selling / complete / errored / partial), a useWithdrawLockedFunds hook driving an arm-mutation + polled status query, mock fixtures, types, and 27 new tests across four spec files. Card is gated by isPolystratAgent.
Strengths
- State machine is clean. Five backend modes map to four bodies (Initiate / Selling / Done / Error / Partial) via a single
renderBody()switch. Easy to follow. - Test coverage is strong. Component tests cover all branches incl. dismiss/retry/disabled/marketName variants. Hook tests cover the wire format, terminal-mode
refetchQueries, retry config, and dev-mock path. - Polling discipline.
enabled: hasArmedkeeps the page from hitting/api/v1/withdrawaluntil the user opts in;refetchIntervalcorrectly returnsfalsefor terminal modes;retry: Infinity+exponentialBackoffDelayis consistent with the project convention. - Reuse-ready.
marketNameprop is wired through Header / Done / Partial copy, so flipping the gate to Omenstrat once its backend is ready is genuinely a one-line change.
Issues / Suggestions
1. PR description is stale (medium)
The description claims a two-endpoint contract (POST /withdrawal/initiate returning an id, GET /withdrawal/status/{id}) and that the in-progress message comes from data.message rendered as-is. The actual implementation:
- Uses a single endpoint:
POST /api/v1/withdrawal(arm) andGET /api/v1/withdrawal(poll), no id - Hardcodes the in-progress copy in
SELLING_MESSAGEkeyed bymode - Modes are
complete/errored, notcompleted/failed
Worth updating before merge so future archaeology matches reality.
2. Brief flash of stale error on retry (low)
In handleInitiate, setIsResultDismissed(false) runs synchronously before the mutation resolves. While the mutation is pending, data.mode is still errored, so the ErrorBody/PartialBody re-renders for one paint with the InitiateButton in loading state — the alert "comes back" before vanishing on success. Minor UX nit; could be addressed by adding isLoading to the dismissed/skip condition or clearing data via setQueryData(undefined) before mutating.
3. `isResultDismissed` is sticky across new sweeps (low)
If the user dismisses an errored/complete alert and the mode later changes (e.g. via cache update from elsewhere), the new alert is suppressed because dismissal is only reset by handleInitiate. Today this is harmless because terminal modes stop polling, but it's a footgun if the hook gets reused. Consider resetting dismissal whenever mode transitions to/from a terminal value via useEffect.
4. Mocked-react-query test couples to internal call shape (low)
useWithdrawLockedFunds.refetch.spec.tsx mocks useMutation/useQuery and asserts on the captured config. It's effective at testing the polling/refetch logic in isolation, but it bypasses the actual react-query lifecycle, so a refactor (e.g. switching to useQueries or composing differently) silently breaks test relevance without breaking the test. Tradeoff is acceptable; consider adding one integration-style assertion (real QueryClient) that the polling stops on terminal mode to backstop it.
5. `fills.length` vs `positions_sold` for partial detection (low)
isPartial = mode === 'errored' && (data?.fills.length ?? 0) > 0. Backend semantics: should "any fill" or "positions_sold > 0" gate the partial UI? If fills is purely an audit log and positions_sold is the source of truth, this could disagree under unusual backend timing. Worth confirming the contract — a one-line comment would help.
6. Minor — alert close icon color inconsistency (cosmetic)
ErrorAlert uses COLOR.SECONDARY for closeIcon while message uses COLOR.RED — inconsistent vs. Done/Partial which match colors. Probably intentional per Figma; flagging for awareness.
Conventions / Style
- ESLint import sort, single quotes, no
any - styled-components for layout, AntD primitives for UI
- Test placement mirrors
src/(depth-3 paths correct) - Uses shared
exponentialBackoffDelayandAPI_V1constants - React Query keys centralized in
reactQueryKeys.ts - Coverage of new code looks complete (every body, mode, dismiss/retry, both enabled gates, mutation error log)
Security / Performance
- No XSS surface — backend
errors[].reasonis not rendered (today). If you later surface error messages, sanitize. retry: Infinityis bounded byexponentialBackoffDelayand the query is disabled until armed, so storm risk is low.- POST has no body — confirm the backend doesn't require CSRF protection for state-changing requests on this endpoint.
Verdict
Approving. The functional work is solid and well-tested. Recommended polish before merge:
- Refresh the PR description to match the actual contract (single endpoint, real mode names).
- Decide whether the brief retry-flash (issue 2) is worth fixing.
- Optionally add the
isPartialsemantics comment (issue 5).
None of the above are blockers.
Summary
isPolystratAgentso Omenstrat continues to render unchanged.POST {LOCAL}/withdrawal/initiate→ id;GET {LOCAL}/withdrawal/status/{id}polled every 2s; halts oncompleted/failed).Implementation notes
useWithdrawLockedFundshook:useMutationfor initiate + polleduseQueryfor status withretry: Infinity+exponentialBackoffDelayso transient network errors don't surface as a hard fail.data.statusand a localisResultDismissedflag.marketNameprop is wired through Header and DoneBody so the same component works for Omenstrat once its backend is ready (just flip the gate inagent.tsxand pass"Omen").messagetext for the in-progress state comes from the backend response and is rendered as-is.Test plan
REACT_APP_AGENT_NAME=polystrat_trader+IS_MOCK_ENABLED=true, runyarn nx dev predict-ui, scroll to bottom of the page.~$120.32and the "Initiate withdrawal" button.mockWithdrawal.ts→status: 'completed'+ a tx link → reload, click Initiate → green "Withdrawal complete!" alert with working tx link, dismiss X falls back to initial body.mockWithdrawal.ts→status: 'failed'+ tx link → reload, click Initiate → red "Withdrawal failed" alert above the initiate body, retry from the button works, dismiss X removes the alert and leaves the initiate body.REACT_APP_AGENT_NAME=omenstrat_trader, no card visible).yarn nx test predict-uipasses (4 new spec files, 27 added tests).yarn nx lint predict-uiclean.yarn nx build predict-uiclean.🤖 Generated with Claude Code