Skip to content

Commit 4190adb

Browse files
snlr308Copilot
andcommitted
Fix some fundamental flaws
Co-authored-by: Copilot <copilot@github.com>
1 parent 83dd510 commit 4190adb

23 files changed

Lines changed: 804 additions & 381 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ npm run generate:manifest
607607
- **POST, not PUT.** All updates use POST. The API does not support PUT.
608608
- **Custom credentials header.** `credentials: username:password` (raw, not base64, not standard Basic Auth).
609609
- **Eventual consistency.** After creating or updating entities, changes may take up to 3 minutes to propagate through the API cache.
610-
- **GET /setting limited.** Only works at merchant and channel level. POST /setting works at all levels (PSP, division, merchant, channel).
610+
- **GET /setting limited.** Only works at merchant and channel level. POST /setting works at all levels (PSP, division, merchant, channel). Settings are inherited, so an empty value means "not set here" rather than "off". The extension resolves effective values upward where the API permits it, e.g. channel -> merchant. If the chain reaches division or PSP, the effective value is unknown through the API; the result includes the known default and the agent can use context binding to inspect the dashboard manually when the user needs the true operational value.
611611

612612
### Settings coverage
613613

background/service-worker.ts

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,14 @@
1111

1212
import { swStartJob, swPauseJob, swCancelJob, swCancelJobById, swGetActiveJobId, type SwJobStartInput } from "./sw-job-executor";
1313
import { clearChatContext, upsertChatContext } from "../src/chat/context-store";
14-
import { createExecuteMap } from "../src/tools/internal-router";
14+
import { createExecuteMap, resolveSession } from "../src/tools/internal-router";
1515
import { TOOL_SCHEMAS } from "../src/webmcp/tool-schemas";
1616
import type { ToolSchema } from "../src/webmcp/tool-schemas";
1717
import type { EntityType } from "../src/lib/entity-types";
18+
import type { WritePreview } from "../src/bridge/confirm-bridge";
19+
import { buildWebMcpWritePreview, isWebMcpReadOnlyInvocation } from "../src/webmcp/execution-policy";
1820

19-
const WEBMCP_EXECUTE_MAP = createExecuteMap();
20-
21-
const WEBMCP_READ_ACTIONS: Record<string, Set<string>> = {
22-
manage_settings: new Set(["get", "batch_get", "list_non_default"]),
23-
};
24-
25-
function isWebMcpReadOnlyInvocation(tool: string, params: Record<string, unknown>): boolean {
26-
const schema = TOOL_SCHEMAS.find((entry) => entry.name === tool);
27-
if (!schema) return false;
28-
if (schema.annotations?.readOnlyHint === true) return true;
29-
30-
const allowedActions = WEBMCP_READ_ACTIONS[tool];
31-
if (!allowedActions) return false;
32-
const action = params.action;
33-
return typeof action === "string" && allowedActions.has(action);
34-
}
21+
const WEBMCP_EXECUTE_MAP = createExecuteMap({ bypassWriteConfirmation: true });
3522

3623
// -- Side panel activation ------------------------------------------------
3724

@@ -135,6 +122,7 @@ export interface WebMcpExecuteToolMessage {
135122
payload: {
136123
tool: string;
137124
params?: Record<string, unknown>;
125+
confirmed?: boolean;
138126
};
139127
}
140128

