Skip to content

feat: Steward wallet migration Phase 1 — dual provider routing#415

Open
0xSolace wants to merge 8 commits intodevfrom
feat/steward-wallet-migration
Open

feat: Steward wallet migration Phase 1 — dual provider routing#415
0xSolace wants to merge 8 commits intodevfrom
feat/steward-wallet-migration

Conversation

@0xSolace
Copy link
Copy Markdown
Collaborator

Summary

Implements Phase 1 of the Privy → Steward wallet migration:

Changes

  • WalletProvider abstraction — pluggable wallet backend interface (packages/lib/services/wallet-provider.ts)
  • Steward client integration — SDK + lightweight fetch helpers (packages/lib/services/steward-client.ts)
  • Dual-provider routing in server-wallets.ts — new wallets go to Steward when USE_STEWARD_FOR_NEW_WALLETS=true, existing Privy wallets unchanged
  • Feature flagswallet-provider-flags.ts with conservative defaults (Privy remains default)
  • Agent APIs — Steward wallet info exposed in agent detail and compat endpoints
  • DB migration 0058 — adds wallet_provider, steward_agent_id, steward_tenant_id columns to agent_server_wallets with CHECK constraint

Testing

  • E2E tested on VPS: new agent provisioned → Steward wallet created (EVM + Solana) → JWT token minted → container launched with STEWARD_* env vars
  • Migration is backwards-compatible: existing Privy wallets continue working unchanged
  • Feature flag controlled: disabled by default, opt-in via env var

Dependencies

  • @stwd/sdk@0.3.0 (published, zero deps)
  • Steward API deployed at api.steward.fi (v0.2.0)

…uting

- Add WalletProvider interface (packages/lib/services/wallet-provider.ts)
- Add StewardClient singleton wrapper (packages/lib/services/steward-client.ts)
- Add wallet migration feature flags (packages/lib/config/wallet-provider-flags.ts)
- Update agent_server_wallets schema: wallet_provider, steward_agent_id, steward_tenant_id columns
- Make privy_wallet_id nullable for Steward-only wallets
- Add DB migration 0058 with CHECK constraint ensuring exactly one provider ID
- Update server-wallets.ts: dual-provider routing for provisionServerWallet() and executeServerWalletRpc()
- New wallets routed to Steward when USE_STEWARD_FOR_NEW_WALLETS=true
- Existing Privy wallets continue working unchanged
- Install @stwd/sdk@0.3.0

Rollback: set USE_STEWARD_FOR_NEW_WALLETS=false to revert to Privy for all new wallets.
- Add steward-client.ts: lightweight client for querying Steward agent/wallet info
- Add GET /api/v1/milady/agents/[agentId]/wallet endpoint for detailed wallet info
- Update agent detail API to include walletAddress, walletProvider, walletStatus
- Update compat API (toCompatAgent) to include wallet_address, wallet_provider
- Update admin docker-containers route to show wallet provider and address
- Update managed env to inject STEWARD_API_URL and STEWARD_AGENT_ID
- Pass sandboxId through to prepareManagedMiladyEnvironment

Docker-backed agents query Steward for wallet data; Privy agents fall back to DB.
All Steward calls are best-effort with timeouts — API degrades gracefully.
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
eliza-cloud-v2 Ready Ready Preview, Comment Mar 27, 2026 9:56pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c134c1d1-c074-41cd-ad7d-493994ee9a5b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/steward-wallet-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude bot commented Mar 27, 2026

Code Review — PR #415: Steward Wallet Migration Phase 1

Overall the dual-provider architecture is solid and the DB migration is well-structured (additive, zero-downtime, good CHECK constraint). A few issues worth addressing before merge:


🐛 Critical: Wrong agent ID passed to Steward API

In multiple API routes, getStewardAgent(agentId) is called with the sandbox UUID, but Steward agents are registered under the name cloud-${characterId} (stored in agent_server_wallets.steward_agent_id). This means the REST call hits /agents/<sandbox-uuid> which will 404 every time.

Affected files:

  • app/api/compat/agents/[id]/route.ts — calls getStewardAgent(agentId) (sandbox UUID)
  • app/api/v1/admin/docker-containers/route.ts — calls getStewardAgent(c.id) (sandbox UUID)
  • app/api/v1/milady/agents/[agentId]/route.ts — calls getStewardAgent(agentId) (sandbox UUID)

Fix: look up agent_server_wallets first to get steward_agent_id, then call getStewardAgent(record.steward_agent_id). The /wallet route (wallet/route.ts) has the same issue when it calls getStewardWalletInfo(agentId).


⚠️ N+1 Steward API calls in admin containers endpoint

app/api/v1/admin/docker-containers/route.ts fires a getStewardAgent() HTTP call for every container in Promise.all. With 50 containers that's 50 concurrent Steward HTTP calls per page load, no caching, no batching. Steward likely doesn't have a batch-agents endpoint yet, but at minimum the results should be cached (wallet address is stable once provisioned).


⚠️ wallet_provider defaulted to "steward" without confirmation

In compat-envelope.ts:

wallet_provider: walletInfo?.provider ?? (sandbox.node_id ? "steward" : null),

If walletInfo is not passed (i.e. the list endpoint compat/agents/route.ts), any agent with a node_id gets wallet_provider: "steward" even if it was provisioned on Privy before the flag was enabled. Clients receiving this field may make incorrect routing decisions. This should default to null and only be set from a confirmed DB/Steward lookup.


⚠️ No caching for Steward reads

getStewardAgent / getStewardWalletInfo make fresh HTTP calls on every request. Wallet addresses are effectively immutable once provisioned. Consider using the existing cache client (already imported in server-wallets.ts) with a reasonable TTL (e.g. 5 min) to avoid hammering Steward on busy endpoints.


