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
41 changes: 41 additions & 0 deletions engine/core/mod.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,50 @@ import {
resolve,
type ResolverMap,
} from "../../engine/core/resolver.ts";
import { ReleaseResolver } from "../../engine/core/mod.ts";
import { fromJSON } from "../../engine/decofile/fetcher.ts";
import defaults from "../manifest/fresh.ts";
import { RequestContext } from "../../deco.ts";

Deno.test(".with({ release }) does not inherit stale resolve hints", async () => {
// Hints are cached by resolveType (block id) and derived from the release's
// resolvables. Fast Preview binds a request-scoped draft via
// `resolver.with({ release: draftProvider })`. If `.with` inherits the base
// resolver's hints, a block whose SHAPE changed between the published release
// and the draft (same id, different content) resolves against the stale shape
// and silently drops everything the old hints don't cover — the exact failure
// that blanked a page whose `sections` went from a plain array (published) to
// a `multivariate` flag (draft).
const resolvers = {
passthrough: (props: unknown) => props,
} as unknown as ResolverMap;

// Published: block "page" has a plain `content` (no nested resolvable).
const published = fromJSON({
page: { __resolveType: "passthrough", content: "plain" },
});
// Draft: SAME id "page", but `content` is now a nested resolvable — the draft
// needs a hint at `content` that the published shape never produced.
const draft = fromJSON({
page: { __resolveType: "passthrough", content: { __resolveType: "inner" } },
inner: { __resolveType: "passthrough", value: "from-draft" },
});

const base = new ReleaseResolver<BaseContext>({ release: published, resolvers });

// Resolve against the published release first — this populates (poisons) the
// base resolver's hint cache for "page" with the plain-content shape.
assertEquals(await base.resolve<{ content: unknown }>("page", {}), {
content: "plain",
});

// Swap the release for the draft, as Fast Preview does.
const drafted = base.with({ release: draft });
assertEquals(await drafted.resolve<{ content: unknown }>("page", {}), {
content: { value: "from-draft" },
});
});

Deno.test("resolve", async (t) => {
const context: BaseContext = {
revision: "",
Expand Down
17 changes: 14 additions & 3 deletions engine/core/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,30 @@ export class ReleaseResolver<TContext extends BaseContext = BaseContext> {
{ resolvers, resolvables, release, danglingRecover }: ExtensionOptions<
TContext
>,
): ReleaseResolver<TContext> =>
new ReleaseResolver<TContext>(
): ReleaseResolver<TContext> => {
// Hints are derived from the release's resolvables and cached by resolveType
// (block id). When the release is swapped — as Fast Preview does to bind a
// request-scoped draft (`resolver.with({ release: draftProvider })`) — the
// base hints are stale: the SAME block id can now resolve to different
// content (e.g. a page whose `sections` was a plain array in the published
// release but a `multivariate` flag in the draft). Inheriting them makes the
// draft resolve against the published shape and silently drop everything the
// old hints don't cover. This mirrors the `release.onChange` invariant in the
// constructor, which already clears hints whenever the release changes.
const releaseChanged = release !== undefined && release !== this.release;
return new ReleaseResolver<TContext>(
{
release: release ?? this.release,
danglingRecover: danglingRecover ?? this.danglingRecover,
resolvables: { ...this.resolvables, ...resolvables },
resolvers: { ...this.resolvers, ...resolvers },
},
{ ...this.resolveHints },
releaseChanged ? {} : { ...this.resolveHints },

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: The fix clears resolveHints on release swap but deliberately leaves runOncePerRelease inherited, even though the release.onChange invariant this comment claims to mirror clears both. runOnce entries like resolveTypeSelector_*/blockSelector in engine/manifest/defaults.ts are built from the previous release's resolvables and stay cached via the shared SyncOnce instances, so a draft bound with .with({ release }) can still resolve against stale published selectors — the same class of bug this PR addresses. Clear runOncePerRelease when releaseChanged too.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At engine/core/mod.ts, line 134:

<comment>The fix clears `resolveHints` on release swap but deliberately leaves `runOncePerRelease` inherited, even though the `release.onChange` invariant this comment claims to mirror clears both. `runOnce` entries like `resolveTypeSelector_*`/`blockSelector` in `engine/manifest/defaults.ts` are built from the *previous* release's `resolvables` and stay cached via the shared `SyncOnce` instances, so a draft bound with `.with({ release })` can still resolve against stale published selectors — the same class of bug this PR addresses. Clear `runOncePerRelease` when `releaseChanged` too.</comment>

<file context>
@@ -113,19 +113,30 @@ export class ReleaseResolver<TContext extends BaseContext = BaseContext> {
         resolvers: { ...this.resolvers, ...resolvers },
       },
-      { ...this.resolveHints },
+      releaseChanged ? {} : { ...this.resolveHints },
       {
         ...this.runOncePerRelease,
</file context>

{
...this.runOncePerRelease,
},
);
};

public getResolvers(): ResolverMap<BaseContext> {
return this._cachedResolvers ??= {
Expand Down
Loading