Skip to content

feat(cli): inject function slug into served fns#6345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raulb wants to merge 5 commits into
base:develop
Choose a base branch
from
Open

feat(cli): inject function slug into served fns #6345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2989c69
feat(cli): inject function slug into served fns
raulb Aug 26, 2026
954c972
refactor: address PR feedback
raulb Aug 27, 2026
c24a039
Merge branch 'develop' into raulb/inject-function-slug-env-var
raulb Aug 27, 2026
cbc9aec
refactor: undo
raulb Aug 27, 2026
abc0a94
fix: address PR feedback
raulb Aug 28, 2026
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
91 changes: 71 additions & 20 deletions apps/cli/src/shared/functions/serve.main.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -122,6 +122,29 @@ const functionsConfig: Record<string, FunctionConfig> = (() => {
}
})();

// EdgeRuntime.userWorkers.create() pools workers by servicePath alone, so two configured
// function names whose entrypoints live in the same directory would share one cached
// worker's environment (and thus its SUPABASE_FUNCTION_SLUG). For those shared directories
// only, serialize worker creation per servicePath and force a fresh worker whenever the
// requested function differs from whichever one currently owns the cached worker. Module
// resolution keeps using each function's real entrypoint path, so imports that reach
// outside the directory (e.g. a sibling `_shared` folder) are unaffected.
const sharedServicePaths = (() => {
const counts = new Map<string, number>();
for (const config of Object.values(functionsConfig)) {
const servicePath = dirname(config.entrypointPath);
counts.set(servicePath, (counts.get(servicePath) ?? 0) + 1);
}
return new Set(
Array.from(counts)
.filter(([, count]) => count > 1)
.map(([servicePath]) => servicePath),
);
})();

const sharedWorkerOwners = new Map<string, string>();
const sharedWorkerQueues = new Map<string, Promise<void>>();

/* --- JWT verification --- */
export function extractBearerToken(rawToken: string) {
const tokenParts = rawToken.split(" ");
Expand DownExpand Up@@ -331,6 +354,8 @@ Deno.serve({
([name, _]) => !name.startsWith("SUPABASE_"),
),
),
// Listed after the spreads so neither the container env nor function config can shadow it
SUPABASE_FUNCTION_SLUG: functionName,
Comment thread
raulb marked this conversation as resolved.
Comment thread
raulb marked this conversation as resolved.
};
if (SUPABASE_PUBLISHABLE_KEY) {
envVarsObj["SUPABASE_PUBLISHABLE_KEYS"] = JSON.stringify({
Expand All@@ -347,7 +372,6 @@ Deno.serve({
([name, _]) => !EXCLUDED_ENVS.includes(name) && !name.startsWith("SUPABASE_INTERNAL_"),
);

const forceCreate = false;
const customModuleRoot = ""; // empty string to allow any local path
const cpuTimeSoftLimitMs = 1000;
const cpuTimeHardLimitMs = 2000;
Expand All@@ -364,26 +388,53 @@ Deno.serve({

const staticPatterns = functionsConfig[functionName].staticFiles;

try {
const worker = await EdgeRuntime.userWorkers.create({
servicePath,
memoryLimitMb,
workerTimeoutMs,
noModuleCache,
noNpm: !usePackageJson,
importMapPath: functionsConfig[functionName].importMapPath,
envVars,
forceCreate,
customModuleRoot,
cpuTimeSoftLimitMs,
cpuTimeHardLimitMs,
decoratorType,
maybeEntrypoint,
context: {
useReadSyncFileAPI: true,
},
staticPatterns,
// Only shared directories pay for coordination: serialize worker creation for this
// servicePath so a concurrent request for a different function can't race the pool's
// "which slug currently owns this worker" decision, then force a fresh worker only
// when the owner actually changed.
let forceCreate = false;
let releaseSharedWorkerSlot: (() => void) | undefined;
if (sharedServicePaths.has(servicePath)) {
const previousSlot = sharedWorkerQueues.get(servicePath) ?? Promise.resolve();
let releaseSlot: () => void;
const currentSlot = new Promise<void>((resolve) => {
releaseSlot = resolve;
});
sharedWorkerQueues.set(
servicePath,
previousSlot.then(() => currentSlot),
);
await previousSlot;
releaseSharedWorkerSlot = releaseSlot!;
forceCreate = sharedWorkerOwners.get(servicePath) !== functionName;

@jgouxjgouxAug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] The owner map does not identify the worker Edge Runtime will reuse

forceCreate: true only skips the active-worker lookup; it does not retire workers already registered for this servicePath. After A creates worker A and B force-creates worker B, both remain in the same registry. A later B request reaches this line with forceCreate: false, and Edge Runtime can round-robin back to worker A, exposing A’s code/state/environment and SUPABASE_FUNCTION_SLUG under B. See maybe_active_worker, add_user_worker, and the registry’s round-robin selection. Alternating A/B requests also keep forcing new workers, so accumulation remains reachable. A last-owner map cannot provide a stable (servicePath, functionName) cache identity; that identity must exist in the worker pool/caller-owned worker cache, or shared paths need to be rejected until it does.

sharedWorkerOwners.set(servicePath, functionName);

@jgouxjgouxAug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Do not record ownership before worker creation succeeds

If B’s forced userWorkers.create() fails, this map still records B while the only reusable worker may belong to A. The next B request therefore computes forceCreate: false and can reuse A’s worker. Ownership must only change after successful creation while coordination is still held. This fixes the failure-state corruption, although it does not address the separate P1 that multiple same-path workers remain registered and can be selected later.

}

try {
let worker;
try {
worker = await EdgeRuntime.userWorkers.create({
servicePath,
memoryLimitMb,
workerTimeoutMs,
noModuleCache,
noNpm: !usePackageJson,
importMapPath: functionsConfig[functionName].importMapPath,
envVars,
forceCreate,
customModuleRoot,
cpuTimeSoftLimitMs,
cpuTimeHardLimitMs,
decoratorType,
maybeEntrypoint,
context: {
useReadSyncFileAPI: true,
},
staticPatterns,
});
} finally {
releaseSharedWorkerSlot?.();
Comment on lines +435 to +436

@chatgpt-codex-connectorchatgpt-codex-connectorBotAug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Hold the worker slot until the request completes

When requests for two function names sharing a servicePath overlap, this releases the slot immediately after worker creation, before worker.fetch() completes. The second request can therefore acquire the slot and force-replace the sole cached worker while the first request is still executing, potentially cancelling the first request or producing a worker error; the duplicate flow in packages/stack/src/services/edge-runtime-main.ts has the same gap. Keep the critical section through request completion or give each slug an independent worker identity. The fresh evidence beyond the earlier creation-race comment is this new finally explicitly releasing the slot before the fetch.

AGENTS.md reference: AGENTS.md:L142-L148

Useful? React with 👍 / 👎.

}

const userReq = prepareUserRequest(req);
return await worker.fetch(userReq);
Expand Down
45 changes: 44 additions & 1 deletion packages/stack/src/functions.unit.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,7 +19,7 @@ import {
resolveFunctionsRuntimeConfig,
type ResolvedFunctionsBundle,
} from "./functions.ts";
import { verifyRequest } from "./services/edge-runtime-main.ts";
import { buildFunctionEnv, verifyRequest } from "./services/edge-runtime-main.ts";

const testPorts: PortSet = {
apiPort: 40_000,
Expand DownExpand Up@@ -303,6 +303,49 @@ describe("stack Functions runtime config", () => {
});
});

describe("stack Functions runtime env", () => {
const config = {
env: { SHARED: "shared-value" },
supabaseUrl: "http://api-gw:8000",
publishableKey: "publishable-key",
secretKey: "secret-key",
dbUrl: "postgresql://db",
};

it("injects the resolved function name as SUPABASE_FUNCTION_SLUG", () => {
const env = buildFunctionEnv(config, { env: {} }, "notes-mcp");

expect(env.SUPABASE_FUNCTION_SLUG).toBe("notes-mcp");
});

it("keeps the slug per-function across calls", () => {
expect(buildFunctionEnv(config, { env: {} }, "notes-mcp").SUPABASE_FUNCTION_SLUG).toBe(
"notes-mcp",
);
expect(buildFunctionEnv(config, { env: {} }, "echo-headers").SUPABASE_FUNCTION_SLUG).toBe(
"echo-headers",
);
});

it("does not let container or function env shadow the slug", () => {
const env = buildFunctionEnv(
{ ...config, env: { ...config.env, SUPABASE_FUNCTION_SLUG: "container-spoof" } },
{ env: { SUPABASE_FUNCTION_SLUG: "function-spoof" } },
"notes-mcp",
);

expect(env.SUPABASE_FUNCTION_SLUG).toBe("notes-mcp");
});

it("still passes through project env and Supabase connection vars", () => {
const env = buildFunctionEnv(config, { env: { FUNCTION_ONLY: "function-value" } }, "notes-mcp");

expect(env.SHARED).toBe("shared-value");
expect(env.FUNCTION_ONLY).toBe("function-value");
expect(env.SUPABASE_URL).toBe("http://api-gw:8000");
});
});

describe("stack Functions runtime auth", () => {
for (const { name, authorization, code, message } of authFailureCases) {
it(name, async () => {
Expand Down
103 changes: 80 additions & 23 deletions packages/stack/src/services/edge-runtime-main.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -171,11 +171,8 @@
return new URL(`file://${path}`).href;
}

async function serveFunction(req: Request, config: any, functionName: string, functionConfig: any) {
const authError = await verifyRequest(req, config, functionConfig);
if (authError) return authError;

const envVars = Object.entries({
export function buildFunctionEnv(config: any, functionConfig: any, functionName: string) {
return {
...config.env,
...functionConfig.env,
SUPABASE_URL: config.supabaseUrl,
Expand All@@ -184,26 +181,86 @@
SUPABASE_DB_URL: config.dbUrl,
SUPABASE_PUBLISHABLE_KEYS: JSON.stringify({ default: config.publishableKey }),
SUPABASE_SECRET_KEYS: JSON.stringify({ default: config.secretKey }),
});
SUPABASE_FUNCTION_SLUG: functionName,
};
}

try {
const worker = await EdgeRuntime.userWorkers.create({
servicePath: dirname(functionConfig.entrypointPath),
memoryLimitMb: 256,
workerTimeoutMs: 400000,
noModuleCache: false,
noNpm: false,
importMapPath: functionConfig.importMapPath ?? undefined,
envVars,
forceCreate: false,
customModuleRoot: "",
cpuTimeSoftLimitMs: 1000,
cpuTimeHardLimitMs: 2000,
decoratorType: "tc39",
maybeEntrypoint: fileUrl(functionConfig.entrypointPath),
context: { useReadSyncFileAPI: true },
staticPatterns: functionConfig.staticFiles,
// EdgeRuntime.userWorkers.create() pools workers by servicePath alone, so two configured
// functions whose entrypoints live in the same directory would share one cached worker's
// environment (and thus its SUPABASE_FUNCTION_SLUG). For those shared directories only,
// serialize worker creation per servicePath and force a fresh worker whenever the
// requested function differs from whichever one currently owns the cached worker. Module
// resolution keeps using each function's real entrypoint path, so imports that reach
// outside the directory (e.g. a sibling `_shared` folder) are unaffected.
function computeSharedServicePaths(functions: Record<string, { entrypointPath: string }>) {
const counts = new Map<string, number>();
for (const functionConfig of Object.values(functions)) {
const servicePath = dirname(functionConfig.entrypointPath);
counts.set(servicePath, (counts.get(servicePath) ?? 0) + 1);
}
return new Set(
Array.from(counts)
.filter(([, count]) => count > 1)
.map(([servicePath]) => servicePath),
);
}

const sharedWorkerOwners = new Map<string, string>();
const sharedWorkerQueues = new Map<string, Promise<void>>();

async function serveFunction(req: Request, config: any, functionName: string, functionConfig: any) {
const authError = await verifyRequest(req, config, functionConfig);
if (authError) return authError;

const envVars = Object.entries(buildFunctionEnv(config, functionConfig, functionName));
const servicePath = dirname(functionConfig.entrypointPath);
const sharedServicePaths = computeSharedServicePaths(config.functions ?? {});

// Only shared directories pay for coordination: serialize worker creation for this
// servicePath so a concurrent request for a different function can't race the pool's
// "which slug currently owns this worker" decision, then force a fresh worker only
// when the owner actually changed.
let forceCreate = false;
let releaseSharedWorkerSlot: (() => void) | undefined;
if (sharedServicePaths.has(servicePath)) {
const previousSlot = sharedWorkerQueues.get(servicePath) ?? Promise.resolve();
let releaseSlot: () => void;
const currentSlot = new Promise<void>((resolve) => {

@jgouxjgouxAug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: this manual Promise gate currently fails the required quality check

@supabase/root#lint:effect:check rejects this new Promise(...) construction, which is why the PR’s Check code quality job is red. The stack runtime is Effect-scoped and its coordination must use the applicable Effect primitive rather than a Promise gate. Given the worker-identity P1 above, the coordination design should be corrected first instead of mechanically wrapping this gate just to satisfy lint.

releaseSlot = resolve;
});
sharedWorkerQueues.set(
servicePath,

Check warning on line 232 in packages/stack/src/services/edge-runtime-main.ts

View workflow job for this annotation

GitHub Actions/ Check code quality

effecttsgo(new-promise) 

packages/stack/src/services/edge-runtime-main.ts:230:25: This code constructs `new Promise(...)`, prefer Effect APIs such as `Effect.async`, `Effect.promise`, or `Effect.tryPromise` instead of manual Promise construction.
previousSlot.then(() => currentSlot),
);
await previousSlot;
releaseSharedWorkerSlot = releaseSlot!;
forceCreate = sharedWorkerOwners.get(servicePath) !== functionName;

@jgouxjgouxAug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Apply the stable worker-identity fix in the stack path too

This owner comparison duplicates the CLI serve-path bug: after multiple forced creations, Edge Runtime retains multiple active workers for this servicePath, so a later forceCreate: false call may round-robin to a worker with another function’s slug. See the full lifecycle trace in the corresponding CLI finding. Please keep both runtime paths aligned, with integration coverage for A → B → repeated B and alternating A/B requests.

sharedWorkerOwners.set(servicePath, functionName);

@depthfirst-appdepthfirst-appBotAug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Severity: MEDIUM

A caller can select a function whose worker fails during creation. This records the URL-selected function as the owner before the cache is replaced; a later request for that function skips forceCreate and may receive the previous function's worker, exposing its code, state, or environment under the wrong endpoint.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: The root cause is that sharedWorkerOwners.set(servicePath, functionName) is called at line 238 beforeEdgeRuntime.userWorkers.create() is awaited. If worker creation fails, the owner map is already poisoned: a subsequent request for that same functionName sees forceCreate = false and reuses a stale/wrong worker.

The fix requires two coordinated changes:

  1. Remove line 238 (sharedWorkerOwners.set(servicePath, functionName);) from its current location before worker creation.

  2. Move the sharedWorkerOwners.set() call to immediately after userWorkers.create() succeeds (after line 260, the closing }); of the create call), but before the finally block at line 261 that releases the queue slot. This ensures the owner map is only updated when the worker is actually live, while still holding the serialization lock so no concurrent request can race on the owner decision:

worker=awaitEdgeRuntime.userWorkers.create({ ... });// Only record ownership after the worker is confirmed running:sharedWorkerOwners.set(servicePath,functionName);}finally{releaseSharedWorkerSlot?.();}

With this change, a failed create() leaves sharedWorkerOwners pointing at the previous owner (or unset), so the next request will correctly set forceCreate = true and attempt a fresh worker instead of reusing a cached one belonging to a different endpoint.

}

try {
let worker;
try {
worker = await EdgeRuntime.userWorkers.create({
servicePath,
memoryLimitMb: 256,
workerTimeoutMs: 400000,
noModuleCache: false,
noNpm: false,
importMapPath: functionConfig.importMapPath ?? undefined,
envVars,
forceCreate,
customModuleRoot: "",
cpuTimeSoftLimitMs: 1000,
cpuTimeHardLimitMs: 2000,
decoratorType: "tc39",
maybeEntrypoint: fileUrl(functionConfig.entrypointPath),
context: { useReadSyncFileAPI: true },
staticPatterns: functionConfig.staticFiles,
});
} finally {
releaseSharedWorkerSlot?.();
}
return await worker.fetch(req);
} catch (error) {
console.error(`Failed to serve Function ${functionName}`, error);
Expand Down
Loading