Skip to content

Commit 417cbc3

Browse files
authored
fix(computer-use): show file panels and fresh looks in the same call (#7065)
Two round trips cost an extra call on most saves and refusals: - A Save or Open panel that the agent's own step opened came back with the step as file_panel_approval_required. The agent then had to observe again so the broker could apply the organization's file rule. That rule now runs on the step's own look: by default the panel is shown in the same receipt, an organization that asks still asks, and one that blocks still keeps the agent out. - An act the helper refused before any step started (a dialog opened, the window changed or another window came in front), or one refused because the helper session was renewed, came back with no look. These now carry a fresh observation, as refusals inside a batch already did, so the next call can act. Nothing refused is ever sent again. Signed-off-by: Jalil <reachjalil@gmail.com>
1 parent 5f5d307 commit 417cbc3

2 files changed

Lines changed: 101 additions & 12 deletions

File tree

‎apps/desktop/electron/computer-control.test.mjs‎

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,17 @@ test("follow-ups and idle cleanup renew without approvals or replaying old input
100100
await f.control.endTurn({ sessionId: "chat" });
101101
assert.equal(payload(await f.control.execute({ sessionId: "chat", name: "computer_observe", args: {} })).ok, true);
102102
await new Promise(resolve => setTimeout(resolve, 35));
103+
const looks = f.calls.filter(name => name === "computer_observe").length;
103104
const result = await f.control.execute({ sessionId: "chat", name: "computer_act", args: { observation_id: "old", action: { type: "click", x: 1, y: 1 } } });
104105
assert.equal(payload(result).code, "observation_required");
105106
assert.equal(f.calls.filter(name => name === "computer_open_session").length, 3);
106107
assert.ok(!f.calls.includes("computer_act"));
107108
assert.deepEqual(f.selected, []);
109+
// The refusal carries a look at the renewed window, so the next call can act on it.
110+
const [, post, view] = result.content.map(part => JSON.parse(part.text));
111+
assert.deepEqual(post, { post_observation: { ok: true }, actions_replayed: false, next: "act", note: "Nothing was sent. Act on this fresh observation." });
112+
assert.equal(view.observation_id, "observation");
113+
assert.equal(f.calls.filter(name => name === "computer_observe").length, looks + 1);
108114
});
109115

