mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
Merge pull request #16 from NianJiuZst/codex/fail-closed-tab-borrow
fix(extension): fail closed on borrow confirmation
This commit is contained in:
@@ -62,33 +62,25 @@ function BorrowToastStack({ children }: { children: React.ReactNode }) {
|
||||
);
|
||||
}
|
||||
|
||||
/** Matches BorrowProgressBar / CountdownRing transition duration (duration-1000). */
|
||||
const PROGRESS_TRANSITION_MS = 1000;
|
||||
|
||||
function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; isModal: boolean }) {
|
||||
const { t } = useTranslation("extension");
|
||||
const totalSeconds = Math.ceil(request.timeoutMs / 1000);
|
||||
const [secondsLeft, setSecondsLeft] = useState(totalSeconds);
|
||||
const [exiting, setExiting] = useState(false);
|
||||
const [awaitingAutoAllow, setAwaitingAutoAllow] = useState(false);
|
||||
const allowedRef = useRef(false);
|
||||
const [awaitingAutoDeny, setAwaitingAutoDeny] = useState(false);
|
||||
const settledRef = useRef(false);
|
||||
const onAllowRef = useRef(request.onAllow);
|
||||
const onDenyRef = useRef(request.onDeny);
|
||||
onAllowRef.current = request.onAllow;
|
||||
onDenyRef.current = request.onDeny;
|
||||
|
||||
useEffect(() => {
|
||||
if (secondsLeft <= 0) {
|
||||
if (!allowedRef.current) {
|
||||
setAwaitingAutoAllow(true);
|
||||
}
|
||||
return;
|
||||
}
|
||||
const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000);
|
||||
return () => clearTimeout(id);
|
||||
}, [secondsLeft]);
|
||||
|
||||
function triggerAllow() {
|
||||
if (allowedRef.current) return;
|
||||
allowedRef.current = true;
|
||||
setAwaitingAutoAllow(false);
|
||||
if (settledRef.current) return;
|
||||
settledRef.current = true;
|
||||
setAwaitingAutoDeny(false);
|
||||
setExiting(true);
|
||||
setTimeout(() => onAllowRef.current(), EXIT_ANIMATION_MS);
|
||||
}
|
||||
@@ -98,13 +90,34 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i
|
||||
}
|
||||
|
||||
function handleDeny() {
|
||||
if (allowedRef.current) return;
|
||||
allowedRef.current = true;
|
||||
setAwaitingAutoAllow(false);
|
||||
if (settledRef.current) return;
|
||||
settledRef.current = true;
|
||||
setAwaitingAutoDeny(false);
|
||||
setExiting(true);
|
||||
setTimeout(() => onDenyRef.current(), EXIT_ANIMATION_MS);
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (secondsLeft <= 0) {
|
||||
if (!settledRef.current) {
|
||||
setAwaitingAutoDeny(true);
|
||||
}
|
||||
return;
|
||||
}
|
||||
const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000);
|
||||
return () => clearTimeout(id);
|
||||
}, [secondsLeft]);
|
||||
|
||||
// Prefer transitionend for visual sync, but fall back to a timer so
|
||||
// background tabs / suppressed CSS transitions still auto-deny.
|
||||
useEffect(() => {
|
||||
if (!awaitingAutoDeny || settledRef.current) return;
|
||||
const id = setTimeout(() => {
|
||||
handleDeny();
|
||||
}, PROGRESS_TRANSITION_MS);
|
||||
return () => clearTimeout(id);
|
||||
}, [awaitingAutoDeny]);
|
||||
|
||||
const progress = secondsLeft / totalSeconds;
|
||||
const truncatedTitle =
|
||||
request.tabTitle.length > 30 ? `${request.tabTitle.slice(0, 30)}…` : request.tabTitle;
|
||||
@@ -128,13 +141,13 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i
|
||||
progress={progress}
|
||||
seconds={secondsLeft}
|
||||
onProgressTransitionEnd={(propertyName) => {
|
||||
if (!awaitingAutoAllow || secondsLeft !== 0) return;
|
||||
if (!awaitingAutoDeny || secondsLeft !== 0) return;
|
||||
if (propertyName !== "stroke-dashoffset") return;
|
||||
triggerAllow();
|
||||
handleDeny();
|
||||
}}
|
||||
/>
|
||||
<span className="text-[13px] text-gray-500">
|
||||
{t("borrowConfirmation.autoAllow", { count: secondsLeft })}
|
||||
{t("borrowConfirmation.autoDeny", { count: secondsLeft })}
|
||||
</span>
|
||||
</div>
|
||||
<ActionButtons onDeny={handleDeny} onAllow={handleAllow} />
|
||||
@@ -156,9 +169,9 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i
|
||||
<BorrowProgressBar
|
||||
progress={progress}
|
||||
onProgressTransitionEnd={(event) => {
|
||||
if (!awaitingAutoAllow || secondsLeft !== 0) return;
|
||||
if (!awaitingAutoDeny || secondsLeft !== 0) return;
|
||||
if (event.propertyName !== "width") return;
|
||||
triggerAllow();
|
||||
handleDeny();
|
||||
}}
|
||||
/>
|
||||
<ActionButtons onDeny={handleDeny} onAllow={handleAllow} gapClass="gap-2" />
|
||||
|
||||
@@ -12,7 +12,7 @@ describe("BorrowConfirmationOverlay", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("auto-allows after countdown reaches zero and progress transition ends", async () => {
|
||||
it("auto-denies after countdown reaches zero and progress transition ends", async () => {
|
||||
const onAllow = vi.fn();
|
||||
const onDeny = vi.fn();
|
||||
|
||||
@@ -37,10 +37,7 @@ describe("BorrowConfirmationOverlay", () => {
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
});
|
||||
}
|
||||
expect(screen.getByText("0 秒后自动允许")).toBeTruthy();
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
});
|
||||
expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy();
|
||||
const progressCircle = container.querySelectorAll("svg circle")[1];
|
||||
expect(progressCircle).toBeTruthy();
|
||||
await act(() => {
|
||||
@@ -54,8 +51,44 @@ describe("BorrowConfirmationOverlay", () => {
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(150);
|
||||
});
|
||||
expect(onAllow).toHaveBeenCalledTimes(1);
|
||||
expect(onDeny).not.toHaveBeenCalled();
|
||||
expect(onDeny).toHaveBeenCalledTimes(1);
|
||||
expect(onAllow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("auto-denies via timer fallback when progress transitionend never fires", async () => {
|
||||
const onAllow = vi.fn();
|
||||
const onDeny = vi.fn();
|
||||
|
||||
render(
|
||||
<BorrowConfirmationOverlay
|
||||
requests={[
|
||||
{
|
||||
id: "req-fallback",
|
||||
isActiveTab: true,
|
||||
tabTitle: "Example",
|
||||
timeoutMs: 5000,
|
||||
onAllow,
|
||||
onDeny,
|
||||
},
|
||||
]}
|
||||
/>,
|
||||
);
|
||||
|
||||
for (let i = 0; i < 5; i++) {
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
});
|
||||
}
|
||||
expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy();
|
||||
// No transitionend — the PROGRESS_TRANSITION_MS fallback must still deny.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
});
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(150);
|
||||
});
|
||||
expect(onDeny).toHaveBeenCalledTimes(1);
|
||||
expect(onAllow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("only invokes onDeny once when deny is clicked repeatedly", async () => {
|
||||
|
||||
@@ -54,8 +54,7 @@ export default defineBackground(() => {
|
||||
deps: {
|
||||
// Skip every Agent Window when choosing where to render the
|
||||
// overlay — Agent Windows boot on about:blank, which has no
|
||||
// content script, so sendMessage would fail-open and silently
|
||||
// allow the borrow without any UI shown.
|
||||
// content script, so they cannot surface an authorization decision.
|
||||
isAgentWindowId: (windowId) => sessions.findByWindowId(windowId) !== null,
|
||||
// Resolve i18n strings per-borrow so language switches take effect
|
||||
// without re-creating the dispatcher.
|
||||
@@ -82,8 +81,8 @@ export default defineBackground(() => {
|
||||
// The Allow / Deny buttons on the OS notification are the *explicit*
|
||||
// authorization fallback when every candidate user window's content
|
||||
// script was missing (extension just reloaded, page in BFCache, etc.).
|
||||
// Without this listener those button clicks would land nowhere and we'd
|
||||
// be back to fail-open-after-sendMessage-failure.
|
||||
// Without this listener those button clicks would land nowhere and the
|
||||
// request would only resolve via the fail-closed background timeout.
|
||||
if (typeof chrome.notifications?.onButtonClicked?.addListener === "function") {
|
||||
attachBorrowNotificationButtonHandler({
|
||||
onButtonClicked: chrome.notifications.onButtonClicked,
|
||||
|
||||
@@ -162,6 +162,22 @@ describe("requestBorrowConfirmation", () => {
|
||||
expect(await pending).toBe(false);
|
||||
});
|
||||
|
||||
it("denies malformed or missing confirmation responses", async () => {
|
||||
tabs.get.mockResolvedValue({ id: 42, title: "Tab" });
|
||||
windows.getLastFocused.mockResolvedValue(
|
||||
userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }),
|
||||
);
|
||||
|
||||
for (const response of [undefined, {}, { allowed: true }, { type: "borrow-response" }]) {
|
||||
tabs.sendMessage.mockResolvedValueOnce(response);
|
||||
const pending = requestBorrowConfirmation(42, {
|
||||
deps: { tabs, windows, notifications },
|
||||
});
|
||||
await vi.runAllTimersAsync();
|
||||
expect(await pending).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("skips the Agent Window when it is lastFocusedWindow and falls back to a real user window", async () => {
|
||||
tabs.get.mockResolvedValueOnce({
|
||||
id: 42,
|
||||
@@ -250,7 +266,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
expect(tabs.sendMessage.mock.calls[1]?.[0]).toBe(301);
|
||||
});
|
||||
|
||||
it("does NOT fail-open immediately when every candidate's sendMessage fails — waits for notification button or timeout", async () => {
|
||||
it("waits for a notification decision and denies when every UI path times out", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
||||
tabs.get.mockResolvedValueOnce({ id: 42, title: "Tab" });
|
||||
windows.getLastFocused.mockResolvedValueOnce(
|
||||
@@ -282,13 +298,13 @@ describe("requestBorrowConfirmation", () => {
|
||||
|
||||
expect(tabs.sendMessage).toHaveBeenCalledTimes(2);
|
||||
// CRITICAL: previously this would already be settled(true). Now we
|
||||
// require an explicit user choice (notification button) or timeout.
|
||||
// require an explicit user choice (notification button) or safe timeout.
|
||||
expect(resolved).toBe(false);
|
||||
|
||||
// Allow the BACKGROUND_TIMEOUT_MS fail-open to fire so the test resolves.
|
||||
// The background timeout must resolve the request without authorizing it.
|
||||
await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS);
|
||||
expect(await pending).toBe(true);
|
||||
expect(resolvedValue).toBe(true);
|
||||
expect(await pending).toBe(false);
|
||||
expect(resolvedValue).toBe(false);
|
||||
|
||||
const exhaustedWarn = warnSpy.mock.calls.find(
|
||||
(call) =>
|
||||
@@ -375,7 +391,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
expect(await pending).toBe(false);
|
||||
});
|
||||
|
||||
it("fail-opens when no injectable user window exists at all", async () => {
|
||||
it("denies when no injectable user window exists at all", async () => {
|
||||
tabs.get.mockResolvedValueOnce({ id: 42, title: "Stranded Tab" });
|
||||
windows.getLastFocused.mockResolvedValueOnce(
|
||||
userWindowWithActiveTab({ windowId: 500, tabId: 5001, url: "about:blank" }),
|
||||
@@ -394,7 +410,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
});
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await pending).toBe(true);
|
||||
expect(await pending).toBe(false);
|
||||
expect(tabs.sendMessage).not.toHaveBeenCalled();
|
||||
// No notification either — there's no actionable target window.
|
||||
expect(notifications.create).not.toHaveBeenCalled();
|
||||
@@ -421,6 +437,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
expect(notificationId.startsWith(BORROW_NOTIFICATION_PREFIX)).toBe(true);
|
||||
expect(options.type).toBe("basic");
|
||||
expect(options.title).toMatch(/borrow/i);
|
||||
expect(options.requireInteraction).toBe(true);
|
||||
expect(notifications.clear).toHaveBeenCalledWith(notificationId);
|
||||
});
|
||||
|
||||
@@ -447,10 +464,12 @@ describe("requestBorrowConfirmation", () => {
|
||||
userWindowWithActiveTab({ windowId: 77, tabId: 4242, url: "https://docs.example/" }),
|
||||
);
|
||||
// Keep sendMessage pending so the request stays live while we click.
|
||||
// A second call may arrive if the background timeout dismisses the overlay.
|
||||
let resolveSend: (v: { type: string; allowed: boolean }) => void = () => undefined;
|
||||
tabs.sendMessage.mockImplementationOnce(
|
||||
() => new Promise((res) => (resolveSend = res as never)),
|
||||
);
|
||||
tabs.sendMessage.mockResolvedValue(undefined);
|
||||
|
||||
const pending = requestBorrowConfirmation(42, {
|
||||
deps: { tabs, windows, notifications },
|
||||
@@ -460,7 +479,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
|
||||
// Simulate user clicking the OS notification.
|
||||
for (const l of listeners) l(notificationId);
|
||||
await vi.runAllTimersAsync();
|
||||
await Promise.resolve();
|
||||
expect(windows.update).toHaveBeenCalledWith(77, { focused: true });
|
||||
|
||||
// Finish the borrow so the test resolves cleanly.
|
||||
@@ -469,21 +488,29 @@ describe("requestBorrowConfirmation", () => {
|
||||
await pending;
|
||||
});
|
||||
|
||||
it("fail-opens after background timeout when content script never responds", async () => {
|
||||
it("denies after background timeout when content script never responds", async () => {
|
||||
tabs.get.mockResolvedValueOnce({ id: 42, title: "Tab" });
|
||||
windows.getLastFocused.mockResolvedValueOnce(
|
||||
userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }),
|
||||
);
|
||||
tabs.sendMessage.mockImplementationOnce(() => new Promise(() => undefined));
|
||||
tabs.sendMessage.mockResolvedValueOnce(undefined);
|
||||
|
||||
const pending = requestBorrowConfirmation(42, {
|
||||
deps: { tabs, windows, notifications },
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS);
|
||||
expect(await pending).toBe(true);
|
||||
expect(await pending).toBe(false);
|
||||
// Timeout must dismiss any in-flight overlay so Allow cannot linger.
|
||||
expect(tabs.sendMessage).toHaveBeenCalledTimes(2);
|
||||
const cancelMessage = tabs.sendMessage.mock.calls[1][1] as {
|
||||
type: string;
|
||||
requestId: string;
|
||||
};
|
||||
expect(cancelMessage.type).toBe("borrow-cancel");
|
||||
});
|
||||
|
||||
it("dismisses pending overlay and fail-opens when aborted", async () => {
|
||||
it("dismisses the pending overlay and denies when aborted", async () => {
|
||||
tabs.get.mockResolvedValueOnce({ id: 42, title: "Borrow Target" });
|
||||
windows.getLastFocused.mockResolvedValueOnce(
|
||||
userWindowWithActiveTab({ windowId: 11, tabId: 7, url: "https://app.example/" }),
|
||||
@@ -500,7 +527,7 @@ describe("requestBorrowConfirmation", () => {
|
||||
controller.abort();
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(await pending).toBe(true);
|
||||
expect(await pending).toBe(false);
|
||||
expect(tabs.sendMessage).toHaveBeenCalledTimes(2);
|
||||
const requestMessage = tabs.sendMessage.mock.calls[0][1] as { requestId: string };
|
||||
const cancelMessage = tabs.sendMessage.mock.calls[1][1] as {
|
||||
|
||||
@@ -28,8 +28,9 @@ export const CONFIRMATION_TIMEOUT_MS = 5000;
|
||||
const EXIT_ANIMATION_MS = 150;
|
||||
/** Matches BorrowConfirmationOverlay progress ring/bar transition (duration-1000). */
|
||||
export const PROGRESS_TRANSITION_MS = 1000;
|
||||
// UI auto-allows after countdown + progress transition + exit fade. Background
|
||||
// timeout must not fire before that chain completes or deny is ignored.
|
||||
// UI auto-denies after countdown + progress transition + exit fade. Background
|
||||
// timeout must not fire before that chain completes or the UI decision races
|
||||
// with an already-settled deny.
|
||||
export const BACKGROUND_TIMEOUT_MS =
|
||||
CONFIRMATION_TIMEOUT_MS + PROGRESS_TRANSITION_MS + EXIT_ANIMATION_MS + 500;
|
||||
|
||||
@@ -114,9 +115,8 @@ const pendingBorrowNotifications = new Map<string, PendingBorrowNotification>();
|
||||
*
|
||||
* This is the *fallback* authorization path that fires when no candidate
|
||||
* user window could host the in-page overlay (every `chrome.tabs.sendMessage`
|
||||
* was rejected because the content script was not present). Without it the
|
||||
* background-task fail-open at the end of `tryCandidate` would silently
|
||||
* allow the borrow without any user-visible authorization step.
|
||||
* was rejected because the content script was not present). It preserves an
|
||||
* explicit authorization path without weakening the fail-closed timeout.
|
||||
*/
|
||||
const pendingBorrowDecisions = new Map<string, (allowed: boolean) => void>();
|
||||
|
||||
@@ -170,7 +170,7 @@ export function attachBorrowNotificationClickHandler(deps: {
|
||||
* Allow / Deny buttons on the OS notification can resolve the matching
|
||||
* pending `requestBorrowConfirmation`. This is the explicit-authorization
|
||||
* fallback path used when every candidate user window's content script was
|
||||
* missing — without it we'd be back to silently fail-opening the borrow.
|
||||
* missing.
|
||||
*
|
||||
* Button index convention (matches `BorrowNotificationCopy`):
|
||||
* - 0 → Allow → `resolve(true)`
|
||||
@@ -326,8 +326,7 @@ async function listConfirmationCandidates(
|
||||
if (typeof w.id !== "number") continue;
|
||||
// Never bounce the overlay back into a session's Agent Window — those
|
||||
// pages can't host a content script when they're on the about:blank
|
||||
// bootstrap URL, so sendMessage would fail-open and silently allow the
|
||||
// borrow with no UI shown.
|
||||
// bootstrap URL, so they cannot surface an authorization decision.
|
||||
if (isAgentWindowId(w.id)) continue;
|
||||
const activeTab = w.tabs?.find((t) => t.active === true) ?? null;
|
||||
if (!activeTab || typeof activeTab.id !== "number") continue;
|
||||
@@ -360,16 +359,14 @@ async function listConfirmationCandidates(
|
||||
* *explicit-authorization fallback* when every candidate fails sendMessage:
|
||||
* we no longer silently allow the borrow in that case, the promise stays
|
||||
* pending until the user clicks a notification button (resolves true/false)
|
||||
* or `BACKGROUND_TIMEOUT_MS` fires (last-resort fail-open so the agent
|
||||
* doesn't hang forever when even the notification path is broken).
|
||||
* or `BACKGROUND_TIMEOUT_MS` fires and safely denies the request.
|
||||
*
|
||||
* Clicking the notification body (vs. its buttons) focuses the chosen user
|
||||
* window so the overlay becomes visible — the previous behaviour.
|
||||
*
|
||||
* Returns `true` on user-allow (overlay or notification) OR last-resort
|
||||
* fail-open (no candidate window at all; abort signal; timeout). Returns
|
||||
* `false` only on explicit user-deny (overlay button or notification Deny
|
||||
* button).
|
||||
* Returns `true` only on an explicit user allow from the overlay or
|
||||
* notification. Missing UI, aborts, malformed responses, and timeouts all
|
||||
* fail closed and return `false`.
|
||||
*/
|
||||
export async function requestBorrowConfirmation(
|
||||
tabId: number,
|
||||
@@ -385,12 +382,9 @@ export async function requestBorrowConfirmation(
|
||||
|
||||
// The borrow pipeline already fetched this tab moments earlier
|
||||
// (validateBorrowTarget in tabs.ts), so a failure here is a transient
|
||||
// SW/page hiccup, not a missing tab. Do NOT fail-open: silently allowing
|
||||
// a borrow with no confirmation UI contradicts the gate's purpose and is
|
||||
// not one of the documented fail-open cases (no candidate window; abort;
|
||||
// timeout — see this function's doc comment). Fall back to a generic
|
||||
// title and still surface the overlay + notification so the user gets a
|
||||
// real chance to approve or deny.
|
||||
// SW/page hiccup, not a missing tab. Fall back to a generic title and still
|
||||
// surface the overlay + notification so the user gets a real chance to
|
||||
// approve or deny.
|
||||
let tabTitle: string;
|
||||
try {
|
||||
const tab = await tabsApi.get(tabId);
|
||||
@@ -405,10 +399,10 @@ export async function requestBorrowConfirmation(
|
||||
|
||||
const candidates = await listConfirmationCandidates(windowsApi, isAgentWindowId);
|
||||
if (candidates.length === 0) {
|
||||
console.warn("[bsk borrow] no injectable user window available — proceeding without overlay", {
|
||||
console.warn("[bsk borrow] no injectable user window available — denying borrow", {
|
||||
tabId,
|
||||
});
|
||||
return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
const requestId = createBorrowRequestId(tabId);
|
||||
@@ -419,7 +413,7 @@ export async function requestBorrowConfirmation(
|
||||
// overlay immediately.
|
||||
const notificationAnchor = candidates[0];
|
||||
if (!notificationAnchor) {
|
||||
return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
return new Promise<boolean>((resolve) => {
|
||||
@@ -448,22 +442,25 @@ export async function requestBorrowConfirmation(
|
||||
resolve(allowed);
|
||||
};
|
||||
|
||||
const onAbort = () => {
|
||||
if (activeMessageTabId !== null) {
|
||||
const cancelMessage: BorrowCancelMessage = {
|
||||
type: "borrow-cancel",
|
||||
const dismissPendingOverlay = () => {
|
||||
if (activeMessageTabId === null) return;
|
||||
const cancelMessage: BorrowCancelMessage = {
|
||||
type: "borrow-cancel",
|
||||
requestId,
|
||||
};
|
||||
void Promise.resolve(tabsApi.sendMessage(activeMessageTabId, cancelMessage)).catch((err) => {
|
||||
console.debug("[bsk borrow] cancel message failed", {
|
||||
tabId,
|
||||
messageTabId: activeMessageTabId,
|
||||
requestId,
|
||||
};
|
||||
void tabsApi.sendMessage(activeMessageTabId, cancelMessage).catch((err) => {
|
||||
console.debug("[bsk borrow] cancel message failed", {
|
||||
tabId,
|
||||
messageTabId: activeMessageTabId,
|
||||
requestId,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
settle(true);
|
||||
});
|
||||
};
|
||||
|
||||
const onAbort = () => {
|
||||
dismissPendingOverlay();
|
||||
settle(false);
|
||||
};
|
||||
|
||||
if (signal?.aborted) {
|
||||
@@ -473,19 +470,18 @@ export async function requestBorrowConfirmation(
|
||||
signal?.addEventListener("abort", onAbort, { once: true });
|
||||
|
||||
timeout = setTimeout(() => {
|
||||
console.info("[bsk borrow] confirmation timed out — proceeding", {
|
||||
console.info("[bsk borrow] confirmation timed out — denying borrow", {
|
||||
tabId,
|
||||
messageTabId: activeMessageTabId,
|
||||
});
|
||||
settle(true);
|
||||
dismissPendingOverlay();
|
||||
settle(false);
|
||||
}, BACKGROUND_TIMEOUT_MS);
|
||||
|
||||
// Surface the OS notification *before* messaging any candidate so the
|
||||
// user has a parallel signal even if every content script is missing.
|
||||
// The Allow / Deny buttons let the user authorize explicitly when the
|
||||
// in-page overlay never reached them — without those buttons we'd have
|
||||
// to fail-open after `tryCandidate` exhausts its candidates, which is
|
||||
// exactly the silent-allow bug we're fixing here.
|
||||
// in-page overlay never reached them.
|
||||
if (notificationsApi) {
|
||||
pendingBorrowNotifications.set(notificationId, {
|
||||
windowId: notificationAnchor.windowId,
|
||||
@@ -499,7 +495,7 @@ export async function requestBorrowConfirmation(
|
||||
title: copy.title,
|
||||
message: copy.body(tabTitle),
|
||||
priority: 2,
|
||||
requireInteraction: false,
|
||||
requireInteraction: true,
|
||||
silent: false,
|
||||
buttons: [{ title: copy.allowButton }, { title: copy.denyButton }],
|
||||
})
|
||||
@@ -523,15 +519,12 @@ export async function requestBorrowConfirmation(
|
||||
): void => {
|
||||
if (settled) return;
|
||||
if (index >= candidates.length) {
|
||||
// Every candidate rejected sendMessage. We deliberately do NOT
|
||||
// fail-open here anymore: silently allowing a borrow with zero
|
||||
// user-visible authorization UI is the bug. Instead we keep the
|
||||
// promise pending and rely on:
|
||||
// Every candidate rejected sendMessage. Keep the promise pending and
|
||||
// rely on:
|
||||
// • the OS notification's Allow / Deny buttons for an explicit
|
||||
// user choice (preferred), or
|
||||
// • the BACKGROUND_TIMEOUT_MS fail-open as the last-resort
|
||||
// soft-fail so the agent doesn't hang forever when even the
|
||||
// notification API is unavailable.
|
||||
// • the BACKGROUND_TIMEOUT_MS fail-closed result so the agent does
|
||||
// not hang forever when even the notification API is unavailable.
|
||||
// Keep the diagnostics so real-world frequency is still observable.
|
||||
console.warn(
|
||||
"[bsk borrow] every candidate user window failed sendMessage — awaiting notification button or timeout",
|
||||
@@ -560,7 +553,9 @@ export async function requestBorrowConfirmation(
|
||||
.sendMessage(candidate.tabId, message)
|
||||
.then((response) => {
|
||||
if (settled) return;
|
||||
const allowed = (response as BorrowResponseMessage | undefined)?.allowed !== false;
|
||||
const candidateResponse = response as BorrowResponseMessage | undefined;
|
||||
const allowed =
|
||||
candidateResponse?.type === "borrow-response" && candidateResponse.allowed === true;
|
||||
console.info("[bsk borrow] confirmation response", { tabId, allowed });
|
||||
settle(allowed);
|
||||
})
|
||||
|
||||
@@ -52,7 +52,7 @@
|
||||
"titleInactive": "Agent wants to borrow a tab",
|
||||
"body": "An AI agent wants to move this tab into its workspace. You can deny or allow the borrow.",
|
||||
"targetTab": "Target tab:",
|
||||
"autoAllow": "Auto-allowing in {{count}}s",
|
||||
"autoDeny": "Request expires in {{count}}s",
|
||||
"deny": "Deny",
|
||||
"allow": "Allow",
|
||||
"notificationTitle": "BrowserSkill: Agent wants to borrow a tab",
|
||||
|
||||
@@ -52,7 +52,7 @@
|
||||
"titleInactive": "Agent 请求借用标签页",
|
||||
"body": "AI Agent 希望将此标签页移入工作区。您可以拒绝或允许借用。",
|
||||
"targetTab": "目标标签页:",
|
||||
"autoAllow": "{{count}} 秒后自动允许",
|
||||
"autoDeny": "{{count}} 秒后自动拒绝",
|
||||
"deny": "拒绝",
|
||||
"allow": "允许",
|
||||
"notificationTitle": "BrowserSkill:Agent 请求借用标签页",
|
||||
|
||||
Reference in New Issue
Block a user