Skip to content
Merged
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
31 changes: 31 additions & 0 deletions src/cache/keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,37 @@ describe("cache/keys", () => {
assertEquals(release.includes("release:release-1"), true);
});

it("separates preview environments so adapter identities cannot collide", () => {
const unnamed = buildProxyManagerCacheKey("example-project", false, null, "main");
const preview = buildProxyManagerCacheKey("example-project", false, null, "main", "preview");

assertNotEquals(unnamed, preview);
});

it("separates distinct preview environment names on the same branch", () => {
const preview = buildProxyManagerCacheKey("example-project", false, null, "main", "preview");
const staging = buildProxyManagerCacheKey("example-project", false, null, "main", "staging");

assertNotEquals(preview, staging);
});

it("keeps the branch key stable when no environment is named", () => {
assertEquals(
buildProxyManagerCacheKey("example-project", false, null, "main"),
"proxy:example-project:preview:main",
);
});

it("escapes delimiters in an environment name", () => {
const forged = buildProxyManagerCacheKey("example-project", false, null, "main", "a:b");

assertEquals(forged.includes("a:b"), false);
assertNotEquals(
forged,
buildProxyManagerCacheKey("example-project", false, null, "main", "a"),
);
});

it("separates canonical projects and credential principals", () => {
const first = buildProxyManagerCacheKey(
"reusable-slug",
Expand Down
7 changes: 6 additions & 1 deletion src/cache/keys/builders/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,12 @@ export function buildProxyManagerCacheKey(
}

const source = encodeCacheSourceIdentity({ type: "branch", branch: branch ?? "main" });
return `${CacheKeyPrefix.PROXY}:${projectSlug}:${mode}:${source.qualifier}${authorityKey}`;
// ProxyFSAdapterManager asserts environmentName matches on reuse, so it must
// be part of the key. Omitted when unnamed to keep existing keys stable.
const environmentQualifier = environmentName
? `:env:${encodeCacheKeyLiteralSegment(environmentName)}`
: "";
Comment on lines +143 to +145

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed — the collision is closed on the identity side rather than in the key.

proxy-manager.ts:129 now normalizes with the same truthiness predicate the key builder uses:

const effectiveEnvironmentName = environmentName || null;

So "" and null produce both the same key and the same identity — they can no longer diverge, which was the actual failure mode (the key omitted the segment while ?? null kept "" in the identity, so the reuse assertion threw).

The other call site, buildDiagnosticCacheKey at proxy-manager.ts:52, is fed already-normalized identity fields, so it cannot reintroduce the split.

Covered by proxy-manager.test.ts "treats an empty environment name as unnamed", which asserts assertStrictEquals(empty, unnamed) and fails when the normalization is reverted.

return `${CacheKeyPrefix.PROXY}:${projectSlug}:${mode}:${source.qualifier}${environmentQualifier}${authorityKey}`;
}

/**
Expand Down
163 changes: 163 additions & 0 deletions src/platform/adapters/fs/veryfront/proxy-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
assertExists,
assertNotStrictEquals,
assertRejects,
assertStrictEquals,
assertThrows,
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
Expand Down Expand Up @@ -138,6 +139,168 @@ describe("ProxyFSAdapterManager", () => {
});
});

describe("adapter identity", () => {
function stubbedManager(): ProxyFSAdapterManager {
return createManager({
adapterFactory: (config) => {
const adapter = new VeryfrontFSAdapter(config);
adapter.initialize = () => Promise.resolve();
return adapter;
},
});
}

it("keeps distinct preview environments on separate adapters", async () => {
const manager = stubbedManager();
try {
const unnamed = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
null,
"main",
);
const preview = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
"preview",
"main",
);

assertNotStrictEquals(unnamed, preview);
} finally {
manager.dispose();
}
});

it("reuses an unnamed preview adapter after a named one is created", async () => {
const manager = stubbedManager();
try {
const first = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
null,
"main",
);
const named = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
"preview",
"main",
);
const again = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
null,
"main",
);

assertNotStrictEquals(first, named);
assertStrictEquals(first, again);
} finally {
manager.dispose();
}
});

it("treats an empty environment name as unnamed", async () => {
const manager = stubbedManager();
try {
const empty = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
"",
"main",
);
const unnamed = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
null,
"main",
);

assertStrictEquals(empty, unnamed);
} finally {
manager.dispose();
}
});

it("ignores releaseId when resolving a preview adapter identity", async () => {
const manager = stubbedManager();
try {
const withRelease = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
"release-7",
null,
"main",
);
const withoutRelease = await manager.getAdapter(
"my-project",
"test-token",
undefined,
false,
null,
null,
"main",
);

assertStrictEquals(withRelease, withoutRelease);
} finally {
manager.dispose();
}
});

it("ignores branch when resolving a production adapter identity", async () => {
const manager = stubbedManager();
try {
const withBranch = await manager.getAdapter(
"my-project",
"test-token",
undefined,
true,
"release-42",
"Production",
"main",
);
const withoutBranch = await manager.getAdapter(
"my-project",
"test-token",
undefined,
true,
"release-42",
"Production",
null,
);

assertStrictEquals(withBranch, withoutBranch);
} finally {
manager.dispose();
}
});
});

describe("exact production source", () => {
it("rejects mutable environment selection without an immutable release", async () => {
const manager = createManager();
Expand Down
8 changes: 5 additions & 3 deletions src/platform/adapters/fs/veryfront/proxy-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@ export class ProxyFSAdapterManager {
const getAdapterStartTime = performance.now();

const effectiveProductionMode = productionMode ?? false;
const effectiveReleaseId = releaseId ?? null;
const effectiveEnvironmentName = environmentName ?? null;
const effectiveBranch = branch ?? (effectiveProductionMode ? null : "main");
// All three must use the same predicate the cache key uses, or an identity
// that is not part of the key can still differ and fail the reuse assertion.
const effectiveReleaseId = effectiveProductionMode ? (releaseId ?? null) : null;
const effectiveEnvironmentName = environmentName || null;
const effectiveBranch = effectiveProductionMode ? null : (branch ?? "main");

if (
this.baseConfig.veryfront?.proxyMode === true &&
Expand Down
39 changes: 39 additions & 0 deletions src/server/handlers/preview/hmr.handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,4 +547,43 @@ describe("server/handlers/preview/hmr.handler", () => {
assertEquals(result.response!.status, 501);
});
});

describe("ensureAdapterInitialized", () => {
it("warms the adapter for the named environment, not the mode", async () => {
let observed: Record<string, unknown> | undefined;
const handler = new HMRHandler();
const ctx = {
projectSlug: "demo-project",
proxyToken: "test-token",
projectId: "proj_123",
resolvedEnvironment: "preview",
environmentName: "Development",
requestContext: { branch: "main" },
adapter: {
fs: {
isVeryfrontAdapter: () => true,
getUnderlyingAdapter: () => undefined,
isMultiProjectMode: () => true,
runWithContext: (
_slug: string,
_token: string,
run: () => Promise<void>,
_projectId: string,
options: Record<string, unknown>,
) => {
observed = options;
return run();
},
exists: () => Promise.resolve(true),
},
},
} as unknown as HandlerContext;

await (handler as unknown as {
ensureAdapterInitialized(ctx: HandlerContext): Promise<void>;
}).ensureAdapterInitialized(ctx);

assertEquals(observed?.environmentName, "Development");
});
});
});
4 changes: 4 additions & 0 deletions src/server/handlers/preview/hmr.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ export class HMRHandler extends BaseHandler {
{
productionMode: false,
branch: ctx.requestContext?.branch ?? "main",
// The named environment, not the mode in `resolvedEnvironment`. Must
// match what renders resolve, or HMR warms a different adapter and
// its WebSocketManager never receives pokes.
environmentName: ctx.environmentName ?? null,
},
);
} catch (error) {
Expand Down