mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 22:24:16 +01:00
fix(runner): accept late Antigravity login URL during drain (#18254)
## Thinking Path > - Paperclip manages work performed by AI agents. > - The native Runner owns Antigravity subscription sign-in. > - Sign-in receives completion on stdout and the browser URL on stderr. > - The two pipes can deliver the completion result before the valid URL. > - The owner rejected that URL after it entered the completed phase. > - This pull request accepts one validated URL while the same child drains. > - A real child regression now proves the order and the failure controls. ## Linked Issues or Issue Description Refs #18188, #18189, and #18190. Related qualification work: #18345 and #18356. The duplicate search found this existing PR, so this update preserves its history and adds the real child proof here. **What happened?** A valid sign-in can fail when the authenticate response arrives before the authorization URL on the other pipe. The completed phase then rejects the valid URL. The same existing success case failed in [current follow-up CI](https://github.com/paperclipai/paperclip/actions/runs/41138618869/job/124347859445). That log has no stream-order trace, so it does not prove its inner failure branch. **Expected behavior** Both valid delivery orders must succeed. Invalid or duplicate URLs, unexpected protocol requests, failed retirement, and failed credential verification must still fail. **Steps to reproduce** 1. Run a synthetic native child that completes authenticate on stdout. 2. Wait for stdin EOF before it emits the authorization URL on stderr. EOF proves that the parent accepted the result and began retirement. 3. The valid case fails on frozen master `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9`. The invalid and duplicate controls pass there. 4. Run the same regression with this PR's existing production fix. All three cases pass. **Paperclip version or commit** Existing reviewed PR parent: `646f5cdf37898ad54e1fa1cdfe39f59844da3e7c`. Frozen actual master for the real child RED: `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9`. ## What Changed - Accept one fully validated authorization URL in the authentication or completed-drain phase. Keep duplicate rejection and all existing URL checks. - Retain the ten existing in-memory controls for both valid orders and eight failure paths. - Add three real child cases for valid, invalid, and duplicate late URLs. Use stdin EOF to order the streams without sleeps or a mocked retirement handler. - Check exact protocol requests, child exit, close-once behavior, Google-call counts, and persisted refresh rotation. This update changes only the existing test file; it adds no production delta to the reviewed parent. ## Verification - Frozen actual master: the new valid real child case fails, while both negative controls pass. The original eight-control parent calibration also remains recorded in this PR's history. - Existing fixed source plus the new tests: 24 passed. This includes 11 existing authentication tests, ten stream controls, and three real child cases. - Late invalid and duplicate URLs fail with zero Google calls. The valid case makes two mocked identity calls and saves the rotated synthetic refresh token. All three retire the real child and close the command lease once. - Scoped owning TypeScript `noEmit` passes for the auth module and both test files. The temporary configuration is not committed. Diff and credential-pattern checks pass. - The tests use real child pipes and private synthetic files. Google responses and the command installation are mocked. No provider account, network request, model run, app, or QA deployment was used. - Full monorepo typecheck, tests, and build were not run locally. Fresh [normal CI](https://github.com/paperclipai/paperclip/actions/runs/41140757050) passes on `1744c9f3374f78b10c2f15c42f3cf5d3af8c3119`: 56 successful checks, two skipped, and one neutral. Typecheck, build, Rust, static, Runner, server and browser lanes pass. Greptile gives that exact head a fresh 5/5 with zero unresolved threads. The PR is mergeable and clean. The earlier parent's passing CI remains historical evidence only. ## Risks The change accepts a URL while the same authenticated child drains. It still validates the exact OAuth URL and rejects a second URL. Success still requires the authenticate result, verified credentials, and proven process retirement. No provider pins, contracts, timeout bounds, retries, or public APIs change. Existing documentation already specifies these sign-in semantics; no documentation change is needed. ## Model Used OpenAI Codex, based on GPT-6, with tool use and code execution. The exact runtime model ID and context window were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
45a3a92e6d
commit
18b1637b63
2 files changed
+173
-1
No files matched your search
@@ -0,0 +1,172 @@
|
||||
import { spawn, type ChildProcess } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { PassThrough, Writable } from "node:stream";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const setup = vi.hoisted(() => ({ verify: vi.fn(), spawn: vi.fn(), close: vi.fn(), retire: vi.fn() }));
|
||||
vi.mock("./antigravity-installation.js", () => ({ antigravityNativeDistributionSpec: () => ({ pinned: true }) }));
|
||||
vi.mock("./installation-integrity.js", () => ({ verifyNativeAcpxInstallation: setup.verify }));
|
||||
vi.mock("./antigravity-process.js", async importOriginal => ({
|
||||
...await importOriginal<typeof import("./antigravity-process.js")>(), retireAntigravityProcess: setup.retire,
|
||||
}));
|
||||
import { startAntigravityBrowserLogin } from "./antigravity-auth.js";
|
||||
|
||||
const clientId = "1071006060591-tmhssin2h21lcre235vtolojh4g403ep.apps.googleusercontent.com";
|
||||
const scopes = ["https://www.googleapis.com/auth/cloud-platform", "https://www.googleapis.com/auth/userinfo.email", "https://www.googleapis.com/auth/aicode"];
|
||||
const authorizationUrl = "https://accounts.google.com/o/oauth2/v2/auth?" + new URLSearchParams({ client_id: clientId, response_type: "code",
|
||||
redirect_uri: "http://127.0.0.1:18973/", scope: scopes.join(" "), state: "fixture_state_0123456789", code_challenge: "a".repeat(43),
|
||||
code_challenge_method: "S256", access_type: "offline", prompt: "consent" });
|
||||
const roots: string[] = [];
|
||||
const physicalChildren: ChildProcess[] = [];
|
||||
afterEach(async () => {
|
||||
for (const child of physicalChildren.splice(0)) {
|
||||
if (child.exitCode === null && child.signalCode === null) {
|
||||
try { process.kill(-child.pid!, "SIGKILL"); } catch {}
|
||||
}
|
||||
}
|
||||
vi.unstubAllGlobals(); vi.clearAllMocks();
|
||||
await Promise.all(roots.splice(0).map(root => rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
type Mode = "url-first" | "result-first" | "wrong-url" | "duplicate-url" | "unexpected-prompt" | "retirement-failure" | "credential-failure" | "auth-error" | "late-wrong-url" | "late-duplicate-url";
|
||||
async function fixture(mode: Mode) {
|
||||
const home = await realpath(await mkdtemp(join(tmpdir(), "paperclip-antigravity-stream-test-"))); roots.push(home);
|
||||
await mkdir(join(home, "antigravity-acp"), { mode: 0o700 });
|
||||
await writeFile(join(home, "antigravity-acp/acp_token.json"), JSON.stringify({ client_id: clientId, client_secret: "synthetic-client-secret",
|
||||
refresh_token: "synthetic-refresh-token", token_uri: "https://oauth2.googleapis.com/token", scopes }), { mode: 0o600 });
|
||||
const stdout = new PassThrough(), stderr = new PassThrough(), requests: string[] = [];
|
||||
const child = Object.assign(new EventEmitter(), { stdout, stderr, stdin: new Writable({ write(chunk, _encoding, done) {
|
||||
const request = JSON.parse(String(chunk)); requests.push(request.method);
|
||||
queueMicrotask(() => {
|
||||
if (request.method === "initialize") {
|
||||
stdout.write(JSON.stringify({ jsonrpc: "2.0", id: request.id, result: { protocolVersion: 1,
|
||||
agentInfo: { name: "antigravity-acp", version: "1.4.3" }, agentCapabilities: { _meta: { antigravity: { paperclipAdmission: 1 } } },
|
||||
authMethods: [{ id: "oauth-personal" }] } }) + "\n");
|
||||
} else if (request.method === "authenticate") {
|
||||
const url = mode === "wrong-url" || mode === "late-wrong-url" ? "https://example.invalid/synthetic-private-marker" : authorizationUrl;
|
||||
const emitUrl = () => stderr.write("Open the following link to authenticate the ACP server: " + url + "\n");
|
||||
const resultFirst = ["result-first", "late-wrong-url", "late-duplicate-url"].includes(mode);
|
||||
if (!resultFirst) emitUrl();
|
||||
if (mode === "duplicate-url") emitUrl();
|
||||
if (mode === "unexpected-prompt") stdout.write(JSON.stringify({ jsonrpc: "2.0", id: 9, method: "session/prompt", params: {} }) + "\n");
|
||||
stdout.write(JSON.stringify(mode === "auth-error"
|
||||
? { jsonrpc: "2.0", id: request.id, error: { code: -32000, message: "synthetic-private-marker" } }
|
||||
: { jsonrpc: "2.0", id: request.id, result: {} }) + "\n");
|
||||
if (resultFirst) queueMicrotask(() => { emitUrl(); if (mode === "late-duplicate-url") emitUrl(); });
|
||||
} else throw new Error("Unexpected session/model request");
|
||||
});
|
||||
done();
|
||||
} }) });
|
||||
setup.close.mockResolvedValue(undefined);
|
||||
setup.retire.mockImplementation(async (_child, closed: Promise<void>) => {
|
||||
await Promise.resolve(); child.emit("close", 0); await closed;
|
||||
});
|
||||
if (mode === "retirement-failure") setup.retire.mockImplementationOnce(async () => { throw new Error("synthetic-private-marker"); });
|
||||
setup.spawn.mockReturnValue(child);
|
||||
setup.verify.mockResolvedValue({ openCommand: async () => ({ spawn: setup.spawn, close: setup.close }) });
|
||||
const fetch = vi.fn(async (url: string) => {
|
||||
if (mode === "credential-failure") return new Response(null, { status: 401 });
|
||||
if (url === "https://oauth2.googleapis.com/token") return Response.json({ access_token: "synthetic-access-token", token_type: "Bearer" });
|
||||
if (url === "https://www.googleapis.com/oauth2/v3/userinfo") return Response.json({ email: "person@example.test", email_verified: true });
|
||||
throw new Error("Unexpected network request");
|
||||
});
|
||||
vi.stubGlobal("fetch", fetch);
|
||||
return { home, requests, fetch };
|
||||
}
|
||||
|
||||
describe("Antigravity login cross-stream completion", () => {
|
||||
it.each(["url-first", "result-first"] as const)("accepts the same authenticated exchange with %s delivery", async mode => {
|
||||
const { home, requests, fetch } = await fixture(mode);
|
||||
const login = startAntigravityBrowserLogin(home);
|
||||
await vi.waitFor(() => expect(login.outcome).toBeDefined());
|
||||
expect(login.outcome).toBe("success");
|
||||
expect(login.authorizationUrl).toBe(authorizationUrl);
|
||||
expect(requests).toEqual(["initialize", "authenticate"]);
|
||||
expect(setup.retire).toHaveBeenCalledOnce(); expect(setup.close).toHaveBeenCalledOnce();
|
||||
expect(fetch).toHaveBeenCalledTimes(2);
|
||||
await login.abort();
|
||||
});
|
||||
|
||||
it.each(["wrong-url", "duplicate-url", "unexpected-prompt", "retirement-failure", "credential-failure", "auth-error", "late-wrong-url", "late-duplicate-url"] as const)("keeps %s failures closed and private", async mode => {
|
||||
const { home, requests, fetch } = await fixture(mode);
|
||||
const login = startAntigravityBrowserLogin(home);
|
||||
await vi.waitFor(() => expect(login.outcome).toBeDefined());
|
||||
expect(login.outcome).toBe("failure"); expect(login.error).toBe("Google sign-in did not complete. Start sign-in again.");
|
||||
expect(login.error).not.toContain("synthetic-private-marker");
|
||||
await login.abort(); expect(setup.close).toHaveBeenCalledOnce();
|
||||
expect(requests).toEqual(["initialize", "authenticate"]);
|
||||
expect(fetch).toHaveBeenCalledTimes(mode === "credential-failure" ? 1 : 0);
|
||||
if (mode === "wrong-url" || mode === "late-wrong-url") expect(login.authorizationUrl).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each(["valid", "invalid", "duplicate"] as const)("handles a %s URL from a real child after authenticate completion and stdin EOF", async mode => {
|
||||
const home = await realpath(await mkdtemp(join(tmpdir(), "paperclip-antigravity-real-stream-"))); roots.push(home);
|
||||
const peer = join(home, "peer.cjs");
|
||||
await writeFile(peer, `
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const readline = require('node:readline');
|
||||
const home = process.argv[2], mode = process.argv[3], requests = [], events = [];
|
||||
const reader = readline.createInterface({input:process.stdin});
|
||||
reader.on('line', line => {
|
||||
const request = JSON.parse(line); requests.push(request.method);
|
||||
fs.writeFileSync(path.join(home,'requests.json'),JSON.stringify(requests));
|
||||
if(request.method==='initialize') {
|
||||
process.stdout.write(JSON.stringify({jsonrpc:'2.0',id:request.id,result:{protocolVersion:1,
|
||||
agentInfo:{name:'antigravity-acp',version:'1.4.3'},agentCapabilities:{_meta:{antigravity:{paperclipAdmission:1}}},
|
||||
authMethods:[{id:'oauth-personal'}]}})+'\\n');
|
||||
} else if(request.method==='authenticate') {
|
||||
fs.mkdirSync(path.join(home,'antigravity-acp'),{mode:0o700});
|
||||
fs.writeFileSync(path.join(home,'antigravity-acp/acp_token.json'),${JSON.stringify(JSON.stringify({ client_id: clientId,
|
||||
client_secret: "synthetic-client-secret", refresh_token: "synthetic-refresh-token", token_uri: "https://oauth2.googleapis.com/token", scopes }))},{mode:0o600});
|
||||
events.push('authenticate-result');
|
||||
process.stdout.write(JSON.stringify({jsonrpc:'2.0',id:request.id,result:{}})+'\\n');
|
||||
} else throw new Error('Unexpected session/model request');
|
||||
});
|
||||
reader.on('close',()=>{
|
||||
events.push('stdin-eof');
|
||||
const url = mode==='invalid' ? 'https://example.invalid/synthetic-private-marker' : ${JSON.stringify(authorizationUrl)};
|
||||
const message = 'Open the following link to authenticate the ACP server: '+url+'\\n';
|
||||
process.stderr.write(mode==='duplicate' ? message+message : message,()=>{
|
||||
events.push('url-flushed');fs.writeFileSync(path.join(home,'events.json'),JSON.stringify(events));process.exit(0);
|
||||
});
|
||||
});
|
||||
`);
|
||||
setup.spawn.mockImplementation((_args, options) => {
|
||||
const child = spawn(process.execPath, [peer, home, mode], { ...options, detached: true });
|
||||
physicalChildren.push(child); return child;
|
||||
});
|
||||
setup.close.mockResolvedValue(undefined);
|
||||
setup.verify.mockResolvedValue({ openCommand: async () => ({ spawn: setup.spawn, close: setup.close }) });
|
||||
const realProcess = await vi.importActual<typeof import("./antigravity-process.js")>("./antigravity-process.js");
|
||||
setup.retire.mockImplementation(realProcess.retireAntigravityProcess);
|
||||
const fetch = vi.fn(async (url: string) => {
|
||||
if (url === "https://oauth2.googleapis.com/token") return Response.json({ access_token: "synthetic-access-token",
|
||||
refresh_token: "synthetic-rotated-refresh", token_type: "Bearer" });
|
||||
if (url === "https://www.googleapis.com/oauth2/v3/userinfo") return Response.json({ email: "person@example.test", email_verified: true });
|
||||
throw new Error("Unexpected network request");
|
||||
});
|
||||
vi.stubGlobal("fetch", fetch);
|
||||
const login = startAntigravityBrowserLogin(home);
|
||||
await vi.waitFor(() => expect(login.outcome).toBeDefined(), { timeout: 5_000 });
|
||||
expect(login.outcome).toBe(mode === "valid" ? "success" : "failure");
|
||||
expect(JSON.parse(await readFile(join(home, "events.json"), "utf8"))).toEqual(["authenticate-result", "stdin-eof", "url-flushed"]);
|
||||
expect(JSON.parse(await readFile(join(home, "requests.json"), "utf8"))).toEqual(["initialize", "authenticate"]);
|
||||
expect(physicalChildren.at(-1)!.exitCode).toBe(0);
|
||||
expect(setup.retire).toHaveBeenCalledOnce(); expect(setup.close).toHaveBeenCalledOnce();
|
||||
expect(fetch).toHaveBeenCalledTimes(mode === "valid" ? 2 : 0);
|
||||
if (mode === "valid") {
|
||||
expect(login.authorizationUrl).toBe(authorizationUrl);
|
||||
expect(JSON.parse(await readFile(join(home, "antigravity-acp", "acp_token.json"), "utf8")).refresh_token).toBe("synthetic-rotated-refresh");
|
||||
} else {
|
||||
expect(login.error).toBe("Google sign-in did not complete. Start sign-in again.");
|
||||
expect(login.error).not.toContain("synthetic-private-marker");
|
||||
if (mode === "invalid") expect(login.authorizationUrl).toBeUndefined();
|
||||
}
|
||||
await login.abort();
|
||||
expect(setup.close).toHaveBeenCalledOnce();
|
||||
}, 15_000);
|
||||
});
|
||||
@@ -203,7 +203,7 @@ export function startAntigravityBrowserLogin(home: string): AntigravityBrowserLo
|
||||
if (!line.startsWith(AUTH_PREFIX)) continue;
|
||||
try {
|
||||
const url = validateAntigravityAuthorizationUrl(line.slice(AUTH_PREFIX.length));
|
||||
if (phase !== 2 || state.authorizationUrl !== undefined) throw new Error(LOGIN_ERROR);
|
||||
if ((phase !== 2 && phase !== 3) || state.authorizationUrl !== undefined) throw new Error(LOGIN_ERROR);
|
||||
state.authorizationUrl = url;
|
||||
} catch { fail(); }
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user