Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe pull request adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
coodos
approved these changes
Feb 4, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/docs/Infrastructure/eVault-Key-Delegation.md (1)
140-152:⚠️ Potential issue | 🟡 MinorDocument the nullable
evaultIdcase in the /whois response.The response schema allows
string | null, but the text implies it’s always present. Add an explicit null note to avoid client assumptions.✏️ Suggested doc tweak
-`evaultId` is the eVault instance identifier (when configured); it matches the `evault` value registered with the Registry. +`evaultId` is the eVault instance identifier when configured; otherwise it is `null`. It matches the `evault` value registered with the Registry.
🤖 Fix all issues with AI agents
In `@docs/docs/Infrastructure/eVault.md`:
- Around line 276-297: Update the /whois documentation to explicitly state that
the evaultId field can be null (type "string | null") in responses so clients
handle it safely; modify the JSON example and the bullet describing `evaultId`
to show that it may be null (e.g., "evaultId: string | null" and note "May be
null when not configured") and reference the /whois endpoint and the evaultId
property in the explanatory text so callers implement null checks before using
the value.
In `@infrastructure/evault-core/src/core/http/server.ts`:
- Around line 164-202: Initialize evaultId from the configured value before
calling the Registry (e.g., set evaultId = evault.evaultId or
process.env.EVAULT_ID) so the response contains the configured ID if registry
resolution fails; then keep the existing registry resolution logic that only
overrides evaultId when resolveResponse.data?.evault is present (or skip the
axios call entirely if evaultId is already truthy). Update the block that
defines evaultId, registryUrlForResolve, and the axios resolution to respect
this fallback and only replace the pre-set evaultId on successful resolution.
🧹 Nitpick comments (1)
platforms/emover/src/app/(app)/migrate/page.tsx (1)
82-130: Stop polling after terminal status to reduce redundant requests.Once the status is
completedorfailed, the interval can be cleared to avoid extra traffic.♻️ Suggested change
- const pollStatus = async () => { + let interval: ReturnType<typeof setInterval> | null = null; + const pollStatus = async () => { try { const response = await apiClient.get(`/api/migration/status/${migrationId}`); const data = response.data; @@ - if (data.status === "completed") { + if (data.status === "completed") { setIsActivated(true); setQrData(null); + if (interval) { + clearInterval(interval); + interval = null; + } } else if (data.status === "failed") { setError(data.error || "Migration failed"); + if (interval) { + clearInterval(interval); + interval = null; + } } } } catch (error) { console.error("Error polling migration status:", error); } }; @@ - const interval = setInterval(pollStatus, 2000); + interval = setInterval(pollStatus, 2000); - return () => clearInterval(interval); + return () => interval && clearInterval(interval);
Comment on lines
276
to
+297
| ```json | ||
| { | ||
| "w3id": "@user-a.w3id", | ||
| "evaultId": "@evault-identifier", | ||
| "keyBindingCertificates": [ | ||
| "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9...", | ||
| "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9..." | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| - `w3id`: The W3ID (eName) from the request. | ||
| - `evaultId`: The eVault instance identifier (when configured via `EVAULT_ID`). Matches the `evault` value registered with the Registry. | ||
| - `keyBindingCertificates`: JWTs binding the eName to public keys. | ||
|
|
||
| **Example**: | ||
| ```bash | ||
| curl -X GET http://localhost:4000/whois \ | ||
| -H "X-ENAME: @user-a.w3id" | ||
| ``` | ||
|
|
||
| **Use Case**: Platforms use this endpoint to retrieve public keys for signature verification. | ||
| **Use Case**: Platforms use this endpoint to retrieve public keys for signature verification and the eVault instance id for routing or auditing. |
Contributor
There was a problem hiding this comment.
Clarify evaultId nullability in the /whois docs.
The API returns string | null; make the docs explicit so clients handle null safely.
✏️ Suggested doc tweak
-- `evaultId`: The eVault instance identifier (when configured via `EVAULT_ID`). Matches the `evault` value registered with the Registry.
+- `evaultId`: The eVault instance identifier (when configured via `EVAULT_ID`); otherwise `null`. Matches the `evault` value registered with the Registry.🤖 Prompt for AI Agents
In `@docs/docs/Infrastructure/eVault.md` around lines 276 - 297, Update the /whois
documentation to explicitly state that the evaultId field can be null (type
"string | null") in responses so clients handle it safely; modify the JSON
example and the bullet describing `evaultId` to show that it may be null (e.g.,
"evaultId: string | null" and note "May be null when not configured") and
reference the /whois endpoint and the evaultId property in the explanatory text
so callers implement null checks before using the value.
Comment on lines
+164
to
202
| // Resolve eName via Registry (same logic as /resolve) to get evault id | ||
| let evaultId: string | null = null; | ||
| const registryUrlForResolve = | ||
| process.env.PUBLIC_REGISTRY_URL || process.env.REGISTRY_URL; | ||
| if (registryUrlForResolve) { | ||
| try { | ||
| const resolveResponse = await axios.get<{ | ||
| ename: string; | ||
| uri: string; | ||
| evault: string; | ||
| originalUri?: string; | ||
| resolved?: boolean; | ||
| }>( | ||
| new URL( | ||
| `/resolve?w3id=${encodeURIComponent(eName)}`, | ||
| registryUrlForResolve, | ||
| ).toString(), | ||
| { timeout: 10000 }, | ||
| ); | ||
| if (resolveResponse.data?.evault) { | ||
| evaultId = resolveResponse.data.evault; | ||
| } | ||
| } catch (error) { | ||
| // 404 or network error: evault not registered for this eName, or registry unavailable | ||
| if ( | ||
| axios.isAxiosError(error) && | ||
| error.response?.status !== 404 | ||
| ) { | ||
| console.error( | ||
| "Error resolving eName via Registry for whois evaultId:", | ||
| error.message, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| const result = { | ||
| w3id: eName, | ||
| evaultId, | ||
| keyBindingCertificates: keyBindingCertificates, |
Contributor
There was a problem hiding this comment.
Return configured evaultId even when Registry resolution fails.
evaultId is currently populated only from /resolve. If the Registry is unavailable (or the eName isn’t registered), the response returns null even when EVAULT_ID is configured and passed in. Initialize from evault.evaultId and only override on successful resolution (optionally skip the call when already set).
🛠️ Suggested fix (fallback to configured value)
- let evaultId: string | null = null;
+ let evaultId: string | null =
+ typeof evault?.evaultId === "string" ? evault.evaultId : null;
const registryUrlForResolve =
process.env.PUBLIC_REGISTRY_URL || process.env.REGISTRY_URL;
- if (registryUrlForResolve) {
+ if (registryUrlForResolve && !evaultId) {
try {
const resolveResponse = await axios.get<{
ename: string;
uri: string;
evault: string;🤖 Prompt for AI Agents
In `@infrastructure/evault-core/src/core/http/server.ts` around lines 164 - 202,
Initialize evaultId from the configured value before calling the Registry (e.g.,
set evaultId = evault.evaultId or process.env.EVAULT_ID) so the response
contains the configured ID if registry resolution fails; then keep the existing
registry resolution logic that only overrides evaultId when
resolveResponse.data?.evault is present (or skip the axios call entirely if
evaultId is already truthy). Update the block that defines evaultId,
registryUrlForResolve, and the axios resolution to respect this fallback and
only replace the pre-set evaultId on successful resolution.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
updates emover styling and adds evault id to the whois endpoint of evault core
Issue Number
n/a
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style