.github/workflows/e2e-api-tests-local-emulator.yaml (4)

113-113: Redundant & operators in background service commands.

Similar to the Docker Compose step, the & at the end of each run command is redundant when using background-action. The action already handles backgrounding the process.

🧹 Suggested cleanup for all background services

-          run: pnpm run start:backend --log-order=stream &
+          run: pnpm run start:backend --log-order=stream

Apply the same pattern to lines 123, 133, 143, and 153.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e-api-tests-local-emulator.yaml at line 113, Remove the
redundant trailing ampersand (&) from backgrounded commands that use the
background-action; specifically update each run step that currently uses the
string "pnpm run start:backend --log-order=stream &" (and the other similar run
commands on the same pattern at the other service steps) to omit the trailing
"&" so the command becomes "pnpm run start:backend --log-order=stream" — apply
the same change for the analogous run entries referenced in the workflow.

---

160-161: Consider documenting the sleep duration.

The 10-second sleep before running tests appears to be a timing workaround. If this is required for services to stabilize after startup, a comment explaining the reason would be helpful.

📝 Suggested documentation

+      # Allow background services to fully initialize before running tests
       - name: Wait 10 seconds
         run: sleep 10

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e-api-tests-local-emulator.yaml around lines 160 - 161,
Add a short inline comment above the "Wait 10 seconds" workflow step explaining
why the 10s sleep is necessary (e.g., to allow the emulator/service to
initialize before tests) and what conditions it is covering, and if applicable
replace or augment the step name or comment to reference a link or note on a
more robust readiness check (so maintainers know this is a timing workaround and
where to find alternatives).

---

166-172: Consider documenting the repeated test runs.

Running tests 3 times on main/dev branches is an unusual pattern. If this is for flakiness detection or stability verification, adding a comment would help future maintainers understand the intent.

📝 Suggested documentation

+      # Run tests multiple times on main/dev to detect flaky tests
       - name: Run tests again (attempt 1)
         if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/dev'
         run: pnpm test run

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e-api-tests-local-emulator.yaml around lines 166 - 172,
Add a short comment above the repeated test steps to document why tests are run
multiple times on main/dev (e.g., flakiness mitigation or stability checks) and
clarify intent; update the step names "Run tests again (attempt 1)" and "Run
tests again (attempt 2)" (or add inline comments) to state the reason (e.g.,
"retry for flaky test detection" or "stability verification") so future
maintainers understand the pattern.

---

46-46: Redundant background operator & in command.

The & at the end of the docker compose command is redundant since background-action already runs the command in the background. The command will work, but the trailing & is unnecessary.

🧹 Suggested cleanup

-          run: docker compose -f docker/dependencies/docker.compose.yaml up --pull always -d &
+          run: docker compose -f docker/dependencies/docker.compose.yaml up --pull always -d

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e-api-tests-local-emulator.yaml at line 46, Remove the
redundant background operator by editing the GitHub Actions step that currently
uses the command string "docker compose -f
docker/dependencies/docker.compose.yaml up --pull always -d &" and drop the
trailing "&"; keep the rest of the run command identical so the
background-action semantics handle detaching without the extra ampersand.

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/emails/page-client.tsx (1)

149-183: Add email configuration UI state to Playground for documentation and testing.

