Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/code/src/renderer/di/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ container.bind(UPDATES_CLIENT).toConstantValue(updatesClient);
// connectivity client — passthrough over the renderer host client
const connectivityClient: ConnectivityClient = {
getStatus: () => trpcClient.connectivity.getStatus.query(),
checkNow: () => trpcClient.connectivity.checkNow.mutate(),
onStatusChange: (sub) =>
trpcClient.connectivity.onStatusChange.subscribe(undefined, sub),
};
Expand Down
108 changes: 108 additions & 0 deletions packages/core/src/auth/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ describe("AuthService", () => {
string,
{ name: string; projects: { id: number; name: string }[] }
>;
// Overrides the /api/code/invites/check-access/ response (defaults to
// granting access). May throw to simulate a network error.
checkAccess?: () => Response;
} = {},
) => {
const accountKey = options.accountKey ?? "user-1";
Expand Down Expand Up @@ -186,6 +189,9 @@ describe("AuthService", () => {
} as unknown as Response;
}

if (options.checkAccess) {
return options.checkAccess();
}
return {
ok: true,
json: vi.fn().mockResolvedValue({ has_access: true }),
Expand Down Expand Up @@ -1342,4 +1348,106 @@ describe("AuthService", () => {
expect(redeemCallCount).toBe(2);
});
});

describe("code access resilience", () => {
const okBody = (body: unknown): Response =>
({
ok: true,
json: vi.fn().mockResolvedValue(body),
}) as unknown as Response;

beforeEach(() => {
seedStoredSession();
oauthFlow.refreshToken.mockResolvedValue(
mockTokenResponse({
accessToken: "access-token",
refreshToken: "rotated-refresh-token",
}),
);
});

it.each([
{
name: "grants access when the server reports has_access true",
checkAccess: () => okBody({ has_access: true }),
expected: true,
},
{
name: "denies access when the server explicitly reports no access",
checkAccess: () => okBody({ has_access: false }),
expected: false,
},
{
name: "stays indeterminate when the check throws",
checkAccess: () => {
throw new Error("network down");
},
expected: null,
},
{
name: "stays indeterminate on a 2xx response without a has_access flag",
checkAccess: () => okBody({}),
expected: null,
},
])("$name", async ({ checkAccess, expected }) => {
stubAuthFetch({ checkAccess });

await service.initialize();

const state = service.getState();
expect(state.status).toBe("authenticated");
expect(state.hasCodeAccess).toBe(expected);
});

it.each([
{
name: "a network error",
fail: (): Response => {
throw new Error("network down");
},
},
{
name: "a 401",
fail: (): Response =>
({
ok: false,
status: 401,
json: vi.fn().mockResolvedValue({}),
}) as unknown as Response,
},
])(
"keeps a confirmed grant when a later check hits $name",
async ({ fail }) => {
let shouldFail = false;
stubAuthFetch({
checkAccess: () =>
shouldFail ? fail() : okBody({ has_access: true }),
});

await service.initialize();
expect(service.getState().hasCodeAccess).toBe(true);

shouldFail = true;
await service.refreshAccessToken();

expect(service.getState().hasCodeAccess).toBe(true);
},
);

it("recovers within the retry loop when a later attempt succeeds", async () => {
let attempts = 0;
stubAuthFetch({
checkAccess: () => {
attempts += 1;
if (attempts === 1) throw new Error("transient");
return okBody({ has_access: true });
},
});

await service.initialize();

expect(service.getState().hasCodeAccess).toBe(true);
expect(attempts).toBeGreaterThanOrEqual(2);
});
});
});
88 changes: 73 additions & 15 deletions packages/core/src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -910,26 +910,84 @@ export class AuthService extends TypedEventEmitter<AuthServiceEvents> {
return;
}

