Skip to content

Commit ee67b0e

Browse files
committed
health: classify Google disabled-API 403 as misconfigured, not expired
A SERVICE_DISABLED / accessNotConfigured 403 means the API is disabled in the OAuth client's project; the credential is fine. Probes now classify it as a new 'misconfigured' status: amber 'API disabled' badge, the provider's own remediation text (console link clickable) on the row, and no reconnect prompt.
1 parent a546d2c commit ee67b0e

9 files changed

Lines changed: 262 additions & 37 deletions

File tree

e2e/scenarios/google-disabled-api-expired.test.ts renamed to e2e/scenarios/google-disabled-api.test.ts

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
1-
// Repro: a Google 403 for a DISABLED API reads as an expired credential.
1+
// A Google 403 for a DISABLED API must NOT read as an expired credential.
22
//
33
// Production case (2026-07-10): a user's "Personal Google" connection showed
44
// the red Expired badge although its OAuth token was alive and refreshing.
55
// The org's Google OAuth client lives in a GCP project without the Gmail /
66
// Drive / Calendar APIs enabled, so the health probe gets Google's
77
// SERVICE_DISABLED 403 ("Gmail API has not been used in project ... or it is
8-
// disabled"). `classifyHttpStatus` maps every 401/403 to "expired", so a
9-
// perfectly healthy credential renders as Expired and the UI tells the user
10-
// to reconnect - which cannot fix a disabled upstream API.
8+
// disabled"). The old status-only classifier mapped every 401/403 to
9+
// "expired", so a healthy credential rendered as Expired and the UI told the
10+
// user to reconnect - advice that cannot fix a disabled upstream API.
1111
//
1212
// The scenario replays that exact journey against the Google emulator: a real
1313
// OAuth grant probes healthy; then the upstream starts answering the probe
1414
// operation with Google's real SERVICE_DISABLED body (fault injection - the
15-
// token stays valid, exactly like prod); the connection now reads Expired on
16-
// screen with the reconnect toast. The assertions pin today's (wrong)
17-
// classification on purpose: when the classifier learns to tell "API disabled
18-
// in the project" from "credential expired", this scenario is the one that
19-
// must change.
15+
// token stays valid, exactly like prod). The fixed classifier reads the error
16+
// body and reports "misconfigured": on screen that is the amber "API
17+
// disabled" badge with Google's own remediation text (including the console
18+
// link that enables the API), and no reconnect prompt.
2019
import { randomBytes } from "node:crypto";
2120

2221
import { expect } from "@effect/vitest";
@@ -205,7 +204,7 @@ const connectGoogleAccount = (input: {
205204
});
206205

