From 5f430772c8ef73084a05ccb7c1ecc4213511c3f1 Mon Sep 17 00:00:00 2001 From: Umut Date: Wed, 2 Sep 2026 14:00:39 +0300 Subject: [PATCH 01/15] fix(server): skip PR polling for unknown providers --- apps/server/src/git/GitManager.test.ts | 148 +++++++++++++++++++++++-- apps/server/src/git/GitManager.ts | 90 +++++++++------ 2 files changed, 197 insertions(+), 41 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index fc2a2c81279d..a365de6bc89a 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -14,6 +14,7 @@ import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as References from "effect/References"; import * as Scope from "effect/Scope"; +import * as TestClock from "effect/testing/TestClock"; import { ChildProcessSpawner } from "effect/unstable/process"; import { expect } from "vite-plus/test"; import type { @@ -27,6 +28,7 @@ import { GitCommandError, ProviderDriverKind, ProviderInstanceId, + type SourceControlProviderKind, TextGenerationError, } from "@t3tools/contracts"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; @@ -621,6 +623,7 @@ function makeManager(input?: { serverSettings?: Parameters[0]; setupScriptRunner?: ProjectSetupScriptRunner.ProjectSetupScriptRunner["Service"]; gitConfigReads?: string[]; + sourceControlProviderKind?: SourceControlProviderKind | (() => SourceControlProviderKind); }) { const { service: gitHubCli, ghCalls } = createGitHubCliWithFakeGh(input?.ghScenario); const textGeneration = createTextGeneration(input?.textGeneration); @@ -657,14 +660,22 @@ function makeManager(input?: { const sourceControlRegistryLayer = Layer.effect( SourceControlProviderRegistry.SourceControlProviderRegistry, GitHubSourceControlProvider.make.pipe( - Effect.map((provider) => - SourceControlProviderRegistry.SourceControlProviderRegistry.of({ - get: () => Effect.succeed(provider), - resolveHandle: () => Effect.succeed({ provider, context: null }), - resolve: () => Effect.succeed(provider), + Effect.map((provider) => { + const sourceControlProvider = () => ({ + ...provider, + kind: + typeof input?.sourceControlProviderKind === "function" + ? input.sourceControlProviderKind() + : (input?.sourceControlProviderKind ?? provider.kind), + }); + return SourceControlProviderRegistry.SourceControlProviderRegistry.of({ + get: () => Effect.sync(sourceControlProvider), + resolveHandle: () => + Effect.sync(() => ({ provider: sourceControlProvider(), context: null })), + resolve: () => Effect.sync(sourceControlProvider), discover: Effect.succeed([]), - }), - ), + }); + }), Effect.provide(Layer.succeed(GitHubCli.GitHubCli, gitHubCli)), ), ); @@ -943,6 +954,129 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("status skips PR lookup when the source-control provider is unknown", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-unknown-provider-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/unknown-provider"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/unknown-provider"]); + + const { manager, ghCalls } = yield* makeManager({ + sourceControlProviderKind: "unknown", + }); + + const status = yield* manager.status({ cwd: repoDir }); + + expect(status.pr).toBeNull(); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + }), + ); + + it.effect("status retries an unknown provider on the short PR lookup cadence", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-provider-refinement-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/provider-refinement"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/provider-refinement"]); + + let providerKind: SourceControlProviderKind = "unknown"; + const existingPr = { + number: 411, + title: "Provider refinement PR", + url: "https://github.com/pingdotgg/t3code/pull/411", + baseRefName: "main", + headRefName: "feature/provider-refinement", + }; + const { manager, ghCalls } = yield* makeManager({ + sourceControlProviderKind: () => providerKind, + ghScenario: { + // @effect-diagnostics-next-line preferSchemaOverJson:off + prListSequence: [JSON.stringify([existingPr])], + }, + }); + + const first = yield* manager.status({ cwd: repoDir }); + expect(first.pr).toBeNull(); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + + providerKind = "github"; + yield* manager.invalidateRemoteStatus(repoDir); + + const cached = yield* manager.status({ cwd: repoDir }); + expect(cached.pr).toBeNull(); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + + yield* TestClock.adjust(Duration.seconds(20)); + + const second = yield* manager.status({ cwd: repoDir }); + expect(second.pr?.number).toBe(411); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(1); + }), + ); + + it.effect("status keeps the last known PR while the provider is temporarily unknown", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-provider-hiccup-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/provider-hiccup"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/provider-hiccup"]); + + let providerKind: SourceControlProviderKind = "github"; + const existingPr = { + number: 412, + title: "Provider hiccup PR", + url: "https://github.com/pingdotgg/t3code/pull/412", + baseRefName: "main", + headRefName: "feature/provider-hiccup", + }; + const { manager } = yield* makeManager({ + sourceControlProviderKind: () => providerKind, + ghScenario: { + // @effect-diagnostics-next-line preferSchemaOverJson:off + prListSequence: [JSON.stringify([existingPr])], + }, + }); + + const first = yield* manager.status({ cwd: repoDir }); + expect(first.pr?.number).toBe(412); + + providerKind = "unknown"; + yield* manager.invalidateStatus(repoDir); + + const second = yield* manager.status({ cwd: repoDir }); + expect(second.pr?.number).toBe(412); + }), + ); + + it.effect("branch PR lookup returns null when the source-control provider is unknown", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-unknown-branch-provider-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/unknown-branch-provider"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/unknown-branch-provider"]); + + const { manager, ghCalls } = yield* makeManager({ + sourceControlProviderKind: "unknown", + }); + + const pullRequest = yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/unknown-branch-provider", + }); + + expect(pullRequest).toBeNull(); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + }), + ); + it.effect("status briefly caches repeated lookups for the same cwd", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 393a8fd05592..104efd63ce6e 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -162,6 +162,10 @@ interface PullRequestInfo extends OpenPrInfo, PullRequestHeadRemoteInfo { updatedAt: Option.Option; } +type PrLookupOutcome = + | { readonly _tag: "Complete"; readonly latest: PullRequestInfo | null } + | { readonly _tag: "ProviderUnknown" }; + const pullRequestUpdatedAtDescOrder: Order.Order = Order.mapInput( Order.flip(Option.makeOrder(DateTime.Order)), (pullRequest) => pullRequest.updatedAt, @@ -934,11 +938,12 @@ export const make = Effect.gen(function* () { normalizeStatusCacheKey(cwd).pipe( Effect.flatMap((cacheKey) => Cache.invalidate(localStatusResultCache, cacheKey)), ); - // PR lookups hit the hosting provider's API (gh/glab/...), so they refresh - // on their own, slower cadence: ahead/behind counts stay fresh on every - // status poll while the PR association is re-fetched at most once per - // PR_LOOKUP_CACHE_TTL per branch. Git actions and user-driven refreshes bump - // the epoch (invalidateStatus) to bypass the cache immediately. + // PR lookups hit the hosting provider's API (gh/glab/...), so definitive + // results refresh on the slower PR_LOOKUP_CACHE_TTL cadence. An unresolved + // provider starts at the shorter failure cadence and backs off while it stays + // unresolved, capped at the healthy lookup cadence. Git actions and + // user-driven refreshes bump the epoch (invalidateStatus) to bypass the cache + // immediately. const prLookupEpochByCwd = new Map(); const prLookupEpoch = (cwd: string) => prLookupEpochByCwd.get(cwd) ?? 0; const bumpPrLookupEpoch = (cwd: string) => @@ -968,8 +973,9 @@ export const make = Effect.gen(function* () { details.remoteName ?? "", String(prLookupEpoch(cwd)), ].join("\u0000"); - // Consecutive failures per cache key, so a branch that keeps failing waits - // longer before the next attempt. Cleared as soon as a lookup succeeds. + // Consecutive failed or non-definitive attempts per cache key, so a branch + // that keeps failing waits longer before the next attempt. Cleared as soon + // as the lookup produces a definitive result. const prLookupFailureStreakByKey = new Map(); const nextPrLookupFailureTtl = (key: string) => { if ( @@ -1017,7 +1023,10 @@ export const make = Effect.gen(function* () { upstreamHeadIsDefault && !headContext.isCrossRepository ) { - return { latest: null, headContext }; + return { + outcome: { _tag: "Complete", latest: null } satisfies PrLookupOutcome, + headContext, + }; } // Only skip when the branch is untracked as well: anything carrying an // upstream keeps the old behaviour. @@ -1026,16 +1035,22 @@ export const make = Effect.gen(function* () { details.upstreamRef === null && (yield* isUnpublishedBranch(cwd, headContext)) ) { - return { latest: null, headContext }; + return { + outcome: { _tag: "Complete", latest: null } satisfies PrLookupOutcome, + headContext, + }; } - const latest = yield* findLatestPrForHeadContext(cwd, headContext); - return { latest, headContext }; + const outcome = yield* findLatestPrForHeadContext(cwd, headContext); + return { outcome, headContext }; }); }, { capacity: PR_LOOKUP_CACHE_CAPACITY, timeToLive: (exit, key) => { if (Exit.isSuccess(exit)) { + if (exit.value.outcome._tag === "ProviderUnknown") { + return Duration.min(nextPrLookupFailureTtl(key), PR_LOOKUP_CACHE_TTL); + } prLookupFailureStreakByKey.delete(key); return PR_LOOKUP_CACHE_TTL; } @@ -1114,27 +1129,29 @@ export const make = Effect.gen(function* () { // `push -u`) must not orphan the fallback value for the same branch. const branchKey = `${cwd}\u0000${details.branch}`; return yield* Cache.get(prLookupCache, prLookupCacheKey(cwd, details)).pipe( - Effect.map(({ latest, headContext }) => { - if (!latest) return { pr: null, headContext }; - // On the default branch, only surface open PRs. - // Merged/closed matches are usually reverse-merge history, not the thread's PR context. - if (details.isDefaultBranch && latest.state !== "open") { - return { pr: null, headContext }; + Effect.flatMap(({ outcome, headContext }) => { + const lastKnownContext = { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }; + if (outcome._tag === "ProviderUnknown") { + return Effect.succeed(resolveLastKnownPr(branchKey, lastKnownContext)); } - return { pr: toStatusPr(latest), headContext }; + + const pr = + outcome.latest === null || + // On the default branch, only surface open PRs. Merged/closed + // matches are usually reverse-merge history, not the thread's PR. + (details.isDefaultBranch && outcome.latest.state !== "open") + ? null + : toStatusPr(outcome.latest); + return Effect.sync(() => { + rememberLastKnownPr(branchKey, { pr, ...lastKnownContext }); + return pr; + }); }), - Effect.tap(({ pr, headContext }) => - Effect.sync(() => - rememberLastKnownPr(branchKey, { - pr, - upstreamRef: details.upstreamRef, - headBranch: headContext.headBranch, - remoteName: headContext.remoteName, - headRemoteUrlKey: headContext.headRemoteUrlKey, - }), - ), - ), - Effect.map(({ pr }) => pr), Effect.catch((error) => Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe( Effect.annotateLogs({ @@ -1433,10 +1450,14 @@ export const make = Effect.gen(function* () { cwd: string, headContext: BranchHeadContext, ) { + const provider = yield* sourceControlProvider(cwd); + if (provider.kind === "unknown") { + return { _tag: "ProviderUnknown" } satisfies PrLookupOutcome; + } const parsedByNumber = new Map(); for (const headSelector of headContext.headSelectors) { - const pullRequests = yield* (yield* sourceControlProvider(cwd)).listChangeRequests({ + const pullRequests = yield* provider.listChangeRequests({ cwd, headSelector, state: "all", @@ -1455,9 +1476,9 @@ export const make = Effect.gen(function* () { const latestOpenPr = parsed.find((pr) => pr.state === "open"); if (latestOpenPr) { - return latestOpenPr; + return { _tag: "Complete", latest: latestOpenPr } satisfies PrLookupOutcome; } - return parsed[0] ?? null; + return { _tag: "Complete", latest: parsed[0] ?? null } satisfies PrLookupOutcome; }); const buildCompletionToast = Effect.fn("buildCompletionToast")(function* ( cwd: string, @@ -2039,7 +2060,8 @@ export const make = Effect.gen(function* () { }); } } - const { latest } = cached; + if (cached.outcome._tag === "ProviderUnknown") return null; + const { latest } = cached.outcome; if (latest === null) return null; if ( (branch === defaultBranch || From 8998149a8e30283aef186684096e3e29609800bd Mon Sep 17 00:00:00 2001 From: Umut Date: Wed, 2 Sep 2026 14:09:19 +0300 Subject: [PATCH 02/15] test(server): document raw JSON diagnostic suppressions --- apps/server/src/git/GitManager.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index a365de6bc89a..09f9f2e8abe0 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -994,6 +994,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager, ghCalls } = yield* makeManager({ sourceControlProviderKind: () => providerKind, ghScenario: { + // Fake gh returns raw JSON stdout, matching the CLI boundary under test. // @effect-diagnostics-next-line preferSchemaOverJson:off prListSequence: [JSON.stringify([existingPr])], }, @@ -1038,6 +1039,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { const { manager } = yield* makeManager({ sourceControlProviderKind: () => providerKind, ghScenario: { + // Fake gh returns raw JSON stdout, matching the CLI boundary under test. // @effect-diagnostics-next-line preferSchemaOverJson:off prListSequence: [JSON.stringify([existingPr])], }, From 6f9ec22b81bdf5068c7f254d24918904e3d470a4 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 07:44:43 +0300 Subject: [PATCH 03/15] chore: repair PR 9212 against current main --- .github/workflows/repair-pr-9212.yml | 198 +++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) create mode 100644 .github/workflows/repair-pr-9212.yml diff --git a/.github/workflows/repair-pr-9212.yml b/.github/workflows/repair-pr-9212.yml new file mode 100644 index 000000000000..bfde23e3e05d --- /dev/null +++ b/.github/workflows/repair-pr-9212.yml @@ -0,0 +1,198 @@ +name: Repair PR 9212 + +on: + push: + branches: + - fix/unknown-provider-pr-polling + +permissions: + contents: write + +jobs: + repair: + if: github.actor != 'github-actions[bot]' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: fix/unknown-provider-pr-polling + + - name: Merge current upstream main and resolve semantic conflicts + shell: bash + run: | + set -euo pipefail + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git remote add upstream https://github.com/pingdotgg/t3code.git 2>/dev/null || git remote set-url upstream https://github.com/pingdotgg/t3code.git + git fetch upstream main + git merge --no-commit --no-ff -Xours upstream/main + + python3 <<'PY' + from pathlib import Path + import subprocess + + path = "apps/server/src/git/GitManager.ts" + p = Path(path) + text = p.read_text() + main = subprocess.check_output(["git", "show", f"upstream/main:{path}"], text=True) + + # Preserve current main's published-local-branch lookup resolution. + helper_marker = " // The remote that holds a ref named after the local branch" + unpublished_marker = " /**\n * Whether git has no record of this branch on any remote" + if helper_marker not in text: + hs = main.index(helper_marker) + he = main.index(unpublished_marker, hs) + insert_at = text.index(unpublished_marker) + text = text[:insert_at] + main[hs:he] + text[insert_at:] + + # Keep main's resolveLookupHeadContext behavior while retaining the PR's + # non-definitive ProviderUnknown cache outcome. + cache_anchor = text.index(" const prLookupCache = yield* Cache.makeWith(") + gen_start = text.index(" return Effect.gen(function* () {", cache_anchor) + skip_comment = text.index( + " // Only skip when the branch is untracked as well:", gen_start + ) + cache_prefix = ''' return Effect.gen(function* () { + const { headContext, lookup } = yield* resolveLookupHeadContext(cwd, details); + if (!lookup) { + return { + outcome: { _tag: "Complete", latest: null } satisfies PrLookupOutcome, + headContext, + }; + } + ''' + text = text[:gen_start] + cache_prefix + text[skip_comment:] + + # Merge current main's refreshMissingPullRequest behavior with the new + # outcome shape. ProviderUnknown must keep its retry backoff and must + # not be treated as a definitive missing PR. + lookup_start = text.index(' const lookupStatusPr = Effect.fn("lookupStatusPr")') + lookup_end = text.index(' const remoteStatusResultCache =', lookup_start) + merged_lookup = ''' const lookupStatusPr = Effect.fn("lookupStatusPr")(function* ( + cwd: string, + details: { + branch: string; + upstreamRef: string | null; + defaultBranch: string | null; + isDefaultBranch: boolean; + }, + refreshMissingPullRequest = false, + ) { + // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first + // `push -u`) must not orphan the fallback value for the same branch. + const branchKey = `${cwd}\\u0000${details.branch}`; + const cacheKey = prLookupCacheKey(cwd, details); + if (refreshMissingPullRequest) { + const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe( + Effect.orElseSucceed(() => Option.none()), + ); + if ( + Option.isSome(cached) && + cached.value.outcome._tag === "Complete" && + cached.value.outcome.latest === null + ) { + yield* Cache.invalidate(prLookupCache, cacheKey); + } + } + return yield* Cache.get(prLookupCache, cacheKey).pipe( + Effect.flatMap(({ outcome, headContext }) => { + const lastKnownContext = { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }; + if (outcome._tag === "ProviderUnknown") { + return Effect.succeed(resolveLastKnownPr(branchKey, lastKnownContext)); + } + + const pr = + outcome.latest === null || + // On the default branch, only surface open PRs. Merged/closed + // matches are usually reverse-merge history, not the thread's PR. + (details.isDefaultBranch && outcome.latest.state !== "open") + ? null + : toStatusPr(outcome.latest); + return Effect.sync(() => { + rememberLastKnownPr(branchKey, { pr, ...lastKnownContext }); + return pr; + }); + }), + Effect.catch((error) => + Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe( + Effect.annotateLogs({ + operation: "lookupStatusPr", + branch: details.branch, + errorTag: + typeof error === "object" && error !== null && "_tag" in error + ? String(error._tag) + : typeof error, + ...(isSourceControlProviderError(error) + ? { + provider: error.provider, + providerOperation: error.operation, + providerCommand: error.command ?? "unknown", + errorDetail: error.detail, + } + : {}), + }), + Effect.andThen(resolveLookupHeadContext(cwd, details)), + Effect.map(({ headContext }) => + resolveLastKnownPr(branchKey, { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }), + ), + ), + ), + ); + }); + const readRemoteStatus = Effect.fn("readRemoteStatus")(function* ( + cwd: string, + options?: GitRemoteStatusOptions, + ) { + const details = yield* gitCore + .statusDetailsRemote(cwd, options) + .pipe(Effect.catchIf(isNotGitRepositoryError, () => Effect.succeed(null))); + if (details === null || !details.isRepo) { + return null; + } + + const pr = + details.branch !== null + ? yield* lookupStatusPr( + cwd, + { + branch: details.branch, + upstreamRef: details.upstreamRef, + defaultBranch: details.defaultBranch, + isDefaultBranch: details.isDefaultBranch, + }, + options?.refreshMissingPullRequest, + ) + : null; + + return { + hasUpstream: details.hasUpstream, + aheadCount: details.aheadCount, + behindCount: details.behindCount, + aheadOfDefaultCount: details.aheadOfDefaultCount, + pr, + } satisfies VcsStatusRemoteResult; + }); + ''' + text = text[:lookup_start] + merged_lookup + text[lookup_end:] + + p.write_text(text) + PY + + # This workflow is only a transport for the repair; do not leave it in the PR. + rm -f .github/workflows/repair-pr-9212.yml + + git add -A + git diff --cached --check + git commit -m "fix(server): rebase unknown-provider PR lookup fix" + git push origin HEAD:fix/unknown-provider-pr-polling From fc5267af15f43cef1f265ec656bda0327d00dff6 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 07:46:01 +0300 Subject: [PATCH 04/15] chore: scope repair whitespace check --- .github/workflows/repair-pr-9212.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/repair-pr-9212.yml b/.github/workflows/repair-pr-9212.yml index bfde23e3e05d..cec169ed1768 100644 --- a/.github/workflows/repair-pr-9212.yml +++ b/.github/workflows/repair-pr-9212.yml @@ -193,6 +193,6 @@ jobs: rm -f .github/workflows/repair-pr-9212.yml git add -A - git diff --cached --check + git diff --cached --check -- apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts git commit -m "fix(server): rebase unknown-provider PR lookup fix" git push origin HEAD:fix/unknown-provider-pr-polling From 8a27cf660d048dde52485a0bd67e5bdabe172c0e Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 07:47:34 +0300 Subject: [PATCH 05/15] chore: format PR 9212 repair --- .github/workflows/repair-pr-9212-format.yml | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 .github/workflows/repair-pr-9212-format.yml diff --git a/.github/workflows/repair-pr-9212-format.yml b/.github/workflows/repair-pr-9212-format.yml new file mode 100644 index 000000000000..7bfc9752ce3d --- /dev/null +++ b/.github/workflows/repair-pr-9212-format.yml @@ -0,0 +1,57 @@ +name: Format PR 9212 repair + +on: + push: + branches: + - fix/unknown-provider-pr-polling + +permissions: + contents: write + +jobs: + format: + if: github.actor != 'github-actions[bot]' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + ref: fix/unknown-provider-pr-polling + + - name: Format and validate repaired files + shell: bash + run: | + set -euo pipefail + export VP_BIN_DIR="$HOME/.local/share/vite-plus/bin" + export VP_DATA_DIR="$HOME/.local/share/vite-plus" + export VP_CACHE_DIR="$HOME/.cache/vite-plus" + installer="$(mktemp)" + curl -fsSL https://vite.plus -o "$installer" + VP_NODE_MANAGER=no bash "$installer" + rm -f "$installer" + export PATH="$VP_BIN_DIR:$PATH" + + vp fmt apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts + + python3 <<'PY' + from pathlib import Path + text = Path("apps/server/src/git/GitManager.ts").read_text() + required = [ + 'cached.value.outcome._tag === "Complete"', + 'cached.value.outcome.latest === null', + 'Effect.andThen(resolveLookupHeadContext(cwd, details))', + 'if (outcome._tag === "ProviderUnknown")', + 'const { headContext, lookup } = yield* resolveLookupHeadContext(cwd, details);', + ] + missing = [item for item in required if item not in text] + if missing: + raise SystemExit(f"missing required merged behavior: {missing}") + PY + + rm -f .github/workflows/repair-pr-9212-format.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add -A + git diff --cached --check -- apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts + git commit -m "style(server): format PR lookup conflict resolution" + git push origin HEAD:fix/unknown-provider-pr-polling From 056fb329f189a7107710ead590500692cf277d56 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 07:49:02 +0300 Subject: [PATCH 06/15] chore: use standalone formatter for PR 9212 --- .github/workflows/repair-pr-9212-format.yml | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/.github/workflows/repair-pr-9212-format.yml b/.github/workflows/repair-pr-9212-format.yml index 7bfc9752ce3d..52ec8cfc7717 100644 --- a/.github/workflows/repair-pr-9212-format.yml +++ b/.github/workflows/repair-pr-9212-format.yml @@ -22,16 +22,7 @@ jobs: shell: bash run: | set -euo pipefail - export VP_BIN_DIR="$HOME/.local/share/vite-plus/bin" - export VP_DATA_DIR="$HOME/.local/share/vite-plus" - export VP_CACHE_DIR="$HOME/.cache/vite-plus" - installer="$(mktemp)" - curl -fsSL https://vite.plus -o "$installer" - VP_NODE_MANAGER=no bash "$installer" - rm -f "$installer" - export PATH="$VP_BIN_DIR:$PATH" - - vp fmt apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts + npx --yes oxfmt@latest apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts python3 <<'PY' from pathlib import Path From 9983f8935b2b5b9e0fb6f60afcf783447c08d7d7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 04:49:24 +0000 Subject: [PATCH 07/15] style(server): format PR lookup conflict resolution --- .github/workflows/repair-pr-9212-format.yml | 48 ----- apps/server/src/git/GitManager.ts | 226 ++++++++++---------- 2 files changed, 113 insertions(+), 161 deletions(-) delete mode 100644 .github/workflows/repair-pr-9212-format.yml diff --git a/.github/workflows/repair-pr-9212-format.yml b/.github/workflows/repair-pr-9212-format.yml deleted file mode 100644 index 52ec8cfc7717..000000000000 --- a/.github/workflows/repair-pr-9212-format.yml +++ /dev/null @@ -1,48 +0,0 @@ -name: Format PR 9212 repair - -on: - push: - branches: - - fix/unknown-provider-pr-polling - -permissions: - contents: write - -jobs: - format: - if: github.actor != 'github-actions[bot]' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - ref: fix/unknown-provider-pr-polling - - - name: Format and validate repaired files - shell: bash - run: | - set -euo pipefail - npx --yes oxfmt@latest apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts - - python3 <<'PY' - from pathlib import Path - text = Path("apps/server/src/git/GitManager.ts").read_text() - required = [ - 'cached.value.outcome._tag === "Complete"', - 'cached.value.outcome.latest === null', - 'Effect.andThen(resolveLookupHeadContext(cwd, details))', - 'if (outcome._tag === "ProviderUnknown")', - 'const { headContext, lookup } = yield* resolveLookupHeadContext(cwd, details);', - ] - missing = [item for item in required if item not in text] - if missing: - raise SystemExit(f"missing required merged behavior: {missing}") - PY - - rm -f .github/workflows/repair-pr-9212-format.yml - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add -A - git diff --cached --check -- apps/server/src/git/GitManager.ts apps/server/src/git/GitManager.test.ts - git commit -m "style(server): format PR lookup conflict resolution" - git push origin HEAD:fix/unknown-provider-pr-polling diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 1ce42e79498e..e0478e5bb867 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -1050,13 +1050,13 @@ export const make = Effect.gen(function* () { ...(remoteName.length > 0 ? { remoteName } : {}), }; return Effect.gen(function* () { - const { headContext, lookup } = yield* resolveLookupHeadContext(cwd, details); - if (!lookup) { - return { - outcome: { _tag: "Complete", latest: null } satisfies PrLookupOutcome, - headContext, - }; - } + const { headContext, lookup } = yield* resolveLookupHeadContext(cwd, details); + if (!lookup) { + return { + outcome: { _tag: "Complete", latest: null } satisfies PrLookupOutcome, + headContext, + }; + } // Only skip when the branch is untracked as well: anything carrying an // upstream keeps the old behaviour. if ( @@ -1146,119 +1146,119 @@ export const make = Effect.gen(function* () { return lastKnown.pr; }; const lookupStatusPr = Effect.fn("lookupStatusPr")(function* ( - cwd: string, - details: { - branch: string; - upstreamRef: string | null; - defaultBranch: string | null; - isDefaultBranch: boolean; - }, - refreshMissingPullRequest = false, -) { - // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first - // `push -u`) must not orphan the fallback value for the same branch. - const branchKey = `${cwd}\u0000${details.branch}`; - const cacheKey = prLookupCacheKey(cwd, details); - if (refreshMissingPullRequest) { - const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe( - Effect.orElseSucceed(() => Option.none()), - ); - if ( - Option.isSome(cached) && - cached.value.outcome._tag === "Complete" && - cached.value.outcome.latest === null - ) { - yield* Cache.invalidate(prLookupCache, cacheKey); - } - } - return yield* Cache.get(prLookupCache, cacheKey).pipe( - Effect.flatMap(({ outcome, headContext }) => { - const lastKnownContext = { - upstreamRef: details.upstreamRef, - headBranch: headContext.headBranch, - remoteName: headContext.remoteName, - headRemoteUrlKey: headContext.headRemoteUrlKey, - }; - if (outcome._tag === "ProviderUnknown") { - return Effect.succeed(resolveLastKnownPr(branchKey, lastKnownContext)); + cwd: string, + details: { + branch: string; + upstreamRef: string | null; + defaultBranch: string | null; + isDefaultBranch: boolean; + }, + refreshMissingPullRequest = false, + ) { + // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first + // `push -u`) must not orphan the fallback value for the same branch. + const branchKey = `${cwd}\u0000${details.branch}`; + const cacheKey = prLookupCacheKey(cwd, details); + if (refreshMissingPullRequest) { + const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe( + Effect.orElseSucceed(() => Option.none()), + ); + if ( + Option.isSome(cached) && + cached.value.outcome._tag === "Complete" && + cached.value.outcome.latest === null + ) { + yield* Cache.invalidate(prLookupCache, cacheKey); } + } + return yield* Cache.get(prLookupCache, cacheKey).pipe( + Effect.flatMap(({ outcome, headContext }) => { + const lastKnownContext = { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }; + if (outcome._tag === "ProviderUnknown") { + return Effect.succeed(resolveLastKnownPr(branchKey, lastKnownContext)); + } - const pr = - outcome.latest === null || - // On the default branch, only surface open PRs. Merged/closed - // matches are usually reverse-merge history, not the thread's PR. - (details.isDefaultBranch && outcome.latest.state !== "open") - ? null - : toStatusPr(outcome.latest); - return Effect.sync(() => { - rememberLastKnownPr(branchKey, { pr, ...lastKnownContext }); - return pr; - }); - }), - Effect.catch((error) => - Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe( - Effect.annotateLogs({ - operation: "lookupStatusPr", - branch: details.branch, - errorTag: - typeof error === "object" && error !== null && "_tag" in error - ? String(error._tag) - : typeof error, - ...(isSourceControlProviderError(error) - ? { - provider: error.provider, - providerOperation: error.operation, - providerCommand: error.command ?? "unknown", - errorDetail: error.detail, - } - : {}), - }), - Effect.andThen(resolveLookupHeadContext(cwd, details)), - Effect.map(({ headContext }) => - resolveLastKnownPr(branchKey, { - upstreamRef: details.upstreamRef, - headBranch: headContext.headBranch, - remoteName: headContext.remoteName, - headRemoteUrlKey: headContext.headRemoteUrlKey, + const pr = + outcome.latest === null || + // On the default branch, only surface open PRs. Merged/closed + // matches are usually reverse-merge history, not the thread's PR. + (details.isDefaultBranch && outcome.latest.state !== "open") + ? null + : toStatusPr(outcome.latest); + return Effect.sync(() => { + rememberLastKnownPr(branchKey, { pr, ...lastKnownContext }); + return pr; + }); + }), + Effect.catch((error) => + Effect.logWarning("PR lookup failed; keeping last known PR state.").pipe( + Effect.annotateLogs({ + operation: "lookupStatusPr", + branch: details.branch, + errorTag: + typeof error === "object" && error !== null && "_tag" in error + ? String(error._tag) + : typeof error, + ...(isSourceControlProviderError(error) + ? { + provider: error.provider, + providerOperation: error.operation, + providerCommand: error.command ?? "unknown", + errorDetail: error.detail, + } + : {}), }), + Effect.andThen(resolveLookupHeadContext(cwd, details)), + Effect.map(({ headContext }) => + resolveLastKnownPr(branchKey, { + upstreamRef: details.upstreamRef, + headBranch: headContext.headBranch, + remoteName: headContext.remoteName, + headRemoteUrlKey: headContext.headRemoteUrlKey, + }), + ), ), ), - ), - ); -}); -const readRemoteStatus = Effect.fn("readRemoteStatus")(function* ( - cwd: string, - options?: GitRemoteStatusOptions, -) { - const details = yield* gitCore - .statusDetailsRemote(cwd, options) - .pipe(Effect.catchIf(isNotGitRepositoryError, () => Effect.succeed(null))); - if (details === null || !details.isRepo) { - return null; - } + ); + }); + const readRemoteStatus = Effect.fn("readRemoteStatus")(function* ( + cwd: string, + options?: GitRemoteStatusOptions, + ) { + const details = yield* gitCore + .statusDetailsRemote(cwd, options) + .pipe(Effect.catchIf(isNotGitRepositoryError, () => Effect.succeed(null))); + if (details === null || !details.isRepo) { + return null; + } - const pr = - details.branch !== null - ? yield* lookupStatusPr( - cwd, - { - branch: details.branch, - upstreamRef: details.upstreamRef, - defaultBranch: details.defaultBranch, - isDefaultBranch: details.isDefaultBranch, - }, - options?.refreshMissingPullRequest, - ) - : null; + const pr = + details.branch !== null + ? yield* lookupStatusPr( + cwd, + { + branch: details.branch, + upstreamRef: details.upstreamRef, + defaultBranch: details.defaultBranch, + isDefaultBranch: details.isDefaultBranch, + }, + options?.refreshMissingPullRequest, + ) + : null; - return { - hasUpstream: details.hasUpstream, - aheadCount: details.aheadCount, - behindCount: details.behindCount, - aheadOfDefaultCount: details.aheadOfDefaultCount, - pr, - } satisfies VcsStatusRemoteResult; -}); + return { + hasUpstream: details.hasUpstream, + aheadCount: details.aheadCount, + behindCount: details.behindCount, + aheadOfDefaultCount: details.aheadOfDefaultCount, + pr, + } satisfies VcsStatusRemoteResult; + }); const remoteStatusResultCache = yield* Cache.makeWith((cwd: string) => readRemoteStatus(cwd), { capacity: STATUS_RESULT_CACHE_CAPACITY, timeToLive: (exit) => (Exit.isSuccess(exit) ? STATUS_RESULT_CACHE_TTL : Duration.zero), From 1f88238e04cce0eb3b4f6c07d49938b700af8f46 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:43:23 +0300 Subject: [PATCH 08/15] fix(server): short-circuit unknown PR providers before identity checks --- apps/server/src/git/GitManager.ts | 115 +----------------------------- 1 file changed, 2 insertions(+), 113 deletions(-) diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index e0478e5bb867..048594a0a9f5 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -129,28 +129,12 @@ const SHORT_SHA_LENGTH = 7; const TOAST_DESCRIPTION_MAX = 72; const STATUS_RESULT_CACHE_TTL = Duration.seconds(1); const STATUS_RESULT_CACHE_CAPACITY = 2_048; -// Matches the automatic settlement sweep cadence so every background sweep -// reads fresh branch state: an external merge settles within about a minute -// instead of waiting out a longer cache. Unpublished branches never reach the -// host (a local probe answers first), and failed lookups still back off -// exponentially via prLookupFailureTtl, so throttling pressure still drops -// under 429s instead of amplifying it. const PR_LOOKUP_CACHE_TTL = Duration.seconds(60); const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20); const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15); const PR_LOOKUP_CACHE_CAPACITY = 2_048; const isSourceControlProviderError = Schema.is(SourceControlProviderError); -/** - * How long a failed PR lookup is cached, given the number of consecutive - * failures for that branch. - * - * A hosting provider rejects a throttled request immediately, so caching every - * failure for a flat 20s made a rate-limited poller re-ask *faster* than a - * healthy one does (which waits PR_LOOKUP_CACHE_TTL), turning a transient 429 - * into sustained pressure. Backing off per branch keeps the retry rate below - * the healthy rate once a branch has failed more than a couple of times. - */ export function prLookupFailureTtl(consecutiveFailures: number): Duration.Duration { const exponent = Math.max(0, consecutiveFailures - 1); const backoffMs = Duration.toMillis(PR_LOOKUP_FAILURE_BASE_TTL) * Math.pow(2, exponent); @@ -293,7 +277,6 @@ function parseRepositoryOwnerLogin(nameWithOwner: string | null): string | null if (trimmed.length === 0) { return null; } - // GitLab reports the top-level group as owner. The full path distinguishes subgroups. const [ownerLogin] = trimmed.split("/"); const normalizedOwnerLogin = ownerLogin?.trim() ?? ""; return normalizedOwnerLogin.length > 0 ? normalizedOwnerLogin : null; @@ -979,12 +962,6 @@ export const make = Effect.gen(function* () { normalizeStatusCacheKey(cwd).pipe( Effect.flatMap((cacheKey) => Cache.invalidate(localStatusResultCache, cacheKey)), ); - // PR lookups hit the hosting provider's API (gh/glab/...), so definitive - // results refresh on the slower PR_LOOKUP_CACHE_TTL cadence. An unresolved - // provider starts at the shorter failure cadence and backs off while it stays - // unresolved, capped at the healthy lookup cadence. Git actions and - // user-driven refreshes bump the epoch (invalidateStatus) to bypass the cache - // immediately. const prLookupEpochByCwd = new Map(); const prLookupEpoch = (cwd: string) => prLookupEpochByCwd.get(cwd) ?? 0; const bumpPrLookupEpoch = (cwd: string) => @@ -993,8 +970,6 @@ export const make = Effect.gen(function* () { prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1); }), ); - // Cache keys are NUL-joined. Automatic settlement validates repository URLs - // against the cached value before it uses a pull request decision. const prLookupCacheKey = ( cwd: string, details: { @@ -1014,9 +989,6 @@ export const make = Effect.gen(function* () { details.remoteName ?? "", String(prLookupEpoch(cwd)), ].join("\u0000"); - // Consecutive failed or non-definitive attempts per cache key, so a branch - // that keeps failing waits longer before the next attempt. Cleared as soon - // as the lookup produces a definitive result. const prLookupFailureStreakByKey = new Map(); const nextPrLookupFailureTtl = (key: string) => { if ( @@ -1057,8 +1029,6 @@ export const make = Effect.gen(function* () { headContext, }; } - // Only skip when the branch is untracked as well: anything carrying an - // upstream keeps the old behaviour. if ( details.localBranchExists && details.upstreamRef === null && @@ -1087,10 +1057,6 @@ export const make = Effect.gen(function* () { }, }, ); - // A transient lookup failure (rate limit, network blip) must not clear an - // already-known PR badge, so the last successful answer per branch sticks - // around as the fallback. Keep the resolved head context with it so a - // branch retargeted to another remote/fork cannot inherit the old badge. interface LastKnownPr { readonly pr: ReturnType | null; readonly upstreamRef: string | null; @@ -1121,20 +1087,10 @@ export const make = Effect.gen(function* () { return null; } - // The normalized URL catches both remote-alias changes and an existing - // alias being repointed. Both sides must be resolved before treating a - // mismatch as real: `readConfigValueNullable` swallows any git-config - // read failure into `null`, so a transient failure to resolve the - // *current* remote URL must read as "unknown", not as "no remote" — the - // latter would otherwise drop an already-known PR badge on every hiccup. if (lastKnown.headRemoteUrlKey !== null && current.headRemoteUrlKey !== null) { return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; } - // If the remote URL can't be compared, fall back to the remote identity - // encoded by tracked branches — same "both sides known" requirement, for - // the same reason. A null-to-non-null transition (upstream/remoteName) - // is allowed because that is the expected first-push case. if ( lastKnown.upstreamRef !== null && current.upstreamRef !== null && @@ -1155,8 +1111,6 @@ export const make = Effect.gen(function* () { }, refreshMissingPullRequest = false, ) { - // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first - // `push -u`) must not orphan the fallback value for the same branch. const branchKey = `${cwd}\u0000${details.branch}`; const cacheKey = prLookupCacheKey(cwd, details); if (refreshMissingPullRequest) { @@ -1185,8 +1139,6 @@ export const make = Effect.gen(function* () { const pr = outcome.latest === null || - // On the default branch, only surface open PRs. Merged/closed - // matches are usually reverse-merge history, not the thread's PR. (details.isDefaultBranch && outcome.latest.state !== "open") ? null : toStatusPr(outcome.latest); @@ -1404,10 +1356,6 @@ export const make = Effect.gen(function* () { } satisfies BranchHeadContext; }); - // The remote that holds a ref named after the local branch, or null when - // none does. Remote names may contain slashes, so refs are matched literally - // per remote instead of with a glob. When several remotes hold the name, the - // preferred remote wins, then origin, then the first configured remote. const findRemoteTrackingRemote = Effect.fn("findRemoteTrackingRemote")(function* ( cwd: string, branch: string, @@ -1449,18 +1397,6 @@ export const make = Effect.gen(function* () { }).pipe(Effect.orElseSucceed(() => null)); }); - // `git worktree add -b feature origin/main` makes the new local branch track - // origin/main. That upstream is the branch's base, not its published PR - // head. Looking up PRs for it can attach an old reverse merge from main and - // auto-settle an unrelated feature thread. - // - // The branch may still have been pushed under its own name by a plain - // `git push feature` that never moved the upstream. When a remote - // holds a ref for the local name, look the PR up by that name on that - // remote. Without such a ref there is nothing to ask the host about, so - // `lookup` is false and no API call is spent. Both the cached lookup and the - // failure fallback resolve through here so the last-known PR compares - // against the same head branch. const resolveLookupHeadContext = Effect.fn("resolveLookupHeadContext")(function* ( cwd: string, details: { @@ -1494,19 +1430,6 @@ export const make = Effect.gen(function* () { return { headContext: ownNameContext, lookup: true }; }); - /** - * Whether git has no record of this branch on any remote, so a change request - * cannot exist for it and asking the provider is a guaranteed-empty API call. - * - * `git push` writes the remote-tracking ref even without `-u` (how most - * terminal and agent pushes land), and configured upstream metadata survives - * when a merged change request's remote branch is deleted. Together they - * distinguish branches known to have reached a host from genuinely local - * branches. The ref glob spans every remote so a fork branch still counts. A - * repository that tracks no remotes at all cannot answer the question, - * because then every branch looks unpublished; it, and any failed probe, - * keeps the lookup. - */ const isUnpublishedBranch = Effect.fn("isUnpublishedBranch")(function* ( cwd: string, headContext: Pick, @@ -1733,10 +1656,6 @@ export const make = Effect.gen(function* () { return defaultFromProvider; } - // The provider lookup can fail for reasons unrelated to the branch, so fall - // back to what the remote itself records before assuming a name. A repository - // whose default branch is master would otherwise get a base branch that does - // not exist. const defaultFromRemote = yield* gitCore.resolvePrimaryRemoteName(cwd).pipe( Effect.flatMap((remoteName) => gitCore.resolveDefaultBranchName(cwd, remoteName)), Effect.orElseSucceed(() => null), @@ -1774,7 +1693,6 @@ export const make = Effect.gen(function* () { cwd: string; branch: string | null; commitMessage?: string; - /** When true, also produce a semantic feature branch name. */ includeBranch?: boolean; filePaths?: readonly string[]; settings: SourceControlTextGenerationSettings; @@ -2154,17 +2072,13 @@ export const make = Effect.gen(function* () { ...(localBranchExists ? {} : { remoteName }), }); if (options?.refresh) { - // A completed turn can create a PR or reuse a merged PR's branch. - // Refresh successful answers, but keep failed lookups' retry backoff. const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe( Effect.orElseSucceed(() => Option.none()), ); if (Option.isSome(cached)) yield* Cache.invalidate(prLookupCache, cacheKey); } let cached = yield* Cache.get(prLookupCache, cacheKey); - // The cached head context may have resolved on a different remote than - // the saved upstream: a branch tracking origin/main but pushed to a fork - // is looked up on the fork. Verify against the remote the lookup used. + if (cached.outcome._tag === "ProviderUnknown") return null; const identityRemoteName = (headContext: BranchHeadContext) => headContext.remoteName ?? remoteName ?? undefined; const currentIdentity = yield* resolvePrLookupRepositoryIdentity( @@ -2190,6 +2104,7 @@ export const make = Effect.gen(function* () { if (!hasSameIdentity(cached.headContext, currentIdentity)) { yield* Cache.invalidate(prLookupCache, cacheKey); cached = yield* Cache.get(prLookupCache, cacheKey); + if (cached.outcome._tag === "ProviderUnknown") return null; const refreshedIdentity = yield* resolvePrLookupRepositoryIdentity( cacheCwd, branch, @@ -2206,7 +2121,6 @@ export const make = Effect.gen(function* () { }); } } - if (cached.outcome._tag === "ProviderUnknown") return null; const { latest } = cached.outcome; if (latest === null) return null; if ( @@ -2220,8 +2134,6 @@ export const make = Effect.gen(function* () { ...toStatusPr(latest), closedAt: latest.closedAt ?? null, mergedAt: latest.mergedAt ?? null, - // Hosting CLIs can select an upstream repository instead of origin. - // The returned PR URL names the repository that actually owns it. repositoryKey: pullRequestRepositoryKey(latest.url), }; }); @@ -2239,9 +2151,6 @@ export const make = Effect.gen(function* () { function* (cwd) { yield* invalidateLocalStatusResultCache(cwd); yield* invalidateRemoteStatusResultCache(cwd); - // Full invalidation is the explicit-freshness path (git actions, user - // refresh); it also bypasses the slow PR-lookup cache. The periodic - // status poll only invalidates local/remote and keeps the PR cache warm. yield* bumpPrLookupEpoch(cwd); }, ); @@ -2335,19 +2244,11 @@ export const make = Effect.gen(function* () { const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo); - // Git refuses to move a branch that is checked out in a worktree, so the - // reuse paths cannot go through materializePullRequestHeadBranch and instead - // advance the checkout from inside the worktree. A worktree that cannot be - // moved (no reachable head, local commits, dirty tree) is still handed - // back, because stranding the thread is worse than reporting the staleness. const reuseExistingWorktree = Effect.fn("reuseExistingWorktree")(function* ( worktreePath: string, checkedOutBranch: string, ) { if (checkedOutBranch !== localPullRequestBranch) { - // findLocalHeadBranch also accepts a branch that merely shares the head's bare name — - // a fork PR opened from "main" matches the user's own local main. That checkout is - // somebody else's work, so it keeps its tracking config and nothing else. yield* ensureExistingWorktreeUpstream(worktreePath); return { pullRequest, @@ -2357,9 +2258,6 @@ export const make = Effect.gen(function* () { }; } - // Read before ensureExistingWorktreeUpstream: it force-updates the remote-tracking ref, - // and once that has jumped to a rewritten head there is no way left to tell a checkout - // that holds nothing of its own from one carrying local commits. const upstreamCommitBeforeFetch = yield* gitCore .resolveCommit({ cwd: worktreePath, revision: "@{upstream}" }) .pipe( @@ -2370,15 +2268,8 @@ export const make = Effect.gen(function* () { yield* ensureExistingWorktreeUpstream(worktreePath); const refreshed = yield* gitCore - // The pull request's own ref, because it is the only thing that certainly names its - // head. The branch's upstream does not: configuring it is best-effort, so a branch cut - // from `origin/main` whose head branch has since been deleted still resolves — and - // following it would move the checkout onto main and call that the pull request. .fetchPullRequestHeadCommit({ cwd: worktreePath, prNumber: pullRequest.number }) .pipe( - // A host that publishes no `refs/pull//head` leaves the remote-tracking branch, - // taken only where it is the head branch's own rather than whatever the checkout - // happened to be cut from. Effect.catch(() => Effect.gen(function* () { const details = yield* gitCore.statusDetails(worktreePath); @@ -2417,8 +2308,6 @@ export const make = Effect.gen(function* () { ), ); - // Only when the checkout actually moved: another thread may be running in this worktree, - // and re-running the setup script under it buys nothing when the code did not change. if (refreshed.moved) { yield* maybeRunSetupScript(worktreePath); } From 6abb2903bd66df5a41e7b111a2311798c087eead Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:47:22 +0300 Subject: [PATCH 09/15] chore(server): preserve PR lookup documentation --- apps/server/src/git/GitManager.ts | 112 ++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 048594a0a9f5..da2133ec2546 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -129,12 +129,28 @@ const SHORT_SHA_LENGTH = 7; const TOAST_DESCRIPTION_MAX = 72; const STATUS_RESULT_CACHE_TTL = Duration.seconds(1); const STATUS_RESULT_CACHE_CAPACITY = 2_048; +// Matches the automatic settlement sweep cadence so every background sweep +// reads fresh branch state: an external merge settles within about a minute +// instead of waiting out a longer cache. Unpublished branches never reach the +// host (a local probe answers first), and failed lookups still back off +// exponentially via prLookupFailureTtl, so throttling pressure still drops +// under 429s instead of amplifying it. const PR_LOOKUP_CACHE_TTL = Duration.seconds(60); const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20); const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15); const PR_LOOKUP_CACHE_CAPACITY = 2_048; const isSourceControlProviderError = Schema.is(SourceControlProviderError); +/** + * How long a failed PR lookup is cached, given the number of consecutive + * failures for that branch. + * + * A hosting provider rejects a throttled request immediately, so caching every + * failure for a flat 20s made a rate-limited poller re-ask *faster* than a + * healthy one does (which waits PR_LOOKUP_CACHE_TTL), turning a transient 429 + * into sustained pressure. Backing off per branch keeps the retry rate below + * the healthy rate once a branch has failed more than a couple of times. + */ export function prLookupFailureTtl(consecutiveFailures: number): Duration.Duration { const exponent = Math.max(0, consecutiveFailures - 1); const backoffMs = Duration.toMillis(PR_LOOKUP_FAILURE_BASE_TTL) * Math.pow(2, exponent); @@ -277,6 +293,7 @@ function parseRepositoryOwnerLogin(nameWithOwner: string | null): string | null if (trimmed.length === 0) { return null; } + // GitLab reports the top-level group as owner. The full path distinguishes subgroups. const [ownerLogin] = trimmed.split("/"); const normalizedOwnerLogin = ownerLogin?.trim() ?? ""; return normalizedOwnerLogin.length > 0 ? normalizedOwnerLogin : null; @@ -962,6 +979,12 @@ export const make = Effect.gen(function* () { normalizeStatusCacheKey(cwd).pipe( Effect.flatMap((cacheKey) => Cache.invalidate(localStatusResultCache, cacheKey)), ); + // PR lookups hit the hosting provider's API (gh/glab/...), so definitive + // results refresh on the slower PR_LOOKUP_CACHE_TTL cadence. An unresolved + // provider starts at the shorter failure cadence and backs off while it stays + // unresolved, capped at the healthy lookup cadence. Git actions and + // user-driven refreshes bump the epoch (invalidateStatus) to bypass the cache + // immediately. const prLookupEpochByCwd = new Map(); const prLookupEpoch = (cwd: string) => prLookupEpochByCwd.get(cwd) ?? 0; const bumpPrLookupEpoch = (cwd: string) => @@ -970,6 +993,8 @@ export const make = Effect.gen(function* () { prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1); }), ); + // Cache keys are NUL-joined. Automatic settlement validates repository URLs + // against the cached value before it uses a pull request decision. const prLookupCacheKey = ( cwd: string, details: { @@ -989,6 +1014,9 @@ export const make = Effect.gen(function* () { details.remoteName ?? "", String(prLookupEpoch(cwd)), ].join("\u0000"); + // Consecutive failed or non-definitive attempts per cache key, so a branch + // that keeps failing waits longer before the next attempt. Cleared as soon + // as the lookup produces a definitive result. const prLookupFailureStreakByKey = new Map(); const nextPrLookupFailureTtl = (key: string) => { if ( @@ -1029,6 +1057,8 @@ export const make = Effect.gen(function* () { headContext, }; } + // Only skip when the branch is untracked as well: anything carrying an + // upstream keeps the old behaviour. if ( details.localBranchExists && details.upstreamRef === null && @@ -1057,6 +1087,10 @@ export const make = Effect.gen(function* () { }, }, ); + // A transient lookup failure (rate limit, network blip) must not clear an + // already-known PR badge, so the last successful answer per branch sticks + // around as the fallback. Keep the resolved head context with it so a + // branch retargeted to another remote/fork cannot inherit the old badge. interface LastKnownPr { readonly pr: ReturnType | null; readonly upstreamRef: string | null; @@ -1087,10 +1121,20 @@ export const make = Effect.gen(function* () { return null; } + // The normalized URL catches both remote-alias changes and an existing + // alias being repointed. Both sides must be resolved before treating a + // mismatch as real: `readConfigValueNullable` swallows any git-config + // read failure into `null`, so a transient failure to resolve the + // *current* remote URL must read as "unknown", not as "no remote" — the + // latter would otherwise drop an already-known PR badge on every hiccup. if (lastKnown.headRemoteUrlKey !== null && current.headRemoteUrlKey !== null) { return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; } + // If the remote URL can't be compared, fall back to the remote identity + // encoded by tracked branches — same "both sides known" requirement, for + // the same reason. A null-to-non-null transition (upstream/remoteName) + // is allowed because that is the expected first-push case. if ( lastKnown.upstreamRef !== null && current.upstreamRef !== null && @@ -1111,6 +1155,8 @@ export const make = Effect.gen(function* () { }, refreshMissingPullRequest = false, ) { + // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first + // `push -u`) must not orphan the fallback value for the same branch. const branchKey = `${cwd}\u0000${details.branch}`; const cacheKey = prLookupCacheKey(cwd, details); if (refreshMissingPullRequest) { @@ -1139,6 +1185,8 @@ export const make = Effect.gen(function* () { const pr = outcome.latest === null || + // On the default branch, only surface open PRs. Merged/closed + // matches are usually reverse-merge history, not the thread's PR. (details.isDefaultBranch && outcome.latest.state !== "open") ? null : toStatusPr(outcome.latest); @@ -1356,6 +1404,10 @@ export const make = Effect.gen(function* () { } satisfies BranchHeadContext; }); + // The remote that holds a ref named after the local branch, or null when + // none does. Remote names may contain slashes, so refs are matched literally + // per remote instead of with a glob. When several remotes hold the name, the + // preferred remote wins, then origin, then the first configured remote. const findRemoteTrackingRemote = Effect.fn("findRemoteTrackingRemote")(function* ( cwd: string, branch: string, @@ -1397,6 +1449,18 @@ export const make = Effect.gen(function* () { }).pipe(Effect.orElseSucceed(() => null)); }); + // `git worktree add -b feature origin/main` makes the new local branch track + // origin/main. That upstream is the branch's base, not its published PR + // head. Looking up PRs for it can attach an old reverse merge from main and + // auto-settle an unrelated feature thread. + // + // The branch may still have been pushed under its own name by a plain + // `git push feature` that never moved the upstream. When a remote + // holds a ref for the local name, look the PR up by that name on that + // remote. Without such a ref there is nothing to ask the host about, so + // `lookup` is false and no API call is spent. Both the cached lookup and the + // failure fallback resolve through here so the last-known PR compares + // against the same head branch. const resolveLookupHeadContext = Effect.fn("resolveLookupHeadContext")(function* ( cwd: string, details: { @@ -1430,6 +1494,19 @@ export const make = Effect.gen(function* () { return { headContext: ownNameContext, lookup: true }; }); + /** + * Whether git has no record of this branch on any remote, so a change request + * cannot exist for it and asking the provider is a guaranteed-empty API call. + * + * `git push` writes the remote-tracking ref even without `-u` (how most + * terminal and agent pushes land), and configured upstream metadata survives + * when a merged change request's remote branch is deleted. Together they + * distinguish branches known to have reached a host from genuinely local + * branches. The ref glob spans every remote so a fork branch still counts. A + * repository that tracks no remotes at all cannot answer the question, + * because then every branch looks unpublished; it, and any failed probe, + * keeps the lookup. + */ const isUnpublishedBranch = Effect.fn("isUnpublishedBranch")(function* ( cwd: string, headContext: Pick, @@ -1656,6 +1733,10 @@ export const make = Effect.gen(function* () { return defaultFromProvider; } + // The provider lookup can fail for reasons unrelated to the branch, so fall + // back to what the remote itself records before assuming a name. A repository + // whose default branch is master would otherwise get a base branch that does + // not exist. const defaultFromRemote = yield* gitCore.resolvePrimaryRemoteName(cwd).pipe( Effect.flatMap((remoteName) => gitCore.resolveDefaultBranchName(cwd, remoteName)), Effect.orElseSucceed(() => null), @@ -1693,6 +1774,7 @@ export const make = Effect.gen(function* () { cwd: string; branch: string | null; commitMessage?: string; + /** When true, also produce a semantic feature branch name. */ includeBranch?: boolean; filePaths?: readonly string[]; settings: SourceControlTextGenerationSettings; @@ -2072,6 +2154,8 @@ export const make = Effect.gen(function* () { ...(localBranchExists ? {} : { remoteName }), }); if (options?.refresh) { + // A completed turn can create a PR or reuse a merged PR's branch. + // Refresh successful answers, but keep failed lookups' retry backoff. const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe( Effect.orElseSucceed(() => Option.none()), ); @@ -2079,6 +2163,9 @@ export const make = Effect.gen(function* () { } let cached = yield* Cache.get(prLookupCache, cacheKey); if (cached.outcome._tag === "ProviderUnknown") return null; + // The cached head context may have resolved on a different remote than + // the saved upstream: a branch tracking origin/main but pushed to a fork + // is looked up on the fork. Verify against the remote the lookup used. const identityRemoteName = (headContext: BranchHeadContext) => headContext.remoteName ?? remoteName ?? undefined; const currentIdentity = yield* resolvePrLookupRepositoryIdentity( @@ -2134,6 +2221,8 @@ export const make = Effect.gen(function* () { ...toStatusPr(latest), closedAt: latest.closedAt ?? null, mergedAt: latest.mergedAt ?? null, + // Hosting CLIs can select an upstream repository instead of origin. + // The returned PR URL names the repository that actually owns it. repositoryKey: pullRequestRepositoryKey(latest.url), }; }); @@ -2151,6 +2240,9 @@ export const make = Effect.gen(function* () { function* (cwd) { yield* invalidateLocalStatusResultCache(cwd); yield* invalidateRemoteStatusResultCache(cwd); + // Full invalidation is the explicit-freshness path (git actions, user + // refresh); it also bypasses the slow PR-lookup cache. The periodic + // status poll only invalidates local/remote and keeps the PR cache warm. yield* bumpPrLookupEpoch(cwd); }, ); @@ -2244,11 +2336,19 @@ export const make = Effect.gen(function* () { const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo); + // Git refuses to move a branch that is checked out in a worktree, so the + // reuse paths cannot go through materializePullRequestHeadBranch and instead + // advance the checkout from inside the worktree. A worktree that cannot be + // moved (no reachable head, local commits, dirty tree) is still handed + // back, because stranding the thread is worse than reporting the staleness. const reuseExistingWorktree = Effect.fn("reuseExistingWorktree")(function* ( worktreePath: string, checkedOutBranch: string, ) { if (checkedOutBranch !== localPullRequestBranch) { + // findLocalHeadBranch also accepts a branch that merely shares the head's bare name — + // a fork PR opened from "main" matches the user's own local main. That checkout is + // somebody else's work, so it keeps its tracking config and nothing else. yield* ensureExistingWorktreeUpstream(worktreePath); return { pullRequest, @@ -2258,6 +2358,9 @@ export const make = Effect.gen(function* () { }; } + // Read before ensureExistingWorktreeUpstream: it force-updates the remote-tracking ref, + // and once that has jumped to a rewritten head there is no way left to tell a checkout + // that holds nothing of its own from one carrying local commits. const upstreamCommitBeforeFetch = yield* gitCore .resolveCommit({ cwd: worktreePath, revision: "@{upstream}" }) .pipe( @@ -2268,8 +2371,15 @@ export const make = Effect.gen(function* () { yield* ensureExistingWorktreeUpstream(worktreePath); const refreshed = yield* gitCore + // The pull request's own ref, because it is the only thing that certainly names its + // head. The branch's upstream does not: configuring it is best-effort, so a branch cut + // from `origin/main` whose head branch has since been deleted still resolves — and + // following it would move the checkout onto main and call that the pull request. .fetchPullRequestHeadCommit({ cwd: worktreePath, prNumber: pullRequest.number }) .pipe( + // A host that publishes no `refs/pull//head` leaves the remote-tracking branch, + // taken only where it is the head branch's own rather than whatever the checkout + // happened to be cut from. Effect.catch(() => Effect.gen(function* () { const details = yield* gitCore.statusDetails(worktreePath); @@ -2308,6 +2418,8 @@ export const make = Effect.gen(function* () { ), ); + // Only when the checkout actually moved: another thread may be running in this worktree, + // and re-running the setup script under it buys nothing when the code did not change. if (refreshed.moved) { yield* maybeRunSetupScript(worktreePath); } From 575f3ae7f8099e71c1728912fe8e4dcf67b36a08 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:52:30 +0300 Subject: [PATCH 10/15] chore: apply targeted CodeRabbit fixture fix --- .github/workflows/tmp-coderabbit-9212-fix.yml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .github/workflows/tmp-coderabbit-9212-fix.yml diff --git a/.github/workflows/tmp-coderabbit-9212-fix.yml b/.github/workflows/tmp-coderabbit-9212-fix.yml new file mode 100644 index 000000000000..de5d6af90c22 --- /dev/null +++ b/.github/workflows/tmp-coderabbit-9212-fix.yml @@ -0,0 +1,60 @@ +name: Temporary CodeRabbit 9212 fix + +on: + push: + branches: + - fix/unknown-provider-pr-polling + +permissions: + contents: write + +jobs: + patch: + if: github.actor != 'github-actions[bot]' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: fix/unknown-provider-pr-polling + + - name: Patch GitManager test fixture + shell: bash + run: | + python - <<'PY' + from pathlib import Path + + path = Path("apps/server/src/git/GitManager.test.ts") + text = path.read_text() + old = ''' GitHubSourceControlProvider.make.pipe( + Effect.map((provider) => { + const sourceControlProvider = () => ({ + ...provider, + kind: + typeof input?.sourceControlProviderKind === "function" + ? input.sourceControlProviderKind() + : (input?.sourceControlProviderKind ?? provider.kind), + });''' + new = ''' GitHubSourceControlProvider.make.pipe( + Effect.map((provider) => { + const baseProvider = input?.sourceControlProvider ?? provider; + const sourceControlProvider = () => ({ + ...baseProvider, + kind: + typeof input?.sourceControlProviderKind === "function" + ? input.sourceControlProviderKind() + : (input?.sourceControlProviderKind ?? baseProvider.kind), + });''' + if old not in text: + raise SystemExit("target makeManager provider block not found") + path.write_text(text.replace(old, new, 1)) + PY + + - name: Commit targeted fix + shell: bash + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add apps/server/src/git/GitManager.test.ts + git rm .github/workflows/tmp-coderabbit-9212-fix.yml + git commit -m "test(server): preserve injected source-control provider" + git push origin HEAD:fix/unknown-provider-pr-polling From 040b74b226cca7e61121873a9d33fb40590533a1 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:54:22 +0300 Subject: [PATCH 11/15] chore: retry targeted CodeRabbit fixes --- .github/workflows/tmp-coderabbit-9212-fix.yml | 59 ++++++++++++------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/.github/workflows/tmp-coderabbit-9212-fix.yml b/.github/workflows/tmp-coderabbit-9212-fix.yml index de5d6af90c22..48d6058a6902 100644 --- a/.github/workflows/tmp-coderabbit-9212-fix.yml +++ b/.github/workflows/tmp-coderabbit-9212-fix.yml @@ -17,7 +17,7 @@ jobs: with: ref: fix/unknown-provider-pr-polling - - name: Patch GitManager test fixture + - name: Patch CodeRabbit findings shell: bash run: | python - <<'PY' @@ -25,36 +25,55 @@ jobs: path = Path("apps/server/src/git/GitManager.test.ts") text = path.read_text() - old = ''' GitHubSourceControlProvider.make.pipe( - Effect.map((provider) => { + + old_provider = ''' Effect.map((provider) => { const sourceControlProvider = () => ({ ...provider, - kind: - typeof input?.sourceControlProviderKind === "function" - ? input.sourceControlProviderKind() - : (input?.sourceControlProviderKind ?? provider.kind), - });''' - new = ''' GitHubSourceControlProvider.make.pipe( - Effect.map((provider) => { + ''' + new_provider = ''' Effect.map((provider) => { const baseProvider = input?.sourceControlProvider ?? provider; const sourceControlProvider = () => ({ ...baseProvider, - kind: - typeof input?.sourceControlProviderKind === "function" - ? input.sourceControlProviderKind() - : (input?.sourceControlProviderKind ?? baseProvider.kind), - });''' - if old not in text: - raise SystemExit("target makeManager provider block not found") - path.write_text(text.replace(old, new, 1)) + ''' + if text.count(old_provider) != 1: + raise SystemExit(f"expected one provider fixture block, found {text.count(old_provider)}") + text = text.replace(old_provider, new_provider, 1) + + old_kind = ' : (input?.sourceControlProviderKind ?? provider.kind),\n' + new_kind = ' : (input?.sourceControlProviderKind ?? baseProvider.kind),\n' + if text.count(old_kind) != 1: + raise SystemExit(f"expected one provider-kind fallback, found {text.count(old_kind)}") + text = text.replace(old_kind, new_kind, 1) + + old_regression = ''' expect(pullRequest).toBeNull(); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + ''' + new_regression = ''' expect(pullRequest).toBeNull(); + + yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); + + expect( + yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/unknown-branch-provider", + }), + ).toBeNull(); + + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + ''' + if text.count(old_regression) != 1: + raise SystemExit(f"expected one unknown-provider assertion block, found {text.count(old_regression)}") + text = text.replace(old_regression, new_regression, 1) + + path.write_text(text) PY - - name: Commit targeted fix + - name: Commit targeted fixes shell: bash run: | git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git add apps/server/src/git/GitManager.test.ts git rm .github/workflows/tmp-coderabbit-9212-fix.yml - git commit -m "test(server): preserve injected source-control provider" + git commit -m "test(server): cover CodeRabbit PR lookup fixes" git push origin HEAD:fix/unknown-provider-pr-polling From 0fe1fcc71a972ad223645d9977cc826913872681 Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:55:09 +0300 Subject: [PATCH 12/15] chore: make targeted CodeRabbit patch whitespace-safe --- .github/workflows/tmp-coderabbit-9212-fix.yml | 36 +++++++------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/.github/workflows/tmp-coderabbit-9212-fix.yml b/.github/workflows/tmp-coderabbit-9212-fix.yml index 48d6058a6902..563e62fa554e 100644 --- a/.github/workflows/tmp-coderabbit-9212-fix.yml +++ b/.github/workflows/tmp-coderabbit-9212-fix.yml @@ -26,41 +26,31 @@ jobs: path = Path("apps/server/src/git/GitManager.test.ts") text = path.read_text() - old_provider = ''' Effect.map((provider) => { - const sourceControlProvider = () => ({ - ...provider, - ''' - new_provider = ''' Effect.map((provider) => { - const baseProvider = input?.sourceControlProvider ?? provider; - const sourceControlProvider = () => ({ - ...baseProvider, - ''' + old_provider = 'Effect.map((provider) => {\n const sourceControlProvider = () => ({\n ...provider,' + new_provider = 'Effect.map((provider) => {\n const baseProvider = input?.sourceControlProvider ?? provider;\n const sourceControlProvider = () => ({\n ...baseProvider,' if text.count(old_provider) != 1: raise SystemExit(f"expected one provider fixture block, found {text.count(old_provider)}") text = text.replace(old_provider, new_provider, 1) - old_kind = ' : (input?.sourceControlProviderKind ?? provider.kind),\n' - new_kind = ' : (input?.sourceControlProviderKind ?? baseProvider.kind),\n' + old_kind = ': (input?.sourceControlProviderKind ?? provider.kind),' + new_kind = ': (input?.sourceControlProviderKind ?? baseProvider.kind),' if text.count(old_kind) != 1: raise SystemExit(f"expected one provider-kind fallback, found {text.count(old_kind)}") text = text.replace(old_kind, new_kind, 1) - old_regression = ''' expect(pullRequest).toBeNull(); - expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); - ''' + old_regression = ' expect(pullRequest).toBeNull();\n expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' new_regression = ''' expect(pullRequest).toBeNull(); - yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); + yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); - expect( - yield* manager.branchPullRequest({ - cwd: repoDir, - branch: "feature/unknown-branch-provider", - }), - ).toBeNull(); + expect( + yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/unknown-branch-provider", + }), + ).toBeNull(); - expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); - ''' + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);''' if text.count(old_regression) != 1: raise SystemExit(f"expected one unknown-provider assertion block, found {text.count(old_regression)}") text = text.replace(old_regression, new_regression, 1) From 2b47defe2b42f42f013f663cde94aecfbda9966d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:55:25 +0000 Subject: [PATCH 13/15] test(server): cover CodeRabbit PR lookup fixes --- .github/workflows/tmp-coderabbit-9212-fix.yml | 69 ------------------- apps/server/src/git/GitManager.test.ts | 17 ++++- 2 files changed, 14 insertions(+), 72 deletions(-) delete mode 100644 .github/workflows/tmp-coderabbit-9212-fix.yml diff --git a/.github/workflows/tmp-coderabbit-9212-fix.yml b/.github/workflows/tmp-coderabbit-9212-fix.yml deleted file mode 100644 index 563e62fa554e..000000000000 --- a/.github/workflows/tmp-coderabbit-9212-fix.yml +++ /dev/null @@ -1,69 +0,0 @@ -name: Temporary CodeRabbit 9212 fix - -on: - push: - branches: - - fix/unknown-provider-pr-polling - -permissions: - contents: write - -jobs: - patch: - if: github.actor != 'github-actions[bot]' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - ref: fix/unknown-provider-pr-polling - - - name: Patch CodeRabbit findings - shell: bash - run: | - python - <<'PY' - from pathlib import Path - - path = Path("apps/server/src/git/GitManager.test.ts") - text = path.read_text() - - old_provider = 'Effect.map((provider) => {\n const sourceControlProvider = () => ({\n ...provider,' - new_provider = 'Effect.map((provider) => {\n const baseProvider = input?.sourceControlProvider ?? provider;\n const sourceControlProvider = () => ({\n ...baseProvider,' - if text.count(old_provider) != 1: - raise SystemExit(f"expected one provider fixture block, found {text.count(old_provider)}") - text = text.replace(old_provider, new_provider, 1) - - old_kind = ': (input?.sourceControlProviderKind ?? provider.kind),' - new_kind = ': (input?.sourceControlProviderKind ?? baseProvider.kind),' - if text.count(old_kind) != 1: - raise SystemExit(f"expected one provider-kind fallback, found {text.count(old_kind)}") - text = text.replace(old_kind, new_kind, 1) - - old_regression = ' expect(pullRequest).toBeNull();\n expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' - new_regression = ''' expect(pullRequest).toBeNull(); - - yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); - - expect( - yield* manager.branchPullRequest({ - cwd: repoDir, - branch: "feature/unknown-branch-provider", - }), - ).toBeNull(); - - expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);''' - if text.count(old_regression) != 1: - raise SystemExit(f"expected one unknown-provider assertion block, found {text.count(old_regression)}") - text = text.replace(old_regression, new_regression, 1) - - path.write_text(text) - PY - - - name: Commit targeted fixes - shell: bash - run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add apps/server/src/git/GitManager.test.ts - git rm .github/workflows/tmp-coderabbit-9212-fix.yml - git commit -m "test(server): cover CodeRabbit PR lookup fixes" - git push origin HEAD:fix/unknown-provider-pr-polling diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 9c83b86ff99a..d466cc5c4332 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -671,12 +671,13 @@ function makeManager(input?: { SourceControlProviderRegistry.SourceControlProviderRegistry, GitHubSourceControlProvider.make.pipe( Effect.map((provider) => { + const baseProvider = input?.sourceControlProvider ?? provider; const sourceControlProvider = () => ({ - ...provider, + ...baseProvider, kind: typeof input?.sourceControlProviderKind === "function" ? input.sourceControlProviderKind() - : (input?.sourceControlProviderKind ?? provider.kind), + : (input?.sourceControlProviderKind ?? baseProvider.kind), }); return SourceControlProviderRegistry.SourceControlProviderRegistry.of({ get: () => Effect.sync(sourceControlProvider), @@ -1087,7 +1088,17 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }); expect(pullRequest).toBeNull(); - expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + +yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); + +expect( + yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/unknown-branch-provider", + }), +).toBeNull(); + +expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); }), ); From 83ba7a41c82f821c10019b6cdfe664d1b81193dd Mon Sep 17 00:00:00 2001 From: umutcagand Date: Fri, 11 Sep 2026 12:56:06 +0300 Subject: [PATCH 14/15] chore: normalize CodeRabbit regression test indentation --- .../workflows/tmp-coderabbit-9212-indent.yml | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/workflows/tmp-coderabbit-9212-indent.yml diff --git a/.github/workflows/tmp-coderabbit-9212-indent.yml b/.github/workflows/tmp-coderabbit-9212-indent.yml new file mode 100644 index 000000000000..867825ac9488 --- /dev/null +++ b/.github/workflows/tmp-coderabbit-9212-indent.yml @@ -0,0 +1,43 @@ +name: Temporary CodeRabbit 9212 indentation fix + +on: + push: + branches: + - fix/unknown-provider-pr-polling + +permissions: + contents: write + +jobs: + patch: + if: github.actor != 'github-actions[bot]' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: fix/unknown-provider-pr-polling + + - name: Fix regression test indentation + shell: bash + run: | + python - <<'PY' + from pathlib import Path + path = Path("apps/server/src/git/GitManager.test.ts") + text = path.read_text() + old = ' expect(pullRequest).toBeNull();\n\nyield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]);\n\nexpect(\n yield* manager.branchPullRequest({\n cwd: repoDir,\n branch: "feature/unknown-branch-provider",\n }),\n).toBeNull();\n\nexpect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' + new = ' expect(pullRequest).toBeNull();\n\n yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]);\n\n expect(\n yield* manager.branchPullRequest({\n cwd: repoDir,\n branch: "feature/unknown-branch-provider",\n }),\n ).toBeNull();\n\n expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' + if text.count(old) != 1: + raise SystemExit(f"expected one malformed regression block, found {text.count(old)}") + path.write_text(text.replace(old, new, 1)) + PY + git diff --check + + - name: Commit indentation fix + shell: bash + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add apps/server/src/git/GitManager.test.ts + git rm .github/workflows/tmp-coderabbit-9212-indent.yml + git commit -m "test(server): fix unknown-provider regression formatting" + git push origin HEAD:fix/unknown-provider-pr-polling From f91e487bf963bc24262080dde54721898e851e96 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:56:23 +0000 Subject: [PATCH 15/15] test(server): fix unknown-provider regression formatting --- .../workflows/tmp-coderabbit-9212-indent.yml | 43 ------------------- apps/server/src/git/GitManager.test.ts | 16 +++---- 2 files changed, 8 insertions(+), 51 deletions(-) delete mode 100644 .github/workflows/tmp-coderabbit-9212-indent.yml diff --git a/.github/workflows/tmp-coderabbit-9212-indent.yml b/.github/workflows/tmp-coderabbit-9212-indent.yml deleted file mode 100644 index 867825ac9488..000000000000 --- a/.github/workflows/tmp-coderabbit-9212-indent.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Temporary CodeRabbit 9212 indentation fix - -on: - push: - branches: - - fix/unknown-provider-pr-polling - -permissions: - contents: write - -jobs: - patch: - if: github.actor != 'github-actions[bot]' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - ref: fix/unknown-provider-pr-polling - - - name: Fix regression test indentation - shell: bash - run: | - python - <<'PY' - from pathlib import Path - path = Path("apps/server/src/git/GitManager.test.ts") - text = path.read_text() - old = ' expect(pullRequest).toBeNull();\n\nyield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]);\n\nexpect(\n yield* manager.branchPullRequest({\n cwd: repoDir,\n branch: "feature/unknown-branch-provider",\n }),\n).toBeNull();\n\nexpect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' - new = ' expect(pullRequest).toBeNull();\n\n yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]);\n\n expect(\n yield* manager.branchPullRequest({\n cwd: repoDir,\n branch: "feature/unknown-branch-provider",\n }),\n ).toBeNull();\n\n expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0);' - if text.count(old) != 1: - raise SystemExit(f"expected one malformed regression block, found {text.count(old)}") - path.write_text(text.replace(old, new, 1)) - PY - git diff --check - - - name: Commit indentation fix - shell: bash - run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add apps/server/src/git/GitManager.test.ts - git rm .github/workflows/tmp-coderabbit-9212-indent.yml - git commit -m "test(server): fix unknown-provider regression formatting" - git push origin HEAD:fix/unknown-provider-pr-polling diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index d466cc5c4332..ca57e9d8146b 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1089,16 +1089,16 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { expect(pullRequest).toBeNull(); -yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); + yield* runGit(repoDir, ["config", "--unset", "remote.origin.url"]); -expect( - yield* manager.branchPullRequest({ - cwd: repoDir, - branch: "feature/unknown-branch-provider", - }), -).toBeNull(); + expect( + yield* manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/unknown-branch-provider", + }), + ).toBeNull(); -expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); + expect(ghCalls.filter((call) => call.startsWith("pr list "))).toHaveLength(0); }), );