Skip to content

Commit e6caca2

Browse files
Report onboarding duplicate-callback recovery through reportError (CL-7255) (#594)
## Summary `recentlyConnectedCredential` still treats a hub/parse failure as found-nothing so the OAuth callback ends at `state_expired`. The catch now calls `reportError` with `operation: "onboarding_duplicate_callback_recovery"` and `extra.userId`, and no longer interpolates the raw `cause.message` into the operational log. ## Verification - `bun test packages/onboarding/test/report-error-routing.test.ts` before the implementation: 4 pass, 1 fail, exit 1 (`reportErrorCalls` length 0). - Same file after the implementation: 5 pass, 0 fail, exit 0. - `bun test packages/onboarding/src packages/onboarding/test/report-error-routing.test.ts packages/onboarding/test/openrouter-connect-routes.test.ts packages/onboarding/test/huggingface-connect-routes.test.ts packages/onboarding/test/routes.test.ts`: 72 pass, 0 fail, exit 0. - `cd packages/onboarding && bunx tsc --noEmit`: exit 0. - `bun run typecheck`: exit 0. - `bun run check:report-error`: exit 0 (baseline shrunk by the fixed catch). - `bun run check`: typecheck and lint passed (eslint 0 errors / 10 pre-existing warnings). The test step failed in `scripts/db-setup.test.ts` with `PostgresError: password authentication failed for user "thegreataxios"` (pre-existing local `DATABASE_URL`, unrelated to this change). Fixes CL-7255
1 parent 7f407ca commit e6caca2

3 files changed

Lines changed: 85 additions & 3 deletions

File tree

‎packages/onboarding/src/routes.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,14 @@ async function recentlyConnectedCredential(
287287
);
288288
return match ? tenant : undefined;
289289
} catch (cause) {
290-
const message = cause instanceof Error ? cause.message : String(cause);
290+
// Never the raw cause detail — this path lists credentials after a
291+
// connect and can see secret-shaped hub/parse errors (CL-7255).
292+
const refId = reportError(cause, {
293+
operation: "onboarding_duplicate_callback_recovery",
294+
extra: { userId: args.userId },
295+
});
291296
args.log(
292-
`duplicate-callback recovery check failed for user ${args.userId}: ${message}`,
297+
`duplicate-callback recovery check failed for user ${args.userId} [${refId}]`,
293298
);
294299
return undefined;
295300
}

‎packages/onboarding/test/report-error-routing.test.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,81 @@ describe("plant-env-credentials.ts catches report through reportError and never
231231
expect(cause).toBeInstanceOf(Error);
232232
});
233233
});
234+
235+
describe("recentlyConnectedCredential reports through reportError and still finds nothing", () => {
236+
test("a hub failure during duplicate-callback recovery reports operation + userId and still ends as state_expired", async () => {
237+
// A principals page arktype will reject, with the secret in a union
238+
// field so the parse summary echoes it. `not.toContain` is otherwise a
239+
// tautology — a type-mismatch summary is `was string` and never
240+
// mentions the value (CL-7255).
241+
const SECRET = "sk-or-v1-thisisafakesecretvalue";
242+
const logs: string[] = [];
243+
const hub = new Hono();
244+
hub.get("/api/me/principals", (c) =>
245+
c.json({
246+
data: [
247+
{
248+
principalId: "prn_1",
249+
tenantId: "ten_1",
250+
tenantName: "user_1's workbench",
251+
tenantSlug: "user-1-user1",
252+
kind: SECRET,
253+
status: "active",
254+
roles: [],
255+
},
256+
],
257+
nextCursor: null,
258+
}),
259+
);
260+
const server = Bun.serve({ port: 0, fetch: hub.fetch });
261+
try {
262+
const routes = createOnboardingRoutes({
263+
hubUrl: `http://localhost:${server.port}`,
264+
pushWorkflow: async () => ({
265+
outcome: "pushed" as const,
266+
commitSha: "a".repeat(40),
267+
}),
268+
log: (line) => {
269+
logs.push(line);
270+
},
271+
pendingSeedStore,
272+
});
273+
const app = mountAuthenticated(routes);
274+
275+
const response = await app.request(
276+
"/oauth/openrouter/callback?code=auth_code_1",
277+
{
278+
headers: {
279+
cookie: "workbench_openrouter_connect=not-a-real-state",
280+
},
281+
},
282+
);
283+
284+
expect(response.status).toBe(302);
285+
const redirect = new URL(
286+
response.headers.get("location") ?? "",
287+
"https://x",
288+
);
289+
expect(redirect.searchParams.get("outcome")).toBe("error");
290+
expect(redirect.searchParams.get("code")).toBe("state_expired");
291+
292+
expect(reportErrorCalls).toHaveLength(1);
293+
const [cause, context] = reportErrorCalls[0] as [
294+
unknown,
295+
Record<string, unknown>,
296+
];
297+
expect(context.operation).toBe("onboarding_duplicate_callback_recovery");
298+
expect(context.tenantId).toBeUndefined();
299+
expect((context.extra as { userId: string }).userId).toBe("user_1");
300+
// reportError is the one place the unredacted cause may travel —
301+
// error-sink redacts before anything reaches a log sink.
302+
expect(cause).toBeInstanceOf(Error);
303+
const causeMessage =
304+
cause instanceof Error ? cause.message : String(cause);
305+
expect(causeMessage).toContain(SECRET);
306+
expect(JSON.stringify({ logs, context })).not.toContain(SECRET);
307+
} finally {
308+
server.stop(true);
309+
}
310+
});
311+
});

‎scripts/checks/report-error-baseline.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ packages/ollama-adapter/src/inline-tool-json.ts 1 return false;
211211
packages/ollama-adapter/src/inline-tool-json.ts 1 return { kind: "incomplete" };
212212
packages/onboarding/src/pending-seed.ts 1 return drop();
213213
packages/onboarding/src/routes.ts 1 // Neither `ProvisionError` nor `CliError` messages are safe to show
214-
packages/onboarding/src/routes.ts 1 const message = cause instanceof Error ? cause.message : String(cause);
215214
packages/onboarding/src/routes.ts 1 if (cause instanceof ProvisionError) {
216215
packages/onboarding/src/routes.ts 1 return c.json(
217216
packages/presence/src/artifact-persistence.ts 1 deps.onSnapshotError?.(key, error);

0 commit comments

Comments
 (0)