-
Notifications
You must be signed in to change notification settings - Fork 15
openapi spec / docs poc #1550
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
base: main
Are you sure you want to change the base?
openapi spec / docs poc #1550
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
🦋 Changeset detectedLatest commit: 98bece8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds CI OpenAPI sync validation, a CLI script to fetch/generate Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant ENS as ENSApi (OPENAPI_CI_CHECK)
participant Script as generate-openapi.ts
participant FS as Repo filesystem
participant Mintlify as Mintlify API
participant Comparator as CI comparator
GH->>ENS: start ENSApi with OPENAPI_CI_CHECK=true
Note right of ENS: ENSApi boots with mocked config
GH->>ENS: poll /openapi.json until ready
Script->>ENS: GET /openapi.json
ENS-->>Script: 200 + JSON
Script->>FS: write formatted `openapi.json`
Comparator->>FS: read committed `openapi.json`
Comparator->>FS: read generated `openapi.json`
Comparator-->>GH: pass / fail result
GH->>Mintlify: POST validate + (optionally) trigger rebuild
Mintlify-->>GH: validation / rebuild response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 unit tests (beta)
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.
Pull request overview
This PR adds OpenAPI specification generation and documentation capabilities to the ENSNode project. It introduces a script to fetch and save the OpenAPI spec from a running ENSApi instance, integrates it with Mintlify documentation, and adds CI checks to ensure the spec stays in sync with production.
Changes:
- Added a 5,106-line OpenAPI specification file documenting all ENSApi endpoints
- Created a generation script to fetch and update the spec from a running instance
- Configured Mintlify docs to reference both production and local OpenAPI specs
- Added CI workflow to validate spec synchronization on the main branch
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/docs.ensnode.io/openapi.json | Complete OpenAPI 3.1.0 specification documenting all ENSApi endpoints including resolution, meta, explore, and ENSAwards APIs |
| apps/ensapi/scripts/generate-openapi.ts | TypeScript script to fetch OpenAPI spec from running instance and save to docs directory |
| apps/ensapi/package.json | Added npm script openapi:generate to run the generation script |
| docs/docs.ensnode.io/docs.json | Updated configuration to reference production API spec and added hidden preview section for local spec |
| .github/workflows/test_ci.yml | Added CI job to verify OpenAPI spec stays in sync with production on main branch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 5
🤖 Fix all issues with AI agents
In @.github/workflows/test_ci.yml:
- Around line 51-55: The openapi-sync-check job currently only runs when
github.ref == 'refs/heads/main', which lets PRs merge with out-of-sync specs;
update the condition on the openapi-sync-check job (the if: line) to also run
for pull requests (e.g., include github.event_name == 'pull_request' or
github.head_ref checks) and mark it non-blocking for PRs by using
continue-on-error: true (or alternatively remove the if entirely and rely on
branch protection or add contributor documentation). Target the job named
openapi-sync-check and the existing if: github.ref == 'refs/heads/main'
condition when making the change.
In `@apps/ensapi/scripts/generate-openapi.ts`:
- Around line 23-24: ensapiUrl may end with a trailing slash causing openapiUrl
to become "//openapi.json"; normalize ensapiUrl before composing openapiUrl (the
variables to change are ensapiUrl and openapiUrl in generate-openapi.ts) by
trimming any trailing '/' from ensapiUrl (or using a URL-safe join) so
openapiUrl is built as `${normalizedEnsapiUrl}/openapi.json` even when
ENSAPI_URL or process.argv[2] includes a trailing slash; ensure
DEFAULT_ENSAPI_URL remains fallback and normalization runs after selecting the
value.
- Line 12: Update the header comment string that reads "Writes openapi.json to
the docs directory for Mintilify" and correct the product name typo to
"Mintlify" so the comment reads "Writes openapi.json to the docs directory for
Mintlify"; locate this exact comment text in generate-openapi.ts and make the
single-word change.
- Around line 28-33: Add a timeout to the fetch in generate-openapi.ts by
creating an AbortSignal via AbortSignal.timeout(ms) and passing it as the signal
option to the fetch(openapiUrl) call; handle the abort case by catching the
thrown error (check for AbortError or error.name === 'AbortError') and log a
clear timeout message before exiting, and ensure the existing non-ok response
handling remains unchanged.
In `@docs/docs.ensnode.io/docs.json`:
- Around line 29-34: The docs.json "Preview" group references a non-existent
page "ensapi/preview"; fix by either adding a new page file named preview.mdx
under the docs/docs.ensnode.io/ensapi/ folder (so the path matches
"ensapi/preview") or remove the entire Preview group entry (the object with
"group": "Preview", "pages": ["ensapi/preview"], "openapi": "./openapi.json",
"hidden": true) from docs.json; update whichever you choose and ensure the
"pages" array references only existing MDX/MD files.
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
🤖 Fix all issues with AI agents
In @.github/workflows/test_ci.yml:
- Around line 58-70: The CI step runs the openapi:generate npm script which
currently relies on a hardcoded production default; make the behavior explicit
by passing the production URL via the ENSAPI_URL env in the workflow step or by
updating the openapi:generate script to accept an ENSAPI_URL argument and
default it to https://api.alpha.ensnode.io; update the step that invokes
openapi:generate to export ENSAPI_URL or call the script with the URL so the
action is explicit and maintainable.
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.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| curl --request POST \ | ||
| --url "https://api.mintlify.com/v1/project/update/${{ secrets.MINTLIFY_PROJECT_ID }}" \ | ||
| --header "Authorization: Bearer ${{ secrets.MINTLIFY_API_TOKEN }}" \ | ||
| --fail |
Copilot
AI
Jan 23, 2026
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.
The curl command uses --fail which is good, but it doesn't include error output redirection. If the curl fails, the error message from Mintlify's API won't be visible in the workflow logs, making it harder to debug failures.
Consider adding -v or --show-error flags, or wrapping the curl in a script that captures and logs the response on failure.
| --fail | |
| --fail --show-error |
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.
@notrab This looks like important feedback.
Would it be fair to use .github/actions/send_slack_notification to send us a Slack notification both for the success and fail cases of this?
| const EnsApiConfigSchemaForOpenApiCiCheck = z.object({ | ||
| port: PortSchema.default(ENSApi_DEFAULT_PORT), | ||
| databaseUrl: DatabaseUrlSchema, | ||
| databaseSchemaName: DatabaseSchemaNameSchema, | ||
| ensIndexerUrl: EnsIndexerUrlSchema, | ||
| theGraphApiKey: TheGraphApiKeySchema, | ||
| namespace: ENSNamespaceSchema, | ||
| rpcConfigs: RpcConfigsSchema, | ||
| ensIndexerPublicConfig: makeENSIndexerPublicConfigSchema("ensIndexerPublicConfig"), | ||
| ensHolidayAwardsStart: z.number(), | ||
| ensHolidayAwardsEnd: z.number(), | ||
| }); | ||
|
|
||
| function buildConfigForOpenApiCiCheck(env: EnsApiEnvironment): EnsApiConfig { | ||
| logger.info("OPENAPI_CI_CHECK mode enabled - using minimal mock config"); | ||
|
|
||
| return EnsApiConfigSchemaForOpenApiCiCheck.parse({ | ||
| port: env.PORT || ENSApi_DEFAULT_PORT, | ||
| databaseUrl: "postgresql://openapi:openapi@localhost:5432/openapi", | ||
| databaseSchemaName: "public", | ||
| ensIndexerUrl: "http://localhost:42069", | ||
| theGraphApiKey: undefined, | ||
| namespace: "mainnet", | ||
| rpcConfigs: { | ||
| "1": "https://rpc.example.com", | ||
| }, | ||
| ensIndexerPublicConfig: { | ||
| labelSet: { | ||
| labelSetId: "ens-default", | ||
| labelSetVersion: 1, | ||
| }, | ||
| indexedChainIds: [1], | ||
| isSubgraphCompatible: false, | ||
| namespace: "mainnet", | ||
| plugins: ["subgraph"], | ||
| databaseSchemaName: "public", | ||
| versionInfo: { | ||
| nodejs: process.version, | ||
| ponder: "0.0.0", | ||
| ensDb: packageJson.version, | ||
| ensIndexer: packageJson.version, | ||
| ensNormalize: "0.0.0", | ||
| ensRainbow: packageJson.version, | ||
| ensRainbowSchema: 1, | ||
| }, | ||
| }, | ||
| ensHolidayAwardsStart: getUnixTime(new Date(ENS_HOLIDAY_AWARDS_START_DATE)), | ||
| ensHolidayAwardsEnd: getUnixTime(new Date(ENS_HOLIDAY_AWARDS_END_DATE)), | ||
| }) as EnsApiConfig; | ||
| } |
Copilot
AI
Jan 23, 2026
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.
The type assertion as EnsApiConfig at line 127 bypasses the full validation schema used for normal configurations. The EnsApiConfigSchemaForOpenApiCiCheck omits the .check() calls for invariants like invariant_rpcConfigsSpecifiedForRootChain, invariant_ensIndexerPublicConfigVersionInfo, and invariant_ensHolidayAwardsEndAfterStart.
While this may be intentional for CI, it creates a risk that the CI check mode could have different validation behavior than production. Consider either:
- Including these invariant checks in the CI schema to ensure consistency
- Adding a comment explaining why these checks are intentionally omitted
| # Wait for server to be ready | ||
| for i in {1..30}; do | ||
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | ||
| echo "ENSApi is ready" | ||
| break | ||
| fi | ||
| echo "Waiting for ENSApi to start... ($i/30)" | ||
| sleep 1 | ||
| done | ||
Copilot
AI
Jan 23, 2026
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.
The server readiness check in the CI workflow doesn't verify if the loop completed successfully. If all 30 attempts fail, the script continues to the next step instead of failing early. This could lead to confusing error messages when the generate step fails because the server never started.
Consider adding a check after the loop to fail if the server didn't start within the timeout period.
| # Wait for server to be ready | |
| for i in {1..30}; do | |
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | |
| echo "ENSApi is ready" | |
| break | |
| fi | |
| echo "Waiting for ENSApi to start... ($i/30)" | |
| sleep 1 | |
| done | |
| # Wait for server to be ready | |
| server_ready=false | |
| for i in {1..30}; do | |
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | |
| echo "ENSApi is ready" | |
| server_ready=true | |
| break | |
| fi | |
| echo "Waiting for ENSApi to start... ($i/30)" | |
| sleep 1 | |
| done | |
| if [ "$server_ready" = false ]; then | |
| echo "ENSApi failed to start within the timeout (30 seconds)." | |
| exit 1 | |
| fi |
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.
This looks like important feedback.
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: 2
🤖 Fix all issues with AI agents
In @.changeset/openapi-generator-and-ci.md:
- Line 6: The markdown file lacks a top-level H1 as the first non-frontmatter
line, triggering MD041; add an H1 heading (for example: "# OpenAPI generator and
CI") as the first non-frontmatter line in .changeset/openapi-generator-and-ci.md
so the file begins with a top-level heading before the existing body text.
In @.github/workflows/deploy_switch_ensnode_environment.yml:
- Around line 84-90: The "Trigger Mintlify Docs Rebuild" step's curl invocation
can hang indefinitely; update the curl call in that step to include explicit
timeouts and optional retry behavior: add --connect-timeout (e.g. 10) and
--max-time (e.g. 60) to bound connection and total request time, and optionally
add --retry (e.g. 3) and --retry-delay (e.g. 5) so transient failures are
retried but still bounded; keep the existing Authorization header and --fail
flag and ensure the step still uses continue-on-error: true if desired.
| "@docs/mintlify": patch | ||
| --- | ||
|
|
||
| Add OpenAPI spec generation tooling and CI validation |
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.
Add a top-level heading to satisfy MD041 (first-line-heading).
If markdownlint applies to .changeset files, the first non-frontmatter line should be an H1. Consider adding a heading before the body text.
✏️ Proposed fix
---
"ensapi": patch
"@docs/mintlify": patch
---
-Add OpenAPI spec generation tooling and CI validation
+# OpenAPI spec generation tooling and CI validation
+
+Add OpenAPI spec generation tooling and CI validation📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Add OpenAPI spec generation tooling and CI validation | |
| --- | |
| "ensapi": patch | |
| "@docs/mintlify": patch | |
| --- | |
| # OpenAPI spec generation tooling and CI validation | |
| Add OpenAPI spec generation tooling and CI validation |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
6-6: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In @.changeset/openapi-generator-and-ci.md at line 6, The markdown file lacks a
top-level H1 as the first non-frontmatter line, triggering MD041; add an H1
heading (for example: "# OpenAPI generator and CI") as the first non-frontmatter
line in .changeset/openapi-generator-and-ci.md so the file begins with a
top-level heading before the existing body text.
| - name: Trigger Mintlify Docs Rebuild | ||
| continue-on-error: true | ||
| run: | | ||
| curl --request POST \ | ||
| --url "https://api.mintlify.com/v1/project/update/${{ secrets.MINTLIFY_PROJECT_ID }}" \ | ||
| --header "Authorization: Bearer ${{ secrets.MINTLIFY_API_TOKEN }}" \ | ||
| --fail |
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.
Add timeouts to the Mintlify API call to avoid hung deployments.
Without connect/total timeouts, a stalled Mintlify endpoint can hang the job indefinitely even with continue-on-error: true, delaying the environment switch workflow. Add explicit timeouts (and optionally retries) to bound the step duration.
⚡ Proposed fix
- name: Trigger Mintlify Docs Rebuild
continue-on-error: true
run: |
curl --request POST \
--url "https://api.mintlify.com/v1/project/update/${{ secrets.MINTLIFY_PROJECT_ID }}" \
--header "Authorization: Bearer ${{ secrets.MINTLIFY_API_TOKEN }}" \
- --fail
+ --fail \
+ --connect-timeout 10 \
+ --max-time 30📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Trigger Mintlify Docs Rebuild | |
| continue-on-error: true | |
| run: | | |
| curl --request POST \ | |
| --url "https://api.mintlify.com/v1/project/update/${{ secrets.MINTLIFY_PROJECT_ID }}" \ | |
| --header "Authorization: Bearer ${{ secrets.MINTLIFY_API_TOKEN }}" \ | |
| --fail | |
| - name: Trigger Mintlify Docs Rebuild | |
| continue-on-error: true | |
| run: | | |
| curl --request POST \ | |
| --url "https://api.mintlify.com/v1/project/update/${{ secrets.MINTLIFY_PROJECT_ID }}" \ | |
| --header "Authorization: Bearer ${{ secrets.MINTLIFY_API_TOKEN }}" \ | |
| --fail \ | |
| --connect-timeout 10 \ | |
| --max-time 30 |
🤖 Prompt for AI Agents
In @.github/workflows/deploy_switch_ensnode_environment.yml around lines 84 -
90, The "Trigger Mintlify Docs Rebuild" step's curl invocation can hang
indefinitely; update the curl call in that step to include explicit timeouts and
optional retry behavior: add --connect-timeout (e.g. 10) and --max-time (e.g.
60) to bound connection and total request time, and optionally add --retry (e.g.
3) and --retry-delay (e.g. 5) so transient failures are retried but still
bounded; keep the existing Authorization header and --fail flag and ensure the
step still uses continue-on-error: true if desired.
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.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| When switching production environments (green/blue), the `deploy_switch_ensnode_environment` workflow triggers a Mintlify rebuild to fetch the latest OpenAPI spec from the production API. | ||
|
|
||
| Changes pushed to the main branch are automatically deployed to production. | ||
| This requires `MINTLIFY_API_TOKEN` and `MINTLIFY_PROJECT_ID` to be configured as repository secrets. |
Copilot
AI
Jan 23, 2026
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.
The documentation states that MINTLIFY_API_TOKEN and MINTLIFY_PROJECT_ID need to be "configured as repository secrets", but according to the PR description, MINTLIFY_PROJECT_ID should be a variable, not a secret. Consider updating the documentation to correctly state that MINTLIFY_API_TOKEN should be a secret and MINTLIFY_PROJECT_ID should be a variable.
| This requires `MINTLIFY_API_TOKEN` and `MINTLIFY_PROJECT_ID` to be configured as repository secrets. | |
| This requires `MINTLIFY_API_TOKEN` to be configured as a repository secret and `MINTLIFY_PROJECT_ID` to be configured as a repository variable. |
| description: Preview upcoming API changes from the current branch. | ||
| --- | ||
|
|
||
| This page shows the OpenAPI specification from the current branch, which may include unreleased API changes. |
Copilot
AI
Jan 23, 2026
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.
The Preview page description mentions "current branch" but Mintlify PR previews are generated from the PR's head branch, not necessarily the "current" branch from a local development perspective. Consider clarifying this as "the branch in this PR" or "this branch" to be more precise about what preview is being shown.
| description: Preview upcoming API changes from the current branch. | |
| --- | |
| This page shows the OpenAPI specification from the current branch, which may include unreleased API changes. | |
| description: Preview upcoming API changes from this branch. | |
| --- | |
| This page shows the OpenAPI specification for this branch (for pull requests, the PR's head branch), which may include unreleased API changes. |
Greptile OverviewGreptile SummaryThis PR establishes automated OpenAPI documentation generation and validation for ENSApi, addressing production docs drift by fetching specs from the live API while enabling PR previews via committed files. Key Changes
ArchitectureThe implementation uses a two-tier approach:
This ensures production docs always reflect the live API while allowing developers to preview API changes before deployment. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant PR as Pull Request
participant CI as CI Workflow
participant ENSApi as ENSApi Server
participant Mintlify as Mintlify Docs
participant Prod as Production API
Note over Dev,Prod: PR Review Flow
Dev->>PR: Create PR with API changes
PR->>CI: Trigger openapi-sync-check job
CI->>ENSApi: Start in OPENAPI_CI_CHECK mode
ENSApi-->>CI: Server ready (mock config)
CI->>ENSApi: Fetch /openapi.json
ENSApi-->>CI: Return OpenAPI spec
CI->>CI: Generate openapi.json locally
CI->>CI: Compare with committed file
alt Spec is in sync
CI-->>PR: ✅ Check passes
else Spec is out of sync
CI-->>PR: ❌ Check fails with diff
end
CI->>Mintlify: Validate openapi.json
Mintlify-->>CI: Validation result
Note over Dev,Prod: PR Preview Flow
PR->>Mintlify: Build preview docs
Mintlify->>PR: Read committed openapi.json
Mintlify-->>Dev: Preview shows upcoming API changes
Note over Dev,Prod: Production Deployment Flow
Dev->>PR: Merge to main
PR->>Mintlify: Trigger production build
Mintlify->>Prod: Fetch /openapi.json at runtime
Prod-->>Mintlify: Return live OpenAPI spec
Mintlify-->>Mintlify: Render production docs
Note over Dev,Prod: Environment Switch Flow
Dev->>CI: Trigger environment switch
CI->>CI: Switch Traefik routing
CI->>Mintlify: POST /v1/project/update
Mintlify->>Prod: Fetch updated /openapi.json
Prod-->>Mintlify: Return OpenAPI spec
Mintlify-->>Mintlify: Rebuild docs with latest spec
|
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.
1 file reviewed, 1 comment
| - name: Start ENSApi in OpenAPI CI check mode | ||
| run: | | ||
| OPENAPI_CI_CHECK=true pnpm --filter ensapi start & | ||
| # Wait for server to be ready | ||
| for i in {1..30}; do | ||
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | ||
| echo "ENSApi is ready" | ||
| break | ||
| fi | ||
| echo "Waiting for ENSApi to start... ($i/30)" | ||
| sleep 1 | ||
| done | ||
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.
Loop doesn't fail if server never starts - add exit check after loop
| - name: Start ENSApi in OpenAPI CI check mode | |
| run: | | |
| OPENAPI_CI_CHECK=true pnpm --filter ensapi start & | |
| # Wait for server to be ready | |
| for i in {1..30}; do | |
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | |
| echo "ENSApi is ready" | |
| break | |
| fi | |
| echo "Waiting for ENSApi to start... ($i/30)" | |
| sleep 1 | |
| done | |
| - name: Start ENSApi in OpenAPI CI check mode | |
| run: | | |
| OPENAPI_CI_CHECK=true pnpm --filter ensapi start & | |
| # Wait for server to be ready | |
| SERVER_READY=false | |
| for i in {1..30}; do | |
| if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then | |
| echo "ENSApi is ready" | |
| SERVER_READY=true | |
| break | |
| fi | |
| echo "Waiting for ENSApi to start... ($i/30)" | |
| sleep 1 | |
| done | |
| if [ "$SERVER_READY" = false ]; then | |
| echo "❌ ENSApi failed to start within 30 seconds" | |
| exit 1 | |
| fi |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/test_ci.yml
Line: 58:70
Comment:
Loop doesn't fail if server never starts - add exit check after loop
```suggestion
- name: Start ENSApi in OpenAPI CI check mode
run: |
OPENAPI_CI_CHECK=true pnpm --filter ensapi start &
# Wait for server to be ready
SERVER_READY=false
for i in {1..30}; do
if curl -s http://localhost:4334/openapi.json > /dev/null 2>&1; then
echo "ENSApi is ready"
SERVER_READY=true
break
fi
echo "Waiting for ENSApi to start... ($i/30)"
sleep 1
done
if [ "$SERVER_READY" = false ]; then
echo "❌ ENSApi failed to start within 30 seconds"
exit 1
fi
```
How can I resolve this? If you propose a fix, please make it concise.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.
This looks like important feedback.
| } | ||
| } | ||
|
|
||
| main(); |
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.
@notrab This seems like a fair idea
| }; | ||
|
|
||
| // Pretty-print the JSON for readability in git diffs | ||
| const content = `${JSON.stringify(spec, null, 2)}\n`; |
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.
It seems we should remove this since we perform formatting with biome in the next step?
| * pnpm openapi:generate http://localhost:4334 | ||
| * | ||
| * Output: | ||
| * Writes openapi.json to the docs directory for Mintlify to consume. Note that a rebuild of Mintlify is required for it to reflect an updated openapi.json. |
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.
We should also mention the biome step.
| LogLevelEnvironment & | ||
| TheGraphEnvironment & | ||
| EnsHolidayAwardsEnvironment; | ||
| EnsHolidayAwardsEnvironment & { |
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.
We should follow the same pattern here as we did for all the other environment variables. Let's not break patterns unless a very special reason.
| TheGraphEnvironment & | ||
| EnsHolidayAwardsEnvironment; | ||
| EnsHolidayAwardsEnvironment & { | ||
| OPENAPI_CI_CHECK?: string; |
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.
All environment variables must always be fully documented (up to a high standard) in apps/ensapi/.env.local.example
| - uses: actions/checkout@v4 | ||
| - uses: ./.github/actions/setup_node_environment | ||
|
|
||
| - name: Start ENSApi in OpenAPI CI check mode |
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.
Why is this called "OpenAPI CI check mode"?
And why is the environment variable called OPENAPI_CI_CHECK?
This doesn't make sense to me. Isn't this actually about generating the openapi.json file?
| - Run `pnpm mint --help` for more Mintlify CLI details | ||
|
|
||
| 2. Navigate to the docs directory: | ||
| ## Publishing Changes |
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.
It seems this section is actually about change management.
| ```bash | ||
| cd ensnode/docs/docs.ensnode.io | ||
| ``` | ||
| Mintlify automatically deploys when changes are pushed to `main`. This presents a tradeoff: |
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.
It automatically deploys on each branch, no?
A preview deploy is still a deploy.
We should always write with precision. This is just referencing production deployments, correct?
| This ensures documentation stays aligned with what users can actually access, while still allowing PR previews for review. | ||
|
|
||
| ## Publishing Changes | ||
| > **Note:** Mintlify currently doesn't support separating auto-deploy from PR previews, or an API for triggering preview builds for specific branches. If this changes, we could decouple the workflows. |
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'm struggling to understand what this means.
Doesn't Mintlify deploy previews on all branches?
Please talk more about Mintlify previews in this readme file.
|
|
||
| export type EnsApiConfig = z.infer<typeof EnsApiConfigSchema>; | ||
|
|
||
| const EnsApiConfigSchemaForOpenApiCiCheck = z.object({ |
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.
We put so much effort into the code here and this looks like trash. Clean it up.
Where is the care and craftsmanship?
Lite PR
Summary
openapi:generatescript to fetch and save OpenAPI spec from ENSApiopenapi.jsonstays in sync with productionCloses #1532
Closes #1475
Why
Production API docs were getting out of sync with deployed API, and there was no way to preview API doc changes in PRs.
This setup ensures:
Testing
pnpm --filter ensapi openapi:generatelocally to verify spec generationNotes for Reviewer (Optional)
MINTLIFY_API_TOKENsecret andMINTLIFY_PROJECT_IDvariable need added to to GitHub for the environment switch workflowPre-Review Checklist (Blocking)