Skip to content
Draft
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
5 changes: 5 additions & 0 deletions .changeset/artifact-bracket-binding-guards.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"executor": patch
---

Recognize single-quoted artifact integration roots and reject hardcoded connection addresses consistently in dot and bracket notation.
41 changes: 41 additions & 0 deletions e2e/scenarios/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,31 @@ scenario(
"infiniteQueryOptions",
);

// Bracket notation must follow the same binding contract as dotted
// references: missing integrations are not silently ignored, and a
// literal connection path is rejected before persistence. The slug is
// unique and deliberately unregistered; no upstream service is needed.
const missingIntegration = `bracket-check-${suffix}`;
const beforeRejected = yield* client.artifacts.list();
const missingBinding = yield* session.call("create-artifact", {
code: `function App(){ useQuery(tools['${missingIntegration}'].query.queryOptions({})); return <div/>; }`,
title: `Missing bracket binding ${suffix}`,
});
expect(missingBinding.ok, "a single-quoted root still requires a real connection").toBe(
false,
);
expect(missingBinding.text).toContain(`No ${missingIntegration} connection`);

const pinnedBinding = yield* session.call("create-artifact", {
code: `function App(){ useQuery(tools['${missingIntegration}']["org"]['shared'].query.queryOptions({})); return <div/>; }`,
title: `Pinned bracket binding ${suffix}`,
});
expect(pinnedBinding.ok, "bracket syntax cannot pin a connection in saved code").toBe(false);
expect(pinnedBinding.text).toContain("Artifact code must not name a connection");
expect((yield* client.artifacts.list()).length, "neither rejection saved an artifact").toBe(
beforeRejected.length,
);

