Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/create-new-settlemint-project.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
await updatePackageJsonToUseLinkedDependencies(dAppDir);
await updatePackageJsonToUseLinkedDependencies(contractsDir);
await updatePackageJsonToUseLinkedDependencies(subgraphDir);
await $`bun install --no-cache`.cwd(projectDir).env(env);
await $`bun install --no-cache --linker=hoisted`.cwd(projectDir).env(env);
});

test("Connect to platform", async () => {
Expand Down Expand Up @@ -281,7 +281,7 @@
const shellError = err as $.ShellError;
console.log(shellError.stdout.toString());
console.log(shellError.stderr.toString());
throw new Error("Build failed");

Check failure on line 284 in test/create-new-settlemint-project.e2e.test.ts

View workflow job for this annotation

GitHub Actions / E2E

error: Build failed

at <anonymous> (/home/runner/work/sdk/sdk/test/create-new-settlemint-project.e2e.test.ts:284:13)
}
});
});
2 changes: 1 addition & 1 deletion test/create-new-standalone-project.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
await updatePackageJsonToUseLinkedDependencies(dAppDir);
await updatePackageJsonToUseLinkedDependencies(contractsDir);
await updatePackageJsonToUseLinkedDependencies(subgraphDir);
await $`bun install --no-cache`.cwd(projectDir).env(env);
await $`bun install --no-cache --linker=hoisted`.cwd(projectDir).env(env);
});

test("Connect to standalone environment", async () => {
Expand Down Expand Up @@ -300,7 +300,7 @@
const shellError = err as $.ShellError;
console.log(shellError.stdout.toString());
console.log(shellError.stderr.toString());
throw new Error("Build failed");

Check failure on line 303 in test/create-new-standalone-project.e2e.test.ts

View workflow job for this annotation

GitHub Actions / E2E

error: Build failed

at <anonymous> (/home/runner/work/sdk/sdk/test/create-new-standalone-project.e2e.test.ts:303:13)
}
});

Expand Down
19 changes: 18 additions & 1 deletion test/restart-resources.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test";
import { createSettleMintClient } from "@settlemint/sdk-js";
import { loadEnv } from "@settlemint/sdk-utils/environment";
import { fetchWithRetry } from "@settlemint/sdk-utils/http";
import type { DotEnv } from "@settlemint/sdk-utils/validation";
import { NODE_NAME_3_WITHOUT_PK } from "./constants/test-resources";
import { forceExitAllCommands, runCommand } from "./utils/run-command";
import { setupSettleMintClient } from "./utils/test-resources";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The setupSettleMintClient function is imported but never used in this file. Unused imports should be removed to improve code readability and maintainability.

Comment on lines +2 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove unused import and use existing helper instead.

The setupSettleMintClient import on line 8 is unused. Additionally, test/utils/test-resources.ts already exports a findBlockchainNodeByName function (lines 31-36) that uses setupSettleMintClient. Import and use that existing helper instead of redefining it locally.

Apply this diff to remove the unused import:

-import { setupSettleMintClient } from "./utils/test-resources";
+import { findBlockchainNodeByName } from "./utils/test-resources";
📝 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.

Suggested change
import { createSettleMintClient } from "@settlemint/sdk-js";
import { loadEnv } from "@settlemint/sdk-utils/environment";
import { fetchWithRetry } from "@settlemint/sdk-utils/http";
import type { DotEnv } from "@settlemint/sdk-utils/validation";
import { NODE_NAME_3_WITHOUT_PK } from "./constants/test-resources";
import { forceExitAllCommands, runCommand } from "./utils/run-command";
import { setupSettleMintClient } from "./utils/test-resources";
import { createSettleMintClient } from "@settlemint/sdk-js";
import { loadEnv } from "@settlemint/sdk-utils/environment";
import { fetchWithRetry } from "@settlemint/sdk-utils/http";
import type { DotEnv } from "@settlemint/sdk-utils/validation";
import { NODE_NAME_3_WITHOUT_PK } from "./constants/test-resources";
import { forceExitAllCommands, runCommand } from "./utils/run-command";
import { findBlockchainNodeByName } from "./utils/test-resources";
🧰 Tools
🪛 Biome (2.1.2)

