mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
## Thinking Path > - Paperclip manages AI agents that do work for users. > - Product E2E journeys check real browser actions and saved results. > - A rich editor can store plain underscores as escaped Markdown. > - The QA reply check treated that valid saved message as a failure. > - A private target adapter also needs to cancel credential-safe HTTP requests before cleanup. > - This PR fixes reply comparison and adds abort and timeout options to the existing sensitive transport. ## Linked Issues or Issue Description Refs #18222. **What happened?** The artifact revision and Stop journeys rejected the correct single user reply because its underscore was stored as `\_`. Sensitive connection setup also lacked a way to pass its lifecycle signal to the actual HTTP request. **Expected behavior** The check accepts the editor's underscore escape and still rejects changed, duplicate, missing, or foreign replies. A cancelled sensitive request closes before setup cleanup proceeds. **Steps to reproduce** Run an artifact revision with an underscore in the reply. For the transport case, abort a pending sensitive request or let its deadline expire. ## What Changed - Compare the fixture reply after undoing only underscore escaping. Keep exact text, count, and author checks. - Let `ConnectionApi.postSensitive` accept a bounded timeout and abort signal. Check cancellation again after cookie loading and before dispatch. - Test the retained reply failure and both cancellation paths with an actual loopback HTTP server. Document the adapter contract. ## Verification - Focused Product E2E tests: 30 passed across two files. - Product E2E TypeScript check passed. After rebase, a final check first found four diagnostics from stale runner/plugin SDK declarations; rebuilding those two workspace packages cleared all four, and the unchanged E2E check passed. - The first sandbox run failed because loopback `listen` returned `EPERM`. The unchanged HTTP tests passed with loopback permission. The test now reports listener errors directly. - Both independent reviewers found a possible pre-arrival deadline-test hang. The follow-up races arrival against request settlement and adds one second of deadline headroom. All 30 affected tests passed after that fix. - Full current-head CI passed in run 40982914525. Greptile and Paperclip Review both scored 5/5 at `f0526f5fbbf13de5ea43e868eb012069ca3d8907`; all review threads are resolved. Live QA requalification follows the merged public and private fixes; no live pass is claimed here. ## Risks The text normalization applies only to plain fixture replies and only removes underscore escapes. It does not remove arbitrary Markdown. Sensitive calls retain their existing 30-second default and closed error messages. API wrappers must forward the optional signal and await the actual request. ## Model Used OpenAI Codex, GPT-6 family. The serving model ID and context window are not exposed by this session. Used reasoning, code editing, shell execution, and test inspection. ## 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>
75 lines
5.1 KiB
TypeScript
75 lines
5.1 KiB
TypeScript
import { afterEach, expect, it, vi } from "vitest";
|
|
import type { APIRequestContext } from "@playwright/test";
|
|
import { ConnectionApi } from "./connection-target.js";
|
|
import { createServer } from "node:http";
|
|
|
|
afterEach(() => vi.unstubAllGlobals());
|
|
it("uses same-origin session cookies without sending fixture credentials through the traced request transport", async () => {
|
|
const cookie = (domain: string, path = "/") => ({ name: domain, value: "session", domain, path, expires: -1, secure: true });
|
|
const request = { storageState: vi.fn(async () => ({ cookies: [cookie("qa.example.test"), cookie("other.example.test"), cookie("qa.example.test", "/other"), cookie("qa.example.test", "/api/company")] })),
|
|
fetch: vi.fn(() => { throw new Error("Sensitive body reached the traced transport"); }) };
|
|
const send = vi.fn(async (_url: URL, options: RequestInit) => {
|
|
expect(options.redirect).toBe("error");
|
|
expect(options.headers).toMatchObject({ Origin: "https://qa.example.test", Cookie: "qa.example.test=session" });
|
|
expect(options.body).toBe(JSON.stringify({ credential: "fixture-only" }));
|
|
return Response.json({ id: "owned-connection" });
|
|
});
|
|
vi.stubGlobal("fetch", send);
|
|
const api = new ConnectionApi(request as unknown as APIRequestContext, "https://qa.example.test");
|
|
await expect(api.postSensitive("/api/companies/owned/tools/apps/connect", { credential: "fixture-only" })).resolves.toEqual({ id: "owned-connection" });
|
|
expect(request.fetch).not.toHaveBeenCalled();
|
|
await expect(api.postSensitive("https://other.example.test/api/key", {})).rejects.toThrow("invalid_api_route");
|
|
await expect(api.postSensitive("/login", {})).rejects.toThrow("invalid_api_route");
|
|
expect(send).toHaveBeenCalledTimes(1);
|
|
});
|
|
it.each(["/api/companies", "/api/companies/", "/api/companies/owned/tools/apps/connect"])("sends a cookie scoped to the exact path or a containing directory: %s", async (path) => {
|
|
const request = { storageState: async () => ({ cookies: [{ name: "scoped", value: "session", domain: "qa.example.test", path, expires: -1, secure: true }] }) } as unknown as APIRequestContext;
|
|
const send = vi.fn(async (_url: URL, options: RequestInit) => {
|
|
expect(options.headers).toMatchObject({ Cookie: "scoped=session" });
|
|
return Response.json({ id: "owned" });
|
|
});
|
|
vi.stubGlobal("fetch", send);
|
|
await new ConnectionApi(request, "https://qa.example.test").postSensitive("/api/companies/owned/tools/apps/connect", {});
|
|
expect(send).toHaveBeenCalledOnce();
|
|
});
|
|
it("withholds provider bodies and thrown transport details on sensitive failures", async () => {
|
|
const request = { storageState: async () => ({ cookies: [] }) } as unknown as APIRequestContext;
|
|
const api = new ConnectionApi(request, "https://qa.example.test");
|
|
vi.stubGlobal("fetch", async () => new Response("SECRET response", { status: 400 }));
|
|
await expect(api.postSensitive("/api/connection", {})).rejects.toThrow("api_status_400");
|
|
vi.stubGlobal("fetch", async () => { throw new Error("SECRET transport"); });
|
|
await expect(api.postSensitive("/api/connection", {})).rejects.toThrow("sensitive_api_request_failed");
|
|
});
|
|
|
|
it.each(["cancel", "deadline"])("retires the actual sensitive HTTP request on %s", async mode => {
|
|
let entered!: () => void, retired!: () => void;
|
|
const received = new Promise<void>(resolve => { entered = resolve; });
|
|
const closed = new Promise<void>(resolve => { retired = resolve; });
|
|
const server = createServer((_req, response) => { response.once("close", retired); entered(); });
|
|
await new Promise<void>((resolve, reject) => { server.once("error", reject); server.listen(0, "127.0.0.1", resolve); });
|
|
try {
|
|
const address = server.address() as { port: number };
|
|
const request = { storageState: async () => ({ cookies: [] }) } as unknown as APIRequestContext;
|
|
const controller = new AbortController();
|
|
const send = new ConnectionApi(request, `http://127.0.0.1:${address.port}`).postSensitive("/api/owned", { fixture: true },
|
|
{ signal: controller.signal, timeoutMs: mode === "deadline" ? 1000 : 3000 });
|
|
const rejected = expect(send).rejects.toThrow("sensitive_api_request_failed");
|
|
// A heavily delayed dispatch can time out before reaching the listener.
|
|
// Report that failure and still clean up, rather than hanging on `received`.
|
|
const first = await Promise.race([received.then(() => "received"), send.then(() => "settled", () => "settled")]);
|
|
if (mode === "cancel") controller.abort();
|
|
await rejected;
|
|
expect(first).toBe("received");
|
|
await closed;
|
|
} finally { server.closeAllConnections(); await new Promise<void>(resolve => server.close(() => resolve())); }
|
|
});
|
|
|
|
it("does not dispatch a sensitive mutation if cancellation happens while loading cookies", async () => {
|
|
const controller = new AbortController();
|
|
const request = { storageState: async () => { controller.abort(); return { cookies: [] }; } } as unknown as APIRequestContext;
|
|
const send = vi.fn(); vi.stubGlobal("fetch", send);
|
|
await expect(new ConnectionApi(request, "https://qa.example.test").postSensitive("/api/owned", {}, { signal: controller.signal }))
|
|
.rejects.toThrow("sensitive_api_request_failed");
|
|
expect(send).not.toHaveBeenCalled();
|
|
});
|