🔍 Migration journal gap

packages/db/migrations/meta/_journal.json jumps from idx 52 (0053_add_milady_billing_columns) to idx 53 (0056_add_billing_status_check), skipping migrations 0054 and 0055. If those migrations exist in other branches/PRs, this journal will conflict when merged. Please verify the journal ordering is correct relative to main.


Minor

  • packages/lib/config/feature-flags.ts has a double blank line added before the new comment — cosmetic but easy to clean up.
  • wallet_provider column uses plain text() in the Drizzle schema with no enum enforcement at the ORM layer (only the DB CHECK constraint). Consider text("wallet_provider", { enum: ["privy", "steward"] }) for type safety.
  • isStewardAvailable() is exported but never imported anywhere in this PR — could be internal-only or deferred until it's actually used.

The check-types-split.ts script was scanning 'lib/', 'db/', and 'components/'
— none of which exist at the project root. The actual source lives at
packages/lib, packages/db, and packages/ui/src/components.

This meant packages/lib (including feature-gate.ts, use-feature-flags.ts,
and feature-flags.ts) was never actually type-checked in CI. The split
type-checker silently skipped all those files.

Fix: update getDirectoriesToCheck() to use the correct package paths.
…ess fallback

- CloudFormation: default-deny direct container port; opt-in via DirectContainerPortCidr param
- milady-web-ui: getClientSafeMiladyAgentWebUiUrl returns null instead of falling back to headscale direct access
- Dashboard pages: remove webUiUrl gating on connect button (always use pairing flow for running agents)
- agent-actions/sandboxes-table: drop getConnectUrl and webUiUrl prop threading
- Add cloudformation-template unit test
- Gitignore env backup files
@claude
Copy link
Copy Markdown

claude bot commented Mar 27, 2026

Code Review — PR #415: Steward wallet migration Phase 1

Overall, this is a well-structured dual-provider abstraction with good backwards-compatibility. The feature flag defaults are conservative, the DB migration is additive and zero-downtime safe, and the test coverage is solid. A few things to address before merging:


Bugs / Correctness

1. Incorrect wallet_provider inference in compat-envelope.ts (line ~855)

wallet_provider: walletInfo?.provider ?? (sandbox.node_id ? "steward" : null),

This fallback assumes all Docker agents use Steward — but during the migration period, Docker agents with existing Privy wallets will have node_id set and no walletInfo passed, and will incorrectly report as "steward". The list endpoint (/compat/agents) never resolves walletInfo at all, so every Docker agent in that list will be reported as Steward regardless of their actual provider. The fallback should be null (unknown), not "steward".

2. Copy icon imported but unused in milady/agents/[id]/page.tsx

import { ..., Copy, ... } from "lucide-react";

Copy is imported but there's no copy-to-clipboard implementation in the rendered WalletSection. Either wire it up or drop the import.

3. Comment refers to STEWARD_AGENT_TOKEN set in docker-sandbox-provider.ts but that file has no changes in this PR

// STEWARD_AGENT_TOKEN is set during provisioning in docker-sandbox-provider.ts.

If this is deferred to a follow-up PR, it's worth an explicit TODO comment. Otherwise containers will launch without the token and Steward auth will fail silently.


Performance

4. N parallel Steward HTTP calls in admin /docker-containers route

const enrichedContainers = await Promise.all(
  containers.map(async (c) => {
    const stewardAgent = await getStewardAgent(c.id);
    ...
  }),
);

With the default page size this fires up to 100 concurrent outbound HTTP requests per admin API call. This will be slow under load and could easily saturate or rate-limit the Steward API. Consider:

  • Batching with a concurrency limit (e.g. p-limit)
  • Or making wallet enrichment lazy/on-demand for the admin table (it's admin-only so lower priority to optimize, but still worth noting)

Code Smell

5. Unnecessary isDockerBacked_early intermediate variable in milady/agents/[id]/page.tsx

const isDockerBacked_early = !!agent.node_id;
const walletInfo = isDockerBacked_early ? await getStewardWalletInfo(...) : null;
const isDockerBacked = isDockerBacked_early;

Just use const isDockerBacked = !!agent.node_id; once and reference it in the walletInfo expression.

6. Hard-coded Basescan link ignores non-Base chains

href={`https://basescan.org/address/${walletInfo.walletAddress}`}

The normalizeChain function handles other EVM chain IDs and Solana, but the explorer link always points to Basescan. If walletInfo.chain isn't eip155:8453 or Base, the link will be wrong. Guard it behind chain === "Base" || !chain or derive the explorer URL from the chain.


Minor

7. DB query directly in route handler (app/api/v1/milady/agents/[agentId]/route.ts):

const walletRecord = await db.query.agentServerWallets.findFirst(...)

Most DB access in this project goes through repository classes (db/repositories/). The same pattern exists in the new /wallet sub-route. Not blocking, but worth a follow-up to keep the pattern consistent.

8. Migration journal gap_journal.json adds entries for 0056 and 0057 which are not in this PR's diff. If those migration files don't exist on the branch this will break db:migrate. Are they on this branch or a dependency?


Positives worth noting

  • Feature flag defaults are correctly conservative (Privy stays default)
  • The stewardFetch helper with AbortSignal.timeout(10_000) is good defensive programming
  • DB migration correctly uses IF NOT EXISTS, is wrapped in a transaction, and adds partial indexes
  • Test coverage for provisioning routing, schema validation, and RPC routing is thorough
  • CloudFormation change to opt-in direct port exposure is a security improvement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant