Skip to content

Commit cb80e81

Browse files
fix: pin the user's credential while the app's reach is unknown
credentialFor treated a failed installations read as covered, so pinned work (routing, stack pushes) could hold an app token that cannot reach the repository and cannot hand it on. Only confirmed coverage keeps the app; otherwise use the user's credential when there is one. The UI still reads an unknown answer as covered, so a GitHub hiccup does not nag. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent be46d4f commit cb80e81

2 files changed

Lines changed: 50 additions & 17 deletions

File tree

‎packages/source-control-github/src/server/GitHubAppAccess.ts‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ export class GitHubAppAccess extends Context.Service<
5555
readonly reach: (host: string, repository: string) => Effect.Effect<GitHubAppReach>;
5656
/** The login the app sign-in acts as. */
5757
readonly viewer: (host: string) => Effect.Effect<string, GitHubRequestPolicy.GitHubApiError>;
58-
/** The credential for work on one repository: the app's only where it is installed. */
58+
/**
59+
* The credential for work pinned to one repository: the app's only where an installation is
60+
* confirmed to cover it, the user's own otherwise when there is one.
61+
*/
5962
readonly credentialFor: (
6063
host: string,
6164
repository: string,
@@ -155,20 +158,27 @@ export const make = Effect.gen(function* () {
155158
timeToLive: (exit) => (Exit.isSuccess(exit) ? COVERAGE_TTL : COVERAGE_FAILURE_TTL),
156159
});
157160

158-
const reach: GitHubAppAccess["Service"]["reach"] = Effect.fn("GitHubAppAccess.reach")(
159-
function* (rawHost, rawRepository) {
160-
const host = rawHost.trim().toLowerCase();
161-
if (!appSignIn.enabled) return "signed-out";
162-
const known = yield* Cache.get(coverage, host).pipe(Effect.option);
163-
if (Option.isNone(known)) return "covered";
164-
if (known.value === null) return "signed-out";
165-
const repository = ownerAndName(rawRepository);
166-
const owner = repository.split("/")[0] ?? "";
167-
return known.value.accounts.has(owner) || known.value.repositories.has(repository)
168-
? "covered"
169-
: "not-covered";
170-
},
171-
);
161+
/** `unknown` when GitHub could not list the installations just now. */
162+
const coverageFor = Effect.fn("GitHubAppAccess.coverageFor")(function* (
163+
rawHost: string,
164+
rawRepository: string,
165+
) {
166+
const host = rawHost.trim().toLowerCase();
167+
if (!appSignIn.enabled) return "signed-out" as const;
168+
const known = yield* Cache.get(coverage, host).pipe(Effect.option);
169+
if (Option.isNone(known)) return "unknown" as const;
170+
if (known.value === null) return "signed-out" as const;
171+
const repository = ownerAndName(rawRepository);
172+
const owner = repository.split("/")[0] ?? "";
173+
return known.value.accounts.has(owner) || known.value.repositories.has(repository)
174+
? ("covered" as const)
175+
: ("not-covered" as const);
176+
});
177+
178+
const reach: GitHubAppAccess["Service"]["reach"] = (host, repository) =>
179+
coverageFor(host, repository).pipe(
180+
Effect.map((answer) => (answer === "unknown" ? "covered" : answer)),
181+
);
172182

173183
const viewer: GitHubAppAccess["Service"]["viewer"] = Effect.fn("GitHubAppAccess.viewer")(
174184
function* (rawHost) {
@@ -193,8 +203,11 @@ export const make = Effect.gen(function* () {
193203
const credentialFor: GitHubAppAccess["Service"]["credentialFor"] = Effect.fn(
194204
"GitHubAppAccess.credentialFor",
195205
)(function* (host, repository) {
196-
const personal = (yield* reach(host, repository)) === "not-covered";
197-
return yield* api.credential(host, { personal });
206+
// Pinned work cannot be handed on, so only confirmed app access keeps the app credential.
207+
if ((yield* coverageFor(host, repository)) === "covered") return yield* api.credential(host);
208+
return yield* api
209+
.credential(host, { personal: true })
210+
.pipe(Effect.catch(() => api.credential(host)));
198211
});
199212

200213
return GitHubAppAccess.of({ reach, viewer, credentialFor });

‎packages/source-control-github/src/server/GitHubAppSignIn.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,26 @@ describe("GitHubAppAccess", () => {
560560
}).pipe(Effect.provide(layer));
561561
});
562562

563+
it.effect("pins the user's credential while the app's reach is unknown", () => {
564+
const failing = (sent: Sent): Reply =>
565+
sent.path.startsWith("/user/installations") ? { status: 502, body: {} } : { body: {} };
566+
const withGh = harness({ signIns: { "github.com": session() }, handle: failing });
567+
const appOnly = harness({
568+
signIns: { "github.com": session() },
569+
ghSignedIn: false,
570+
handle: failing,
571+
});
572+
const tokenFor = Effect.flatMap(Effect.service(GitHubAppAccess.GitHubAppAccess), (access) =>
573+
access
574+
.credentialFor("github.com", "pingdotgg/t3code")
575+
.pipe(Effect.map((held) => Redacted.value(held.token))),
576+
);
577+
return Effect.gen(function* () {
578+
expect(yield* tokenFor.pipe(Effect.provide(withGh.layer))).toBe("gh-token");
579+
expect(yield* tokenFor.pipe(Effect.provide(appOnly.layer))).toBe("ghu_current");
580+
});
581+
});
582+
563583
it.effect("names the account the app sign-in acts as", () => {
564584
const { layer } = harness({
565585
signIns: { "github.com": session() },

0 commit comments

Comments
 (0)