207206
scenario(
208-
"Google · repro: a disabled upstream API reads as an expired connection (SERVICE_DISABLED 403)",
207+
"Google · a disabled upstream API reads as API disabled with the enable link, not Expired",
209208
{ timeout: 300_000 },
210209
Effect.gen(function* () {
211210
const target = yield* Target;
@@ -259,16 +258,16 @@ scenario(
259258
);
260259
yield* Effect.promise(() => emulator.client.ledger.clear());
261260

262-
// THE BUG, pinned: a 403 that means "enable this API in your GCP
263-
// project" classifies as an expired credential.
261+
// The fix under test: a 403 that means "enable this API in your GCP
262+
// project" classifies as misconfigured, not as a dead credential.
264263
const verdict = yield* client.connections.checkHealth({
265264
params: { owner: "org", integration: slug, name: CONNECTION },
266265
query: { ifStaleMs: 0 },
267266
});
268267
expect(
269268
verdict.status,
270-
"BUG: the SERVICE_DISABLED 403 classifies as an expired credential",
271-
).toBe("expired");
269+
`the SERVICE_DISABLED 403 classifies as misconfigured: ${JSON.stringify(verdict)}`,
270+
).toBe("misconfigured");
272271
expect(verdict.httpStatus, "the probe observed the 403").toBe(403);
273272
expect(
274273
verdict.detail,
@@ -287,24 +286,49 @@ scenario(
287286
});
288287

289288
await step(
290-
"The healthy-yesterday connection now shows Expired, with no clicks",
289+
"The connection shows the amber API disabled badge - not Expired - with no clicks",
291290
async () => {
292291
await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
293-
await connections.getByText("Expired", { exact: true }).waitFor({ timeout: 30_000 });
294-
await connections.getByLabel("Status: Expired").waitFor();
292+
await connections
293+
.getByText("API disabled", { exact: true })
294+
.waitFor({ timeout: 30_000 });
295+
await connections.getByLabel("Status: API disabled").waitFor();
296+
// The dead-credential story must be gone entirely.
297+
expect(
298+
await connections.getByText("Expired", { exact: true }).count(),
299+
"no Expired badge on a misconfigured connection",
300+
).toBe(0);
301+
},
302+
);
303+
304+
await step(
305+
"The row carries Google's own instruction, console link included",
306+
async () => {
307+
await connections.getByText(/has not been used in project/).waitFor();
308+
const consoleLink = connections.getByRole("link", {
309+
name: /console\.developers\.google\.com/,
310+
});
311+
await consoleLink.waitFor();
312+
expect(
313+
await consoleLink.getAttribute("href"),
314+
"the enable-API console link is clickable",
315+
).toContain("console.developers.google.com");
295316
},
296317
);
297318

298319
await step(
299-
"Check now tells the user to reconnect - advice that cannot fix a disabled API",
320+
"Check now explains the disabled API instead of prescribing reconnect",
300321
async () => {
301322
await connections.locator('button[aria-haspopup="menu"]').click();
302323
await page.getByRole("menuitem", { name: "Check now" }).click();
303-
// The misleading UX in one line: the toast prescribes reconnecting
304-
// while the upstream error says "enable the API in your project".
305324
await page
306-
.getByText("Connection expired, reconnect to restore access")
325+
.getByText(/has not been used in project/)
326+
.first()
307327
.waitFor({ timeout: 30_000 });
328+
expect(
329+
await page.getByText("Connection expired, reconnect to restore access").count(),
330+
"the reconnect toast never fires for a configuration 403",
331+
).toBe(0);
308332
},
309333
);
310334
});

packages/core/sdk/src/health-check.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { describe, expect, it } from "@effect/vitest";
55

66
import {
77
candidateIdentityTier,
8+
classifyHttpStatus,
9+
classifyProbeResponse,
810
rankResponseSample,
911
sortHealthCheckCandidatesByIdentity,
1012
} from "./health-check";
@@ -66,3 +68,73 @@ describe("candidateIdentityTier", () => {
6668
).toBe("user.getAuthUser");
6769
});
6870
});
71+
72+
// Probe classification: 401/403 mean "credential dead" EXCEPT the
73+
// configuration 403 (Google accessNotConfigured / SERVICE_DISABLED), which
74+
// must read misconfigured: the token authenticated, the API is disabled in
75+
// the OAuth client's project, and reconnecting cannot fix it. This is the
76+
// production case behind the "Expired when it is not" report (2026-07-10).
77+
describe("classifyProbeResponse", () => {
78+
const disabledMessage =
79+
"Gmail API has not been used in project 000000000000 before or it is disabled. " +
80+
"Enable it by visiting https://console.developers.google.com/apis/api/gmail.googleapis.com/overview?project=000000000000 then retry.";
81+
82+
it("classifies Google's classic accessNotConfigured 403 as misconfigured", () => {
83+
const body = {
84+
error: {
85+
code: 403,
86+
message: disabledMessage,
87+
errors: [
88+
{ message: disabledMessage, domain: "usageLimits", reason: "accessNotConfigured" },
89+
],
90+
status: "PERMISSION_DENIED",
91+
},
92+
};
93+
expect(classifyProbeResponse(403, body)).toBe("misconfigured");
94+
});
95+
96+
it("classifies the newer SERVICE_DISABLED ErrorInfo detail as misconfigured", () => {
97+
const body = {
98+
error: {
99+
code: 403,
100+
message: disabledMessage,
101+
status: "PERMISSION_DENIED",
102+
details: [
103+
{
104+
"@type": "type.googleapis.com/google.rpc.ErrorInfo",
105+
reason: "SERVICE_DISABLED",
106+
domain: "googleapis.com",
107+
},
108+
],
109+
},
110+
};
111+
expect(classifyProbeResponse(403, body)).toBe("misconfigured");
112+
});
113+
114+
it("keeps a plain 403 (no recognized reason) as expired: false-expired is the safe default", () => {
115+
expect(classifyProbeResponse(403, { error: { code: 403, message: "Forbidden" } })).toBe(
116+
"expired",
117+
);
118+
expect(classifyProbeResponse(403, undefined)).toBe("expired");
119+
expect(classifyProbeResponse(403, "Forbidden")).toBe("expired");
120+
// A permission reason that is NOT a configuration marker stays expired.
121+
expect(
122+
classifyProbeResponse(403, {
123+
error: { errors: [{ reason: "insufficientPermissions" }] },
124+
}),
125+
).toBe("expired");
126+
});
127+
128+
it("never rewrites a 401: an authentication failure is a credential problem regardless of body", () => {
129+
const body = { error: { errors: [{ reason: "accessNotConfigured" }] } };
130+
expect(classifyProbeResponse(401, body)).toBe("expired");
131+
});
132+
133+
it("matches the status-only classifier everywhere else", () => {
134+
for (const status of [200, 204, 404, 429, 500, 503]) {
135+
expect(classifyProbeResponse(status, { error: {} }), `status ${status}`).toBe(
136+
classifyHttpStatus(status),
137+
);
138+
}
139+
});
140+
});

