mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix(adapter-utils): retry GitHub broker transport failures before falling back (#14856)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run `git` and `gh` through a managed launcher. The launcher gets a GitHub credential from the Paperclip control plane > - The launcher sends one request to the credential broker for each command > - If that request fails at the transport level, for example after a 10-second timeout, the launcher continues without managed credentials > - So a slow or restarting control plane removes the managed GitHub identity from that command. Some agents then use other GitHub identities that do not have the necessary permissions > - This pull request retries a failed broker request two more times, with a short backoff, before the launcher gives up > - The benefit is that a short control-plane delay does not remove the managed identity from an agent's GitHub operation ## Linked Issues or Issue Description Refs #14175. That pull request changes the same broker request loop for a different failure: sandbox network denials. The pull request that merges second must rebase. **What happened** A Codex agent ran `git` and `gh` through the managed launcher while the control plane was under heavy memory pressure. Each command printed `Paperclip: GitHub broker_transport_unavailable; continuing without managed credentials.` The agent then tried to open the pull request through a different GitHub integration. GitHub rejected the request with `403 Resource not accessible by integration`. **Expected behavior** A short broker delay or a short transport failure must not remove the managed GitHub identity from the command. The launcher must try the broker again before it continues without credentials. **Steps to reproduce** 1. Set `PAPERCLIP_GITHUB_BROKER_URL` to a closed port. 2. Start a broker on that port after about 300 ms. 3. Run `gh` through the launcher. 4. Before this change, the launcher prints `broker_transport_unavailable` and runs `gh` without the managed token. **Version or commit** `4ac374103` on master. Commit `3166e93a7` has the same code. **Deployment mode** Local trusted instance that runs as a launchd service, with `codex_local` agents. ## What Changed - `packages/adapter-utils/src/github-launcher.ts`: the broker request loop now catches transport errors and retries up to two more times, after 0.5 s and then after 1 s. The loop reads the response body inside the retry, so a failed or slow body read is also retried. Busy (409) responses keep their own budget of 30 attempts, separate from transport retries. After the third transport failure, the launcher prints `broker_transport_unavailable` as before. - `packages/adapter-utils/src/github-launcher.test.ts`: two new tests make the broker fail the first request and answer the second. In one, the connection drops before the response. In the other, the connection drops in the middle of the body. Each test checks that `gh` gets the managed token, that the broker receives exactly two requests, and that no `broker_transport_unavailable` message appears. - The existing `broker-offline` test now has a 15-second timeout, because each command now retries twice before it falls back. ## Verification - `npx vitest run packages/adapter-utils/src/github-launcher.test.ts`: 9 of 9 tests pass. - The body-read test fails on the first commit of this pull request and passes with the second commit. - `pnpm --filter @paperclipai/adapter-utils typecheck`: passes. - The existing `broker-offline` test confirms that the launcher still falls back after the retries, and that local Git still works. ## Risks - When the broker is unreachable, each `git` or `gh` command now waits about 1.5 s more before it continues without credentials. When the broker times out, the worst case is about 31.5 s instead of 10 s. - The change only adds retries. It does not change which credentials the launcher accepts or which environment variables it copies. - #14175 changes the same loop. The pull request that merges second needs a small rebase. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use: shell commands, file edits and test runs. The model wrote the change, the test and this description. The repository owner approved the change before it was made. ## 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 — *targeted tests and the package typecheck; see Verification* - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes — *no documentation describes the broker retry* - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green — *CI has not run yet* - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — *Greptile has not reviewed yet* - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8ec4b84e1c
commit
b721d24cac
2 files changed
+56
-12
No files matched your search
@@ -61,7 +61,7 @@ describe("managed GitHub launchers", () => {
|
||||
...process.env, ...githubBrokerEnvironment({}, { url: `http://127.0.0.1:${port}`, token: "private-capability" }),
|
||||
GH_CONFIG_DIR: configRoot, PATH: `${bin}:${process.env.PATH}`,
|
||||
} });
|
||||
});
|
||||
}, 15_000); // broker-offline retries the transport twice per command before it falls back
|
||||
|
||||
it("explains unavailable access while allowing local work without credentials", async () => {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-diagnostic-"));
|
||||
@@ -82,6 +82,35 @@ describe("managed GitHub launchers", () => {
|
||||
expect(result.stderr).toContain("More than one managed GitHub identity matches this run");
|
||||
expect(result.stderr).not.toMatch(/host-token|must-not-be-used|run-capability/);
|
||||
});
|
||||
// The first broker request fails, the second succeeds: the managed token must still reach gh.
|
||||
it.each([
|
||||
["connection drops before the response", (res: import("node:http").ServerResponse) => { res.socket?.destroy(); }],
|
||||
["body read fails mid-response", (res: import("node:http").ServerResponse) => {
|
||||
res.writeHead(200, {"content-type":"application/json"}); res.write('{"status":'); setTimeout(() => res.socket?.destroy(), 20);
|
||||
}],
|
||||
])("retries when the %s and still uses managed credentials", async (_label, fail) => {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-retry-"));
|
||||
cleanups.push(() => rm(root, {recursive:true,force:true}));
|
||||
const bin = path.join(root,"managed"), realBin = path.join(root,"real");
|
||||
await mkdir(bin); await mkdir(realBin);
|
||||
await writeFile(path.join(bin,"gh"), githubLauncherSource(), {mode:0o700});
|
||||
await writeFile(path.join(realBin,"gh"), '#!/usr/bin/env node\nprocess.stdout.write(JSON.stringify({token:process.env.GH_TOKEN ?? null}));', {mode:0o700});
|
||||
let requests = 0;
|
||||
const server = createServer((_req,res) => {
|
||||
requests++;
|
||||
if (requests === 1) return fail(res);
|
||||
res.setHeader("content-type","application/json");
|
||||
res.end(JSON.stringify({status:"available",env:{GH_TOKEN:"managed-token"}}));
|
||||
});
|
||||
await new Promise<void>(resolve => server.listen(0,"127.0.0.1",resolve));
|
||||
cleanups.push(() => new Promise<void>(resolve => server.close(() => resolve())));
|
||||
const {port} = server.address() as {port:number};
|
||||
const result = await exec(path.join(bin,"gh"), [], {env:{...process.env,...githubBrokerEnvironment({GH_TOKEN:"host-token"},{url:`http://127.0.0.1:${port}`,token:"run-capability"}),PATH:`${bin}:${realBin}:${process.env.PATH}`}});
|
||||
expect(JSON.parse(result.stdout)).toEqual({token:"managed-token"});
|
||||
expect(requests).toBe(2);
|
||||
expect(result.stderr).not.toContain("broker_transport_unavailable");
|
||||
expect(result.stderr).not.toMatch(/host-token|run-capability/);
|
||||
});
|
||||
it("captures each command's identity and clears host credentials when the next person has none", async () => {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-launcher-test-"));
|
||||
cleanups.push(() => rm(root, { recursive: true, force: true }));
|
||||
|
||||
@@ -54,21 +54,36 @@ async function main() {
|
||||
let response;
|
||||
if (base && env.PAPERCLIP_GITHUB_BROKER_TOKEN) {
|
||||
const url = base.replace(/\/+$/, '').replace(/\/api$/, '') + '/runtime-tools/github/credentials';
|
||||
for (let attempt = 0; attempt < 30; attempt++) {
|
||||
response = await fetch(url, {
|
||||
method: 'POST', redirect: 'error', signal: AbortSignal.timeout(10000),
|
||||
headers: { authorization: 'Bearer ' + (env.PAPERCLIP_GITHUB_BRIDGE_TOKEN || env.PAPERCLIP_API_KEY || env.PAPERCLIP_GITHUB_BROKER_TOKEN),
|
||||
'x-paperclip-github-capability': env.PAPERCLIP_GITHUB_BROKER_TOKEN, 'content-type': 'application/json' },
|
||||
body: '{}',
|
||||
});
|
||||
if (response.status !== 409) break;
|
||||
await response.arrayBuffer();
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
// A slow or restarting control plane must not cost the operation its
|
||||
// managed identity, so a failed request is retried before giving up.
|
||||
// Busy (409) responses and transport failures keep separate budgets, and
|
||||
// the body is read inside the retry so a failed read is retried too.
|
||||
let transportFailures = 0, conflicts = 0, result;
|
||||
for (;;) {
|
||||
try {
|
||||
response = await fetch(url, {
|
||||
method: 'POST', redirect: 'error', signal: AbortSignal.timeout(10000),
|
||||
headers: { authorization: 'Bearer ' + (env.PAPERCLIP_GITHUB_BRIDGE_TOKEN || env.PAPERCLIP_API_KEY || env.PAPERCLIP_GITHUB_BROKER_TOKEN),
|
||||
'x-paperclip-github-capability': env.PAPERCLIP_GITHUB_BROKER_TOKEN, 'content-type': 'application/json' },
|
||||
body: '{}',
|
||||
});
|
||||
if (response.status === 409 && conflicts < 29) {
|
||||
conflicts += 1;
|
||||
await response.arrayBuffer();
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
continue;
|
||||
}
|
||||
result = response.ok ? await response.json() : null;
|
||||
break;
|
||||
} catch (error) {
|
||||
transportFailures += 1;
|
||||
if (transportFailures >= 3) throw error;
|
||||
await new Promise(resolve => setTimeout(resolve, 500 * transportFailures));
|
||||
}
|
||||
}
|
||||
if (!response.ok) {
|
||||
diagnostic(response.status === 401 || response.status === 403 ? 'capability_rejected' : 'broker_response_unavailable');
|
||||
} else {
|
||||
const result = await response.json();
|
||||
if (result.status === 'unavailable') {
|
||||
const reason = typeof result.reason === 'string'
|
||||
? result.reason.replace(/[\x00-\x1f\x7f]/g, ' ').slice(0, 500)
|
||||
|
||||
Reference in new issue
Block a user