History

feat(sources): persist auth-related sync failures

Rhys Sullivan committed 7 months ago

Commit cd677391 parent dbff25f5 files · +107 −11Browse files

Unified · Split

5 files
  1. packages/platform/sdk/src/runtime/sources/source-store.ts +5 −1
  2. packages/platform/sdk/src/runtime/sources/source-store/lifecycle.ts +5 −1
  3. packages/platform/sdk/src/sources/operations.test.ts +68 −2
  4. packages/platform/sdk/src/sources/operations.ts +17 −4
  5. packages/platform/server/src/index.test.ts +12 −3
packages/platform/sdk/src/runtime/sources/source-store.ts+5−1
@@ -59,7 +59,10 @@ type RuntimeSourceStoreShape = { · 58 unmodified lines
59 }) => ReturnType<typeof removeSourceByIdWithDeps>;59 }) => ReturnType<typeof removeSourceByIdWithDeps>;
60 persistSource: (60 persistSource: (
61 source: Source,61 source: Source,
62 options?: { actorScopeId?: ScopeId | null },62 options?: {
63 actorScopeId?: ScopeId | null;
64 lastError?: string | null;
65 },
63 ) => ReturnType<typeof persistSourceWithDeps>;66 ) => ReturnType<typeof persistSourceWithDeps>;
64};67};
65 68
@@ -136,6 +139,7 @@ export const persistSource = ( · 70 unmodified lines
136 source: Source,139 source: Source,
137 options: {140 options: {
138 actorScopeId?: ScopeId | null;141 actorScopeId?: ScopeId | null;
142 lastError?: string | null;
139 } = {},143 } = {},
140): Effect.Effect<144): Effect.Effect<
141 Source,145 Source,
packages/platform/sdk/src/runtime/sources/source-store/lifecycle.ts+5−1
@@ -70,6 +70,7 @@ export const persistSourceWithDeps = ( · 69 unmodified lines
70 source: Source,70 source: Source,
71 options: {71 options: {
72 actorScopeId?: ScopeId | null;72 actorScopeId?: ScopeId | null;
73 lastError?: string | null;
73 } = {},74 } = {},
74): Effect.Effect<Source, Error, never> =>75): Effect.Effect<Source, Error, never> =>
75 Effect.gen(function* () {76 Effect.gen(function* () {
@@ -111,7 +112,10 @@ export const persistSourceWithDeps = ( · 35 unmodified lines
111 ...localScope.scopeState.sources,112 ...localScope.scopeState.sources,
112 [nextSource.id]: {113 [nextSource.id]: {
113 status: nextSource.status,114 status: nextSource.status,
114 lastError: existingSourceState?.lastError ?? null,115 lastError:
116 options.lastError !== undefined
117 ? options.lastError
118 : existingSourceState?.lastError ?? null,
115 sourceHash: existingSourceState?.sourceHash ?? null,119 sourceHash: existingSourceState?.sourceHash ?? null,
116 createdAt: existingSourceState?.createdAt ?? nextSource.createdAt,120 createdAt: existingSourceState?.createdAt ?? nextSource.createdAt,
117 updatedAt: nextSource.updatedAt,121 updatedAt: nextSource.updatedAt,
packages/platform/sdk/src/sources/operations.test.ts+68−2
@@ -4,6 +4,7 @@ import { · 3 unmodified lines
4import {4import {
5 NodeFileSystem,5 NodeFileSystem,
6} from "@effect/platform-node";6} from "@effect/platform-node";
7import { resolve } from "node:path";
7import {8import {
8 describe,9 describe,
9 expect,10 expect,
@@ -31,8 +32,11 @@ type BrokenStoredSource = { · 21 unmodified lines
31 readonly name: string;32 readonly name: string;
32};33};
33 34
34const makeBrokenSourcePlugin = () => {35const makeBrokenSourcePlugin = (input: {
36 syncErrorMessage?: string;
37} = {}) => {
35 const storage = new Map<string, BrokenStoredSource>();38 const storage = new Map<string, BrokenStoredSource>();
39 const syncErrorMessage = input.syncErrorMessage ?? "sync boom";
36 40
37 return defineExecutorSourcePlugin<41 return defineExecutorSourcePlugin<
38 "broken",42 "broken",
@@ -98,7 +102,7 @@ const makeBrokenSourcePlugin = () => { · 59 unmodified lines
98 kind: "imported",102 kind: "imported",
99 sync: () =>103 sync: () =>
100 Effect.fail(104 Effect.fail(
101 runtimeEffectError("sources/operations.test", "sync boom"),105 runtimeEffectError("sources/operations.test", syncErrorMessage),
102 ),106 ),
103 invoke: () =>107 invoke: () =>
104 Effect.fail(108 Effect.fail(
@@ -154,4 +158,66 @@ describe("source operations", () => { · 49 unmodified lines
154 expect(persisted.name).toBe("Broken Source");158 expect(persisted.name).toBe("Broken Source");
155 }).pipe(Effect.provide(NodeFileSystem.layer)),159 }).pipe(Effect.provide(NodeFileSystem.layer)),
156 );160 );
161
162 it.scoped("marks credential failures as auth_required and records the last sync error", () =>
163 Effect.gen(function* () {
164 const fs = yield* FileSystem.FileSystem;
165 const workspaceRoot = yield* fs.makeTempDirectory({
166 prefix: "executor-source-ops-auth-",
167 });
168
169 const executor = yield* Effect.acquireRelease(
170 createLocalExecutorEffect({
171 localDataDir: ":memory:",
172 workspaceRoot,
173 plugins: [
174 makeBrokenSourcePlugin({
175 syncErrorMessage:
176 "GraphQL introspection requires credentials (status 403)",
177 }),
178 ] as const,
179 }),
180 (executor) =>
181 Effect.promise(() => executor.close()).pipe(
182 Effect.orDie,
183 Effect.zipRight(
184 fs.remove(workspaceRoot, {
185 recursive: true,
186 force: true,
187 }),
188 ),
189 ),
190 );
191
192 const created = yield* executor.broken.createSource({
193 name: "Protected Source",
194 });
195
196 expect(created.status).toBe("auth_required");
197
198 const persisted = yield* executor.broken.getSource(created.id);
199
200 expect(persisted.status).toBe("auth_required");
201 expect(persisted.name).toBe("Protected Source");
202
203 const inspection = yield* executor.sources.inspection.get(created.id);
204
205 expect(inspection.source.status).toBe("auth_required");
206 expect(inspection.toolCount).toBe(0);
207 expect(inspection.tools).toEqual([]);
208
209 const workspaceState = JSON.parse(
210 yield* fs.readFileString(
211 resolve(workspaceRoot, ".executor", "state", "workspace-state.json"),
212 "utf8",
213 ),
214 ) as {
215 sources?: Record<string, { lastError?: string | null }>;
216 };
217
218 expect(workspaceState.sources?.[created.id]?.lastError).toBe(
219 "GraphQL introspection requires credentials (status 403)",
220 );
221 }).pipe(Effect.provide(NodeFileSystem.layer)),
222 );
157});223});
packages/platform/sdk/src/sources/operations.ts+17−4
@@ -36,6 +36,16 @@ const sourceOps = { · 35 unmodified lines
36 remove: operationErrors("sources.remove"),36 remove: operationErrors("sources.remove"),
37} as const;37} as const;
38 38
39const syncFailureDetails = (cause: unknown): string =>
40 cause instanceof Error ? cause.message : String(cause);
41
42const syncFailureStatus = (cause: unknown): Source["status"] =>
43 /(?:status\s*(?:401|403)|unauthorized|forbidden|requires credentials|requires authentication|authentication required|authorization required|missing credential|missing credentials|oauth)/i.test(
44 syncFailureDetails(cause),
45 )
46 ? "auth_required"
47 : "error";
48
39const syncArtifactsForSource = (input: {49const syncArtifactsForSource = (input: {
40 store: ExecutorStateStoreShape;50 store: ExecutorStateStoreShape;
41 sourceStore: Effect.Effect.Success<typeof RuntimeSourceStoreService>;51 sourceStore: Effect.Effect.Success<typeof RuntimeSourceStoreService>;
@@ -58,10 +68,12 @@ const syncArtifactsForSource = (input: { · 16 unmodified lines
58 onLeft: (error) =>68 onLeft: (error) =>
59 Effect.gen(function* () {69 Effect.gen(function* () {
60 if (input.source.enabled && input.source.status === "connected") {70 if (input.source.enabled && input.source.status === "connected") {
61 const erroredSource = yield* normalizeSourceForSave({71 const failedStatus = syncFailureStatus(error);
72 const lastError = syncFailureDetails(error);
73 const failedSource = yield* normalizeSourceForSave({
62 source: {74 source: {
63 ...input.source,75 ...input.source,
64 status: "error",76 status: failedStatus,
65 },77 },
66 now: Date.now(),78 now: Date.now(),
67 }).pipe(79 }).pipe(
@@ -75,12 +87,13 @@ const syncArtifactsForSource = (input: { · 7 unmodified lines
75 87
76 yield* mapPersistenceError(88 yield* mapPersistenceError(
77 input.operation.child("source_error"),89 input.operation.child("source_error"),
78 input.sourceStore.persistSource(erroredSource, {90 input.sourceStore.persistSource(failedSource, {
79 actorScopeId: input.actorScopeId,91 actorScopeId: input.actorScopeId,
92 lastError,
80 }),93 }),
81 );94 );
82 95
83 return erroredSource;96 return failedSource;
84 }97 }
85 98
86 return yield* input.operation.unknownStorage(99 return yield* input.operation.unknownStorage(
packages/platform/server/src/index.test.ts+12−3
@@ -13,6 +13,7 @@ import { · 12 unmodified lines
13import { NodeFileSystem } from "@effect/platform-node";13import { NodeFileSystem } from "@effect/platform-node";
14import { describe, expect, it } from "@effect/vitest";14import { describe, expect, it } from "@effect/vitest";
15import { assertTrue } from "@effect/vitest/utils";15import { assertTrue } from "@effect/vitest/utils";
16import * as Data from "effect/Data";
16import * as Effect from "effect/Effect";17import * as Effect from "effect/Effect";
17import * as Layer from "effect/Layer";18import * as Layer from "effect/Layer";
18import * as Schema from "effect/Schema";19import * as Schema from "effect/Schema";
@@ -65,6 +66,13 @@ import { · 46 unmodified lines
65} from "../../../sources/core/src/index";66} from "../../../sources/core/src/index";
66import { createMcpCatalogFragment } from "../../../../plugins/mcp/sdk/catalog";67import { createMcpCatalogFragment } from "../../../../plugins/mcp/sdk/catalog";
67 68
69class MissingCredentialEnvVarError extends Data.TaggedError(
70 "MissingCredentialEnvVarError",
71)<{
72 readonly message: string;
73 readonly tokenEnvVar: string;
74}> {}
75
68const serverHttpPlugins = [76const serverHttpPlugins = [
69 graphqlHttpPlugin(),77 graphqlHttpPlugin(),
70 googleDiscoveryHttpPlugin(),78 googleDiscoveryHttpPlugin(),
@@ -493,9 +501,10 @@ const seedGithubOpenApiSourceInWorkspace = (input: { · 422 unmodified lines
493 const tokenEnvVar = input.credentialEnvVar ?? "GITHUB_TOKEN";501 const tokenEnvVar = input.credentialEnvVar ?? "GITHUB_TOKEN";
494 const tokenValue = process.env[tokenEnvVar];502 const tokenValue = process.env[tokenEnvVar];
495 if (!tokenValue) {503 if (!tokenValue) {
496 return yield* Effect.fail(504 return yield* new MissingCredentialEnvVarError({
497 new Error(`Missing token value in environment variable ${tokenEnvVar}`),505 message: `Missing token value in environment variable ${tokenEnvVar}`,
498 );506 tokenEnvVar,
507 });
499 }508 }
500 509
501 const existingSecrets = yield* input.client.local.listSecrets({});510 const existingSecrets = yield* input.client.local.listSecrets({});