110116
test("human takeover survives end-of-turn and idle cleanup until Continue", async t => {
@@ -507,6 +513,14 @@ test("an act refused before anything was sent returns a fresh observation instea
507513
await act({ action: { type: "key", key: "return" }, screenshot: false });
508514
assert.equal(seen[1]?.args.include_image, false, "the model's screenshot choice still holds");
509515

516+
// A refusal the helper gives before any step starts carries no step status: it gets a look too.
517+
refusal = { ok: false, code: "observation_required", message: "A dialog opened or closed since the last observation. Observe again; nothing was sent.", next: "observe" };
518+
await act({ action: { type: "press", ref: "e2" } });
519+
assert.deepEqual(seen.map((call) => call.name), ["computer_act", "computer_observe"], "a dialog the last step opened is looked at in this call");
520+
refusal = { ok: false, code: "protected_input", message: "Take over for this step.", next: "human_takeover" };
521+
await act({ action: { type: "type", text: "x" } });
522+
assert.deepEqual(seen.map((call) => call.name), ["computer_act"], "a refusal that hands over gets no look");
523+
510524
refusal = { ...unsent, code: "scope_denied", next: "open_session" };
511525
await act({ action: { type: "click", x: 1, y: 1 } });
512526
assert.deepEqual(seen.map((call) => call.name), ["computer_act"], "only refusals that ask for a new look get one");
@@ -1567,18 +1581,22 @@ test("the agent saves what it sees as an image under the same file rule as save_
15671581
/** A Mac helper with an Open panel in front: it asks before showing the panel, and picks the file once its path is approved. */
15681582
function openPanelFixture(options = {}) {
15691583
const seen = [];
1584+
// As the helper does, a panel once allowed stays allowed until it closes; picking a file closes it.
1585+
let open = false, allowed = false;
15701586
const f = fixture({ seen, platform: "darwin", ...options, toolResult: (name) => {
15711587
const args = seen.at(-1).args;
1572-
const looks = seen.filter((call) => call.name === "computer_observe").length;
1573-
if (name === "computer_observe" && looks > 1 && args.file_panel_access !== 31) {
1574-
return text({ ok: false, code: "file_panel_approval_required", app: "Photoshop", panel_id: 31, dispatched: false, next: "observe" });
1588+
if (name === "computer_observe" && open && !allowed) {
1589+
if (args.file_panel_access !== 31) return text({ ok: false, code: "file_panel_approval_required", app: "Photoshop", panel_id: 31, dispatched: false, next: "observe" });
1590+
allowed = true;
15751591
}
15761592
if (name !== "computer_act") return undefined;
1577-
return text(JSON.stringify(args.file_approval) === JSON.stringify([args.action.path])
1578-
? { ok: true, status: "dispatched", path: "file_chosen" }
1593+
const chosen = JSON.stringify(args.file_approval) === JSON.stringify([args.action.path]);
1594+
if (chosen) open = false;
1595+
return text(chosen ? { ok: true, status: "dispatched", path: "file_chosen" }
15791596
: { ok: false, code: "files_approval_required", files: [args.action.path], open: true, app: "Photoshop", dispatched: false, next: "observe" });
15801597
} });
1581-
const look = () => f.control.execute({ sessionId: "chat", name: "computer_observe", args: {} });
1598+
// Each explicit look is at a panel the app has just opened.
1599+
const look = () => { open = true; allowed = false; return f.control.execute({ sessionId: "chat", name: "computer_observe", args: {} }); };
15821600
const pick = (path) => f.control.execute({ sessionId: "chat", name: "computer_act", args: { observation_id: "observation", action: { type: "open_in_panel", path } } });
15831601
const cards = () => f.requests.filter((request) => request?.reason === "files");
15841602
const acts = () => seen.filter((call) => call.name === "computer_act").map((call) => call.args.file_approval ?? null);
@@ -1589,6 +1607,46 @@ function openPanelFixture(options = {}) {
15891607
const answer = async (count, reply) => f.control.answerRequest({ sessionId: "chat", answer: reply, id: (await card(count)).id });
15901608
return { ...f, look, pick, cards, acts, card, answer };
15911609
}
1610+
/** A Mac helper whose next act opens a Save panel: looks after it ask first, as the helper does for any file panel. */
1611+
function savePanelFixture(movePolicy) {
1612+
const seen = [];
1613+
let opened = false;
1614+
const f = fixture({ seen, platform: "darwin", movePolicy, toolResult: (name) => {
1615+
const args = seen.at(-1).args;
1616+
if (name === "computer_act") { opened = true; return text({ ok: true, status: "dispatched", completed: 1, next: "observe", steps: [{ index: 0, action: "key", status: "dispatched", path: "menu_for_shortcut" }] }); }
1617+
if (name === "computer_observe" && opened && args.file_panel_access !== 31) {
1618+
return text({ ok: false, code: "file_panel_approval_required", app: "TextEdit", panel_id: 31, dispatched: false, next: "observe" });
1619+
}
1620+
return undefined;
1621+
} });
1622+
const looks = () => seen.filter((call) => call.name === "computer_observe").map((call) => call.args.file_panel_access ?? null);
1623+
const save = () => f.control.execute({ sessionId: "chat", name: "computer_act", args: { observation_id: "observation", action: { type: "key", key: "s", modifiers: ["command"] } } });
1624+
const open = () => f.control.setFeatureEnabled(true).then(() => f.control.execute({ sessionId: "chat", name: "computer_open", args: { app_id: "editor", mode: "control", purpose: "Save the letter" } }));
1625+
return { ...f, looks, save, open };
1626+
}
1627+
1628+
test("a Save panel the agent's own step opened is shown in that step's receipt, with no second look", async (t) => {
1629+
const asked = [];
1630+
const f = savePanelFixture(async (input) => { asked.push(input); return { effect: "allow" }; }); t.after(() => f.control.reset(true));
1631+
await f.open();
1632+
const before = f.looks().length;
1633+
const parts = (await f.save()).content.map((part) => JSON.parse(part.text));
1634+
assert.deepEqual(parts[1], { post_observation: { ok: true }, actions_replayed: false }, "the panel is looked at in the same call");
1635+
assert.deepEqual(f.looks().slice(before), [null, 31], "the second look carries the panel the organization's rule allowed");
1636+
assert.deepEqual(asked, [{ kind: "computer_files", sessionId: "chat" }]);
1637+
assert.deepEqual(f.requests.filter((request) => request?.reason === "files"), [], "by default nothing asks");
1638+
});
1639+
1640+
test("an organization that blocks file panels still keeps the agent out of one its step opened", async (t) => {
1641+
const f = savePanelFixture(async () => ({ effect: "deny", message: "Blocked here." })); t.after(() => f.control.reset(true));
1642+
await f.open();
1643+
const before = f.looks().length;
1644+
const parts = (await f.save()).content.map((part) => JSON.parse(part.text));
1645+
assert.equal(parts[1].post_observation.code, "file_panel_approval_required");
1646+
assert.equal(parts.length, 2, "no view of the panel");
1647+
assert.deepEqual(f.looks().slice(before), [null], "never looked at with access");
1648+
});
1649+
15921650
const openForPlacing = (f) => f.control.setFeatureEnabled(true).then(() => f.control.execute({ sessionId: "chat", name: "computer_open", args: { app_id: "editor", mode: "control", purpose: "Place the map" } }));
15931651

15941652
test("an Open panel is looked through and a file picked in it without a card by default", async (t) => {

‎packages/computer-use/computer-control.mjs‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ const withPostObservation = (receipt, status, content = [], extra = {}) => ({ ..
4242
{ type: "text", text: JSON.stringify({ post_observation: status, actions_replayed: false,
4343
...(!status.ok ? { next: status.next, fresh_observation_required: true } : {}), ...extra }) }, ...content] });
4444
const COORDINATE_ACTIONS = new Set(["move", "click", "double_click", "triple_click", "scroll", "drag"]);
45+
/** Refusals before any input that a fresh look answers: the receipt carries that look, so the next call can act. */
46+
const LOOK_AGAIN = new Set(["observation_required", "stale_observation", "focus_lost", "window_not_frontmost"]);
4547
const actionEntries = (args) => [args.action, ...(Array.isArray(args.actions) ? args.actions : [])].filter((entry) => entry && typeof entry === "object");
4648
/** Command-C, Command-X or Command-V: a copy, cut or paste through the agent's clipboard. */
4749
const isClipboardStep = (entry) => entry.type === "key" && typeof entry.key === "string" && ["c", "x", "v"].includes(entry.key.toLowerCase())
@@ -1100,7 +1102,19 @@ export function createComputerControl({ adapters, runProgram, runScript, onTimin
11001102
if (["background", "foreground"].includes(state.delivery)) current.delivery = state.delivery;
11011103
invalidateObservation(current); current.needsObservation = true;
11021104
updateSession(current, opened, args.purpose);
1103-
if (["computer_act", "computer_zoom"].includes(name)) return failure("observation_required", "The internal session was renewed. Observe the current window before input; nothing was dispatched.", { next: "observe", actions_replayed: false });
1105+
if (["computer_act", "computer_zoom"].includes(name)) {
1106+
const refused = failure("observation_required", "The internal session was renewed. Observe the current window before input; nothing was dispatched.", { next: "observe", actions_replayed: false });
1107+
// The model would observe next: it gets that look now, in this call, as after any refusal before input.
1108+
const look = await current.transport.callTool("computer_observe", { session_id: current.nativeId }, { signal: active });
1109+
progress();
1110+
const seen = stateOf(look);
1111+
if (look.isError || seen?.ok !== true || typeof seen.observation_id !== "string" || needsPerson(look)) return refused;
1112+
updateSession(current, look);
1113+
current.needsObservation = false;
1114+
const listed = listChanges(null, look.content);
1115+
current.listing = listed.listing;
1116+
return withPostObservation(refused, { ok: true }, listed.content, { next: "act", note: "Nothing was sent. Act on this fresh observation." });
1117+
}
11041118
}
11051119
if (!["computer_discover", "computer_open"].includes(name) && !current.nativeId) return failure("session_required", "No session is open. Call computer_open first. Nothing was dispatched.");
11061120
if (name === "computer_open") {
@@ -1141,8 +1155,11 @@ export function createComputerControl({ adapters, runProgram, runScript, onTimin
11411155
// The helper refused before sending anything and asks for a new look, such as when
11421156
// the window image changed: the model would observe next, so it gets that look now.
11431157
// The refused input is never retried; the model decides again from the fresh view.
1144-
const refusedUnsent = name === "computer_act" && !call.program && nativeResult !== undefined && state.ok === false
1145-
&& state.status === "failed" && state.completed === 0 && state.may_have_acted === false && state.next === "observe";
1158+
// So does a refusal the helper gives before any step starts (it carries no step status):
1159+
// a dialog opened, the window changed, or another window came to the front.
1160+
const refusedUnsent = name === "computer_act" && !call.program && nativeResult !== undefined && state.ok === false && state.next === "observe"
1161+
&& (state.status === "failed" && state.completed === 0 && state.may_have_acted === false
1162+
|| state.status === undefined && state.may_have_acted === undefined && LOOK_AGAIN.has(state.code));
11461163
if (current.nativeId && (inputLanded || refusedUnsent || name === "computer_open" && !result.isError && state.ok && state.state === "active")) {
11471164
call.post = { current, generation, nativeId: current.nativeId, receipt: result, captured: false, refused: refusedUnsent,
11481165
observationGeneration: name === "computer_open" ? current.observationGeneration : observationGeneration };
@@ -1175,12 +1192,26 @@ export function createComputerControl({ adapters, runProgram, runScript, onTimin
11751192
// continuation across a person takeover. Watch frames are unrelated.
11761193
// Unless the model chose, the computer decides whether an image adds anything.
11771194
// Opening an app also lists its menu-bar commands, so the agent can run one at once.
1178-
const observation = await current.transport.callTool("computer_observe", { session_id: current.nativeId,
1195+
const look = { session_id: current.nativeId,
11791196
...(call.postImage === undefined ? {} : { include_image: call.postImage }),
1180-
...(name === "computer_open" && platform === "darwin" && current.adapter?.placement === "local" && !call.program ? { menus: true } : {}) }, { signal: active });
1197+
...(name === "computer_open" && platform === "darwin" && current.adapter?.placement === "local" && !call.program ? { menus: true } : {}) };
1198+
let observation = await current.transport.callTool("computer_observe", look, { signal: active });
11811199
progress(); assertUiActive(current);
11821200
if (!postStillOwned(call)) return postWarning(result, "observation_withheld", "human_takeover");
1183-
const observed = stateOf(observation);
1201+
let observed = stateOf(observation);
1202+
// The action opened a file panel: the organization's rule decides here, as it does for the
1203+
// model's own look, so the agent sees the panel in this same call. By default nothing asks.
1204+
if (observed?.code === "file_panel_approval_required" && observed.dispatched === false && Number.isSafeInteger(observed.panel_id)) {
1205+
const checked = await approveMove(grant, { kind: "files", app: typeof observed.app === "string" ? observed.app : "the app", names: [] }, signal);
1206+
progress(); assertUiActive(current);
1207+
if (!postStillOwned(call)) return postWarning(result, "observation_withheld", "human_takeover");
1208+
if (checked.answer === "allow") {
1209+
observation = await current.transport.callTool("computer_observe", { ...look, file_panel_access: observed.panel_id }, { signal: active });
1210+
progress(); assertUiActive(current);
1211+
if (!postStillOwned(call)) return postWarning(result, "observation_withheld", "human_takeover");
1212+
observed = stateOf(observation);
1213+
}
1214+
}
11841215
if (observation.isError || observed?.ok !== true || typeof observed.observation_id !== "string" || !observed.observation_id || needsPerson(observation)) {
11851216
current.needsObservation = true;
11861217
nativeStopped(current, observation); updateSession(current, observation);

0 commit comments

Comments
 (0)