mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native remote runners use a session-owned broker for managed GitHub operations. > - That broker uses the existing authenticated sandbox callback bridge. > - The native caller omitted the server-selected duplex flag. An enabled and capable target still used file polling. > - Warm sessions must observe a changed flag and channel loss before the next admission. > - This pull request forwards the existing flag and retires unavailable or changed bridges before retained instruction preparation. > - The benefit is less idle remote traffic without changing credential authority or the default transport. ## Linked Issues or Issue Description **What happened?** A native GitHub callback bridge selected `queue_v1` even when its sandbox target had duplex enabled and supported it. The file bridge continues remote queue checks while the warm session remains available. **Expected behavior** An enabled, capable target should use the existing HTTP/2 bridge. Disabled or unsupported targets and failed channel startup must retain the authenticated file fallback. A changed server flag must take effect at the next normal warm-session boundary. **Steps to reproduce** 1. Create native GitHub access on a sandbox target with the server-selected duplex flag enabled, verified duplex capability and a working channel. 2. Observe the selected callback transport. 3. Before this fix, it is `queue_v1`. The added real-wire regression expects `http2_v1` and fails on the old source. ## What Changed - Forward the existing server-selected duplex flag through the native GitHub broker. - Record that transport selection on the broker. Retire the warm provider and broker through the existing cleanup path when the flag changes. - Read live channel failure when checking broker readiness. Mark normal session cleanup as an orderly completion. - Include the selected flag in instruction preparation identity. Retire a failed broker before adopting retained instructions, while preserving existing ownership and cleanup fences. - Add real local HTTP/2 credential-rotation and file-fallback controls. Add native warm-session flag-change controls. - Document selection, fallback and warm-session behavior. - Scan CI secret expressions in one forward pass, including quoted braces and escaped apostrophes. Preserve direct, bracket, multiline and whole-context references while rejecting filename and literal-string false positives. ## Verification - Current head `41440b07ea45e9e882e36ed495a66e501c8dd4df` resolves the actual CI-policy conflict with master `307eb16c0c59abd9e0626fde9201939b262e7f5f`. It retains upstream's stricter whole-expression credential guard, including quoted secret text, and uses the forward scanner instead of a backtracking regex. All **16 CI policy tests** pass, including the bounded 28- and 10,000-apostrophe controls in both policy modes. The reported old regex still has its recorded two-second timeout (14 passed, one failed). On this final merged source, **809 affected native cases** pass with zero selected skips (65.20s). Full repository typecheck (31.44s) and build (22.13s) pass. Frozen source hashes stayed unchanged. Fresh exact-head full CI, configured security and completed Apex remain pending. - Before master integration, the frozen native repair passed repository `pnpm -r typecheck` passed (33.0s) and `pnpm build` passed (32.1s). - Before the latest master integration, all **800 affected cases passed with zero selected skips**: complete native executor (725), resume (50), GitHub broker (16), environment duplex-gate (4), and heartbeat instruction-copy (5) suites. The final run took 95.3s. Source hashes stayed unchanged through these checks. - The initial head passed 788 affected cases and full CI, but Apex scored it **3/5**. It found a dead HTTP/2 broker could remain reusable and a flag change could fail after retained instruction preparation. Both findings required the new repair. Three added before-fix regressions failed on channel availability, orderly close, and preparation identity. - The first repair run failed three new warm-instruction fixtures because fake provider close left its recorded state ready. The corrected fixture writes a suspended checkpoint without weakening the production ownership guard. A separate real-wire run then failed two availability assertions because its helper spread had frozen the readiness getter. The helper now preserves the getter. All failed attempts remain in the local evidence. - One disposable real Daytona sandbox exercised both transports on the initial transport patch through the production native broker and callback bridge. The provider shell wrapper used the then-current production declarations. During equal two-second idle windows, file mode issued **9 remote commands** and HTTP/2 issued **0**. Both variants rotated synthetic credentials between two run bindings and denied idle requests. Cleanup deleted the sandbox. - Daytona component setup took 3.64s for file mode and 4.74s for HTTP/2. This is an idle-traffic reduction, not a startup-speed claim. That component run predates the availability and preparation repair; it does not qualify the repaired paths on Daytona. The new failure controls use a real local HTTP/2 channel. The credential resolver was synthetic and no model was called. Real GitHub-account grants, staging replies, and the end-to-end latency target remain unverified by this component test. - Repaired-head CI found a policy-test false positive: a broad `secrets.` regex matched an upstream filename. The unchanged test reproduced the failure, and the corrected version passed all 15 controls. Master later supplied a stronger direct/bracket check, which we retained when resolving the merge conflict. Review then found that formatted expressions could stop that check at a quoted brace. The new reported-case baseline failed (one failed, 14 passed). The final check treats quoted strings as complete tokens, respects expression boundaries, and passes all 15 cases. The later master conflict also added `toJSON(secrets)` controls. The old branch pattern failed that whole-context case. Review subsequently found exponential backtracking with escaped apostrophes; the final forward scanner retains the quoted-boundary and whole-context controls and passes all 15 tests, including the bounded long-expression regression. No workflow or credential handling changed. All failed attempts remain recorded. - The full local repository test suite was not rerun. [Prior-head full CI](https://github.com/paperclipai/paperclip/actions/runs/38775801821), including `ci / verify` and `ci / e2e`, passed on previous head `e5f8bdb425081cf08110c0f7423f96609e405920`. All 52 check runs succeeded, two optional Storybook jobs skipped, and legacy Snyk passed. Completed Apex review is **5/5** on that exact head, with no unresolved review threads. ## Risks - Only an explicit server-selected opt-in can use HTTP/2. The existing provider-capability, channel-startup and authenticated file-fallback checks remain in the shared bridge. - Toggling the flag or losing the HTTP/2 channel replaces an idle retained provider at the next run boundary. Instruction preparation must make the same retirement decision. Normal ownership, settled history and cleanup checks remain in effect. - Lower idle traffic may reduce provider/controller pressure, but no staging latency improvement is claimed. This feature remains inactive when the existing flag is off. Other preparation and finalization costs remain. - No schema, API, UI, credential-selection, tracing endpoint or default-setting changes. ## Model Used - OpenAI Codex, based on GPT-6, with tool-assisted reasoning and code execution. The exact hosted snapshot ID and context-window size are 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 - [ ] All Paperclip CI gates are green - [ ] 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>
152 lines
6.4 KiB
TypeScript
152 lines
6.4 KiB
TypeScript
import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto";
|
|
import { createServer } from "node:http";
|
|
import {
|
|
cleanupGitHubOperationLaunchers,
|
|
adapterExecutionTargetEnablesSandboxDuplexBridge,
|
|
githubOperationLauncherDirectory,
|
|
prepareGitHubOperationLaunchers,
|
|
startAdapterExecutionTargetPaperclipBridge,
|
|
} from "@paperclipai/adapter-utils/execution-target";
|
|
import { githubBrokerEnvironment } from "@paperclipai/adapter-utils/github-launcher";
|
|
|
|
type Binding = { companyId: string; agentId: string; issueId: string; runId: string };
|
|
type LauncherInput = Parameters<typeof prepareGitHubOperationLaunchers>[0];
|
|
export type NativeGitHubAccess = Awaited<ReturnType<typeof createNativeGitHubAccess>>;
|
|
|
|
/** A process-lifetime transport, with authority only while its controller owns a run.
|
|
* No run token or GitHub credential is stored in the provider's environment/files.
|
|
* The resolver still checks the active run and current identity/grants per operation.
|
|
*/
|
|
export async function createNativeGitHubAccess(input: {
|
|
scope: Omit<Binding, "runId">;
|
|
target: LauncherInput["target"];
|
|
cwd: string;
|
|
env: NodeJS.ProcessEnv;
|
|
resolveCredentials: (binding: Binding) => Promise<unknown>;
|
|
onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise<void>;
|
|
}, startBridge = startAdapterExecutionTargetPaperclipBridge) {
|
|
const duplexRequested = adapterExecutionTargetEnablesSandboxDuplexBridge(input.target);
|
|
const token = randomBytes(32).toString("hex");
|
|
const location = { runId: `native-session-${randomUUID()}`, target: input.target };
|
|
let active: Binding | null = null;
|
|
let stopped = false;
|
|
let ready = true;
|
|
let stopping: Promise<void> | undefined;
|
|
let bridge: Awaited<ReturnType<typeof startAdapterExecutionTargetPaperclipBridge>> = null;
|
|
const server = createServer(async (req, res) => {
|
|
res.setHeader("Cache-Control", "no-store");
|
|
res.setHeader("Content-Type", "application/json");
|
|
req.resume();
|
|
const reply = (status: number, body: unknown) => {
|
|
res.writeHead(status);
|
|
res.end(JSON.stringify(body));
|
|
};
|
|
const bearer = req.headers.authorization ?? "";
|
|
const expected = `Bearer ${token}`;
|
|
if (req.headers.origin || req.headers.cookie || req.headers["sec-fetch-site"] ||
|
|
Buffer.byteLength(bearer) !== Buffer.byteLength(expected) ||
|
|
!timingSafeEqual(Buffer.from(bearer), Buffer.from(expected))) {
|
|
reply(403, { error: "GitHub session authentication required" });
|
|
return;
|
|
}
|
|
if (req.method !== "POST" || req.url !== "/runtime-tools/github/credentials") {
|
|
reply(404, { error: "Unknown GitHub session operation" });
|
|
return;
|
|
}
|
|
const binding = active;
|
|
if (!binding || stopped) {
|
|
reply(403, { error: "No active GitHub run" });
|
|
return;
|
|
}
|
|
try {
|
|
// Bind at receipt; body/headers cannot select a run or responsible user.
|
|
const result = await input.resolveCredentials({ ...binding });
|
|
if (active !== binding || stopped) {
|
|
reply(403, { error: "GitHub run ended during credential acquisition" });
|
|
return;
|
|
}
|
|
reply(200, result);
|
|
} catch (error) {
|
|
const status = (error as { status?: number })?.status;
|
|
// Never leak secret-store/provider errors. Preserve steering's retry signal.
|
|
reply(status === 409 ? 409 : status === 403 ? 403 : 503,
|
|
{ error: "GitHub credentials unavailable" });
|
|
}
|
|
});
|
|
server.requestTimeout = 15_000;
|
|
server.headersTimeout = 10_000;
|
|
const stop = () => stopping ??= (async () => {
|
|
stopped = true;
|
|
active = null;
|
|
bridge?.markOrderlyCompletion?.();
|
|
server.closeAllConnections();
|
|
const results = await Promise.allSettled([
|
|
bridge?.stop(),
|
|
new Promise<void>(resolve => server.close(() => resolve())),
|
|
cleanupGitHubOperationLaunchers(location),
|
|
]);
|
|
if (results.some(result => result.status === "rejected")) {
|
|
await input.onLog?.("stderr", "[paperclip] GitHub session cleanup incomplete.\n").catch(() => undefined);
|
|
}
|
|
})();
|
|
try {
|
|
await new Promise<void>((resolve, reject) => {
|
|
server.once("error", reject);
|
|
server.listen(0, "127.0.0.1", () => { server.off("error", reject); resolve(); });
|
|
});
|
|
const address = server.address();
|
|
if (!address || typeof address === "string") throw new Error("GitHub broker did not listen");
|
|
const url = `http://127.0.0.1:${address.port}`;
|
|
try {
|
|
bridge = await startBridge({
|
|
...location,
|
|
runtimeRootDir: input.target?.kind === "remote"
|
|
? githubOperationLauncherDirectory(location)
|
|
: null,
|
|
adapterKey: "native-github",
|
|
hostApiToken: token,
|
|
hostApiUrl: url,
|
|
enableSandboxDuplexBridge: duplexRequested,
|
|
onLog: input.onLog,
|
|
});
|
|
if (input.target?.kind === "remote" && !bridge) {
|
|
throw new Error("GitHub session requires a remote callback bridge");
|
|
}
|
|
} catch {
|
|
// GitHub remains optional. Stage anonymous wrappers for this session;
|
|
// the supervisor retries transport setup on the next run.
|
|
ready = false;
|
|
await new Promise<void>(resolve => server.close(() => resolve()));
|
|
await input.onLog?.("stderr", "[paperclip] GitHub runtime transport unavailable; continuing without managed GitHub access.\n").catch(() => undefined);
|
|
}
|
|
const env = await prepareGitHubOperationLaunchers({
|
|
...location, cwd: input.cwd,
|
|
env: {
|
|
...githubBrokerEnvironment({ PATH: input.env.PATH }, {
|
|
url: ready ? bridge?.env.PAPERCLIP_API_URL ?? url : "",
|
|
token: ready ? bridge?.env.PAPERCLIP_API_KEY ?? token : "",
|
|
}),
|
|
// Never retain an old run's bridge authentication override.
|
|
PAPERCLIP_GITHUB_BRIDGE_TOKEN: ready ? bridge?.env.PAPERCLIP_API_KEY ?? token : "",
|
|
},
|
|
});
|
|
return {
|
|
env,
|
|
get ready() { return ready && !stopped && bridge?.readRunDisposition?.().failed !== true; },
|
|
duplexRequested,
|
|
activate(binding: Binding) {
|
|
if (stopped || active) throw new Error("GitHub session is closed or busy");
|
|
if (binding.companyId !== input.scope.companyId || binding.agentId !== input.scope.agentId ||
|
|
binding.issueId !== input.scope.issueId) throw new Error("GitHub session scope mismatch");
|
|
const owner = { ...binding };
|
|
active = owner;
|
|
return () => { if (active === owner) active = null; };
|
|
},
|
|
stop,
|
|
};
|
|
} catch (error) {
|
|
await stop().catch(() => undefined);
|
|
throw error;
|
|
}
|
|
}
|