Conversation
…e suite (QA-91) PR #2049 removed the SDK's client-side signer-count pre-check, relying on the backend's enforcement, which exposed that the long-lived smoke wallet had accumulated well over the signer cap (prior device-signer accumulation bug). PR #2056 worked around it by rotating the wallet identity every run, masking the regression instead of covering it. Add two scenarios instead of masking it further: - signer-limit-regression.spec.ts re-authenticates into the original over-the-cap wallet (fixed "e2e" suffix, from before the per-run rotation) and transfers funds, asserting the backend keeps grandfathering it. - signer-limit-cap.spec.ts seeds a brand new wallet with external-wallet signers via a server key until the backend rejects one, asserting that rejection is SIGNER_LIMIT_EXCEEDED, so the cap itself stays enforced. TestConfiguration gains an optional emailOverride so a test can authenticate into a specific wallet instead of the suite's rotating-per-run identity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
🔥 Smoke Test Results❌ Status: Failed Statistics
Test DetailsThis is a non-blocking smoke test. Full regression tests run separately. |
…ation email (QA-91) handleEmailPhoneSignerFlow resolved the OTP email via getEmailForSigner directly, ignoring testConfig.emailOverride, so signer-limit-regression.spec.ts logged into the legacy wallet correctly but then searched for the transaction-confirmation OTP under the rotating per-run email and timed out. Thread emailOverride through handleSignerConfirmation/transferFunds so the confirmation step checks the same inbox as the login. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…QA-91) It only used validateAPITestConfig to check TESTS_CROSSMINT_API_KEY, which this test never reads — it creates its own wallet entirely through the server key. That check tied it to whichever project the client key belongs to for no reason, which matters now that scenario A and scenario B are meant to run against two different projects. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Scenario A's project is staying grandfathered (its cap gets raised in ConfigCat); scenario B now creates its wallet against a separate project so it tests the cap-enforcement mechanism itself without depending on, or contributing load to, the project behind the legacy wallet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
One-off debug step to answer "how many signers does it have right now" without putting any key in chat. Will revert once read. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nt in CI" Debug step used the wrong key type (client key; the GET /:walletLocator route needs a server key) and printed a bogus count. The real number came from the actual production error instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cap-test wallet (QA-91) Greptile review on #2133: - getLegacySmokeWalletEmail used the "e2e" suffix, but smoke-tests.yml pinned TESTS_WALLET_EMAIL_SUFFIX to "smoke" before PR #2056 introduced per-run rotation (see smoke-tests.yml's git history). The regression test was authenticating into a different, unrelated wallet. - signer-limit-cap.spec.ts created a wallet with a Date.now()-based owner every run, leaving an ever-growing set of abandoned staging wallets. owner already guarantees idempotency server-side (getOrCreateWallet), so a fixed owner reuses the same wallet and plateaus at the project's cap instead of growing forever. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…run (QA-91) Confirmed the removal can't be scripted via a plain API/SDK call: both addSigner and removeSigner route through withRecoverySigner, and for an email recovery method the approval is a TEE handshake (ncs-signer.ts encrypts the OTP client-side and posts it to the signer iframe) — there is no REST endpoint to replicate that from Node. Adds a Remove button to the devkit's existing Permissions component (it already lists delegated signers and holds a live wallet instance; it was missing only the remove affordance). The regression test captures the locator of whichever device signer the transfer step silently registers, reloads to force a fresh signer list, and removes that one signer through the same UI + OTP confirmation flow already used elsewhere in the suite. Net effect: this wallet's signer count stays flat across runs instead of growing by one every time, so a modest cap raise stays correct going forward instead of needing to be re-raised periodically. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The legacy-wallet regression test now does a second OTP round-trip to clean up the signer it adds, pushing the suite past the previous 10-minute budget — the last run was killed by the job timeout mid-test rather than by an actual failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…llet (QA-91) Placed under tests/sdk, a Playwright project defined in playwright.config.ts but never invoked by any workflow, so it never runs as part of the regular smoke suite and never re-seeds on a normal CI run. Logs into test-email-cap-fixture@<mailosaurServerId>.mailosaur.net and adds external-wallet delegated signers through the existing Permissions UI (the same one the QA-91 cleanup step uses) until it holds 1 admin + 15 delegated signers. Each add needs its own OTP confirmation (no server-key shortcut — approval for an email recovery method is a TEE handshake, not a scriptable HTTP call), so this is meant to be run once via `pnpm exec playwright test --project=sdk --grep "cap-fixture"`, not as a recurring test. signer-limit-regression.spec.ts will be pointed at this wallet once it is seeded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR #2049 removed the SDK's client-side signer-count pre-check and relied entirely on the backend's enforcement. That exposed that the long-lived smoke-test wallet had silently accumulated well over the signer cap (root cause:
DeviceRecoveryService#resolveStaleDeviceSignerLocatoronly prunes a stale device signer when exactly one candidate is unambiguous — with 2+ it prunes nothing), which brokeWallet Smoke › transfers fundswithSIGNER_LIMIT_EXCEEDED.PR #2056 fixed CI by rotating the wallet identity every run (
TESTS_WALLET_EMAIL_SUFFIX: smoke-${{ github.run_id }}), but that only masked the issue — a fresh wallet per run never reaches the cap, so a real enforcement regression would go unnoticed again.This PR adds the two regression scenarios instead:
signer-limit-regression.spec.tsre-authenticates into the original smoke wallet (the one that regressed, now well past the cap) via a newemailOverrideonTestConfiguration, and transfers funds — asserting the backend keeps grandfathering wallets that were already over the cap before it existed.signer-limit-cap.spec.tscreates a brand-new wallet and addsexternal-walletsigners one at a time (via a server key,@crossmint/wallets-sdk'sWalletsApiClientdirectly) until the backend rejects one — asserting that rejection is specificallySIGNER_LIMIT_EXCEEDED, so the cap itself stays enforced on new wallets.Together they cover both directions of the regression: old wallets must keep working, new wallets must still be capped.
Needs the
TESTS_CROSSMINT_SERVER_API_KEY_SMOKE_TESTSsecret (now set) — a staging server key, since adding a signer after wallet creation needs server-only usage, unlike the suite's existing client key.Linear: QA-91.
Test plan
signer-limit-regression.spec.ts: full Playwright UI run against the existing over-the-cap wallet; passes if the transfer completes and no response ever carriesSIGNER_LIMIT_EXCEEDED.signer-limit-cap.spec.ts: no browser — direct SDK/API calls against staging; passes once aregisterSignercall returnscode: "SIGNER_LIMIT_EXCEEDED"(discovered dynamically, not hardcoded to today's cap value).tsc --noEmit) andbiome checkclean against all changed/added files.node_modulesinstalled (pnpm installneeds a token this environment doesn't have), so there is no local Playwright run backing this.Package updates
None — tests only, no package changes.