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
2 changes: 1 addition & 1 deletion electron/ai-edition/agent-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1769,7 +1769,7 @@ describe("moveClip — the tool the prompt used to promise", () => {

// ── D-CONSENT ───────────────────────────────────────────────────────────────
//
// Settings offers "Project edits — when off, the agent must ask before changing
// Settings offers "Agent edits — when off, the agent must ask before changing
// the timeline". `chat-service.ts` computed `editsAllowed` and then did
// `void editsAllowed;`. The flag reached nothing: not the tools, not the prompt.
describe("allowAgentEdits=false refuses every write", () => {
Expand Down
18 changes: 14 additions & 4 deletions electron/ai-edition/agent-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,9 @@ function clipRoster(document: AxcutDocument): string {
return `The timeline is: ${clips.map((c) => `${c.id}${c.reason ? ` (${c.reason})` : ""}`).join(", ")}.`;
}

/** The in-app agent's edit switch, as AI settings label it (`providerSettings.projectEditsLabel`). */
export const AGENT_EDITS_SETTING = "'Agent edits' in AI settings";

/**
* The refusal the model gets for every write while `allowAgentEdits` is off.
*
Expand All @@ -1215,22 +1218,29 @@ function clipRoster(document: AxcutDocument): string {
*
* The `{error}` envelope is the shape the workbench's wire oracle already
* recognises as a failed tool result — a new one would read as a success.
*
* `setting` names the switch as the user sees it: the in-app agent's by default,
* the MCP server's when an MCP client is refused (`openscreen-mcp-server.ts`).
*/
function consentRequired(name: string, args: unknown): AgentToolExecution {
export function consentRequired(
name: string,
args: unknown,
setting: string = AGENT_EDITS_SETTING,
): AgentToolExecution {
return {
ok: false,
resultJson: JSON.stringify({
error:
"Project edits are turned off for this project: the user asked to be consulted " +
"before the timeline changes. Nothing was modified.",
"Edits are turned off: the user asked to be consulted before the timeline changes. " +
"Nothing was modified.",
code: "consent_required",
tool: name,
requestedArgs: args,
howToProceed:
"Describe the exact edit you would make — the tool, the times, the ids — and ask the " +
"user to confirm it. Do NOT retry this call and do NOT reach for another write tool: " +
"every one of them is refused while the setting is off. Never say an edit was applied. " +
"If they want you to go ahead, they can re-enable 'Project edits' in Settings → AI.",
`If they want you to go ahead, they can turn on ${setting}.`,
}),
};
}
Expand Down
2 changes: 1 addition & 1 deletion electron/ai-edition/chat-service.toolloop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ describe("runChat tool loop", () => {

// ── D-CONSENT ───────────────────────────────────────────────────────────────
//
// `allowAgentEdits` is offered in Settings as "Project edits — when off, the
// `allowAgentEdits` is offered in Settings as "Agent edits — when off, the
// agent must ask before changing the timeline". `runChat` computed
// `const editsAllowed = config.allowAgentEdits !== false;` and then, twenty
// lines later, `void editsAllowed;`. That was every use of it: it reached
Expand Down
16 changes: 16 additions & 0 deletions electron/ai-edition/deep-agent/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,22 @@ describe("the prompt when the user has turned project edits off", () => {
// selection rules the model needs to describe the edit it is proposing.
expect(buildSystemPrompt({ editsAllowed: false }).startsWith(SYSTEM_PROMPT)).toBe(true);
});

it("names the switch in force: the agent's by default, the caller's when it has its own", () => {
expect(buildSystemPrompt({ editsAllowed: false })).toContain("'Agent edits' in AI settings");
const mcp = buildSystemPrompt({ editsAllowed: false, editsSetting: "'MCP edits' over there" });
expect(mcp).toContain("'MCP edits' over there");
expect(mcp).not.toContain("Agent edits");
});
});

describe("the prompt for a caller that can export", () => {
it("no longer gives exporting as the example of a missing tool", () => {
expect(SYSTEM_PROMPT).toMatch(/deleting an asset\/recording, exporting\)/);
const prompt = buildSystemPrompt({ editsAllowed: true, canExport: true });
expect(prompt).not.toMatch(/exporting/);
expect(prompt).toMatch(/deleting an asset\/recording\)/);
});
});

describe("the tools when the user has turned project edits off", () => {
Expand Down
46 changes: 31 additions & 15 deletions electron/ai-edition/deep-agent/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type { AxcutDocument } from "../../../src/lib/ai-edition/schema";
import { ZOOM_DEPTH_LEGEND } from "../../../src/lib/ai-edition/timeline/zoom-scale";
import { ZOOM_TRANSITION_LEGEND } from "../../../src/lib/ai-edition/timeline/zoom-transitions";
import {
AGENT_EDITS_SETTING,
type AgentToolExecution,
addAnnotationArgs,
addAudioArgs,
Expand Down Expand Up @@ -83,7 +84,8 @@ export interface OpenScreenAgentSink {
// inline read-tool schemas below stay local.

/**
* The block appended when the user has turned "Project edits" off.
* The block appended when the user has turned edits off: "Agent edits" for the
* in-app agent, "MCP edits" for an MCP client; `setting` names the one in force.
*
* ponytail: the executor's refusal is the wall (see `consentRequired` in
* agent-tools.ts) but a wall alone does not fix the measured defect. The
Expand All @@ -93,14 +95,15 @@ export interface OpenScreenAgentSink {
* rightly: the model tried. Only the prompt can stop the emission. Two layers,
* neither sufficient alone.
*/
const CONSENT_PROMPT_BLOCK = [
"",
"PROJECT EDITS ARE CURRENTLY DISABLED by the user, who asked to be consulted before the timeline changes.",
"- Read freely: getCurrentDocument and getTranscript work as usual.",
"- Do NOT call any tool that writes (removeWords, addTrim, setTrim, setClipRange, moveClip, replaceTimeline, add*/set* effects, remove*). Every one of them will be refused, so calling them wastes the turn and tells the user nothing.",
"- Instead: say precisely what you would change — which tool, which times, which ids — and ask the user to confirm. Be specific enough that they can say yes to it.",
"- Never state or imply that an edit was applied. If the user confirms and you are still refused, tell them the 'Project edits' setting in Settings → AI has to be re-enabled first.",
].join("\n");
const consentPromptBlock = (setting: string) =>
[
"",
"PROJECT EDITS ARE CURRENTLY DISABLED by the user, who asked to be consulted before the timeline changes.",
"- Read freely: getCurrentDocument and getTranscript work as usual.",
"- Do NOT call any tool that writes (removeWords, addTrim, setTrim, setClipRange, moveClip, replaceTimeline, add*/set* effects, remove*). Every one of them will be refused, so calling them wastes the turn and tells the user nothing.",
"- Instead: say precisely what you would change — which tool, which times, which ids — and ask the user to confirm. Be specific enough that they can say yes to it.",
`- Never state or imply that an edit was applied. If the user confirms and you are still refused, tell them to turn on ${setting} first.`,
].join("\n");

// ponytail: exported so a test can assert that what the model receives is this
// string and NOTHING else — the deepagents regression was invisible precisely
Expand Down Expand Up @@ -134,14 +137,27 @@ const BASE_SYSTEM_PROMPT = [
"Cursor telemetry: while the screen was captured, OpenScreen recorded where the pointer went. assets[].hasCursorTelemetry in getCurrentDocument says which assets carry it, and getCursorTrack returns the recorded track for one of them — positions over time, and the pointer shape at each moment. What it means is yours to read.",
"Blindness is not evidence. When a tool reports it could not read something (reason 'unavailable'), that is a limit of your runtime, not a fact about the project. Only an explicit negative — reason 'no-sidecar', or a false flag — supports telling the user the data is not there.",
"",
"Honesty rules: if a request has NO matching tool (e.g. deleting an asset/recording, exporting), say so plainly — do not substitute a different edit and report it as the requested one. After your edits, if you are at all unsure the document ended up as intended, call getCurrentDocument and reconcile what you claim with the real state; each tool result already tells you exactly what it did, so never report a change the results don't support.",
].join("\n");

/** The system prompt for a turn, which depends on ONE thing: whether the user
* currently allows the agent to write. Exported so the consent wording can be
* tested without a provider. */
export function buildSystemPrompt(options: { editsAllowed: boolean }): string {
return options.editsAllowed ? BASE_SYSTEM_PROMPT : BASE_SYSTEM_PROMPT + CONSENT_PROMPT_BLOCK;
// Exporting is the example of a missing tool only where there is none: `openscreen mcp` has exportVideo.
const honestyRules = (canExport: boolean) =>
`Honesty rules: if a request has NO matching tool (e.g. deleting an asset/recording${canExport ? "" : ", exporting"}), say so plainly — do not substitute a different edit and report it as the requested one. After your edits, if you are at all unsure the document ended up as intended, call getCurrentDocument and reconcile what you claim with the real state; each tool result already tells you exactly what it did, so never report a change the results don't support.`;

/** The system prompt for a turn: whether the user currently allows writes, and through
* which setting. The MCP server reuses it as its instructions, naming its own setting
* and, over stdio, the export tool it serves. Exported so the wording can be tested
* without a provider. */
export function buildSystemPrompt(options: {
editsAllowed: boolean;
/** Where the user turns writes back on. Default: the in-app agent's switch. */
editsSetting?: string;
/** The caller also has a tool that exports the video. */
canExport?: boolean;
}): string {
const prompt = BASE_SYSTEM_PROMPT + honestyRules(options.canExport === true);
return options.editsAllowed
? prompt
: prompt + consentPromptBlock(options.editsSetting ?? AGENT_EDITS_SETTING);
}

/** The prompt of a normal (edits-allowed) turn — the overwhelmingly common
Expand Down
5 changes: 3 additions & 2 deletions electron/ipc/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5029,10 +5029,11 @@ export function registerIpcHandlers(
// dependencies live, but NOT started here: the headless CLI shares this
// function and must never bind the port a running app is listening on.
// `main.ts` starts it. Its writes have their own switch, off by default and
// read on every call — separate from the in-app agent's "Project edits", so
// turning the server on grants a client read access and nothing more.
// read on every call: "MCP edits", separate from the in-app agent's "Agent edits",
// so turning the server on grants a client read access and nothing more.
const mcpSettings = new McpSettingsStore(app.getPath("userData"), safeStorage);
const mcpToolDeps: McpToolDeps = {
transport: "http",
host: new EditorDocumentHost(ipcMain),
projects: aiEditionDocuments,
editsAllowed: () => mcpSettings.getSettings().allowEdits,
Expand Down
1 change: 1 addition & 0 deletions electron/mcp/mcp-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const fakeCrypto: McpTokenCrypto = {
};

const deps: McpToolDeps = {
transport: "http",
host: { snapshot: async () => null, apply: async () => "no-editor" },
projects: {
listProjects: async () => [],
Expand Down
4 changes: 2 additions & 2 deletions electron/mcp/mcp-settings-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export interface McpSettings {
enabled: boolean;
port: number;
/**
* Whether MCP clients may run the tools that change the project. Off by
* default and separate from the in-app agent's "Project edits": turning the
* Whether MCP clients may run the tools that change the project ("MCP edits"). Off by
* default and separate from the in-app agent's "Agent edits": turning the
* server on gives a client read access, and writing is a second decision.
*/
allowEdits: boolean;
Expand Down
131 changes: 131 additions & 0 deletions electron/mcp/media-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ beforeEach(async () => {
openVideoFails = false;

const toolDeps: McpToolDeps = {
transport: "stdio",
host: { snapshot: async () => null, apply: async () => "no-editor" },
projects: documents,
editsAllowed: () => true,
Expand Down Expand Up @@ -175,6 +176,49 @@ async function recordTake(title = "Demo") {
};
}

describe("what `openscreen mcp` tells its client", () => {
it("composes the media block with the checkpoint rule, and nothing true only in the app", () => {
const instructions = client.getInstructions() ?? "";
expect(instructions.startsWith(MEDIA_PREAMBLE)).toBe(true);
expect(instructions).toContain("Before a series of edits, call createCheckpoint");
expect(instructions).toContain("lost when it exits");
expect(instructions).not.toMatch(
/project open in the editor|Project edits|Agent edits|Nothing here records|undo step|exporting\)/,
);
});

it("describes projectId and restoreCheckpoint as they work here", async () => {
const { tools } = await client.listTools();
const byName = new Map(tools.map((tool) => [tool.name, tool]));
const projectId = byName.get("addTrim")?.inputSchema.properties?.projectId as {
description?: string;
};
expect(projectId.description).toContain("Pass it on every call");
const restore = byName.get("restoreCheckpoint")?.description ?? "";
expect(restore).toContain("pointer track");
expect(restore).not.toContain("undo step");
});

it("answers in its own terms: no editor to fall back on, no Ctrl/Cmd+Z, its own lifetime", async () => {
const noProject = await call("getCurrentDocument");
expect(textOf(noProject)).toContain("this server has no editor");
expect(textOf(noProject)).not.toContain("No project is open");

const other = await documents.createProject("Other");
const unguarded = await call("setAspectRatio", {
projectId: other.project.id,
aspectRatio: "1:1",
});
expect(textOf(unguarded)).toContain("straight to the project's file");

const unknown = await call("restoreCheckpoint", {
projectId: other.project.id,
checkpointId: "cp_nope",
});
expect(textOf(unknown)).toContain("lost when it exits");
});
});

describe("frameTimes", () => {
it("centres `count` frames in equal slices, and clamps asked-for times inside the video", () => {
expect(frameTimes(12, null, 4)).toEqual([1.5, 4.5, 7.5, 10.5]);
Expand Down Expand Up @@ -337,6 +381,93 @@ describe("setPointerTrack", () => {
expect(click?.cx).toBe(0.6);
});

/** The sidecar beside the take, byte for byte; `null` when there is none. */
const sidecarFile = (video: string) =>
fs.readFile(`${video}.cursor.json`, "utf8").catch(() => null);
const clickAt = (take: { recordingStartedAt: number }, x: number) => [
{ at: take.recordingStartedAt + 5_000, x, y: 0.5, click: true },
];

it("refuses a project open in the app's editor before writing anything", async () => {
const take = await recordTake();
const before = await sidecarFile(take.videoPath);
documents.markOpenInEditor(take.projectId);
const refused = await call("setPointerTrack", {
projectId: take.projectId,
events: clickAt(take, 0.2),
});
expect(refused.isError).toBe(true);
expect(textOf(refused)).toContain("open in the OpenScreen editor");
expect(await sidecarFile(take.videoPath)).toBe(before);
expect((await documents.getProject(take.projectId)).zoomRanges).toHaveLength(take.autoZooms);
});

it("refuses a project with no checkpoint before writing anything", async () => {
const take = await recordTake();
const before = await sidecarFile(take.videoPath);
// Twenty newer checkpoints push out the one stopRecording made.
const other = await documents.createProject("Other");
for (let i = 0; i < 20; i++) {
await call("createCheckpoint", { projectId: other.project.id });
}
const refused = await call("setPointerTrack", {
projectId: take.projectId,
events: clickAt(take, 0.2),
});
expect(refused.isError).toBe(true);
expect(textOf(refused)).toContain("createCheckpoint");
expect(await sidecarFile(take.videoPath)).toBe(before);
});

it("is undone by restoreCheckpoint, the take's own pointer track included", async () => {
const take = await recordTake();
const before = await sidecarFile(take.videoPath);
const set = await call("setPointerTrack", {
projectId: take.projectId,
events: clickAt(take, 0.2),
});
expect(set.isError).toBeFalsy();
expect(await sidecarFile(take.videoPath)).not.toBe(before);

const restored = await call("restoreCheckpoint", {
projectId: take.projectId,
checkpointId: take.checkpointId,
});
expect(restored.isError).toBeFalsy();
expect(await sidecarFile(take.videoPath)).toBe(before);
expect((await documents.getProject(take.projectId)).zoomRanges).toHaveLength(take.autoZooms);
});

it("takes away a track the take never had, and each checkpoint gets back its own", async () => {
await fs.rm(`${videoPath}.cursor.json`);
const take = await recordTake();
await call("setPointerTrack", { projectId: take.projectId, events: clickAt(take, 0.2) });
const first = await sidecarFile(take.videoPath);
const second = JSON.parse(
textOf(await call("createCheckpoint", { projectId: take.projectId })),
).checkpointId;
await call("setPointerTrack", { projectId: take.projectId, events: clickAt(take, 0.8) });

await call("restoreCheckpoint", { projectId: take.projectId, checkpointId: take.checkpointId });
expect(await sidecarFile(take.videoPath)).toBeNull();
await call("restoreCheckpoint", { projectId: take.projectId, checkpointId: second });
expect(await sidecarFile(take.videoPath)).toBe(first);
});

it("says the edit is partial when the project saved but the track could not be written", async () => {
await fs.rm(`${videoPath}.cursor.json`);
const take = await recordTake();
// A directory where the sidecar goes: the save succeeds, the sidecar write cannot.
await fs.mkdir(`${take.videoPath}.cursor.json`);
const set = await call("setPointerTrack", {
projectId: take.projectId,
events: clickAt(take, 0.2),
});
expect(set.isError).toBe(true);
expect(textOf(set)).toContain("The project was saved, but the pointer track");
expect(textOf(set)).not.toContain("Tool failed");
});

it("refuses a project whose take this server did not record", async () => {
const other = await documents.createProject("Imported");
const refused = await call("setPointerTrack", {
Expand Down
Loading
Loading