The Playground was updated in this PR but does not showcase the new local-emulator read-only state pattern introduced in the email configuration section. Update the Playground to document this state (read-only copy, conditional buttons, informational alert) to keep design behavior testable and referenceable alongside other dashboard UI patterns.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/emails/page-client.tsx
around lines 149 - 183, Add a Playground variant that demonstrates the new
email-config read-only emulator state by rendering the same header and controls
using the isLocalEmulator flag and emailConfig.isShared check: display the
Typography copy for the emulator ("Email server settings are read-only in the
local emulator") when isLocalEmulator is true, hide the TestSendingDialog
trigger when emailConfig.isShared || isLocalEmulator, hide the
EditEmailServerDialog trigger when isLocalEmulator is true, and render the Alert
with AlertDescription when isLocalEmulator is true so the Playground shows the
read-only copy, conditional buttons, and informational alert; reference the
existing components/props used in the page (isLocalEmulator,
emailConfig.isShared, TestSendingDialog, EditEmailServerDialog, Alert,
AlertDescription, Typography, Button) so the Playground mirrors the real UI
behavior.

apps/dashboard/.env.development (1)

12-12: Consider documenting the reason for disabling the debugger.

The change from true to false for NEXT_PUBLIC_STACK_DEBUGGER_ON_ASSERTION_ERROR appears unrelated to the local emulator feature. If intentional, a comment explaining the rationale would help future maintainers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/.env.development` at line 12, The environment variable
NEXT_PUBLIC_STACK_DEBUGGER_ON_ASSERTION_ERROR was changed from true to false
without explanation; either revert it if that was accidental or add a concise
comment above NEXT_PUBLIC_STACK_DEBUGGER_ON_ASSERTION_ERROR in the
.env.development indicating the reason for disabling the debugger (e.g., to
avoid noisy logs during local emulator runs or to prevent exposing debug UI),
and include any conditions when it should be re-enabled to guide future
maintainers.

apps/dashboard/src/lib/config-update.tsx (1)

272-275: Consider using a toast instead of alert() for better UX.

While alert() works, it blocks the UI and is inconsistent with the modern toast-based feedback used elsewhere in the dashboard. A toast would provide a non-blocking notification.

💡 Suggested improvement using toast

+import { toast } from "@/components/ui/use-toast";
+
 // In the callback:
 if (isLocalEmulator) {
-  alert("These settings are read-only in the local emulator. Update them in your production deployment instead.");
+  toast({
+    title: "Read-only in local emulator",
+    description: "Update these settings in your production deployment instead.",
+    variant: "destructive",
+  });
   return false;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/lib/config-update.tsx` around lines 272 - 275, Replace the
blocking alert() call in the isLocalEmulator branch with the dashboard's
non-blocking toast notification so UX stays consistent: instead of alert("These
settings are read-only..."), call the existing toast/notification utility used
elsewhere in the app (e.g., toast or notify) with the same message and an
appropriate level (info/warning), keep the return false behavior, and add/import
the toast helper if not already present so the change is local to the
isLocalEmulator conditional.

apps/backend/src/lib/local-emulator.ts (1)

6-7: Consider adding a comment explaining the intentional hardcoded password.

LOCAL_EMULATOR_ADMIN_PASSWORD is hardcoded, which is intentional for local development. A brief comment would help future maintainers understand this is deliberate and not a security oversight.

📝 Suggested documentation

 export const LOCAL_EMULATOR_ADMIN_EMAIL = "local-emulator@stack-auth.com";
+// Intentionally hardcoded for local development only. This password is used
+// to seed the local emulator admin user and is not used in production.
 export const LOCAL_EMULATOR_ADMIN_PASSWORD = "LocalEmulatorPassword";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/local-emulator.ts` around lines 6 - 7, Add a brief
inline comment above LOCAL_EMULATOR_ADMIN_PASSWORD (and optionally above
LOCAL_EMULATOR_ADMIN_EMAIL) stating that the hardcoded password is intentional
for local development/emulation only, not used in production, and include
guidance to change or override it via environment variables or secrets for any
shared/dev environments; reference the constants LOCAL_EMULATOR_ADMIN_PASSWORD
and LOCAL_EMULATOR_ADMIN_EMAIL so maintainers understand the intent and where to
look.

apps/e2e/tests/backend/endpoints/api/v1/internal/config-local-emulator.test.ts (1)

21-26: Same partial projectKeys pattern as in local-emulator-project.test.ts.

This sets only projectId and superSecretAdminKey. Consider creating a shared helper that consistently sets all required keys from the provisioned response, including secretServerKey which is also returned by the endpoint.

♻️ More complete projectKeys

   backendContext.set({
     projectKeys: {
       projectId: response.body.project_id,
+      secretServerKey: response.body.secret_server_key,
       superSecretAdminKey: response.body.super_secret_admin_key,
     },
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/e2e/tests/backend/endpoints/api/v1/internal/config-local-emulator.test.ts`
around lines 21 - 26, The test is only setting partial projectKeys via
backendContext.set({ projectKeys: { projectId, superSecretAdminKey } }) and
misses other keys (e.g., secretServerKey) returned by the provision endpoint;
add a shared helper (e.g., setProjectKeysFromResponse(response) or
populateProjectKeys) that extracts all required keys from the provision response
(project_id, super_secret_admin_key, secret_server_key, etc.) and calls
backendContext.set({ projectKeys: { ... } }), then replace the inline object in
this test and local-emulator-project.test.ts with that helper to ensure
consistency.

apps/e2e/tests/backend/endpoints/api/v1/internal/local-emulator-project.test.ts (1)

7-7: Consider importing LOCAL_EMULATOR_OWNER_TEAM_ID from the source module.

This constant is duplicated from apps/backend/src/lib/local-emulator.ts. If the value changes in the source, this test will silently use a stale value and may pass incorrectly or fail misleadingly.

♻️ Suggested import

-const LOCAL_EMULATOR_OWNER_TEAM_ID = "5a0c858b-d9e9-49d4-9943-8ce385d86428";
+import { LOCAL_EMULATOR_OWNER_TEAM_ID } from "@/lib/local-emulator";

Note: You may need to adjust the import path based on how the E2E test environment resolves backend modules.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/e2e/tests/backend/endpoints/api/v1/internal/local-emulator-project.test.ts`
at line 7, Replace the duplicated hardcoded LOCAL_EMULATOR_OWNER_TEAM_ID in the
test with a single import of that constant from the source module that defines
it (the local-emulator module) so the test always uses the canonical value;
remove the const declaration in the test, add an import for
LOCAL_EMULATOR_OWNER_TEAM_ID, and if necessary adjust the test's module
resolution/import path so the test can import from the local-emulator module.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

apps/dashboard/src/lib/config-update.tsx (1)

266-285: Consider extracting the local emulator check to a shared constant or utility.

The pattern getPublicEnvVar("NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR") === "true" is duplicated on lines 47 and 266. Extracting it to a module-level constant or a dedicated hook (e.g., useIsLocalEmulator) would improve maintainability and reduce the risk of inconsistency.

♻️ Example extraction

+const IS_LOCAL_EMULATOR = getPublicEnvVar("NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR") === "true";
+
 export function ConfigUpdateDialogProvider({ children }: { children: React.ReactNode }) {
   // ...
   const showPushableDialog = useCallback(async (...) => {
     // ...
-    const isLocalEmulator = getPublicEnvVar("NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR") === "true";
+    const isLocalEmulator = IS_LOCAL_EMULATOR;
     // ...
   }, []);
   // ...
 }

 export function useUpdateConfig() {
   const { showPushableDialog } = useConfigUpdateDialog();
-  const isLocalEmulator = getPublicEnvVar("NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR") === "true";
+  const isLocalEmulator = IS_LOCAL_EMULATOR;
   // ...
-  }, [isLocalEmulator, showPushableDialog]);
+  }, [showPushableDialog]); // isLocalEmulator is now a stable module-level constant
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/lib/config-update.tsx` around lines 266 - 285, Extract the
duplicated environment check
getPublicEnvVar("NEXT_PUBLIC_STACK_IS_LOCAL_EMULATOR") === "true" into a shared
utility (either a module-level constant or a hook like useIsLocalEmulator) and
replace the inline checks in this file (the isLocalEmulator local variable
inside the useCallback) and the other occurrence referenced in the review with
an import/use of that shared symbol; update this file to call the new utility
(e.g., const isLocalEmulator = useIsLocalEmulator() or import {
IS_LOCAL_EMULATOR } from ...) and remove the direct getPublicEnvVar call,
keeping existing behavior in the useCallback that shows an alert and prevents
updates when true, and ensure showPushableDialog and project.updateConfig paths
remain unchanged.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/page-client.tsx (1)

177-187: Wrap async handler in runAsynchronouslyWithAlert for proper error handling.

While the Button component handles loading states, errors thrown in handleOpenConfigFile (lines 55, 62, 69, 89, 98, 100, 109) should be shown to users via alerts rather than potentially being swallowed. The coding guidelines specify using runAsynchronouslyWithAlert for error handling in async handlers.

♻️ Proposed fix

+import { runAsynchronouslyWithAlert } from "@stackframe/stack-shared/dist/utils/promises";

          <Button
            onClick={async () => {
              if (isLocalEmulator) {
                setOpenConfigFileDialog(true);
                return;
              }
              router.push("/new-project");
              return await wait(2000);
            }}
          >{isLocalEmulator ? "Open config file" : "Create Project"}
          </Button>

And for line 219:

-          <Button onClick={handleOpenConfigFile} loading={openingConfigFile}>
+          <Button onClick={() => runAsynchronouslyWithAlert(handleOpenConfigFile())} loading={openingConfigFile}>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/(outside-dashboard)/projects/page-client.tsx
around lines 177 - 187, The inline async onClick handler for the Button should
be wrapped with runAsynchronouslyWithAlert to surface any thrown errors to the
user; replace the current async () => { ... } with () =>
runAsynchronouslyWithAlert(async () => { if (isLocalEmulator) {
setOpenConfigFileDialog(true); return; } router.push("/new-project"); await
wait(2000); }); so the same logic (isLocalEmulator check,
setOpenConfigFileDialog, router.push, wait) runs inside
runAsynchronouslyWithAlert and errors from this handler (and related functions
like handleOpenConfigFile) are shown via alerts.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

apps/backend/src/lib/config.tsx (1)

1079-1108: Consider adding test coverage for the allowing cases.

The test verifies that writes are blocked when all guard conditions are met, which is good. However, it would strengthen confidence to also test that writes are allowed when:

1. STACK_SEED_MODE is "true" (seeding should bypass the guard)
2. The project is not a local emulator project
3. Local emulator mode is disabled

This would ensure the guard doesn't accidentally block legitimate writes.

💡 Example additional test case for seed mode bypass

import.meta.vitest?.test('setEnvironmentConfigOverride allows writes in seed mode', async ({ expect }) => {
  const vi = import.meta.vitest?.vi;
  if (!vi) {
    throw new StackAssertionError("Vitest context is required for in-source tests.");
  }

  const envUtils = await import("@stackframe/stack-shared/dist/utils/env");
  const localEmulator = await import("./local-emulator");

  const getEnvVariableSpy = vi.spyOn(envUtils, "getEnvVariable").mockImplementation((name: string, defaultValue?: string) => {
    if (name === "STACK_SEED_MODE") {
      return "true"; // Seed mode enabled
    }
    return defaultValue ?? "test-value";
  });
  const isLocalEmulatorEnabledSpy = vi.spyOn(localEmulator, "isLocalEmulatorEnabled").mockReturnValue(true);
  const isLocalEmulatorProjectSpy = vi.spyOn(localEmulator, "isLocalEmulatorProject").mockResolvedValue(true);

  try {
    // Should not throw - mock the DB call if needed or use a different assertion
    // This is a conceptual example; actual implementation depends on DB mocking strategy
  } finally {
    isLocalEmulatorProjectSpy.mockRestore();
    isLocalEmulatorEnabledSpy.mockRestore();
    getEnvVariableSpy.mockRestore();
  }
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/config.tsx` around lines 1079 - 1108, Add positive test
coverage confirming setEnvironmentConfigOverride does not throw when the
local-emulator guard should be bypassed: create three new tests that mock
env.utils.getEnvVariable and ./local-emulator helpers and assert success (or
non-throw) for (1) STACK_SEED_MODE === "true", (2) isLocalEmulatorProject
returns false, and (3) isLocalEmulatorEnabled returns false; restore mocks in
finally blocks and reuse the same LOCAL_EMULATOR_ENV_CONFIG_BLOCKED_MESSAGE and
setEnvironmentConfigOverride symbol names so readers can locate the original
failing test and the guard logic.

Configuration used: defaults

Review profile: CHILL

Plan: Pro