try {
const apiHost = getCloudUrlFromRegion(this.session.cloudRegion);
const response = await this.executeAuthenticatedFetch(
fetch,
`${apiHost}/api/code/invites/check-access/`,
{},
this.session.accessToken,
);
const data = (await response.json().catch(() => ({}))) as {
has_access?: boolean;
};
const hasAccess = await this.checkCodeAccess(this.session);

this.updateState({ hasCodeAccess: data.has_access === true });
} catch (error) {
this.logger.warn("Failed to update code access state", { error });
this.updateState({ hasCodeAccess: false });
if (hasAccess !== null) {
this.updateState({ hasCodeAccess: hasAccess });
return;
}

// Indeterminate: a transient/unauthorized failure isn't proof the invite
// was revoked, so keep the prior value and let the next sync re-check.
this.logger.warn(
"Code access check was inconclusive; keeping previous value",
{ hasCodeAccess: this.state.hasCodeAccess },
);
}

/**
* Resolves Code invite access. Only a 2xx response with an explicit boolean
* `has_access` is authoritative; everything else (offline, network error,
* non-2xx, malformed body) is indeterminate, retried with backoff, then
* returned as `null` so the caller keeps the prior value. Uses the synced
* token directly rather than `authenticatedFetch`, which would re-enter the
* refresh flow this runs inside and deadlock.
*/
private async checkCodeAccess(
session: InMemorySession,
): Promise<boolean | null> {
const url = `${getCloudUrlFromRegion(session.cloudRegion)}/api/code/invites/check-access/`;

for (
let attempt = 0;
attempt < AuthService.CODE_ACCESS_MAX_ATTEMPTS;
attempt++
) {
if (!this.connectivity.getStatus().isOnline) {
return null;
}

try {
const response = await this.executeAuthenticatedFetch(
fetch,
url,
{},
session.accessToken,
);

if (response.ok) {
const data = (await response.json().catch(() => null)) as {
has_access?: unknown;
} | null;
if (data && typeof data.has_access === "boolean") {
return data.has_access;
}
this.logger.warn("Code access response missing has_access flag", {
status: response.status,
});
} else {
this.logger.warn("Code access check returned non-OK status", {
status: response.status,
});
}
} catch (error) {
this.logger.warn("Code access check request failed", {
error,
attempt,
});
}

const isLastAttempt =
attempt === AuthService.CODE_ACCESS_MAX_ATTEMPTS - 1;
if (isLastAttempt) break;
await sleepWithBackoff(attempt, AuthService.REFRESH_BACKOFF);
}

return null;
}
private static readonly REFRESH_MAX_ATTEMPTS = 3;
private static readonly ORG_FETCH_MAX_ATTEMPTS = 3;
private static readonly CODE_ACCESS_MAX_ATTEMPTS = 3;
private static readonly ORG_RECOVERY_MAX_ATTEMPTS = 5;
private static readonly REFRESH_BACKOFF: BackoffOptions = {
initialDelayMs: 1_000,
Expand Down
55 changes: 55 additions & 0 deletions packages/core/src/git-interaction/gitInteractionLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ function makeState(overrides: Partial<GitState> = {}): GitState {
ghStatus: { installed: true, authenticated: true },
repoInfo: { owner: "test", repo: "test" },
prStatus: null,
isOnline: true,
...overrides,
};
}
Expand Down Expand Up @@ -249,4 +250,58 @@ describe("computeGitInteractionState", () => {
expect(result.primaryAction.enabled).toBe(false);
});
});