const rendered = yield* session.call("create-artifact", {
code: artifactSource(marker),
title,
Expand All @@ -233,6 +258,22 @@ scenario(
artifactId = structured.artifactId as ArtifactId;
expect(artifactId, "the artifact was persisted and its id returned").toBeTruthy();

const beforePinnedEdit = yield* client.artifacts.get({ params: { artifactId } });
const pinnedEdit = yield* session.call("edit-artifact", {
artifactId,
edits: [
{
oldText: beforePinnedEdit.code,
newText: `function App(){ useQuery(tools["${missingIntegration}"].user['personal'].query.queryOptions({})); return <div/>; }`,
},
],
});
expect(pinnedEdit.ok, "edit-artifact also rejects bracketed pinned connections").toBe(false);
expect(pinnedEdit.text).toContain("Artifact code must not name a connection");
expect((yield* client.artifacts.get({ params: { artifactId } })).code).toBe(
beforePinnedEdit.code,
);

const url = String(structured.url);
expect(rendered.text, "the model is handed the URL to relay to the user").toContain(url);

Expand Down
7 changes: 7 additions & 0 deletions packages/core/execution/src/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ const CREATE_ARTIFACT_SKILL_BODY = [
"",
"## Addressing: Integrations, Not Connections",
"",
"Integration slugs containing hyphens use static bracket notation:",
"`tools[\"cloudflare-bindings\"].<tool>` or `tools['cloudflare-bindings'].<tool>`.",
"Both quote styles work, including named roles such as",
"`tools['cloudflare-bindings'](\"prod\").<tool>`. Bracket notation does not permit",
"hardcoding `.user.<connection>` or `.org.<connection>` into artifact source;",
"supply the connection separately as described below.",
"",
"This is the one place artifact code differs from `execute` code, and getting it",
"wrong is rejected outright.",
"",
Expand Down
124 changes: 124 additions & 0 deletions packages/hosts/mcp/src/artifact-bindings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,56 @@ describe("extractArtifactRoles", () => {
expect(roles).toEqual([{ role: "cloudflare-bindings", integration: "cloudflare-bindings" }]);
});

it.each([
`tools['cloudflare-bindings']`,
`tools [ 'cloudflare-bindings' ]`,
`tools [ "cloudflare-bindings" ]`,
])("reads a static bracket root: %s", (root) => {
expect(extractArtifactRoles(`${root}.query.queryOptions({});`)).toEqual([
{ role: "cloudflare-bindings", integration: "cloudflare-bindings" },
]);
});

it("reads both role quote flavours after either bracket quote flavour", () => {
expect(
extractArtifactRoles(`
tools['cloudflare-bindings']("prod").query.queryOptions({});
tools["cloudflare-bindings"]('staging').query.queryOptions({});
`),
).toEqual([
{ role: "prod", integration: "cloudflare-bindings" },
{ role: "staging", integration: "cloudflare-bindings" },
]);
});

it("deduplicates mixed bracket and dotted references", () => {
expect(
extractArtifactRoles(
`tools.linear.issues.list(); tools['linear'].issues.list(); tools["linear"].issues.list();`,
),
).toEqual([{ role: "linear", integration: "linear" }]);
});

it("ignores bracketed system roots, comments and another object's tools", () => {
expect(
extractArtifactRoles(`
tools['search']({}); tools["describe"].tool({}); tools['executor'].coreTools.connections.list({});
// tools['ignored-comment'].query({});
/* tools["ignored-block"].query({}); */
other.tools['ignored-property'].query({});
mytools['ignored-name'].query({});
`),
).toEqual([]);
});

it.each([
`tools[integration].query({});`,
`tools['cloudflare-' + suffix].query({});`,
"tools[`cloudflare-bindings`].query({});",
])("does not infer a static binding from computed syntax: %s", (code) => {
expect(extractArtifactRoles(code)).toEqual([]);
});

it("collapses repeated references to one role", () => {
const roles = extractArtifactRoles(
`useQuery(tools.linear.issues.list.queryOptions({}));
Expand Down Expand Up @@ -113,6 +163,42 @@ describe("oldStyleAddressRejection", () => {
).toContain("tools.inventory.org.");
});

it.each([
`tools['cloudflare-bindings'].user.personal.query`,
`tools["cloudflare-bindings"].org.shared.query`,
`tools['cloudflare-bindings']['user']['personal'].query`,
`tools["cloudflare-bindings"]["org"]["shared"].query`,
`tools ['cloudflare-bindings'] [ "org" ] ['shared'].query`,
`tools.inventory['user'].personal.query`,
`tools.inventory["org"].shared.query`,
`tools.inventory.org['shared'].query`,
])("rejects bracketed connection paths: %s", (path) => {
expect(oldStyleAddressRejection(`useQuery(${path}.queryOptions({}));`)).toContain(
"Artifact code must not name a connection",
);
});

it("uses valid bracket notation in a hyphenated root's rejection guidance", () => {
const message = oldStyleAddressRejection(`tools['cloudflare-bindings'].org.shared.query({});`);
expect(message).toContain('tools["cloudflare-bindings"].org.');
expect(message).toContain('tools["cloudflare-bindings"].<tool>(args)');
expect(message).toContain('tools["cloudflare-bindings"]("prod")');
expect(message).not.toContain("tools.cloudflare-bindings");
});

it.each([
`tools['cloudflare-bindings'].query.queryOptions({});`,
`tools["cloudflare-bindings"]('prod').query.queryOptions({});`,
`tools['cloudflare-bindings'].admin['user'].query({});`,
`tools.inventory['organization'].query({});`,
`tools.inventory.userProfile.query({});`,
`// tools['cloudflare-bindings'].org.shared.query({});`,
`/* tools["cloudflare-bindings"]['user'].personal.query({}); */`,
`other.tools['cloudflare-bindings'].org.shared.query({});`,
])("keeps non-pinned and ignored bracket references legal: %s", (code) => {
expect(oldStyleAddressRejection(code)).toBeNull();
});

it("accepts the short form", () => {
expect(
oldStyleAddressRejection(`useQuery(tools.vercel.domains.getDomains.queryOptions({}));`),
Expand All @@ -139,6 +225,44 @@ describe("oldStyleAddressRejection", () => {
});

describe("resolveArtifactBindings", () => {
it("binds a single-quoted root through the existing connection inventory", () => {
const roles = extractArtifactRoles(`tools['cloudflare-bindings']('prod').query({});`);
expect(
resolveArtifactBindings({
roles,
connections: { prod: "cloudflare-bindings.org.shared" },
available: [connection("cloudflare-bindings", "org", "shared")],
}),
).toEqual({
ok: true,
bindings: {
prod: { integration: "cloudflare-bindings", owner: "org", connection: "shared" },
},
});
});

it("still refuses a mismatched integration for an extracted bracket role", () => {
const result = resolveArtifactBindings({
roles: extractArtifactRoles(`tools['cloudflare-bindings']('prod').query({});`),
connections: { prod: "linear.org.shared" },
available: [connection("linear", "org", "shared")],
});
expect(result.ok).toBe(false);
if (result.ok) return;
expect(result.message).toContain("A role's connection must belong to the integration");
});

it("still refuses unavailable connections for an extracted bracket role", () => {
const result = resolveArtifactBindings({
roles: extractArtifactRoles(`tools['cloudflare-bindings'].query({});`),
connections: { "cloudflare-bindings": "cloudflare-bindings.org.missing" },
available: [connection("cloudflare-bindings", "user", "personal")],
});
expect(result.ok).toBe(false);
if (result.ok) return;
expect(result.message).toContain('No connection "cloudflare-bindings.org.missing"');
});

it("binds a role silently when the author has exactly one connection", () => {
const result = resolveArtifactBindings({
roles: [{ role: "vercel", integration: "vercel" }],
Expand Down
34 changes: 21 additions & 13 deletions packages/hosts/mcp/src/artifact-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,38 +94,46 @@ const withCommentsBlanked = (code: string): string =>
/**
* A tools root reference, with the optional role call that follows it.
*
* The role is captured from either quote flavour. Anything else after the root
* — property access, a call with an object — is left to the caller's own path
* handling; extraction only cares which integration slot is being reached.
* Static bracket roots and roles accept either quote flavour. Anything else
* after the root — property access, a call with an object — is left to the
* caller's own path handling; extraction only cares which integration slot is
* being reached.
*/
const TOOL_ROOT = String.raw`(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*"([A-Za-z_$][\w$-]*)"\s*\])`;
const TOOL_ROOT = String.raw`(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*(?:"([A-Za-z_$][\w$-]*)"|'([A-Za-z_$][\w$-]*)')\s*\])`;
const TOOLS_REFERENCE = new RegExp(
String.raw`(?<![.\w$])tools\s*${TOOL_ROOT}\s*(?:\(\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)')\s*\))?`,
"g",
);

/**
* An old-style address: a tier literal in the segment right after the
* integration, i.e. `tools.<integration>.user.` / `tools.<integration>.org.`.
* integration, in either dot or static bracket notation. The root grammar is
* shared with role extraction so a supported root cannot escape this guard.
*
* Scoped to exactly those two literals in exactly that position. A tool whose
* own first path segment is literally `user` or `org` would be caught by this
* (`tools.acme.user.profile.get`), which is the accepted false-positive
* surface: the two words are reserved by the address grammar itself, the shape
* is vanishingly rare, and the error says precisely what to write instead.
*/
const OLD_STYLE_TIER_SEGMENT = /(?<![.\w$])tools\s*\.\s*([A-Za-z_$][\w$]*)\s*\.\s*(user|org)\s*\./;
const OLD_STYLE_TIER_SEGMENT = new RegExp(
String.raw`(?<![.\w$])tools\s*${TOOL_ROOT}\s*(?:\.\s*(user|org)|\[\s*(?:"(user|org)"|'(user|org)')\s*\])\s*(?=\.|\[)`,
);

/** The message a five-segment path in artifact code is refused with. */
export const oldStyleAddressRejection = (code: string): string | null => {
const match = OLD_STYLE_TIER_SEGMENT.exec(withCommentsBlanked(code));
if (!match) return null;
const [, integration = "", tier = ""] = match;
const integration = match[1] ?? match[2] ?? match[3] ?? "";
const tier = match[4] ?? match[5] ?? match[6] ?? "";
const root = /^[A-Za-z_$][\w$]*$/.test(integration)
? `tools.${integration}`
: `tools[${JSON.stringify(integration)}]`;
return [
`Artifact code must not name a connection: \`tools.${integration}.${tier}.…\` pins this artifact to one account.`,
`Address the integration only — \`tools.${integration}.<tool>(args)\` — and the server binds it to your connection when the artifact runs.`,
`Discovery through \`execute\` still uses the full \`tools.${integration}.${tier}.<connection>.<tool>\` address; only saved artifact code drops the middle segments.`,
`If this artifact needs two accounts of the same integration, tag each one with a role — \`tools.${integration}("prod").<tool>(args)\` — and pass \`connections: { "prod": "${integration}.${tier}.<connection>" }\` to create-artifact.`,
`Artifact code must not name a connection: \`${root}.${tier}.…\` pins this artifact to one account.`,
`Address the integration only — \`${root}.<tool>(args)\` — and the server binds it to your connection when the artifact runs.`,
`Discovery through \`execute\` still uses the full \`${root}.${tier}.<connection>.<tool>\` address; only saved artifact code drops the middle segments.`,
`If this artifact needs two accounts of the same integration, tag each one with a role — \`${root}("prod").<tool>(args)\` — and pass \`connections: { "prod": "${integration}.${tier}.<connection>" }\` to create-artifact.`,
].join(" ");
};

Expand All @@ -141,9 +149,9 @@ export const extractArtifactRoles = (code: string): readonly ArtifactRole[] => {
const scannable = withCommentsBlanked(code);
const found = new Map<string, ArtifactRole>();
for (const match of scannable.matchAll(TOOLS_REFERENCE)) {
const integration = match[1] ?? match[2];
const integration = match[1] ?? match[2] ?? match[3];
if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue;
const role = match[3] ?? match[4] ?? integration;
const role = match[4] ?? match[5] ?? integration;
if (role.length === 0) continue;
if (!found.has(role)) found.set(role, { role, integration });
}
Expand Down
85 changes: 85 additions & 0 deletions packages/hosts/mcp/src/artifacts-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,56 @@ describe("MCP host — edit-artifact", () => {
);
});

it("binds a single-quoted root on edit and refuses a later pinned-path edit atomically", async () => {
const store = makeArtifactStore();
const code =
"function App(){ useQuery(tools['cloudflare-bindings'].query.queryOptions({})); return <div/>; }";
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const edited = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: COUNTER_CODE, newText: code }],
connections: { "cloudflare-bindings": "cloudflare-bindings.org.shared" },
},
});
expect(edited.isError, textOf(edited)).toBeFalsy();
expect(store.rows.get("art_1")?.bindings).toEqual({
"cloudflare-bindings": {
integration: "cloudflare-bindings",
owner: "org",
connection: "shared",
},
});

const refused = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [
{
oldText: "tools['cloudflare-bindings'].query",
newText: "tools['cloudflare-bindings']['org']['shared'].query",
},
],
},
});
expect(refused.isError).toBe(true);
expect(textOf(refused)).toContain("Artifact code must not name a connection");
expect(store.calls).toHaveLength(2);
expect(store.rows.get("art_1")?.code).toBe(code);
},
{
artifacts: store.port,
connections: connectionsPort([conn("cloudflare-bindings", "org", "shared")]),
},
);
});

it("updates the title and description when asked, keeping the patched code", async () => {
const store = makeArtifactStore();
await withClient(
Expand Down Expand Up @@ -2171,6 +2221,30 @@ describe("MCP host — create-artifact binding", () => {
},
);
});

it("refuses bracketed pinned paths before create-artifact can persist them", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
const result = await client.callTool({
name: "create-artifact",
arguments: {
title: "Pinned bracket path",
code: "function App(){ useQuery(tools['cloudflare-bindings']['org']['shared'].query.queryOptions({})); return <div/>; }",
},
});
expect(result.isError).toBe(true);
expect(textOf(result)).toContain('tools["cloudflare-bindings"].<tool>(args)');
expect(store.calls).toEqual([]);
},
{
artifacts: store.port,
connections: connectionsPort([conn("cloudflare-bindings", "org", "shared")]),
},
);
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -2236,6 +2310,17 @@ describe("MCP host — execute-action binding resolution", () => {
]);
});

it("creates a single-quoted root and resolves the shell's JSON-quoted action through its binding", async () => {
const { result, executed } = await runBoundAction({
code: "function App(){ useQuery(tools['cloudflare-bindings']('prod').query.queryOptions({})); return <div/>; }",
available: [conn("cloudflare-bindings", "org", "shared")],
connections: { prod: "cloudflare-bindings.org.shared" },
action: { code: 'return await tools["cloudflare-bindings"]("prod").query({})' },
});
expect(result.isError).toBeFalsy();
expect(executed).toEqual(['return await tools["cloudflare-bindings"].org.shared.query({})']);
});

it("routes each role to its own connection", async () => {
const roleCode = `function App(){
useQuery(tools.linear("prod").issues.list.queryOptions({}));
Expand Down