From 44dc165394c6b08ae0dc3e170d3e26cbfe10687e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 19:46:16 +0000 Subject: [PATCH 1/3] test(forgejo): reproduce #32617 fj reads failing on renamed repositories Forgejo answers the old path of a renamed repository with a same-origin relative 301. requestFj sends requests with redirect: "manual" and fails on the 301 instead of following it to the renamed path. Co-Authored-By: Claude Opus 5.5 --- .../ForgejoSourceControlProvider.test.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index fbfe278b32ba4..ab8043d64cd5b 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -1707,6 +1707,91 @@ it.effect("loads later fj review pages when the server caps pages below the requ ); }); +it.effect("reads an fj pull request through the path of a renamed repository", () => { + const requests: string[] = []; + return Effect.gen(function* () { + const cli = yield* ForgejoCli.make; + const provider = yield* ForgejoPullRequestProvider.make.pipe( + Effect.provideService(ForgejoCli.ForgejoCli, cli), + ); + const readSummary = provider.getChangeRequestSummary; + if (readSummary === undefined) return yield* Effect.die("summary read missing"); + const summary = yield* readSummary({ + cwd: "/repo", + repository: "maria/old-name", + host: "forgejo.test", + number: 7, + }); + assert.strictEqual(summary.number, 7); + assert.strictEqual(summary.url, "https://forgejo.test/maria/new-name/pulls/7"); + assert.deepStrictEqual(requests, [ + "https://forgejo.test/api/v1/repos/maria/old-name/pulls/7", + "https://forgejo.test/api/v1/repos/maria/new-name/pulls/7", + ]); + }).pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.makeNoop({ + exists: () => Effect.succeed(true), + readFileString: () => + Effect.succeed( + encodeJson({ hosts: { "forgejo.test": { type: "Application", token: "test-token" } } }), + ), + }), + ), + Effect.provideService( + HttpClient.HttpClient, + HttpClient.make((request) => { + requests.push(request.url); + assert.strictEqual(request.headers.authorization, "token test-token"); + const { pathname } = new URL(request.url); + if (pathname === "/api/v1/repos/maria/old-name/pulls/7") { + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + new Response( + 'Moved Permanently.', + { + status: 301, + headers: { location: "/api/v1/repos/maria/new-name/pulls/7" }, + }, + ), + ), + ); + } + assert.strictEqual(pathname, "/api/v1/repos/maria/new-name/pulls/7"); + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + new Response( + encodeJson({ + ...requiredChecksPullRequest, + html_url: "https://forgejo.test/maria/new-name/pulls/7", + }), + ), + ), + ); + }), + ), + Effect.provide( + TestSourceControlHost.layer({ + process: { + run: (input) => { + if (input.command === "git") { + return Effect.succeed( + processOutput("origin\thttps://forgejo.test/maria/old-name.git (fetch)"), + ); + } + assert.strictEqual(input.command, "fj"); + return Effect.succeed(processOutput("")); + }, + }, + }), + ), + Effect.provideService(HostProcess.HomeDirectory, "/home/maria"), + ); +}); + it.effect("handles fj mutation statuses without retrying failures or reading absent bodies", () => Effect.gen(function* () { for (const status of [204, 205, 302, 401, 403, 404, 429, 500]) { From 791f2afef7d738ce5a03a13e91db12b2c80d3bbb Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 19:55:03 +0000 Subject: [PATCH 2/3] fix(forgejo): fj requests follow a renamed repository's API redirect Forgejo answers a renamed repository's old API path with a relative 301. requestFj sends with redirect: "manual" and failed on it, so every read and write through the old path failed on every sync. requestFj now follows 301/302/307/308 up to five hops when the Location passes the same same-origin, /api/v1/, no-userinfo check as the request path, re-sending the same method, token and body. Any other redirect is not followed and fails as before. Co-Authored-By: Claude Opus 5.5 --- .../src/server/ForgejoCli.ts | 51 ++++++++++++----- .../ForgejoSourceControlProvider.test.ts | 56 +++++++++++++++++++ 2 files changed, 92 insertions(+), 15 deletions(-) diff --git a/packages/source-control-forgejo/src/server/ForgejoCli.ts b/packages/source-control-forgejo/src/server/ForgejoCli.ts index 4ae39f3ae9626..844321b4d043b 100644 --- a/packages/source-control-forgejo/src/server/ForgejoCli.ts +++ b/packages/source-control-forgejo/src/server/ForgejoCli.ts @@ -350,26 +350,47 @@ export const make = Effect.gen(function* () { readonly body?: string; }) { const base = new URL(`${input.baseUrl}/api/v1/`); - const url = new URL(input.path, base); - if ( - url.origin !== base.origin || - !url.pathname.startsWith(base.pathname) || - url.username || - url.password - ) + const apiUrl = (path: string, from: string) => { + const url = URL.parse(path, from); + return url && + url.origin === base.origin && + url.pathname.startsWith(base.pathname) && + !url.username && + !url.password + ? url + : undefined; + }; + const requested = apiUrl(input.path, base.href); + if (!requested) return yield* new ForgejoCliError({ command: "fj", cwd: input.cwd, detail: "Invalid Forgejo API path.", }); - let request = HttpClientRequest.make(input.method ?? "GET")(url.toString()).pipe( - HttpClientRequest.setHeader("Authorization", `token ${input.token}`), - ); - if (input.body !== undefined) - request = request.pipe(HttpClientRequest.bodyText(input.body, "application/json")); - const response = yield* httpClient - .execute(request) - .pipe(Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" })); + let url = requested; + const send = (target: URL) => { + let request = HttpClientRequest.make(input.method ?? "GET")(target.toString()).pipe( + HttpClientRequest.setHeader("Authorization", `token ${input.token}`), + ); + if (input.body !== undefined) + request = request.pipe(HttpClientRequest.bodyText(input.body, "application/json")); + return httpClient + .execute(request) + .pipe(Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" })); + }; + let response = yield* send(url); + // Forgejo redirects a renamed repository's API paths before running the handler, + // so the same method and body are re-sent. The token never leaves the API base. + for (let hops = 0; hops < 5; hops++) { + const location = response.headers.location; + const next = + location && [301, 302, 307, 308].includes(response.status) + ? apiUrl(location, url.href) + : undefined; + if (!next) break; + url = next; + response = yield* send(url); + } const status = response.status; const body = status === 204 || status === 205 diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index ab8043d64cd5b..55906bd26ee0c 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -1792,6 +1792,62 @@ it.effect("reads an fj pull request through the path of a renamed repository", ( ); }); +it.effect("re-sends an fj write with its method and body to a renamed repository's path", () => { + const requests: string[] = []; + return Effect.gen(function* () { + const cli = yield* ForgejoCli.make; + yield* cli.api({ + cwd: "/repo", + repository: "https://forgejo.local/maria/old-name", + path: "repos/maria/old-name/issues/42/comments", + method: "POST", + body: { body: "only once" }, + }); + assert.deepStrictEqual(requests, [ + "https://forgejo.local/api/v1/repos/maria/old-name/issues/42/comments", + "https://forgejo.local/api/v1/repos/maria/new-name/issues/42/comments", + ]); + }).pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.makeNoop({ + exists: () => Effect.succeed(true), + readFileString: () => + Effect.succeed( + encodeJson({ + hosts: { "forgejo.local": { type: "Application", token: "test-token" } }, + }), + ), + }), + ), + Effect.provideService( + HttpClient.HttpClient, + HttpClient.make((request) => { + requests.push(request.url); + assert.strictEqual(request.method, "POST"); + assert.strictEqual(request.headers.authorization, "token test-token"); + assert.deepStrictEqual(requestJson(request), { body: "only once" }); + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + requests.length === 1 + ? new Response(null, { + status: 301, + headers: { location: "/api/v1/repos/maria/new-name/issues/42/comments" }, + }) + : new Response("{}", { status: 201 }), + ), + ); + }), + ), + Effect.provide( + TestSourceControlHost.layer({ + process: { run: () => Effect.succeed(processOutput("")) }, + }), + ), + ); +}); + it.effect("handles fj mutation statuses without retrying failures or reading absent bodies", () => Effect.gen(function* () { for (const status of [204, 205, 302, 401, 403, 404, 429, 500]) { From fe167c169599b89de948ffbe1ea59943a9a1199c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 10 Nov 2026 19:39:21 +0000 Subject: [PATCH 3/3] test(forgejo): keep one redirect regression test The POST case covers the shared redirect path, including method, body and auth. AI-assisted; verified with the tests and tools listed above. --- .../ForgejoSourceControlProvider.test.ts | 85 ------------------- 1 file changed, 85 deletions(-) diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index 66ed6c5c16153..a5215755134c1 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -1778,91 +1778,6 @@ it.effect("loads later fj review pages when the server caps pages below the requ ); }); -it.effect("reads an fj pull request through the path of a renamed repository", () => { - const requests: string[] = []; - return Effect.gen(function* () { - const cli = yield* ForgejoCli.make; - const provider = yield* ForgejoPullRequestProvider.make.pipe( - Effect.provideService(ForgejoCli.ForgejoCli, cli), - ); - const readSummary = provider.getChangeRequestSummary; - if (readSummary === undefined) return yield* Effect.die("summary read missing"); - const summary = yield* readSummary({ - cwd: "/repo", - repository: "maria/old-name", - host: "forgejo.test", - number: 7, - }); - assert.strictEqual(summary.number, 7); - assert.strictEqual(summary.url, "https://forgejo.test/maria/new-name/pulls/7"); - assert.deepStrictEqual(requests, [ - "https://forgejo.test/api/v1/repos/maria/old-name/pulls/7", - "https://forgejo.test/api/v1/repos/maria/new-name/pulls/7", - ]); - }).pipe( - Effect.provideService( - FileSystem.FileSystem, - FileSystem.makeNoop({ - exists: () => Effect.succeed(true), - readFileString: () => - Effect.succeed( - encodeJson({ hosts: { "forgejo.test": { type: "Application", token: "test-token" } } }), - ), - }), - ), - Effect.provideService( - HttpClient.HttpClient, - HttpClient.make((request) => { - requests.push(request.url); - assert.strictEqual(request.headers.authorization, "token test-token"); - const { pathname } = new URL(request.url); - if (pathname === "/api/v1/repos/maria/old-name/pulls/7") { - return Effect.succeed( - HttpClientResponse.fromWeb( - request, - new Response( - 'Moved Permanently.', - { - status: 301, - headers: { location: "/api/v1/repos/maria/new-name/pulls/7" }, - }, - ), - ), - ); - } - assert.strictEqual(pathname, "/api/v1/repos/maria/new-name/pulls/7"); - return Effect.succeed( - HttpClientResponse.fromWeb( - request, - new Response( - encodeJson({ - ...requiredChecksPullRequest, - html_url: "https://forgejo.test/maria/new-name/pulls/7", - }), - ), - ), - ); - }), - ), - Effect.provide( - TestSourceControlHost.layer({ - process: { - run: (input) => { - if (input.command === "git") { - return Effect.succeed( - processOutput("origin\thttps://forgejo.test/maria/old-name.git (fetch)"), - ); - } - assert.strictEqual(input.command, "fj"); - return Effect.succeed(processOutput("")); - }, - }, - }), - ), - Effect.provideService(HostProcess.HomeDirectory, "/home/maria"), - ); -}); - it.effect("re-sends an fj write with its method and body to a renamed repository's path", () => { const requests: string[] = []; return Effect.gen(function* () {