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
36 changes: 35 additions & 1 deletion .fork/customizations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2067,9 +2067,31 @@
not successes, and caching them would pin a transient failure for the
element's lifetime.

Third, the resolver snapshots the rendering component's primitive props
(strings/numbers/booleans, `children` excluded, capped at 12 entries and
64 chars per string) off the composite fiber whose displayName matches
the reported component name — no match, no props, so the request never
labels props with a component they do not belong to. The snapshot crosses
the bridge only beside a component name, is re-validated by the guest
twin (designProps.ts's readDesignProps, mirroring DesignSourceResult.ts's
normalizeResolvedProps), rides the element as `data-t3-props`, is
re-validated AGAIN when the request builder reads that page-controlled
attribute back, and renders as "Rendered by `<Button>` — props:
`variant="ghost" size="sm"`". It exists to ground the SCOPE guardrail's
component-vs-one-off judgment in the design system's own vocabulary.
bippy is imported by its root export on purpose (the mangled-name d.ts of
bippy/core cannot be imported by name; the root's hook-install side
effect is guarded double work beside react-grab's bundled copy).

Relatedly, the NO_PREVIEW guardrail must not promise upstream's automatic
verification while this fork has not vendored client/verifier.ts; a guard
test pins that wording.
test pins that wording. The SCOPE guardrail likewise diverges from
upstream: instead of call-site-only scoping that skips shared-component
edits, the agent judges component-wide vs one-off intent from the
request's component/source context (selecting a Button and nudging its
padding usually means the Button, not one instance), prefers project
tokens over hard-coded values, and states which scope it chose. The same
guard test pins that wording too.
tier: 4
files:
- apps/web/src/custom/designMode/protocol.ts
Expand All @@ -2078,6 +2100,8 @@
# (the engine tsconfig includes it by path) so the web project can type-check
# and unit-test it, which files under engine/ cannot be.
- apps/web/src/custom/designMode/cssOrigin.ts
- apps/web/src/custom/designMode/designProps.ts
- apps/web/src/custom/designMode/designProps.test.ts
- apps/web/src/custom/designMode/designModeStore.ts
- apps/web/src/custom/designMode/designModeBridge.ts
- apps/web/src/custom/designMode/designModeBridge.test.ts
Expand Down Expand Up @@ -2153,6 +2177,16 @@
# upstream PickPreload entry — the whole desktop half of native source
# recovery hangs off this one line.
- apps/desktop/src/preview-pick-preload.ts
# alwaysBundle for the preview-pick-preload entry must keep listing every
# package the resolver imports (react-grab AND bippy). The webview is
# sandboxed, so anything left external becomes a require() that throws
# before the preload installs the picker or the resolver global — packaged
# builds only, where no dev server catches it.
- apps/desktop/vite.config.ts
# Unfenced (strict JSON): the bippy dependency DesignSourceResolver walks
# fibers with. An upstream dep sweep dropping it breaks the packaged
# preload, not just a type import.
- apps/desktop/package.json
# Thin fenced import + <ForkPreviewDesignMode> in the chrome row's
# trailingActions slot, plus <ForkPreviewLayersToggle> in its leadingActions
# slot; the browser surface tree stays untouched.
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"@t3tools/shared": "workspace:*",
"@t3tools/ssh": "workspace:*",
"@t3tools/tailscale": "workspace:*",
"bippy": "^0.5.41",
"effect": "catalog:",
"electron": "41.5.0",
"electron-store": "^8.2.0",
Expand Down
80 changes: 79 additions & 1 deletion apps/desktop/src/preview/DesignSourceResolver.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { describe, expect, it } from "vite-plus/test";

import { describeResolvedSource, normalizeResolvedSource } from "./DesignSourceResult.ts";
import {
describeResolvedSource,
MAX_PROP_VALUE_LENGTH,
MAX_PROPS,
normalizeResolvedProps,
normalizeResolvedSource,
} from "./DesignSourceResult.ts";

const VALID = {
filePath: "/src/components/Button.tsx",
Expand Down Expand Up @@ -156,3 +162,75 @@ describe("describeResolvedSource", () => {
expect(describeResolvedSource({ filePath: "/src/App.tsx\u0000", stack } as never)).toBeNull();
});
});