describe("offline", () => {
it.each([
{
action: "push",
field: "pushDisabledReason" as const,
overrides: {
currentBranch: "feature/test",
hasChanges: false,
aheadOfRemote: 2,
} satisfies Partial<GitState>,
},
{
action: "create-pr",
field: "createPrDisabledReason" as const,
overrides: {
currentBranch: "feature/test",
hasChanges: true,
} satisfies Partial<GitState>,
},
])(
"gates $action with a no-internet reason while offline",
({ field, overrides }) => {
const result = computeGitInteractionState(
makeState({ ...overrides, isOnline: false }),
);
expect(result[field]).toBe("No internet connection");
},
);

it.each([
{
action: "commit",
overrides: {
currentBranch: "feature/test",
hasChanges: true,
} satisfies Partial<GitState>,
},
{
action: "branch-here",
overrides: { currentBranch: null } satisfies Partial<GitState>,
},
])(
"still allows the local $action action while offline",
({ action, overrides }) => {
const result = computeGitInteractionState(
makeState({ ...overrides, isOnline: false }),
);
const found = result.actions.find((a) => a.id === action);
expect(found?.enabled).toBe(true);
expect(found?.disabledReason).toBeNull();
},
);
});
Comment thread
k11kirky marked this conversation as resolved.
});
11 changes: 11 additions & 0 deletions packages/core/src/git-interaction/gitInteractionLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,18 @@ interface GitState {
headBranch: string | null;
prUrl: string | null;
} | null;
isOnline: boolean;
}

const OFFLINE_REASON = "No internet connection";

interface GitComputed {
actions: GitMenuAction[];
primaryAction: GitMenuAction;
pushDisabledReason: string | null;
// Named like pushDisabledReason: a disabled create-pr is dropped from
// `actions`, so its reason is only readable here.
createPrDisabledReason: string | null;
prBaseBranch: string | null;
prHeadBranch: string | null;
prUrl: string | null;
Expand Down Expand Up @@ -74,6 +80,7 @@ function getPushDisabledReason(
opts?: { assumeWillHaveCommits?: boolean },
): string | null {
if (repoReason) return repoReason;
if (!s.isOnline) return OFFLINE_REASON;

if (s.behind > 0) {
return "Sync branch with remote first.";
Expand All @@ -96,6 +103,7 @@ function getCreatePrDisabledReason(
repoReason: string | null,
): string | null {
if (repoReason) return repoReason;
if (!s.isOnline) return OFFLINE_REASON;

if (!s.ghStatus) return "Checking GitHub CLI status...";
if (!s.ghStatus.installed) return "Install GitHub CLI: `brew install gh`";
Expand Down Expand Up @@ -172,6 +180,7 @@ export function computeGitInteractionState(input: GitState): GitComputed {
actions: [branchAction],
primaryAction: branchAction,
pushDisabledReason: "Create a branch first.",
createPrDisabledReason: "Create a branch first.",
prBaseBranch: input.defaultBranch,
prHeadBranch: null,
prUrl: null,
Expand Down Expand Up @@ -200,6 +209,7 @@ export function computeGitInteractionState(input: GitState): GitComputed {
actions,
primaryAction,
pushDisabledReason: "Create a feature branch first.",
createPrDisabledReason,
prBaseBranch: input.defaultBranch,
prHeadBranch: input.currentBranch,
prUrl: input.prStatus?.prUrl ?? null,
Expand Down Expand Up @@ -231,6 +241,7 @@ export function computeGitInteractionState(input: GitState): GitComputed {
pushDisabledReason: getPushDisabledReason(input, repoReason, {
assumeWillHaveCommits: true,
}),
createPrDisabledReason,
prBaseBranch: input.prStatus?.baseBranch ?? input.defaultBranch,
prHeadBranch: input.prStatus?.headBranch ?? input.currentBranch,
prUrl: input.prStatus?.prUrl ?? null,
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/sessions/sessionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3872,6 +3872,16 @@ export class SessionService {
}
}

/**
* Recovers cloud sessions after reconnect: retries errored streams and
* flushes stranded queues (same steps as the window-focus and auth-restored
* paths). Local sessions recover on their own via `reconcileLocalConnection`.
*/
public recoverAfterReconnect(): void {
this.retryUnhealthyCloudSessions();
this.flushQueuedCloudMessagesAfterAuthRestored();
}

public flushQueuedCloudMessagesAfterAuthRestored(): void {
const sessions = this.d.store.getSessions();
for (const session of Object.values(sessions)) {
Expand Down
Loading
Loading