packages/core/sdk/src/health-check.ts

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,24 @@
1818
import { Schema } from "effect";
1919

2020
// ---------------------------------------------------------------------------
21-
// Status: the four states a connection can be in. `expired` is the one this
21+
// Status: the five states a connection can be in. `expired` is the one this
2222
// whole feature exists for (Google's 7-day dev-token revocation): the credential
23-
// authenticated fine yesterday and now returns 401/403. `degraded` covers any
24-
// other non-2xx (upstream 5xx, a 404 on a mis-picked operation); `unknown` is
25-
// "never checked / no health check configured".
23+
// authenticated fine yesterday and now returns 401/403. `misconfigured` is the
24+
// 403 that is NOT a credential problem: the token authenticated, but the
25+
// upstream rejects on configuration (a Google API not enabled in the OAuth
26+
// client's GCP project): the fix is in the provider console, so telling the
27+
// user to reconnect would mislead. `degraded` covers any other non-2xx
28+
// (upstream 5xx, a 404 on a mis-picked operation); `unknown` is "never checked
29+
// / no health check configured".
2630
// ---------------------------------------------------------------------------
2731

28-
export const HealthStatus = Schema.Literals(["healthy", "expired", "degraded", "unknown"]);
32+
export const HealthStatus = Schema.Literals([
33+
"healthy",
34+
"expired",
35+
"misconfigured",
36+
"degraded",
37+
"unknown",
38+
]);
2939
export type HealthStatus = typeof HealthStatus.Type;
3040

3141
// ---------------------------------------------------------------------------
@@ -139,13 +149,59 @@ export type HealthCheckCandidate = typeof HealthCheckCandidate.Type;
139149
// ---------------------------------------------------------------------------
140150

141151
/** Map an HTTP status to a health state: 2xx healthy, 401/403 expired (the auth
142-
* wall), everything else degraded. */
152+
* wall), everything else degraded. Status-only fallback: when the response
153+
* BODY is available, use `classifyProbeResponse` instead, which tells a
154+
* credential 403 apart from a configuration 403. */
143155
export const classifyHttpStatus = (status: number): HealthStatus => {
144156
if (status >= 200 && status < 300) return "healthy";
145157
if (status === 401 || status === 403) return "expired";
146158
return "degraded";
147159
};
148160