describe("describeResolvedSource — props", () => {
it("carries props on the full location and on the hint alike", () => {
const props = { variant: "ghost", size: "sm" };
expect(
describeResolvedSource({ ...VALID, componentName: "Btn", props } as typeof VALID),
).toEqual({ file: VALID.filePath, line: 12, column: 4, componentName: "Btn", props });
const stack = [{ fileName: VALID.filePath, lineNumber: 12, columnNumber: 4 }];
expect(
describeResolvedSource({ ...VALID, stack, componentName: "Btn", props } as typeof VALID),
).toEqual({ componentName: "Btn", file: VALID.filePath, props });
});

// Rendering gates on the component name (`<Name> — props: ...`), so a props bag with no
// name to hang it on must not cross at all.
it("drops props when there is no component name to label them with", () => {
const props = { variant: "ghost" };
expect(describeResolvedSource({ ...VALID, props } as typeof VALID)).toEqual({
file: VALID.filePath,
line: 12,
column: 4,
});
});
});

describe("normalizeResolvedProps", () => {
it("keeps primitive props and drops everything else", () => {
expect(
normalizeResolvedProps({
variant: "ghost",
count: 3,
disabled: false,
onClick: () => {},
style: { color: "red" },
items: [1, 2],
nested: null,
big: Number.POSITIVE_INFINITY,
}),
).toEqual({ variant: "ghost", count: 3, disabled: false });
});

it("excludes children even when primitive — the request already carries Text:", () => {
expect(normalizeResolvedProps({ children: "Save", variant: "ghost" })).toEqual({
variant: "ghost",
});
});

it("drops names that are not JSX-attribute-shaped and strings carrying control characters", () => {
expect(
normalizeResolvedProps({
"a b": "spaced-name",
"data-testid": "composer-send",
label: "line\nbreak",
}),
).toEqual({ "data-testid": "composer-send" });
});

it("caps the entry count and slices oversized string values", () => {
const bag: Record<string, string> = {};
for (let i = 0; i < MAX_PROPS + 5; i += 1) bag[`p${i}`] = "x".repeat(200);
const out = normalizeResolvedProps(bag);
expect(Object.keys(out ?? {})).toHaveLength(MAX_PROPS);
expect(out?.p0).toBe("x".repeat(MAX_PROP_VALUE_LENGTH));
});

it("returns null for non-objects and for bags where nothing survives", () => {
expect(normalizeResolvedProps(null)).toBeNull();
expect(normalizeResolvedProps("variant=ghost")).toBeNull();
expect(normalizeResolvedProps([1, 2, 3])).toBeNull();
expect(normalizeResolvedProps({ onClick: () => {} })).toBeNull();
});
});
48 changes: 44 additions & 4 deletions apps/desktop/src/preview/DesignSourceResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@
* the same property react-grab already depends on for the element picker.
*
* The surface is deliberately tiny: one frozen object with one async `resolve`
* method returning validated scalars (`file`/`line`/`column`) or null. No Electron
* object, IPC function, Node capability, React Fiber, or raw react-grab context
* ever crosses this boundary.
* method returning validated scalars (`file`/`line`/`column`/`componentName`, plus the
* primitives-only `props` snapshot) or null. No Electron object, IPC function, Node
* capability, React Fiber, or raw react-grab context ever crosses this boundary.
*/
// The ROOT export on purpose, side effect and all: importing `bippy` installs the React
// DevTools hook stub at module load when none exists. In this preload that is guarded
// double work, not new behavior — react-grab's own bundled bippy ships the identical
// installer in the chunk `react-grab/primitives` pulls in, and fiber access (the picker,
// getElementContext) already depends on it. `bippy/core` would skip the install but its
// published d.ts exports mangled names (`getDisplayName as L`), so it cannot be imported
// by name. Same pinned version as react-grab's bundled copy (0.5.41).
import { getDisplayName, isCompositeFiber, type Fiber } from "bippy";
Comment thread
NoahHendrickson marked this conversation as resolved.
Resolved
Comment thread
NoahHendrickson marked this conversation as resolved.
Resolved
import { getElementContext } from "react-grab/primitives";

