Conversation
Replace `version` field with `versionInfo` field.
… the `EnsApiPublicConfig` data model. This change allows tracking the version of `@adraffy/ens-normalize` package used in ENSApi.
Also, enforces the invariat which requires the `@adraffy/ens-normalize` version to match betweenENSApi and ENSIndexer.
Include `ens-normalize.js` info card item
🦋 Changeset detectedLatest commit: d935178 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces top-level Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR enriches the ENSApi version surface by replacing the flat Key changes:
Confidence Score: 5/5Safe to merge — all findings are P2 style/convention issues that do not affect runtime correctness of the happy path. The invariant check, Zod schemas, serializer, and UI changes are all self-consistent and well-tested. The three remaining comments are P2: a missing
Important Files Changed
Sequence DiagramsequenceDiagram
participant FS as Filesystem (node_modules)
participant VI as version-info.ts
participant CS as config.schema.ts
participant VA as validations.ts
participant DB as ENSDb (ENSIndexer config)
participant API as ENSApi HTTP /config
note over VI: Module load (startup)
FS-->>VI: getPackageVersion("@adraffy/ens-normalize")
VI-->>VI: ensApiVersionInfo = { ensApi, ensNormalize }
note over CS: buildConfigFromEnvironment()
DB-->>CS: ensIndexerPublicConfig (includes versionInfo.ensNormalize)
CS->>VA: invariant_ensIndexerPublicConfigVersionInfo()
VA-->>VA: ensApiVersionInfo.ensNormalize === ensIndexerPublicConfig.versionInfo.ensNormalize?
alt version mismatch
VA-->>CS: Zod issue pushed → ZodError thrown
CS-->>CS: process.exit(1)
else versions match
CS-->>API: EnsApiPublicConfig { versionInfo: ensApiVersionInfo, ... }
end
Reviews (2): Last reviewed commit: "Apply AI PR feedback" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR updates ENSApi’s public config model to expose structured version metadata (including @adraffy/ens-normalize) and adds an ENSApi startup invariant to enforce ensNormalize version parity with ENSIndexer, with ENSAdmin UI updates to display the new info.
Changes:
- ENSNode SDK: replace
EnsApiPublicConfig.versionwithEnsApiPublicConfig.versionInfo: EnsApiVersionInfo(and update serialization + tests). - ENSApi: implement
ensApiVersionInfoand validateensNormalizeversion matches the connected ENSIndexer. - ENSAdmin: update connection UI to display ENSApi version via
versionInfoand showens-normalizeversion.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/ensapi/config/zod-schemas.ts | Updates Zod schemas to validate the new versionInfo structure. |
| packages/ensnode-sdk/src/ensapi/config/types.ts | Introduces EnsApiVersionInfo and updates EnsApiPublicConfig shape. |
| packages/ensnode-sdk/src/ensapi/config/serialize.ts | Serializes versionInfo instead of the old version field. |
| packages/ensnode-sdk/src/ensapi/config/conversions.test.ts | Updates config serialization/deserialization tests for versionInfo. |
| packages/ensnode-sdk/src/ensapi/client.test.ts | Updates example config response shape in client tests. |
| apps/ensapi/src/lib/version-info.ts | Adds runtime version discovery for ENSApi + @adraffy/ens-normalize. |
| apps/ensapi/src/config/validations.ts | Enforces ensNormalize version match between ENSApi and ENSIndexer. |
| apps/ensapi/src/config/config.schema.ts | Builds ENSApi public config using ensApiVersionInfo. |
| apps/ensapi/src/config/config.schema.test.ts | Updates tests to expect versionInfo and align ensNormalize values. |
| apps/ensadmin/src/components/connection/cards/ensnode-info.tsx | Updates UI to display ENSApi version + ens-normalize version. |
| .changeset/spotty-pumas-tap.md | Declares a minor bump for @ensnode/ensnode-sdk due to config shape change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensadmin/src/components/connection/cards/ensnode-info.tsx`:
- Around line 339-355: The InfoCardItem displays the ensNormalize version using
ensApiPublicConfig.versionInfo.ensNormalize but constructs the npm link using
ensIndexerPublicConfig.versionInfo.ensNormalize causing an inconsistent source;
update the link to use the same value source
(ensApiPublicConfig.versionInfo.ensNormalize) so both the displayed value and
the ExternalLinkWithIcon href reference the identical symbol (ensure changes
touch the InfoCardItem block, ExternalLinkWithIcon usage, and the ensNormalize
property).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 919f3f89-854c-4143-9528-f92f9eb81822
📒 Files selected for processing (12)
.changeset/spotty-pumas-tap.mdapps/ensadmin/src/components/connection/cards/ensnode-info.tsxapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.schema.tsapps/ensapi/src/config/validations.tsapps/ensapi/src/lib/version-info.tsdocs/docs.ensnode.io/ensapi-openapi.jsonpackages/ensnode-sdk/src/ensapi/client.test.tspackages/ensnode-sdk/src/ensapi/config/conversions.test.tspackages/ensnode-sdk/src/ensapi/config/serialize.tspackages/ensnode-sdk/src/ensapi/config/types.tspackages/ensnode-sdk/src/ensapi/config/zod-schemas.ts
b3bdb63 to
34a0ad3
Compare
34a0ad3 to
3e6b901
Compare
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/lib/version-info.ts
Outdated
| } catch (error) { | ||
| const errorMessage = error instanceof Error ? error.message : "Unknown error"; | ||
|
|
||
| console.warn(`Could not find version for ${packageName}:`, errorMessage); | ||
|
|
||
| return "unknown"; |
There was a problem hiding this comment.
This uses console.warn for logging inside the service runtime. Elsewhere in ENSApi, structured logging goes through the app logger; using console.warn can bypass log formatting/levels and make production logs inconsistent. Consider switching this to the shared logger (or a makeLogger("version-info")) and logging the package name + error as structured fields.
3e6b901 to
473fddc
Compare
473fddc to
aa4eaae
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/lib/version-info.ts`:
- Around line 66-71: The catch currently swallows resolution failures and
returns "unknown", which causes downstream validation to misreport dependency
mismatches; in the catch block in version-info.ts (the one catching errors while
resolving packageName) replace the return "unknown" with throwing the error (or
throw a new Error that includes context and errorMessage) so startup surfaces
the real failure instead of downgrading to "unknown".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd8de25f-3082-415b-a80d-b0a6a9e9b2d0
📒 Files selected for processing (4)
apps/ensadmin/src/components/connection/cards/ensnode-info.tsxapps/ensapi/src/config/validations.tsapps/ensapi/src/lib/version-info.tspackages/ensnode-sdk/src/ensapi/config/zod-schemas.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/ensapi/src/lib/version-info.ts (1)
83-83: 🧹 Nitpick | 🔵 Trivial
String.replace()without global flag only replaces the first occurrence.For
@adraffy/ens-normalizethis works since there's only one/, but this would silently fail for hypothetical packages with multiple slashes in their path (e.g.,@scope/foo/bar).♻️ Suggested fix using replaceAll or regex with global flag
- const normalizedName = packageName.replace("/", "+"); + const normalizedName = packageName.replaceAll("/", "+");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/lib/version-info.ts` at line 83, The current normalization uses packageName.replace("/", "+") which only replaces the first slash; update the normalization in version-info.ts (the normalizedName assignment) to replace all "/" characters (e.g., use packageName.replaceAll("/") or packageName.replace(/\//g, "+")) so multi-slash package names like "@scope/foo/bar" are fully normalized; ensure you pick the method compatible with the runtime target.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/ensapi/src/lib/version-info.ts`:
- Line 83: The current normalization uses packageName.replace("/", "+") which
only replaces the first slash; update the normalization in version-info.ts (the
normalizedName assignment) to replace all "/" characters (e.g., use
packageName.replaceAll("/") or packageName.replace(/\//g, "+")) so multi-slash
package names like "@scope/foo/bar" are fully normalized; ensure you pick the
method compatible with the runtime target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b60c4c95-99f7-445a-99ce-1297b662a215
📒 Files selected for processing (4)
apps/ensadmin/src/components/connection/cards/ensnode-info.tsxapps/ensapi/src/config/validations.tsapps/ensapi/src/lib/version-info.tspackages/ensnode-sdk/src/ensapi/config/zod-schemas.ts
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Looks nice 😄 Please take the lead to merge when ready 👍
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
version: stringfield onEnsApiPublicConfigdata model withversionInfo: EnsApiVersionInfo.ensNormalizeversion info.versionInfofield, including enforcing the required invariant which checks ifensNormalizeversion matches between ENSApi and ENSIndexer.ensNormalizeversion.Why
Testing
ensNormalizeversion and observed how ENSApi refuses to start due to version mismatch.Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)