-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/add x ename header support #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds E2E test infrastructure and workflows, introduces E2E tests and helpers, adjusts GraphQL/token/access behaviors, and removes large portions of the web3-adapter runtime and declaration surfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as E2E Test
participant Setup as setupE2ETestServer
participant Containers as Test Containers
participant Registry as Mock Registry
participant EVault as evault-core GraphQL
participant Client as web3-adapter EVaultClient
Test->>Setup: setupE2ETestServer()
Setup->>Containers: start Neo4j/Postgres
Setup->>Registry: start mock registry (/register,/resolve,/platforms/certification)
Setup->>EVault: start Fastify + GraphQL
Setup-->>Test: return E2ETestServer
Test->>Client: dynamic import / instantiate EVaultClient
Client->>EVault: POST /graphql (Authorization, X-ENAME)
EVault->>EVault: vault-access-guard -> checkAccess() returns {hasAccess, exists}
EVault->>Containers: DB queries
Containers-->>EVault: results
EVault-->>Client: response (metaEnvelope includes parsed)
Client-->>Test: assertions
Test->>Setup: teardownE2ETestServer()
Setup->>Containers: stop containers
Setup->>Registry: stop registry
Setup-->>Test: done
sequenceDiagram
participant Adapter as EVaultClient
participant Headers as Headers
participant Server as evault-core GraphQL
Adapter->>Headers: set Authorization: Bearer <token>
Adapter->>Headers: set X-ENAME: <w3id>
Adapter->>Server: POST /graphql (with headers)
Server->>Server: evaluate access (hasAccess, exists)
Server-->>Adapter: tenant-scoped response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (2)
49-67: EventSource connection is never cleaned up, risking a resource leak.The EventSource created on line 51 has no close() call or error handler. If this component unmounts, the connection persists and continues attempting to reconnect, consuming resources indefinitely. Additionally, there is no onerror handler to manage connection failures gracefully.
Apply this diff to add cleanup and error handling:
function watchEventStream(id: string) { const sseUrl = new URL(`/api/auth/sessions/${id}`, PUBLIC_PICTIQUE_BASE_URL).toString(); const eventSource = new EventSource(sseUrl); eventSource.onopen = () => { console.log('Successfully connected.'); }; eventSource.onerror = (error) => { console.error('EventSource connection failed:', error); eventSource.close(); }; eventSource.onmessage = (e) => { const data = JSON.parse(e.data as string); const { user } = data; setAuthId(user.id); const { token } = data; setAuthToken(token); eventSource.close(); goto('/home'); }; } watchEventStream(new URL(qrData).searchParams.get('session') as string); onDestroy(() => { window.removeEventListener('resize', checkMobile); });Also ensure the EventSource is stored and closed in the destroy hook to prevent leaks if navigation occurs unexpectedly.
46-46: Add error handling for the auth offer API call.The async call to
/api/auth/offerlacks error handling. If this call fails, the component will be left in an invalid state with no qrData, yet the UI will still attempt to render login options.Wrap the call in a try-catch block to handle errors gracefully and provide user feedback.
infrastructure/web3-adapter/src/__tests__/evault.test.ts (1)
23-31: Missing X-ENAME header breaks every GraphQL callThe server-side resolvers now throw unless the
X-ENAMEheader is present, so everyqueryGraphQLcall here will reject with “X-ENAME header is required.” Please propagate the tenant eName when posting the request (e.g. via the W3ID you already configure for the test environment) so the tests keep exercising the real flow.async function queryGraphQL( query: string, variables: Record<string, unknown> = {}, ) { - const response = await fetch(EVaultEndpoint, { + const eName = process.env.W3ID; + if (!eName) { + throw new Error("W3ID must be set for eVault integration tests"); + } + + const response = await fetch(EVaultEndpoint, { method: "POST", headers: { "Content-Type": "application/json", + "X-ENAME": eName, }, body: JSON.stringify({ query, variables }), });
🧹 Nitpick comments (1)
infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts (1)
44-60: Avoid cross-test dependencies on provisioning state.Later suites rely on
evault1/evault2being initialized; if either provisioning test fails or is skipped, everything else cascades. Hoist the provisioning intobeforeAlland keep these tests asserting invariants so fixtures always exist regardless of execution order.@@ - it("should provision an eVault instance", async () => { - evault1 = await provisionTestEVault(server); - - expect(evault1).toBeDefined(); - expect(evault1.w3id).toBeDefined(); - expect(evault1.uri).toBeDefined(); - expect(evault1.uri).toContain("http://"); - }); - - it("should provision multiple eVault instances with different eNames", async () => { - evault2 = await provisionTestEVault(server); - - expect(evault2).toBeDefined(); - expect(evault2.w3id).toBeDefined(); - expect(evault2.w3id).not.toBe(evault1.w3id); - }); + beforeAll(async () => { + evault1 = await provisionTestEVault(server); + evault2 = await provisionTestEVault(server); + }); + + it("should provision an eVault instance", async () => { + expect(evault1).toBeDefined(); + expect(evault1.w3id).toBeDefined(); + expect(evault1.uri).toBeDefined(); + expect(evault1.uri).toContain("http://"); + }); + + it("should provision multiple eVault instances with different eNames", async () => { + expect(evault2).toBeDefined(); + expect(evault2.w3id).toBeDefined(); + expect(evault2.w3id).not.toBe(evault1.w3id); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/tests-evault-core-e2e.yml(1 hunks).github/workflows/tests-evault-core.yml(0 hunks).github/workflows/tests-registry.yml(0 hunks)infrastructure/evault-core/package.json(1 hunks)infrastructure/evault-core/src/core/protocol/graphql-server.ts(2 hunks)infrastructure/evault-core/src/core/protocol/vault-access-guard.ts(1 hunks)infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts(1 hunks)infrastructure/evault-core/src/test-utils/e2e-setup.ts(1 hunks)infrastructure/evault-core/src/test-utils/mock-registry-server.ts(2 hunks)infrastructure/evault-core/src/test-utils/shared-test-keys.ts(1 hunks)infrastructure/web3-adapter/src/__tests__/evault.test.ts(4 hunks)infrastructure/web3-adapter/src/evault/evault.ts(1 hunks)platforms/pictique/src/routes/(auth)/auth/+page.svelte(1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/tests-registry.yml
- .github/workflows/tests-evault-core.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/web3-adapter/src/__tests__/evault.test.tsinfrastructure/evault-core/src/e2e/evault-core.e2e.spec.tsinfrastructure/evault-core/src/test-utils/mock-registry-server.ts
🧬 Code graph analysis (5)
infrastructure/web3-adapter/src/__tests__/evault.test.ts (1)
infrastructure/web3-adapter/src/adapter.ts (1)
Web3Adapter(7-59)
infrastructure/web3-adapter/src/evault/evault.ts (1)
infrastructure/web3-adapter/src/index.js (2)
client(200-200)endpoint(196-196)
infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts (2)
infrastructure/evault-core/src/test-utils/e2e-setup.ts (6)
E2ETestServer(24-35)ProvisionedEVault(37-40)setupE2ETestServer(50-155)teardownE2ETestServer(160-187)provisionTestEVault(192-224)makeGraphQLRequest(229-256)infrastructure/web3-adapter/src/index.js (1)
client(200-200)
infrastructure/evault-core/src/test-utils/mock-registry-server.ts (1)
infrastructure/evault-core/src/test-utils/shared-test-keys.ts (1)
getSharedTestPublicJWK(28-33)
infrastructure/evault-core/src/test-utils/e2e-setup.ts (7)
infrastructure/evault-core/src/core/db/db.service.ts (1)
DbService(18-599)infrastructure/evault-core/src/test-utils/neo4j-setup.ts (2)
setupTestNeo4j(7-22)teardownTestNeo4j(24-33)infrastructure/evault-core/src/core/db/retry-neo4j.ts (1)
connectWithRetry(13-40)infrastructure/evault-core/src/core/db/migrations/add-ename-index.ts (1)
createENameIndex(15-33)infrastructure/evault-core/src/core/protocol/graphql-server.ts (1)
GraphQLServer(12-358)infrastructure/evault-core/src/core/http/server.ts (1)
registerHttpRoutes(17-301)infrastructure/evault-core/src/test-utils/shared-test-keys.ts (1)
getSharedTestKeyPair(10-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: test-web3-adapter-integration
🔇 Additional comments (1)
platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
27-31: Formatting improvement maintains functional equivalence.The userAgent assignment has been reformatted across multiple lines for improved readability. The logic remains unchanged—evaluating navigator.userAgent, navigator.vendor, window.opera, and falling back to an empty string.
ef632f4 to
c5e9e8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
infrastructure/evault-core/src/test-utils/e2e-setup.ts (1)
165-180: Close Neo4j resources during teardown.The Neo4j driver created by
connectWithRetry(line 89) and used byDbServiceis never closed during teardown. This leaves open sockets and can cause the E2E suite to hang.Apply this diff to close the driver properly:
await stopMockRegistryServer(server.registryServer); } + // Close Neo4j driver and database service + try { + await server.dbService.close(); + } catch (error) { + console.error("Error closing DB service:", error); + } + + try { + if (server.neo4jDriver) { + await server.neo4jDriver.close(); + } + } catch (error) { + console.error("Error closing Neo4j driver:", error); + } + if (server.testDataSource?.isInitialized) { await server.testDataSource.destroy(); }
🧹 Nitpick comments (5)
infrastructure/web3-adapter/.gitignore (1)
1-37: Well-structured .gitignore following TypeScript/Node.js best practices.The file covers all standard exclusions: dependencies, build artifacts, IDE settings, OS files, logs, test coverage, and environment secrets. The organization with section comments is clear and maintainable.
Consider adding
.tsbuildinfo(Line 37+) if your TypeScript configuration uses incremental builds. This is a minor optional addition:# Environment .env .env.local .env.*.local + +# TypeScript incremental build cache +.tsbuildinfoVerify that this file aligns with your project's
tsconfig.json(particularly theincrementalflag andoutDirsettings) and confirm whether package lock files (package-lock.json,yarn.lock) should be committed or ignored per your project conventions.infrastructure/evault-core/vitest.config.ts (1)
6-17: Exclude E2E suites from the unit-test projectRight now the base Vitest config still discovers files under
**/e2e/**. That means runningvitest(or using IDE integrations that rely solely on the config) will spin up the new E2E suites—even when we intended to keep them behind the dedicated E2E workflow—leading to long, flaky runs unless the web3-adapter build and containers happen to be ready. Please add**/e2e/**to thistest.excludelist so the exclusion is baked into the shared config rather than depending on every caller to remember the CLI flag.- exclude: ['**/node_modules/**', '**/dist/**'], + exclude: ['**/node_modules/**', '**/dist/**', '**/e2e/**'],infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts (1)
48-61: Lift shared provisioning intobeforeAllThis
itboth provisions the tenants and asserts on them, but every later suite depends on the side effects (evault1/evault2). If this test ever fails or is skipped, the rest of the file crashes with undefined tenants, making the root cause harder to spot. Please move the provisioning logic into abeforeAll(and keep a lightweight assertion-only test if you still want coverage) so the shared fixtures are always prepared before the dependent suites run.- describe("Setup", () => { - it("should provision eVault instances for testing", async () => { - evault1 = await provisionTestEVault(server); - … - }); - }); + beforeAll(async () => { + evault1 = await provisionTestEVault(server); + evault2 = await provisionTestEVault(server); + }); + + describe("Setup", () => { + it("should provision eVault instances for testing", () => { + expect(evault1).toBeDefined(); + … + }); + });infrastructure/web3-adapter/src/evault/evault.ts (1)
230-233: Consider extracting logging logic into a utility function.The conditional logging pattern
if (!process.env.CI && !process.env.VITEST)is repeated throughout the file. While functional, extracting this into a logging utility would improve maintainability and consistency.Example:
private log(message: string, ...args: any[]): void { if (!process.env.CI && !process.env.VITEST) { console.error(message, ...args); } }Then use:
this.log("Error requesting platform token:", error);Also applies to: 274-277, 369-369, 549-552, 579-583, 611-613
infrastructure/evault-core/src/core/protocol/vault-access-guard.ts (1)
154-163: Add guard for empty array case.The check
result[0] && !('acl' in result[0])handles the empty array case, but it could be clearer. Ifresult.length === 0, the code should return early to avoid confusion.Apply this diff to make the logic more explicit:
// If the result is an array if (Array.isArray(result)) { + // Empty array - return as-is + if (result.length === 0) { + return result; + } // Check if it's an array of Envelopes (no ACL) or MetaEnvelopes (has ACL) if (result.length > 0 && result[0] && !('acl' in result[0])) { // It's an array of Envelopes - already filtered by eName, just return as-is return result; } // It's an array of MetaEnvelopes - filter based on access return this.filterEnvelopesByAccess(result, context); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (18)
infrastructure/web3-adapter/src/db/index.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/db/index.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/db/mapping.db.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/db/mapping.db.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/evault/evault.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/evault/evault.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/index.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/index.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/index.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/index.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/logger.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/logger.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/transport.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/logging/transport.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/mapper/mapper.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/mapper/mapper.js.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/mapper/mapper.types.d.ts.mapis excluded by!**/*.mapinfrastructure/web3-adapter/src/mapper/mapper.types.js.mapis excluded by!**/*.map
📒 Files selected for processing (28)
infrastructure/evault-core/package.json(1 hunks)infrastructure/evault-core/src/core/protocol/graphql-server.ts(3 hunks)infrastructure/evault-core/src/core/protocol/vault-access-guard.spec.ts(9 hunks)infrastructure/evault-core/src/core/protocol/vault-access-guard.ts(4 hunks)infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts(1 hunks)infrastructure/evault-core/src/test-utils/e2e-setup.ts(1 hunks)infrastructure/evault-core/vitest.config.e2e.ts(1 hunks)infrastructure/evault-core/vitest.config.ts(2 hunks)infrastructure/web3-adapter/.gitignore(1 hunks)infrastructure/web3-adapter/src/db/index.d.ts(0 hunks)infrastructure/web3-adapter/src/db/index.js(0 hunks)infrastructure/web3-adapter/src/db/mapping.db.d.ts(0 hunks)infrastructure/web3-adapter/src/db/mapping.db.js(0 hunks)infrastructure/web3-adapter/src/evault/evault.d.ts(0 hunks)infrastructure/web3-adapter/src/evault/evault.js(0 hunks)infrastructure/web3-adapter/src/evault/evault.ts(12 hunks)infrastructure/web3-adapter/src/index.d.ts(0 hunks)infrastructure/web3-adapter/src/index.js(0 hunks)infrastructure/web3-adapter/src/logging/index.d.ts(0 hunks)infrastructure/web3-adapter/src/logging/index.js(0 hunks)infrastructure/web3-adapter/src/logging/logger.d.ts(0 hunks)infrastructure/web3-adapter/src/logging/logger.js(0 hunks)infrastructure/web3-adapter/src/logging/transport.d.ts(0 hunks)infrastructure/web3-adapter/src/logging/transport.js(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.d.ts(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.js(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.types.d.ts(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.types.js(0 hunks)
💤 Files with no reviewable changes (18)
- infrastructure/web3-adapter/src/logging/transport.d.ts
- infrastructure/web3-adapter/src/logging/index.js
- infrastructure/web3-adapter/src/evault/evault.js
- infrastructure/web3-adapter/src/mapper/mapper.types.js
- infrastructure/web3-adapter/src/mapper/mapper.d.ts
- infrastructure/web3-adapter/src/logging/transport.js
- infrastructure/web3-adapter/src/mapper/mapper.types.d.ts
- infrastructure/web3-adapter/src/logging/logger.d.ts
- infrastructure/web3-adapter/src/logging/index.d.ts
- infrastructure/web3-adapter/src/db/mapping.db.d.ts
- infrastructure/web3-adapter/src/index.d.ts
- infrastructure/web3-adapter/src/mapper/mapper.js
- infrastructure/web3-adapter/src/index.js
- infrastructure/web3-adapter/src/db/index.d.ts
- infrastructure/web3-adapter/src/evault/evault.d.ts
- infrastructure/web3-adapter/src/logging/logger.js
- infrastructure/web3-adapter/src/db/mapping.db.js
- infrastructure/web3-adapter/src/db/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/evault-core/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/evault-core/src/core/protocol/vault-access-guard.spec.tsinfrastructure/evault-core/src/e2e/evault-core.e2e.spec.tsinfrastructure/web3-adapter/src/evault/evault.ts
🧬 Code graph analysis (4)
infrastructure/evault-core/src/test-utils/e2e-setup.ts (7)
infrastructure/evault-core/src/core/db/db.service.ts (1)
DbService(18-599)infrastructure/evault-core/src/test-utils/neo4j-setup.ts (2)
setupTestNeo4j(7-22)teardownTestNeo4j(24-33)infrastructure/evault-core/src/core/db/retry-neo4j.ts (1)
connectWithRetry(13-40)infrastructure/evault-core/src/core/db/migrations/add-ename-index.ts (1)
createENameIndex(15-33)infrastructure/evault-core/src/core/protocol/graphql-server.ts (1)
GraphQLServer(12-366)infrastructure/evault-core/src/core/http/server.ts (1)
registerHttpRoutes(17-301)infrastructure/evault-core/src/test-utils/shared-test-keys.ts (1)
getSharedTestKeyPair(10-23)
infrastructure/evault-core/src/core/protocol/graphql-server.ts (1)
infrastructure/w3id/src/utils/jwt.ts (1)
getJWTHeader(86-89)
infrastructure/evault-core/src/core/protocol/vault-access-guard.spec.ts (1)
infrastructure/evault-core/src/core/protocol/vault-access-guard.ts (1)
checkAccess(59-106)
infrastructure/evault-core/src/e2e/evault-core.e2e.spec.ts (1)
infrastructure/evault-core/src/test-utils/e2e-setup.ts (5)
E2ETestServer(24-35)ProvisionedEVault(37-40)setupE2ETestServer(50-155)teardownE2ETestServer(160-193)provisionTestEVault(198-230)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test-web3-adapter-integration
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (14)
infrastructure/web3-adapter/src/evault/evault.ts (6)
17-18: LGTM!Reducing retries and delays in CI/test environments improves test execution speed without affecting production behavior.
38-45: LGTM!The query correctly maps to the GraphQL server's
getMetaEnvelopeByIdfield and retrieves the required fields including theparsedfield.
65-71: LGTM!The interface correctly matches the GraphQL response structure with proper typing for the
parsedfield.
304-311: LGTM!The X-ENAME header is correctly added to the GraphQL client configuration, enabling the new architecture mentioned in the PR objectives.
501-528: LGTM!The enhanced error handling and validation significantly improve the robustness of the
storeMetaEnvelopemethod by providing contextual error messages and validating the response before returning.
568-577: LGTM!The response validation and mapping logic correctly transforms the GraphQL response to the MetaEnvelope interface, with appropriate null handling for the
parsedfield.infrastructure/evault-core/src/core/protocol/vault-access-guard.ts (4)
57-62: LGTM!The enhanced return type distinguishes between envelope existence and access permission, enabling more precise error handling downstream. This is a valuable improvement to the API design.
73-78: LGTM!The logic correctly handles the case where a valid platform token grants access but the envelope existence cannot be determined without the eName header.
86-96: LGTM!The ACL fallback logic correctly preserves existing behavior while adding existence information to the response.
176-194: LGTM!The middleware correctly distinguishes between "not found" (returns null) and "access denied" (throws error), providing clear semantics for API consumers. The null result handling is also appropriately defensive.
infrastructure/evault-core/src/test-utils/e2e-setup.ts (4)
24-40: LGTM!The interfaces are well-structured and capture all necessary resources for E2E test orchestration.
50-155: LGTM!The setup function comprehensively orchestrates all necessary test infrastructure, including testcontainers, services, and servers. The structure is clean and well-organized.
198-230: LGTM!The provisioning helper correctly generates signed entropy tokens and validates the provisioning result before returning.
235-262: LGTM!The GraphQL request helper is simple and correct, with appropriate error handling for GraphQL-level errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of "as any" usages in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few logs disappeared here without any CI or VITEST guard, is that intentional?
sosweetham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd recommend we atleast shift to using unknown instead of any, and add back the rule to error for anys
xPathin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but consider my 2 file comments.
Description of change
use
X-ENAMEheader to adapt with new architectureIssue Number
closes #397
Type of change
How the change has been tested
Unit & E2E tests
Change checklist
Summary by CodeRabbit
Tests
Bug Fixes
Chores
Breaking Changes