import {
Expand All @@ -21,6 +29,35 @@ import {
type ResolvedDesignSource,
} from "./DesignSourceResult.ts";

/** How far up the fiber tree to look for the named component. The rendering component is
* normally the first composite ancestor; the budget only exists so a hostile or cyclic
* `return` chain cannot spin. */
const MAX_FIBER_WALK = 25;

/** The `memoizedProps` of the composite fiber whose displayName matches the name react-grab
* reported, walked upward from the context's own fiber. Matching by NAME is the honesty
* guard: the request renders these as `<Name> — props: ...`, so props read off any other
* fiber would be labeled with a component they do not belong to — no match, no props.
* Returns the RAW object; normalizeResolvedProps (via describeResolvedSource) owns making
* it safe to cross the bridge. Best-effort throughout: any surprise returns null rather
* than failing a resolution that already has a location to deliver. */
function readComponentProps(fiber: Fiber | null, componentName: string | null): unknown {
if (!fiber || typeof componentName !== "string" || componentName.length === 0) return null;
try {
let candidate: Fiber | null = fiber;
for (let depth = 0; candidate && depth < MAX_FIBER_WALK; depth += 1) {
if (isCompositeFiber(candidate) && getDisplayName(candidate.type) === componentName) {
const props: unknown = candidate.memoizedProps;
return typeof props === "object" ? props : null;
}
candidate = candidate.return;
}
} catch {
// A hostile props getter or a detached fiber mid-commit — context, never worth throwing.
}
return null;
}

/** react-grab resolution symbolicates through source maps — cap concurrent work so a
* scrubbing cursor can't queue dozens of expensive lookups at once. */
const MAX_CONCURRENT = 2;
Expand Down Expand Up @@ -64,7 +101,10 @@ async function resolveElement(element: Element): Promise<ResolvedDesignSource |
await acquireSlot();
try {
const context = await getElementContext(element);
const normalized = describeResolvedSource(context);
const normalized = describeResolvedSource({
...context,
props: readComponentProps(context.fiber, context.componentName),
});
// Recheck after the await — an element replaced mid-resolution must not hand the
// engine a location for a node that no longer exists.
if (!normalized || !element.isConnected || element.ownerDocument !== document) return null;
Expand Down
57 changes: 57 additions & 0 deletions apps/desktop/src/preview/DesignSourceResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,52 @@ export interface DesignSourceResult {
line: number;
column: number;
componentName?: string;
props?: ResolvedProps;
}

/** The rendering component's primitive props, snapshotted at resolution time. Strings,
* numbers and booleans ONLY — the design-system vocabulary (`variant="ghost" size="sm"`)
* is primitive-shaped, and the primitives-only rule is also what keeps functions, elements,
* styles and refs from ever crossing the bridge. Context for the agent's component-vs-one-off
* scope judgment (SCOPE_GUARDRAIL), not an oracle: the snapshot can go stale with state and
* that is fine. */
export type ResolvedProps = Record<string, string | number | boolean>;

/** Caps are the flood control for a page-controlled object: a hostile (or just enormous)
* props bag must not balloon the request text. TWINNED in readResolvedProps
* (apps/web/src/custom/designMode/designProps.ts) — same trust-boundary duplication as
* normalizeFilePath/readSourceFile; keep the two in step. */
export const MAX_PROPS = 12;
export const MAX_PROP_VALUE_LENGTH = 64;

/** JSX-attribute-shaped names only (identifier, plus the `-` that `data-*`/`aria-*` carry).
* Same posture as COMPONENT_NAME_PATTERN: the name lands in the agent's request text. */
const PROP_NAME_PATTERN = /^[A-Za-z_$][\w$-]{0,63}$/;

/** Validates an unknown value into the primitive props snapshot, or null when nothing
* usable survives. Non-conforming ENTRIES are skipped, not fatal — props are best-effort
* context, unlike the location, where a wrong value poisons the request. `children` is
* excluded even when primitive: the element's own text already rides the request as `Text:`.
* String values are control-character-rejected (the request-injection threat model every
* page-controlled string here shares) and sliced to the cap, the same plain-slice treatment
* contextText applies. */
export function normalizeResolvedProps(value: unknown): ResolvedProps | null {
if (typeof value !== "object" || value === null || Array.isArray(value)) return null;
const out: ResolvedProps = {};
let count = 0;
for (const [name, raw] of Object.entries(value)) {
if (count >= MAX_PROPS) break;
if (name === "children" || !PROP_NAME_PATTERN.test(name)) continue;
if (typeof raw === "boolean") out[name] = raw;
else if (typeof raw === "number" && Number.isFinite(raw)) out[name] = raw;
else if (typeof raw === "string") {
// eslint-disable-next-line no-control-regex
if (/[\u0000-\u001f\u007f]/.test(raw)) continue;
out[name] = raw.slice(0, MAX_PROP_VALUE_LENGTH);
} else continue;
count += 1;
}
return count === 0 ? null : out;
}

/**
Expand All @@ -41,6 +87,7 @@ export function normalizeResolvedSource(
lineNumber?: unknown;
columnNumber?: unknown;
componentName?: unknown;
props?: unknown;
stack?: unknown;
} | null,
): DesignSourceResult | null {
Expand All @@ -60,11 +107,15 @@ export function normalizeResolvedSource(
return null;
}
const componentName = normalizeComponentName(value.componentName);
// Gated on the component name: props are rendered as `<Name> — props: ...` in the request,
// and a props bag with no component to hang it on has nowhere honest to appear.
const props = componentName === null ? null : normalizeResolvedProps(value.props);
return {
file,
line: lineNumber,
column: Math.max(1, columnNumber),
...(componentName === null ? {} : { componentName }),
...(props === null ? {} : { props }),
};
}

Expand Down Expand Up @@ -188,6 +239,7 @@ export function isSymbolicated(
export interface DesignSourceHint {
file?: string;
componentName?: string;
props?: ResolvedProps;
line?: never;
column?: never;
}
Expand All @@ -213,8 +265,13 @@ export function describeResolvedSource(
const componentName = normalizeComponentName(value?.componentName);
const file = normalizeFilePath(value?.filePath);
if (componentName === null && file === null) return null;
// Props survive a rejected location exactly like the component name does — both come off
// the fiber, and both matter MOST when there is no line to point at. Same component-name
// gate as the full-location arm.
const props = componentName === null ? null : normalizeResolvedProps(value?.props);
return {
...(componentName === null ? {} : { componentName }),
...(file === null ? {} : { file }),
...(props === null ? {} : { props }),
};
}
11 changes: 10 additions & 1 deletion apps/desktop/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,16 @@ export default defineConfig({
outExtensions: () => ({ js: ".cjs" }),
entry: ["src/preview-pick-preload.ts"],
deps: {
alwaysBundle: (id) => id === "react-grab" || id.startsWith("react-grab/"),
// This preload runs sandboxed (WebviewPreferences.ts pins `sandbox=true`), so its
// limited `require` cannot resolve npm packages from the packaged ASAR. Anything
// left external here throws before the preload installs the picker or the design
// source resolver, taking the whole preview-pick path down. `bippy` rides along
// with react-grab: DesignSourceResolver imports it directly for the fiber walk.
alwaysBundle: (id) =>
id === "react-grab" ||
id.startsWith("react-grab/") ||
id === "bippy" ||
id.startsWith("bippy/"),
},
},
{
Expand Down
12 changes: 12 additions & 0 deletions apps/web/src/__fork_guards__/forkDesignMode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,18 @@ describe("fork guard: design mode", () => {
expect(preloadEntry).toContain("fork:begin fork-design-mode");
expect(preloadEntry).toContain('import "./preview/DesignSourceResolver.ts"');

// Every package the resolver imports must be BUNDLED into the preload artifact. This
// webview is sandboxed, so a leftover external require() throws before the preload
// installs the picker or the resolver global — the whole preview-pick path dies, and
// only in packaged builds where a dev server never catches it.
const desktopViteConfig = NodeFS.readFileSync(
NodePath.join(desktopRoot, "vite.config.ts"),
"utf8",
);
for (const bundled of ["react-grab", "bippy"]) {
expect(desktopViteConfig).toContain(`id === "${bundled}"`);
}

// react-grab stays a desktop-preload dependency only — never bundled into the web app.
const webPackage = read("package.json");
expect(webPackage).not.toContain("react-grab");
Expand Down
Loading
Loading