161+
// Error `reason` / `status` markers that make a 403 a CONFIGURATION rejection
162+
// rather than a credential one. Deliberately narrow and provider-shaped:
163+
// Google's error envelope carries `errors[].reason: "accessNotConfigured"`
164+
// (message "<API> has not been used in project N before or it is disabled")
165+
// and newer surfaces use `status: "PERMISSION_DENIED"` with
166+
// `details[].reason: "SERVICE_DISABLED"`. Unrecognized 403s keep the expired
167+
// classification: a false "expired" prompts a harmless reconnect, but a false
168+
// "misconfigured" would hide a genuinely dead credential.
169+
const CONFIGURATION_403_REASONS = new Set(["accessnotconfigured", "service_disabled"]);
170+
171+
/** Collect candidate `reason` markers from a Google-shaped error envelope:
172+
* `error.errors[].reason` (classic) and `error.details[].reason` (newer
173+
* google.rpc.ErrorInfo). Tolerant of partial shapes; returns []. */
174+
const errorReasonMarkers = (body: unknown): string[] => {
175+
if (body == null || typeof body !== "object") return [];
176+
const error = (body as Record<string, unknown>)["error"];
177+
if (error == null || typeof error !== "object") return [];
178+
const markers: string[] = [];
179+
for (const key of ["errors", "details"] as const) {
180+
const entries = (error as Record<string, unknown>)[key];
181+
if (!Array.isArray(entries)) continue;
182+
for (const entry of entries) {
183+
if (entry == null || typeof entry !== "object") continue;
184+
const reason = (entry as Record<string, unknown>)["reason"];
185+
if (typeof reason === "string") markers.push(reason.toLowerCase());
186+
}
187+
}
188+
return markers;
189+
};
190+
191+
/** Classify a probe response from its status AND body. Everything is
192+
* `classifyHttpStatus` except one carve-out: a 403 whose error body carries a
193+
* known configuration reason (Google `accessNotConfigured` /
194+
* `SERVICE_DISABLED`) is `misconfigured`, not `expired`: the credential
195+
* authenticated; the upstream API is disabled in the OAuth client's project,
196+
* and only enabling it there (not reconnecting) fixes it. */
197+
export const classifyProbeResponse = (status: number, body: unknown): HealthStatus => {
198+
const byStatus = classifyHttpStatus(status);
199+
if (status !== 403 || byStatus !== "expired") return byStatus;
200+
return errorReasonMarkers(body).some((reason) => CONFIGURATION_403_REASONS.has(reason))
201+
? "misconfigured"
202+
: "expired";
203+
};
204+
149205
/** Resolve a dot-path (`a.b.0.c`) against a JSON value and coerce the leaf to a
150206
* display string. Numeric segments index arrays. Returns undefined when the
151207
* path is empty, missing, or lands on a non-scalar. */

packages/core/sdk/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ export {
114114
HealthCheckCandidateParameter,
115115
HealthCheckResponseField,
116116
classifyHttpStatus,
117+
classifyProbeResponse,
117118
extractIdentity,
118119
compareHealthCheckCandidates,
119120
candidateIdentityTier,

packages/core/sdk/src/shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export {
118118
HealthCheckCandidate,
119119
HealthCheckCandidateParameter,
120120
classifyHttpStatus,
121+
classifyProbeResponse,
121122
extractIdentity,
122123
compareHealthCheckCandidates,
123124
candidateIdentityTier,

packages/plugins/openapi/src/sdk/backing.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
ToolName,
88
ToolResult,
99
authToolFailure,
10-
classifyHttpStatus,
10+
classifyProbeResponse,
1111
detectInsufficientScope,
1212
sortHealthCheckCandidatesByIdentity,
1313
extractIdentity,
@@ -962,7 +962,9 @@ export const checkHealthOpenApi = (input: {
962962
} satisfies HealthCheckResult;
963963
}
964964

965-
const status = classifyHttpStatus(probe.result.status);
965+
// Body-aware: a configuration 403 (Google accessNotConfigured /
966+
// SERVICE_DISABLED) reads misconfigured, not expired.
967+
const status = classifyProbeResponse(probe.result.status, probe.result.error);
966968
const identity =
967969
status === "healthy" ? extractIdentity(probe.result.data, spec.identityField) : undefined;
968970
// Sample the returned body ONLY on a healthy probe: the sample exists to

0 commit comments

Comments
 (0)