Reads used to walk the scope stack with a findOne per scope, short-circuiting on first hit. That's O(N) DB hops on a miss where N = scope-stack depth. Swap for a single findMany by id (the scope adapter injects scope_id IN (scopes) automatically) and sort the resulting rows in-process by innermost-first. Provider calls stay sequential, bounded by the number of registered rows for the id (usually 0 or 1), not by scope depth. Adds a test covering innermost-shadows-outer ordering.
Unified · Split
| @@ -933,4 +933,46 @@ describe("tenant isolation (SDK)", () => { · 932 unmodified lines | ||
| 933 | 933 | expect(value).toBeNull(); |
| 934 | 934 | }), |
| 935 | 935 | ); |
| 936 | + | |
| 937 | + it.effect("secrets.get — innermost scope shadows outer on same id", () => | |
| 938 | + Effect.gen(function* () { | |
| 939 | + const plugins = [tenantPlugin()] as const; | |
| 940 | + const schema = collectSchemas(plugins); | |
| 941 | + const adapter = makeMemoryAdapter({ schema }); | |
| 942 | + const blobs = makeInMemoryBlobStore(); | |
| 943 | + | |
| 944 | + const innerScope = ScopeId.make("user-org:u1:o1"); | |
| 945 | + const outerScope = ScopeId.make("o1"); | |
| 946 | + | |
| 947 | + const exec = yield* createExecutor({ | |
| 948 | + scopes: [ | |
| 949 | + new Scope({ id: innerScope, name: "inner", createdAt: new Date() }), | |
| 950 | + new Scope({ id: outerScope, name: "outer", createdAt: new Date() }), | |
| 951 | + ], | |
| 952 | + adapter, | |
| 953 | + blobs, | |
| 954 | + plugins, | |
| 955 | + }); | |
| 956 | + | |
| 957 | + yield* exec.secrets.set( | |
| 958 | + new SetSecretInput({ | |
| 959 | + id: SecretId.make("token"), | |
| 960 | + scope: outerScope, | |
| 961 | + name: "org token", | |
| 962 | + value: "org-value", | |
| 963 | + }), | |
| 964 | + ); | |
| 965 | + yield* exec.secrets.set( | |
| 966 | + new SetSecretInput({ | |
| 967 | + id: SecretId.make("token"), | |
| 968 | + scope: innerScope, | |
| 969 | + name: "user token", | |
| 970 | + value: "user-value", | |
| 971 | + }), | |
| 972 | + ); | |
| 973 | + | |
| 974 | + const value = yield* exec.secrets.get("token"); | |
| 975 | + expect(value).toBe("user-value"); | |
| 976 | + }), | |
| 977 | + ); | |
| 936 | 978 | }); |
| @@ -537,31 +537,40 @@ export const createExecutor = < · 536 unmodified lines | ||
| 537 | 537 | // walk because their secrets must be registered through set() to |
| 538 | 538 | // be known at all. |
| 539 | 539 | // |
| 540 | − // Multi-scope behavior: the routing-table lookup walks the scope | |
| 541 | − // stack explicitly, innermost first, so that a secret registered | |
| 542 | − // in a deeper scope shadows one with the same id at a shallower | |
| 543 | − // scope (e.g. a user's personal OAuth token wins over an org-wide | |
| 544 | − // one). The provider-enumeration fallback is scope-agnostic — | |
| 545 | − // providers like env or 1password don't partition their inventory | |
| 546 | − // by executor scope. | |
| 540 | + // Multi-scope behavior: the routing-table lookup pulls every row | |
| 541 | + // for this id across the scope stack in a single `IN (...)` query, | |
| 542 | + // then sorts innermost-first so a secret registered in a deeper | |
| 543 | + // scope shadows one with the same id at a shallower scope (e.g. a | |
| 544 | + // user's personal OAuth token wins over an org-wide one). Provider | |
| 545 | + // calls stay sequential — scope-partitioning providers (workos-vault, | |
| 546 | + // 1password-per-vault) have to be asked per scope because the object | |
| 547 | + // name includes the scope — but they're bounded by the number of | |
| 548 | + // registered rows for this id, not by scope-stack depth. The | |
| 549 | + // provider-enumeration fallback is scope-agnostic: providers like | |
| 550 | + // env or 1password don't partition their inventory by executor scope. | |
| 551 | + const scopePrecedence = new Map<string, number>(); | |
| 552 | + scopeIds.forEach((s, i) => scopePrecedence.set(s, i)); | |
| 553 | + | |
| 547 | 554 | const secretsGet = ( |
| 548 | 555 | id: string, |
| 549 | 556 | ): Effect.Effect<string | null, StorageFailure> => |
| 550 | 557 | Effect.gen(function* () { |
| 551 | − for (const scopeId of scopeIds) { | |
| 552 | − const row = yield* core.findOne({ | |
| 553 | − model: "secret", | |
| 554 | − where: [ | |
| 555 | − { field: "id", value: id }, | |
| 556 | − { field: "scope_id", value: scopeId }, | |
| 557 | − ], | |
| 558 | − }); | |
| 559 | − if (row) { | |
| 560 | − const provider = secretProviders.get(row.provider); | |
| 561 | − if (!provider) continue; | |
| 562 | − const value = yield* provider.get(id, scopeId); | |
| 563 | − if (value !== null) return value; | |
| 564 | − } | |
| 558 | + // The scope-wrapped adapter injects `scope_id IN (scopeIds)` | |
| 559 | + // automatically, so we only filter by id here. | |
| 560 | + const rows = yield* core.findMany({ | |
| 561 | + model: "secret", | |
| 562 | + where: [{ field: "id", value: id }], | |
| 563 | + }); | |
| 564 | + const ordered = [...rows].sort( | |
| 565 | + (a, b) => | |
| 566 | + (scopePrecedence.get(a.scope_id as string) ?? Infinity) - | |
| 567 | + (scopePrecedence.get(b.scope_id as string) ?? Infinity), | |
| 568 | + ); | |
| 569 | + for (const row of ordered) { | |
| 570 | + const provider = secretProviders.get(row.provider as string); | |
| 571 | + if (!provider) continue; | |
| 572 | + const value = yield* provider.get(id, row.scope_id as string); | |
| 573 | + if (value !== null) return value; | |
| 565 | 574 | } |
| 566 | 575 | |
| 567 | 576 | // Fallback: ask every enumerating provider in parallel. First |