mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
feat(coding-agent): confirm device-code browser and clipboard actions
Knowing how annoying re-logins in corporate environments can be, I think
we can open the browser and copy the code to the clipboard (what the user
asks for) without forcing it upon users (which I assume was the reason
it was removed in 8f0e9251b9).
This change adds optional browser opening and code copying during
device-code login after an additional confirmation (usually Enter).
It should work for all providers, with an optional opt-out controlled
by the provider author, not user settings.
Fixes #9282.
This commit is contained in:
parent
c60de42e98
commit
7151099d02
5 changed files with 237 additions and 1 deletions
|
|
@ -141,6 +141,10 @@ export type AuthEvent =
|
|||
type: "device_code";
|
||||
userCode: string;
|
||||
verificationUri: string;
|
||||
/** Offer best-effort browser opening on user confirmation. Defaults to true; clients may ignore it. */
|
||||
openBrowserOnConfirm?: boolean;
|
||||
/** Offer copying of userCode on user confirmation, replacing the clipboard. Defaults to true. */
|
||||
copyCodeOnConfirm?: boolean;
|
||||
intervalSeconds?: number;
|
||||
expiresInSeconds?: number;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,6 +17,10 @@ export interface OAuthAuthInfo {
|
|||
export interface OAuthDeviceCodeInfo {
|
||||
userCode: string;
|
||||
verificationUri: string;
|
||||
/** Offer best-effort browser opening on user confirmation. Defaults to true; clients may ignore it. */
|
||||
openBrowserOnConfirm?: boolean;
|
||||
/** Offer copying of userCode on user confirmation, replacing the clipboard. Defaults to true. */
|
||||
copyCodeOnConfirm?: boolean;
|
||||
intervalSeconds?: number;
|
||||
expiresInSeconds?: number;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -138,6 +138,8 @@ An OAuth provider supplies a display name, login flow, token refresh, and access
|
|||
|
||||
OAuth callbacks are UI-neutral. They can open an authorization URL, show a device code, report progress, request input, or ask the user to choose a login method. Honor cancellation and the supplied abort signal during network requests.
|
||||
|
||||
Device-code login offers browser opening and code copying on Enter (`tui.select.confirm`) by default. Confirmation is optional; users can complete login manually without opening a browser or changing the clipboard. Set `openBrowserOnConfirm` or `copyCodeOnConfirm` to `false` in the `onDeviceCode` payload to disable either action. Native `AuthEvent` device-code notifications support the same flags.
|
||||
|
||||
Never write access tokens, refresh tokens, authorization headers, or complete provider responses to ordinary logs.
|
||||
|
||||
## Supply and refresh models
|
||||
|
|
|
|||
|
|
@ -1,5 +1,15 @@
|
|||
import type { AuthInfoLink, OAuthDeviceCodeInfo } from "@earendil-works/pi-ai";
|
||||
import { Container, type Focusable, getKeybindings, Input, Spacer, Text, type TUI } from "@earendil-works/pi-tui";
|
||||
import {
|
||||
Container,
|
||||
type Focusable,
|
||||
getKeybindings,
|
||||
Input,
|
||||
Spacer,
|
||||
Text,
|
||||
TruncatedText,
|
||||
type TUI,
|
||||
} from "@earendil-works/pi-tui";
|
||||
import { copyToClipboard } from "../../../utils/clipboard.ts";
|
||||
import { openBrowser } from "../../../utils/open-browser.ts";
|
||||
import { theme } from "../theme/theme.ts";
|
||||
import { AuthUrlComponent } from "./auth-url.ts";
|
||||
|
|
@ -18,6 +28,8 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
private abortController = new AbortController();
|
||||
private inputResolver?: (value: string) => void;
|
||||
private inputRejecter?: (error: Error) => void;
|
||||
private deviceCodeAction?: { info: OAuthDeviceCodeInfo; hint: Container };
|
||||
private deviceCodeHint?: Container;
|
||||
private onComplete: (success: boolean, message?: string) => void;
|
||||
|
||||
// Focusable implementation - propagate to input for IME cursor positioning
|
||||
|
|
@ -83,7 +95,16 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
);
|
||||
}
|
||||
|
||||
private clearDeviceCodeAction(): void {
|
||||
if (this.deviceCodeHint) {
|
||||
this.contentContainer.removeChild(this.deviceCodeHint);
|
||||
}
|
||||
this.deviceCodeHint = undefined;
|
||||
this.deviceCodeAction = undefined;
|
||||
}
|
||||
|
||||
private cancel(): void {
|
||||
this.clearDeviceCodeAction();
|
||||
this.abortController.abort();
|
||||
if (this.inputRejecter) {
|
||||
this.inputRejecter(new Error("Login cancelled"));
|
||||
|
|
@ -97,6 +118,7 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
* Called by onAuth callback - show URL and optional instructions
|
||||
*/
|
||||
showAuth(url: string, instructions?: string): void {
|
||||
this.clearDeviceCodeAction();
|
||||
this.contentContainer.clear();
|
||||
this.contentContainer.addChild(new Spacer(1));
|
||||
this.authUrl = new AuthUrlComponent(this.tui, url);
|
||||
|
|
@ -116,6 +138,12 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
*/
|
||||
showDeviceCode(info: OAuthDeviceCodeInfo): void {
|
||||
this.authUrl = undefined;
|
||||
info = {
|
||||
...info,
|
||||
openBrowserOnConfirm: info.openBrowserOnConfirm ?? true,
|
||||
copyCodeOnConfirm: info.copyCodeOnConfirm ?? true,
|
||||
};
|
||||
this.clearDeviceCodeAction();
|
||||
this.contentContainer.clear();
|
||||
this.contentContainer.addChild(new Spacer(1));
|
||||
const linkedUrl = `\x1b]8;;${info.verificationUri}\x07${info.verificationUri}\x1b]8;;\x07`;
|
||||
|
|
@ -127,13 +155,52 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
this.contentContainer.addChild(new Spacer(1));
|
||||
this.contentContainer.addChild(new Text(theme.fg("warning", `Enter code: ${info.userCode}`), 1, 0));
|
||||
|
||||
if (!this.signal.aborted && (info.openBrowserOnConfirm || info.copyCodeOnConfirm)) {
|
||||
const actionLabel = info.openBrowserOnConfirm
|
||||
? info.copyCodeOnConfirm
|
||||
? "to open browser and copy code"
|
||||
: "to open browser"
|
||||
: "to copy code";
|
||||
// Reuse this single row for progress/result text, including in narrow terminals.
|
||||
const hint = new Container();
|
||||
hint.addChild(new TruncatedText(`(${keyHint("tui.select.confirm", actionLabel)})`, 1, 0));
|
||||
this.contentContainer.addChild(hint);
|
||||
this.deviceCodeHint = hint;
|
||||
this.deviceCodeAction = { info, hint };
|
||||
}
|
||||
this.tui.requestRender();
|
||||
}
|
||||
|
||||
private setDeviceCodeStatus(hint: Container, message: string): void {
|
||||
if (this.signal.aborted || !this.focused || !this.contentContainer.children.includes(hint)) return;
|
||||
hint.children = [new TruncatedText(theme.fg("dim", message), 1, 0)];
|
||||
this.tui.requestRender();
|
||||
}
|
||||
|
||||
private activateDeviceCode(info: OAuthDeviceCodeInfo, hint: Container): void {
|
||||
this.setDeviceCodeStatus(hint, info.copyCodeOnConfirm ? "Copying code..." : "Browser open requested");
|
||||
if (info.copyCodeOnConfirm) {
|
||||
// Do not delay polling or browser opening while clipboard tools are running.
|
||||
void copyToClipboard(info.userCode)
|
||||
.then(() => this.setDeviceCodeStatus(hint, "Code copied to clipboard"))
|
||||
.catch(() => this.setDeviceCodeStatus(hint, "Copy code manually"));
|
||||
}
|
||||
if (info.openBrowserOnConfirm) {
|
||||
try {
|
||||
// The shared launcher also opens files; device-code login must only open web URLs.
|
||||
const url = new URL(info.verificationUri);
|
||||
if (url.protocol === "https:" || url.protocol === "http:") openBrowser(url.href);
|
||||
} catch {
|
||||
// Invalid URLs or launcher failures must not interrupt login.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Show input for manual code/URL entry (for callback server providers)
|
||||
*/
|
||||
showManualInput(prompt: string): Promise<string> {
|
||||
this.clearDeviceCodeAction();
|
||||
this.input.setValue("");
|
||||
this.contentContainer.addChild(new Spacer(1));
|
||||
this.contentContainer.addChild(new Text(theme.fg("dim", prompt), 1, 0));
|
||||
|
|
@ -152,6 +219,7 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
* Note: Does NOT clear content, appends to existing (preserves URL from showAuth)
|
||||
*/
|
||||
showPrompt(message: string, placeholder?: string): Promise<string> {
|
||||
this.clearDeviceCodeAction();
|
||||
this.contentContainer.addChild(new Spacer(1));
|
||||
this.contentContainer.addChild(new Text(theme.fg("text", message), 1, 0));
|
||||
if (placeholder) {
|
||||
|
|
@ -178,6 +246,7 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
/** Show informational text before another login step. */
|
||||
showDetails(lines: string[]): void {
|
||||
this.authUrl = undefined;
|
||||
this.clearDeviceCodeAction();
|
||||
this.contentContainer.clear();
|
||||
this.contentContainer.addChild(new Spacer(1));
|
||||
for (const line of lines) {
|
||||
|
|
@ -232,6 +301,21 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
this.deviceCodeAction &&
|
||||
!this.inputResolver &&
|
||||
!this.signal.aborted &&
|
||||
this.focused &&
|
||||
kb.matches(data, "tui.select.confirm")
|
||||
) {
|
||||
const action = this.deviceCodeAction;
|
||||
// Consume once so a repeated Enter cannot launch multiple browsers or clipboard writes.
|
||||
this.deviceCodeAction = undefined;
|
||||
this.activateDeviceCode(action.info, action.hint);
|
||||
this.tui.requestRender();
|
||||
return;
|
||||
}
|
||||
|
||||
// Pass to input
|
||||
this.input.handleInput(data);
|
||||
}
|
||||
|
|
|
|||
142
packages/coding-agent/test/login-dialog-device-code.test.ts
Normal file
142
packages/coding-agent/test/login-dialog-device-code.test.ts
Normal file
|
|
@ -0,0 +1,142 @@
|
|||
import { setKeybindings, type TUI } from "@earendil-works/pi-tui";
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { KeybindingsManager } from "../src/core/keybindings.ts";
|
||||
import { LoginDialogComponent } from "../src/modes/interactive/components/login-dialog.ts";
|
||||
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
|
||||
import { stripAnsi } from "../src/utils/ansi.ts";
|
||||
import { copyToClipboard } from "../src/utils/clipboard.ts";
|
||||
import { openBrowser } from "../src/utils/open-browser.ts";
|
||||
|
||||
vi.mock("../src/utils/clipboard.ts", () => ({ copyToClipboard: vi.fn() }));
|
||||
vi.mock("../src/utils/open-browser.ts", () => ({ openBrowser: vi.fn() }));
|
||||
|
||||
const device = { userCode: "ABCD-EFGH", verificationUri: "https://example.com/device" };
|
||||
|
||||
function createDialog() {
|
||||
const onComplete = vi.fn();
|
||||
const dialog = new LoginDialogComponent({ requestRender: vi.fn() } as unknown as TUI, "custom-provider", onComplete);
|
||||
dialog.focused = true;
|
||||
return { dialog, onComplete };
|
||||
}
|
||||
|
||||
function render(dialog: LoginDialogComponent): string {
|
||||
return stripAnsi(dialog.render(120).join("\n"));
|
||||
}
|
||||
|
||||
// Regression #9282: device-code convenience actions require explicit confirmation.
|
||||
describe("device-code login", () => {
|
||||
beforeAll(() => initTheme("dark"));
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(copyToClipboard).mockResolvedValue(undefined);
|
||||
setKeybindings(new KeybindingsManager());
|
||||
});
|
||||
|
||||
it("opens the verification page and copies the code once after confirmation", async () => {
|
||||
const { dialog } = createDialog();
|
||||
const domain = dialog.showPrompt("GitHub Enterprise URL/domain (blank for github.com)");
|
||||
dialog.handleInput("\r");
|
||||
await expect(domain).resolves.toBe("");
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.showWaiting("Waiting for authentication...");
|
||||
expect(render(dialog)).toContain("to open browser and copy code");
|
||||
expect(openBrowser).not.toHaveBeenCalled();
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
|
||||
dialog.handleInput("\r");
|
||||
await Promise.resolve();
|
||||
dialog.handleInput("\r");
|
||||
expect(openBrowser).toHaveBeenCalledExactlyOnceWith(device.verificationUri);
|
||||
expect(copyToClipboard).toHaveBeenCalledExactlyOnceWith(device.userCode);
|
||||
expect(render(dialog)).toContain("Code copied to clipboard");
|
||||
expect(render(dialog)).toContain(`Enter code: ${device.userCode}`);
|
||||
expect(render(dialog)).toContain("Waiting for authentication...");
|
||||
expect(render(dialog)).not.toContain("to open browser and copy code");
|
||||
});
|
||||
|
||||
it.each([
|
||||
[false, false],
|
||||
[true, false],
|
||||
[false, true],
|
||||
] as const)(
|
||||
"honors browser=%s / clipboard=%s provider overrides",
|
||||
async (openBrowserOnConfirm, copyCodeOnConfirm) => {
|
||||
const { dialog } = createDialog();
|
||||
dialog.showDeviceCode({ ...device, openBrowserOnConfirm, copyCodeOnConfirm });
|
||||
dialog.handleInput("\r");
|
||||
await Promise.resolve();
|
||||
expect(openBrowser).toHaveBeenCalledTimes(openBrowserOnConfirm ? 1 : 0);
|
||||
expect(copyToClipboard).toHaveBeenCalledTimes(copyCodeOnConfirm ? 1 : 0);
|
||||
expect(dialog.signal.aborted).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it("clears the previous auth URL copy action when showing a device code", () => {
|
||||
const { dialog } = createDialog();
|
||||
dialog.showAuth("https://example.com/authorize");
|
||||
vi.mocked(openBrowser).mockClear();
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.handleInput("\x18");
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
expect(openBrowser).not.toHaveBeenCalled();
|
||||
dialog.handleInput("\r");
|
||||
expect(copyToClipboard).toHaveBeenCalledExactlyOnceWith(device.userCode);
|
||||
});
|
||||
|
||||
it("clears device actions when switching to browser authentication", () => {
|
||||
const { dialog } = createDialog();
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.showAuth("https://example.com/authorize");
|
||||
vi.mocked(openBrowser).mockClear();
|
||||
dialog.handleInput("\r");
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
expect(openBrowser).not.toHaveBeenCalled();
|
||||
dialog.handleInput("\x18");
|
||||
expect(copyToClipboard).toHaveBeenCalledExactlyOnceWith("https://example.com/authorize");
|
||||
});
|
||||
|
||||
it("clears both action types when showing details", () => {
|
||||
const { dialog } = createDialog();
|
||||
dialog.showAuth("https://example.com/authorize");
|
||||
dialog.showDetails(["Choose a login method"]);
|
||||
dialog.handleInput("\x18");
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.showDetails(["Choose a login method"]);
|
||||
vi.mocked(openBrowser).mockClear();
|
||||
dialog.handleInput("\r");
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
expect(openBrowser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("cancels without opening a browser or changing the clipboard", () => {
|
||||
const { dialog, onComplete } = createDialog();
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.handleInput("\x1b");
|
||||
dialog.handleInput("\r");
|
||||
expect(dialog.signal.aborted).toBe(true);
|
||||
expect(onComplete).toHaveBeenCalledExactlyOnceWith(false, "Login cancelled");
|
||||
expect(openBrowser).not.toHaveBeenCalled();
|
||||
expect(copyToClipboard).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps manual login available when browser and clipboard access fail", async () => {
|
||||
vi.mocked(openBrowser).mockImplementation(() => {
|
||||
throw new Error("Browser unavailable");
|
||||
});
|
||||
vi.mocked(copyToClipboard).mockRejectedValue(new Error("Clipboard unavailable"));
|
||||
const { dialog, onComplete } = createDialog();
|
||||
dialog.showDeviceCode(device);
|
||||
dialog.showWaiting("Waiting for authentication...");
|
||||
expect(() => dialog.handleInput("\r")).not.toThrow();
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
expect(openBrowser).toHaveBeenCalledExactlyOnceWith(device.verificationUri);
|
||||
expect(copyToClipboard).toHaveBeenCalledExactlyOnceWith(device.userCode);
|
||||
expect(render(dialog)).toContain(device.verificationUri);
|
||||
expect(render(dialog)).toContain(`Enter code: ${device.userCode}`);
|
||||
expect(render(dialog)).toContain("Copy code manually");
|
||||
expect(render(dialog)).toContain("Waiting for authentication...");
|
||||
expect(dialog.signal.aborted).toBe(false);
|
||||
expect(onComplete).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue