Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRenames the PostgreSQL service from Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Greptile SummaryThis PR completes a Docker stack cleanup by renaming the Confidence Score: 5/5Safe to merge — all changes are consistent renames, environment restructuring, and a well-scoped version-sync script with no logic errors. Both previously flagged issues (stateful global regex and committed ENSNODE_TAG override) have been addressed. The rename from postgres→ensdb is applied uniformly across all 18 files. The sync script correctly uses separate non-global test and global replace patterns, validates SemVer input, and is only called after changeset version has updated package.json files. No P0 or P1 issues found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR merged to main] --> B[changesets/action triggered]
B --> C["pnpm changeset:version\n(version command)"]
C --> C1[changeset version\nupdates package.json files]
C1 --> C2["pnpm release:postversion"]
C2 --> C3["pnpm docker:version:sync\nreads ensapi version\nvia version:current script"]
C3 --> C4["sync-docker-services-tags.mjs\nupdates ENSNODE_VERSION:-x.y.z\nin all service YAML files"]
C2 --> C5[pnpm generate:openapi]
C4 --> D[changesets/action commits\nversion bump + updated service files\nto Release PR]
D --> E{Release PR merged?}
E -- Yes --> F["pnpm changeset:publish\n(publish command)"]
F --> G[NPM packages published]
F --> H[Docker images built\nand pushed to GHCR]
H --> I["Images tagged with\nENSNODE_VERSION from\nservice file defaults"]
style C4 fill:#d4edda,stroke:#28a745
style D fill:#d4edda,stroke:#28a745
Reviews (6): Last reviewed commit: "fix pr comments" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/envs/.env.docker.common`:
- Line 4: The shared env file currently hardcodes ENSNODE_TAG=1.9.0 which
overrides service fallback defaults; either remove or comment out the
ENSNODE_TAG line in docker/envs/.env.docker.common so services use their
${ENSNODE_TAG:-...} fallbacks, or update the sync script
(scripts/sync-docker-services-tags.mjs) and CI process to also update this file
on releases; pick one approach and apply it consistently so ENSNODE_TAG is no
longer silently desynced across services.
In `@docker/services/ensadmin.yml`:
- Line 4: The Docker service image fallback is pinned one version behind; update
the default tag used in the image declaration (the `${ENSNODE_TAG:-1.10.1}`
fallback) to the current repo version by replacing `1.10.1` with `1.10.2` so
that when `ENSNODE_TAG` is unset the service pulls the latest intended image;
ensure the change is applied to the `image:
ghcr.io/namehash/ensnode/ensadmin:${ENSNODE_TAG:-1.10.1}` line to become
`${ENSNODE_TAG:-1.10.2}`.
In `@scripts/sync-docker-services-tags.mjs`:
- Line 13: The semverRegex constant currently only allows either a prerelease or
build metadata but not both; update the pattern used by semverRegex (and any
duplicate occurrences around the block referenced) to accept an optional
prerelease segment followed optionally by a build metadata segment in the
correct order (core version \\.x\\.x, optional prerelease prefixed by '-', then
optional build metadata prefixed by '+'), and ensure character classes allow
dot-separated identifiers per SemVer (alphanumerics and hyphens for prerelease,
alphanumerics and hyphens for build identifiers) so strings like
"1.2.3-rc.1+build.7" match.
- Around line 36-45: The global regex `pattern` (const pattern =
/\$\{ENSNODE_TAG:-[^}]+\}/g) shares lastIndex across iterations and causes false
negatives when using pattern.test() in the loop over `serviceFiles`; fix by
removing the global flag or recreating/resetting the regex before each use
(e.g., use const pattern = /\$\{ENSNODE_TAG:-[^}]+\}/ without /g, or set
pattern.lastIndex = 0 before calling pattern.test(content)) so the check in the
loop that throws the Error when the ENSNODE_TAG default expression is missing
behaves correctly.
🪄 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: 6fe7ad89-c0e5-4fe4-813a-09d7535cfc7b
📒 Files selected for processing (17)
.github/workflows/release.yml.github/workflows/release_snapshot.ymlapps/ensapi/package.jsondocker/README.mddocker/docker-compose.devnet.ymldocker/docker-compose.orchestrator.ymldocker/docker-compose.ymldocker/envs/.env.docker.commondocker/envs/.env.docker.devnetdocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensdb.ymldocker/services/ensindexer.ymldocker/services/ensrainbow.ymldocs/ensnode.io/src/content/docs/docs/deploying/docker.mdxpackage.jsonscripts/sync-docker-services-tags.mjs
sevenzing
changed the title
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/envs/.env.docker.devnet`:
- Line 19: Remove the extra blank line that appears immediately before the TODO
block (the `TODO` comment) so the TODO line directly follows the previous entry;
this will clear the dotenv-linter ExtraBlankLine warning by ensuring no empty
line exists between the previous env variable and the TODO comment.
In `@package.json`:
- Around line 15-19: The npm script "changeset:version" no longer runs artifact
generation/sync and can leave OpenAPI and Docker tag defaults stale; update the
"changeset:version" script so it runs the post-version steps (either by
appending "&& pnpm docker:tag:sync && pnpm generate:openapi" or by invoking the
existing "release:postversion" script) so that running pnpm changeset:version
produces atomic, up-to-date release artifacts; ensure you reference the exact
script names "changeset:version", "release:postversion", "generate:openapi", and
"docker:tag:sync" in the change.
🪄 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: 4cdcae72-35f7-49ed-bf8c-c04591fc1f27
📒 Files selected for processing (17)
.github/workflows/release.yml.github/workflows/release_snapshot.ymldocker/README.mddocker/docker-compose.devnet.ymldocker/docker-compose.orchestrator.ymldocker/docker-compose.ymldocker/envs/.env.docker.commondocker/envs/.env.docker.devnetdocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensdb.ymldocker/services/ensindexer.ymldocker/services/ensrainbow.ymldocs/ensnode.io/src/content/docs/docs/deploying/docker.mdxpackage.jsonpackages/integration-test-env/src/orchestrator.tsscripts/sync-docker-services-tags.mjs
|
|
||
| async function main() { | ||
| const versionFromArg = process.argv[2]; | ||
| const version = versionFromArg ?? (await getVersionFromEnsapi()); |
There was a problem hiding this comment.
Why don't we make this whole file simpler by only accepting the version in the argument and then as part of the pnpm script we can pass it like so?
cat ./apps/ensapi/package.json | jq '.version' | node scripts/sync-docker-services-tags.mjs
also switch this file to .ts, ensure there's a tsconfig extending the base config in this package, and execute with tsx. then introduce a script in this package called sync-docker-services-version or docker:version:sync and call that script from the monorepo package.json instead of leaking internal details about this package at the monorepo root
There was a problem hiding this comment.
hm, we might do that but I'm almost sure that jq isn't installed by default in workflow container, but I see your direction, let me see what I can do here
There was a problem hiding this comment.
decided to do it this way:
root package.json:
"docker:tag:sync": "node ./scripts/sync-docker-services-tags.mjs \"$(pnpm -F ensapi -s version:current)\"",ensapi package.json:
"version:current": "node -p \"require('./package.json').version\""
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Looks nice! Shared a few suggestions please feel welcome to take the lead to merge when you feel good 👍
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/envs/.env.docker.devnet`:
- Around line 4-18: The env file entries are out of order and failing
dotenv-linter; reorder the keys (PLUGINS, ENSINDEXER_SCHEMA_NAME, RPC_URL_1,
RPC_URL_15658733, LABEL_SET_ID, LABEL_SET_VERSION, DB_SCHEMA_VERSION) into the
project’s expected sorted order so dotenv-linter passes, keeping the existing
comments with their related keys and preserving values for keys like PLUGINS,
ENSINDEXER_SCHEMA_NAME, RPC_URL_1, RPC_URL_15658733, LABEL_SET_VERSION,
LABEL_SET_ID, and DB_SCHEMA_VERSION.
In `@scripts/sync-docker-services-tags.mjs`:
- Around line 36-48: The loop in scripts/sync-docker-services-tags.mjs writes
each file as it iterates which can leave partial updates if a later file fails;
change to a two-pass approach: first iterate serviceFiles and for each
absolutePath (using resolve(rootDir, relativePath)) readFile and validate
testPattern.match exists and compute the updated content (using
replacePattern/replacement) while collecting {absolutePath, previous, updated};
if any validation or I/O error occurs, throw before making any changes; only
after the full pass succeeds, iterate the collected updates and perform
writeFile for each and console.log the `"Updated ..."` messages.
🪄 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: 66c9f1fd-6197-4f62-8434-039049c8152d
📒 Files selected for processing (10)
.github/workflows/release.ymlapps/ensapi/package.jsondocker/docker-compose.ymldocker/envs/.env.docker.devnetdocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensindexer.ymldocker/services/ensrainbow.ymlpackage.jsonscripts/sync-docker-services-tags.mjs
| # ENSIndexer | ||
| PLUGINS=subgraph,ensv2 | ||
| # ENSIndexer and ENSApi | ||
| ENSINDEXER_SCHEMA_NAME=docker_devnet_v1 | ||
| # ENSIndexer and ENSApi | ||
| RPC_URL_1=http://devnet:8545 | ||
| # ENSIndexer and ENSRainbow | ||
| LABEL_SET_VERSION=0 | ||
| # ENSIndexer and ENSRainbow | ||
| LABEL_SET_ID=ens-test-env | ||
| # ENSRainbow | ||
| DB_SCHEMA_VERSION=3 | ||
| # TODO: in future we might migrate devnet to different chain_id like 1337 or 31337 | ||
| # need to update this `RPC_URL_15658733` in that case | ||
| RPC_URL_15658733=http://devnet:8545 |
There was a problem hiding this comment.
Reorder these env keys before merging.
dotenv-linter is already flagging the new ordering here, so this file will keep failing lint until the entries are sorted into the project’s expected order.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 7-7: [UnorderedKey] The ENSINDEXER_SCHEMA_NAME key should go before the PLUGINS key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The LABEL_SET_VERSION key should go before the PLUGINS key
(UnorderedKey)
[warning] 13-13: [UnorderedKey] The LABEL_SET_ID key should go before the LABEL_SET_VERSION key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The DB_SCHEMA_VERSION key should go before the ENSINDEXER_SCHEMA_NAME key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/envs/.env.docker.devnet` around lines 4 - 18, The env file entries are
out of order and failing dotenv-linter; reorder the keys (PLUGINS,
ENSINDEXER_SCHEMA_NAME, RPC_URL_1, RPC_URL_15658733, LABEL_SET_ID,
LABEL_SET_VERSION, DB_SCHEMA_VERSION) into the project’s expected sorted order
so dotenv-linter passes, keeping the existing comments with their related keys
and preserving values for keys like PLUGINS, ENSINDEXER_SCHEMA_NAME, RPC_URL_1,
RPC_URL_15658733, LABEL_SET_VERSION, LABEL_SET_ID, and DB_SCHEMA_VERSION.
| for (const relativePath of serviceFiles) { | ||
| const absolutePath = resolve(rootDir, relativePath); | ||
| const content = await readFile(absolutePath, "utf8"); | ||
|
|
||
| if (!testPattern.test(content)) { | ||
| throw new Error(`Could not find ENSNODE_VERSION default expression in ${relativePath}`); | ||
| } | ||
|
|
||
| const previous = content.match(testPattern)[0]; | ||
| const updated = content.replace(replacePattern, replacement); | ||
| await writeFile(absolutePath, updated, "utf8"); | ||
| console.log(`Updated ${relativePath}:\t"${previous}" -> "${replacement}"`); | ||
| } |
There was a problem hiding this comment.
Avoid partial updates if a later file fails.
Each file is written immediately inside the loop, so a missing placeholder or I/O error on a later service file can leave earlier files already modified. For release automation, validate all targets first and only write after the full pass succeeds.
🔧 Suggested two-pass update
async function updateServiceDefaultVersion(version) {
const testPattern = /\$\{ENSNODE_VERSION:-[^}]+\}/;
const replacePattern = /\$\{ENSNODE_VERSION:-[^}]+\}/g;
const replacement = `\${ENSNODE_VERSION:-${version}}`;
- console.log(`Updating service default tag to ${version}\n`);
-
+ const updates = [];
for (const relativePath of serviceFiles) {
const absolutePath = resolve(rootDir, relativePath);
const content = await readFile(absolutePath, "utf8");
if (!testPattern.test(content)) {
throw new Error(`Could not find ENSNODE_VERSION default expression in ${relativePath}`);
}
const previous = content.match(testPattern)[0];
- const updated = content.replace(replacePattern, replacement);
- await writeFile(absolutePath, updated, "utf8");
- console.log(`Updated ${relativePath}:\t"${previous}" -> "${replacement}"`);
+ updates.push({
+ absolutePath,
+ relativePath,
+ previous,
+ updated: content.replace(replacePattern, replacement),
+ });
+ }
+
+ console.log(`Updating service default tag to ${version}\n`);
+ for (const { absolutePath, relativePath, previous, updated } of updates) {
+ await writeFile(absolutePath, updated, "utf8");
+ console.log(`Updated ${relativePath}:\t"${previous}" -> "${replacement}"`);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sync-docker-services-tags.mjs` around lines 36 - 48, The loop in
scripts/sync-docker-services-tags.mjs writes each file as it iterates which can
leave partial updates if a later file fails; change to a two-pass approach:
first iterate serviceFiles and for each absolutePath (using resolve(rootDir,
relativePath)) readFile and validate testPattern.match exists and compute the
updated content (using replacePattern/replacement) while collecting
{absolutePath, previous, updated}; if any validation or I/O error occurs, throw
before making any changes; only after the full pass succeeds, iterate the
collected updates and perform writeFile for each and console.log the `"Updated
..."` messages.
3 participants
Summary
postgresservice config toensdband refining service/environment wiring across compose files.docker/envs/.env.docker.devnetand removesenvironmentkey from docker-composesscripts/sync-docker-services-tags.mjs(AI GENERATED!!!) and updates package/workflow wiring to keep Docker service tags synchronized for release flows.Why
Resolving comments after previous PR was merged:
#1965 (review)
Testing
ENSNODE_TAG=123 docker compose -f docker/docker-compose.devnet.yml configshows replaced dataPre-Review Checklist (Blocking)