[error] 8-8: This import is unused.

Unused imports might be the result of an incomplete refactoring.
Unsafe fix: Remove the unused imports.

(lint/correctness/noUnusedImports)

🤖 Prompt for AI Agents
In test/restart-resources.e2e.test.ts around lines 2 to 8, remove the unused
import setupSettleMintClient and instead import findBlockchainNodeByName from
the existing test/utils/test-resources module; update the test to call
findBlockchainNodeByName where the local helper was being redefined or used so
the code reuses the shared implementation and eliminates the unused import.


const COMMAND_TEST_SCOPE = __filename;

Expand All @@ -14,11 +17,15 @@ afterEach(() => {

describe("Restart platform resources using the SDK", () => {
test("Restart blockchain node on the platform", async () => {
const blockchainNode = await findBlockchainNodeByName(NODE_NAME_3_WITHOUT_PK);
if (!blockchainNode) {
throw new Error(`Blockchain node ${NODE_NAME_3_WITHOUT_PK} not found`);
}
const { output } = await runCommand(COMMAND_TEST_SCOPE, [
"platform",
"restart",
"blockchain-node",
NODE_NAME_3_WITHOUT_PK,
blockchainNode.uniqueName,
"--wait",
"--accept-defaults",
]).result;
Expand All @@ -30,3 +37,13 @@ describe("Restart platform resources using the SDK", () => {
expect(response.status).toBe(401); // Unauthorized as we did not provide a token
});
});

async function findBlockchainNodeByName(blockchainNodeName: string) {
const env: Partial<DotEnv> = await loadEnv(false, false);
const settlemint = createSettleMintClient({
accessToken: env.SETTLEMINT_ACCESS_TOKEN!,
Copy link

@cubic-dev-ai cubic-dev-ai bot Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the E2E-specific access token environment variable for authentication in tests to avoid targeting the wrong environment. Prefer reusing the shared client setup helper or switch this line to the E2E token.

Prompt for AI agents
Address the following comment on test/restart-resources.e2e.test.ts at line 44:

<comment>Use the E2E-specific access token environment variable for authentication in tests to avoid targeting the wrong environment. Prefer reusing the shared client setup helper or switch this line to the E2E token.</comment>

<file context>
@@ -30,3 +37,13 @@ describe(&quot;Restart platform resources using the SDK&quot;, () =&gt; {
+async function findBlockchainNodeByName(blockchainNodeName: string) {
+  const env: Partial&lt;DotEnv&gt; = await loadEnv(false, false);
+  const settlemint = createSettleMintClient({
+    accessToken: env.SETTLEMINT_ACCESS_TOKEN!,
+    instance: env.SETTLEMINT_INSTANCE!,
+  });
</file context>
Fix with Cubic

instance: env.SETTLEMINT_INSTANCE!,
Comment on lines +41 to +45

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use E2E access token when fetching blockchain node

The new helper builds the SDK client with env.SETTLEMINT_ACCESS_TOKEN loaded from .env, even though the test suite relies on SETTLEMINT_ACCESS_TOKEN_E2E_TESTS for authentication (see test/utils/test-resources.ts and other e2e tests). In environments where only the E2E token is provided and no .env file exists with SETTLEMINT_ACCESS_TOKEN, the client will be created with undefined credentials and settlemint.blockchainNode.list will fail with 401, so the test will still break. Reuse setupSettleMintClient() or read process.env.SETTLEMINT_ACCESS_TOKEN_E2E_TESTS to ensure the call is authenticated.

Useful? React with 👍 / 👎.

});
const nodes = await settlemint.blockchainNode.list(env.SETTLEMINT_APPLICATION!);
return nodes.find((node) => node.name === blockchainNodeName);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect Access Token Usage in Tests

Local findBlockchainNodeByName function uses env.SETTLEMINT_ACCESS_TOKEN instead of the E2E-specific process.env.SETTLEMINT_ACCESS_TOKEN_E2E_TESTS that should be used (as defined in the existing utility function at test/utils/test-resources.ts). This will cause the function to use the wrong access token, potentially failing authentication or accessing the wrong environment. The existing utility function should be imported and used instead, as done in other test files like pause-resume-blockchain-node.e2e.test.ts.

Fix in Cursor Fix in Web

Comment on lines +41 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This findBlockchainNodeByName function is a duplicate of an existing utility in test/utils/test-resources.ts. While the implementation is slightly different regarding the access token, duplicating code increases maintenance overhead and can lead to inconsistencies. A better approach would be to refactor the shared utility to be more flexible. For now, please rename this function to avoid name collisions and confusion with the shared utility (e.g., findBlockchainNodeByNameWithDefaultToken). This will make the code's intent clearer.

Comment on lines +41 to +49
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The findBlockchainNodeByName function uses the wrong environment variable for SettleMint client authentication in E2E tests.
Severity: HIGH | Confidence: 0.95

🔍 Detailed Analysis

The findBlockchainNodeByName function initializes the SettleMint client using env.SETTLEMINT_ACCESS_TOKEN! for authentication. This variable is incorrect for E2E tests, as the test runner populates process.env.SETTLEMINT_ACCESS_TOKEN_E2E_TESTS! with the necessary credentials. The loadEnv() function does not specifically populate SETTLEMINT_ACCESS_TOKEN with E2E test credentials, leading to an authentication failure when the client attempts to list blockchain nodes.

💡 Suggested Fix

Modify the findBlockchainNodeByName function to use process.env.SETTLEMINT_ACCESS_TOKEN_E2E_TESTS! for the accessToken when creating the SettleMint client, or reuse the existing setupSettleMintClient() utility.

🤖 Prompt for AI Agent
Fix this bug. In test/restart-resources.e2e.test.ts at lines 41-49: The
`findBlockchainNodeByName` function initializes the SettleMint client using
`env.SETTLEMINT_ACCESS_TOKEN!` for authentication. This variable is incorrect for E2E
tests, as the test runner populates `process.env.SETTLEMINT_ACCESS_TOKEN_E2E_TESTS!`
with the necessary credentials. The `loadEnv()` function does not specifically populate
`SETTLEMINT_ACCESS_TOKEN` with E2E test credentials, leading to an authentication
failure when the client attempts to list blockchain nodes.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +41 to +49
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove duplicate helper - reuse existing implementation.

This function duplicates findBlockchainNodeByName already defined in test/utils/test-resources.ts (lines 31-36). The existing implementation uses setupSettleMintClient(), which is the appropriate test utility wrapper.

Remove this local function definition and import the existing helper as suggested in the earlier comment (lines 2-8).

-
-async function findBlockchainNodeByName(blockchainNodeName: string) {
-  const env: Partial<DotEnv> = await loadEnv(false, false);
-  const settlemint = createSettleMintClient({
-    accessToken: env.SETTLEMINT_ACCESS_TOKEN!,
-    instance: env.SETTLEMINT_INSTANCE!,
-  });
-  const nodes = await settlemint.blockchainNode.list(env.SETTLEMINT_APPLICATION!);
-  return nodes.find((node) => node.name === blockchainNodeName);
-}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In test/restart-resources.e2e.test.ts around lines 41 to 49, remove the local
duplicate findBlockchainNodeByName function and instead import and reuse the
existing helper from test/utils/test-resources.ts (the existing implementation
at lines 31-36 uses setupSettleMintClient()); delete the local function, add the
appropriate import at the top (per the earlier comment around lines 2-8), and
update any calls to use the imported helper so the test uses the shared utility.

Loading