@@ -278,20 +266,23 @@ chrome.runtime.onMessage.addListener(
278266

279267
async function handleWebMcpExecuteTool(
280268
payload: WebMcpExecuteToolMessage["payload"],
281-
): Promise<{ ok: boolean; result?: string; error?: string }> {
269+
): Promise<{ ok: boolean; result?: string; error?: string; needsConfirmation?: boolean; preview?: WritePreview }> {
282270
const params = payload.params ?? {};
283271
const execute = WEBMCP_EXECUTE_MAP[payload.tool];
284272
if (!execute) {
285273
return { ok: false, error: `Unknown tool: ${payload.tool}` };
286274
}
287275

288276
if (!isWebMcpReadOnlyInvocation(payload.tool, params)) {
289-
return {
290-
ok: false,
291-
error:
292-
"WebMCP write execution is temporarily disabled while the service-worker confirmation path is implemented. " +
293-
"Read tools are available; use the side panel or Chat tab for confirmed writes for now.",
294-
};
277+
const session = await resolveSession();
278+
if (!session) {
279+
return { ok: false, error: "Session not unlocked. Open the side panel and enter your PIN first." };
280+
}
281+
282+
const preview = buildWebMcpWritePreview(payload.tool, params, session.env);
283+
if (preview && payload.confirmed !== true) {
284+
return { ok: false, needsConfirmation: true, preview };
285+
}
295286
}
296287

297288
const result = await execute(params);

background/sw-job-executor.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ async function flushProgress(jobId: string, force = false) {
5151

5252
// -- SDK facade for SW (no confirm bridge) --------------------------------
5353

54-
function buildSwSdk(creds: ApiCredentials, env: Environment, writes: WriteRecord[]) {
55-
const ctx: SdkContext = { creds, env };
54+
function buildSwSdk(creds: ApiCredentials, env: Environment, writes: WriteRecord[], signal?: AbortSignal, throttleRate?: number) {
55+
const ctx: SdkContext = { creds, env, signal, throttleRate };
5656
const virtualSdk = createSdk(ctx);
5757

5858
function recordWrite(
@@ -355,7 +355,7 @@ async function executeInSw(jobId: string, creds: ApiCredentials, env: Environmen
355355
const results: unknown[] = [];
356356
const writes: WriteRecord[] = [];
357357

358-
const sdk = buildSwSdk(creds, env, writes);
358+
const sdk = buildSwSdk(creds, env, writes, signal, job.throttleRate);
359359

360360
const consoleProxy = {
361361
log: (...args: unknown[]) => logs.push({ level: "log", args, timestamp: new Date().toISOString() }),

content/bridge.ts

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,51 @@
1010
* script via chrome.scripting.executeScript (bypasses page CSP).
1111
*/
1212

13+
interface BridgeResponse {
14+
ok?: boolean;
15+
result?: string;
16+
error?: string;
17+
needsConfirmation?: boolean;
18+
preview?: {
19+
tool: string;
20+
action: string;
21+
method: "POST" | "DELETE";
22+
description: string;
23+
params: Record<string, unknown>;
24+
env: "uat" | "prod";
25+
};
26+
}
27+
28+
async function sendExecuteTool(
29+
tool: string,
30+
params: Record<string, unknown>,
31+
confirmed = false,
32+
): Promise<BridgeResponse | undefined> {
33+
return chrome.runtime.sendMessage({
34+
type: "webmcp:execute-tool",
35+
payload: { tool, params, confirmed },
36+
}) as Promise<BridgeResponse | undefined>;
37+
}
38+
39+
function confirmWrite(preview: NonNullable<BridgeResponse["preview"]>): boolean {
40+
const prettyParams = Object.keys(preview.params).length > 0
41+
? JSON.stringify(preview.params, null, 2)
42+
: "(none)";
43+
44+
const message = [
45+
`Confirm write (${preview.env.toUpperCase()})`,
46+
"",
47+
preview.description,
48+
"",
49+
`Method: ${preview.method}`,
50+
`Tool: ${preview.tool}/${preview.action}`,
51+
"",
52+
`Params: ${prettyParams}`,
53+
].join("\n");
54+
55+
return window.confirm(message);
56+
}
57+
1358
// -- Message listener (main world -> isolated world) ----------------------
1459

1560
window.addEventListener("message", async (event: MessageEvent) => {
@@ -24,10 +69,19 @@ window.addEventListener("message", async (event: MessageEvent) => {
2469
};
2570

2671
try {
27-
const response = await chrome.runtime.sendMessage({
28-
type: "webmcp:execute-tool",
29-
payload: { tool, params },
30-
}) as { ok?: boolean; result?: string; error?: string } | undefined;
72+
let response = await sendExecuteTool(tool, params);
73+
74+
if (response?.needsConfirmation && response.preview) {
75+
if (!confirmWrite(response.preview)) {
76+
window.postMessage({
77+
type: "webmcp:tool-result",
78+
callId,
79+
error: "Operation cancelled by user.",
80+
}, "*");
81+
return;
82+
}
83+
response = await sendExecuteTool(tool, params, true);
84+
}
3185

3286
if (!response?.ok) {
3387
window.postMessage({

content/register-main.ts

Lines changed: 0 additions & 123 deletions
This file was deleted.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"test": "vitest run",
1717
"test:watch": "vitest",
1818
"test:live": "WEBAPI_TEST_LIVE=1 vitest run --dir src --testNamePattern=. src/**/*.live.test.ts",
19+
"smoke:webmcp": "node tools-cli/webmcp-smoke.mjs",
1920
"verify:tokens": "node tools-cli/token-benchmark.mjs",
2021
"verify:security": "node tools-cli/security-verify.mjs",
2122
"generate:manifest": "node tools-cli/generate-manifest.mjs",

sidepanel/index.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import React from "react";
22
import ReactDOM from "react-dom/client";
33
import { App } from "./App";
4-
import { registerAllTools } from "../src/webmcp/register-tools";
5-
6-
// Register WebMCP tools with retry (handles async navigator.modelContext injection)
7-
registerAllTools();
84

95
// Tab-close job pausing is handled directly by the service worker (sw-job-executor).
106
// The side panel monitors job state via chrome.storage change events (see job-runner.ts).

src/bridge/write-confirm-utils.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/**
22
* Shared utilities for write confirmation across bridge.ts and register-tools.ts.
33
*
4-
* Extracted to avoid duplication of describeDirectWrite and related helpers
5-
* which had identical copies in content/bridge.ts and src/webmcp/register-tools.ts.
4+
* Keeps write descriptions consistent across Chat, side panel, sandbox, and WebMCP paths.
65
*/
76

87
import type { Environment } from "../lib/types";
@@ -53,6 +52,21 @@ export function describeDirectWrite(tool: string, action: string, params: Record
5352
}
5453
}
5554

55+
export function describeMutatingCall(
56+
tool: string,
57+
params: Record<string, unknown>,
58+
): { action: string; method: "POST" | "DELETE"; description: string } | null {
59+
const actions = MUTATING_ACTIONS[tool];
60+
if (!actions) return null;
61+
const action = params.action as string;
62+
if (!actions.has(action)) return null;
63+
return {
64+
action,
65+
method: httpMethod(action),
66+
description: describeDirectWrite(tool, action, params),
67+
};
68+
}
69+
5670
/**
5771
* Check whether a tool call is mutating; if so, request user confirmation.
5872
* Returns the write description if the call was mutating and confirmed,
@@ -62,21 +76,18 @@ export function describeDirectWrite(tool: string, action: string, params: Record
6276
export async function confirmIfMutating(
6377
tool: string, params: Record<string, unknown>, env: Environment
6478
): Promise<string | undefined> {
65-
const actions = MUTATING_ACTIONS[tool];
66-
if (!actions) return undefined;
67-
const action = params.action as string;
68-
if (!actions.has(action)) return undefined;
79+
const mutatingCall = describeMutatingCall(tool, params);
80+
if (!mutatingCall) return undefined;
6981

70-
const description = describeDirectWrite(tool, action, params);
7182
const preview: WritePreview = {
7283
tool,
73-
action,
74-
method: httpMethod(action),
75-
description,
84+
action: mutatingCall.action,
85+
method: mutatingCall.method,
86+
description: mutatingCall.description,
7687
params,
7788
env,
7889
};
7990
const choice = await requestConfirm(preview);
8091
if (choice === "cancel") throw new Error("Operation cancelled by user.");
81-
return description;
92+
return mutatingCall.description;
8293
}

src/chat/discovery-playbook.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export function buildChatSystemPrompt(options: ChatSystemPromptOptions = {}): st
5858
: "Never attempt writes or code execution.",
5959
...PLAYBOOK.principles,
6060
"Entities with state DISABLED are soft-deleted. List and hierarchy tools automatically hide them unless includeDisabled=true. If the result includes _hiddenDisabled, mention how many were hidden. Do not count or list disabled entities unless the user explicitly asks about deleted items.",
61+
"For settings results with source: unknown, state the API limitation briefly and include the default value if present. Do not offer UI inspection from Chat Tab.",
6162
"Discovery playbook:",
6263
...PLAYBOOK.playbooks.map((entry) => `${entry.trigger}: ${entry.steps.join(" ")}`),
6364
"Response style:",

0 commit comments

Comments
 (0)