fix(next-js): disable health check during build phase

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-next-js-max	Ready	Preview	Comment	Oct 19, 2025 11:33pm
rivetkit-serverless	Canceled			Oct 19, 2025 11:33pm

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(next-js): disable health check during build phase #1409 👈 (View in Graphite)
• fix(next-js): add webpackIgnore for optional dependencies #1408
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• merge queue - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: fix(next-js): disable health check during build phase

Summary

This PR adds automatic detection of Next.js build phase to disable health checks when NEXT_PHASE === "phase-production-build". This is a sensible fix that prevents health check failures during the Next.js build process.

Code Quality ✅

Positives:

• Clean, focused change that solves a specific problem
• Follows existing patterns in the codebase (similar to /home/runner/work/rivetkit/rivetkit/packages/next-js/src/mod.ts:16)
• Proper use of nullish coalescing (??=) to only set if not already configured
• Appropriate logging to help with debugging
• Log message follows codebase style (lowercase)

Minor Suggestions:

1. Config mutation concern - The current implementation mutates the config parameter directly:

   config.disableHealthCheck ??= true;

   This works but modifies the caller's object. While this appears intentional (since it's before the config is used), it might be worth considering if this could cause unexpected side effects for callers who reuse the config object. However, looking at the config flow, this seems acceptable since the config is processed through a Zod schema which creates a new object.

2. Environment variable validation - The code checks for a specific string value "phase-production-build" which is correct according to Next.js conventions. However, there's no validation that the env var format is as expected. Consider if you want to handle other Next.js phases (e.g., "phase-development-server") differently, though the current approach is probably correct.

Potential Bugs / Issues ✅

No major issues found. The implementation is sound:

• Uses getEnvUniversal() consistently with the rest of the codebase
• Proper string comparison for environment variable
• Nullish coalescing prevents overriding explicitly set values

Performance Considerations ✅

• Minimal performance impact: one environment variable read and string comparison
• The check happens once during client construction, not on hot path
• No concerns here

Security Concerns ✅

• No security issues identified
• Environment variable is read-only and used for control flow only
• No user input involved

Test Coverage ⚠️

Recommendation: Add tests

Currently, there are no tests in packages/rivetkit/src/client/ directory. Consider adding:

1. Unit test for the Next.js build phase detection:

   describe('Client constructor', () => {
     it('should disable health check during Next.js build phase', () => {
       // Mock getEnvUniversal to return "phase-production-build"
       // Create client
       // Assert config.disableHealthCheck is true
     });
     
     it('should not override explicitly set disableHealthCheck', () => {
       // Mock getEnvUniversal to return "phase-production-build"
       // Create client with disableHealthCheck: false
       // Assert config.disableHealthCheck remains false
     });
   });

2. Integration test to verify health check is actually skipped during this phase

Additional Observations

1. Consistency with existing pattern: This change aligns well with the pattern already established in packages/next-js/src/mod.ts:16 where disableHealthCheck is also set to true for serverless runners.

2. Documentation: The inline comment is clear and explains the "why" appropriately.

3. Location: The fix is in the right place - the ClientRaw constructor is appropriate since this affects all clients created during the Next.js build phase.

Verdict: ✅ Approved with minor suggestion

This is a solid fix for a legitimate issue. The only recommendation is to add test coverage when time permits, though this shouldn't block the PR.

Questions for the author:

• Have you tested this with an actual Next.js build to confirm it resolves the issue?
• Are there other Next.js phases (like "phase-development-server") that should also disable health checks?

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• example-smoke-test
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/actor@1409

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/cloudflare-workers@1409

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/core@1409

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/db@1409

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/framework-base@1409

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/next-js@1409

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/react@1409

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivetkit@1409

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/sql-loader@1409

commit: 336e41c

[claude]

PR Review: Fix Next.js Health Check During Build Phase

Summary

This PR adds automatic detection of the Next.js build phase to disable health checks when the client is instantiated during build time. This prevents errors from attempting to connect to the /metadata endpoint which doesn't exist during the build phase.

---

✅ Positives

1. Good Problem Identification: The fix correctly identifies that Next.js build phase doesn't have a running server, so health checks will fail.

2. Appropriate Solution: Using the NEXT_PHASE environment variable is the correct way to detect Next.js build phase (as per Next.js source).

3. Follows Codebase Patterns: Uses existing getEnvUniversal utility and logger() pattern consistently with the rest of the codebase.

4. Non-Breaking: Uses nullish coalescing (??=) to respect explicitly configured values.

5. Logging: Includes appropriate info-level logging for debugging.

---

🔍 Issues & Suggestions

1. Inconsistency with Next.js Package (Medium Priority)

The packages/next-js/src/mod.ts file already sets disableHealthCheck: true (line 16), but this PR adds the same logic to the core client. This creates two places handling the same problem:

In packages/next-js/src/mod.ts:16:

// Metadata endpoint will not exist at 127.0.0.1:6420
inputConfig.disableHealthCheck = true;

Recommendation:

• If the Next.js package always disables health checks, this PR may be redundant for Next.js users using that package
• If this PR targets users who use rivetkit directly in Next.js without the @rivetkit/next-js package, that should be documented
• Consider whether the logic in packages/next-js/src/mod.ts can be removed now

2. Missing Test Coverage (High Priority)

No tests were added to verify this behavior. Based on CLAUDE.md guidelines, you should add tests for:

• Detecting NEXT_PHASE=phase-production-build correctly
• Not overriding explicitly set disableHealthCheck: false
• Respecting explicitly set disableHealthCheck: true

Suggested test location: packages/rivetkit/src/client/client.test.ts

3. Environment Variable Mutation (Low Priority)

The code modifies the config object passed to the constructor:

config.disableHealthCheck ??= true;

While this works, it's mutating the input parameter. Consider whether this is intentional or if a copy should be made first.

4. Log Message Style (Low Priority - Style Consistency)

Per CLAUDE.md: "Write log messages in lowercase"

Current: "detected next.js build phase, disabling health check"

This is already lowercase, so it's good! ✅

5. Potential Runtime Check (Enhancement)

The check only happens during client construction. If someone creates a client in a separate module that gets loaded during build, it will be caught. However, consider if there are edge cases where this might not work as expected.

---

🔒 Security Considerations

No security concerns identified. The change only affects when health checks run and doesn't introduce new attack vectors.

---

⚡ Performance Considerations

• Positive Impact: Prevents unnecessary network requests during build time
• Negligible Overhead: Single environment variable check during client construction

---

📝 Recommendations

High Priority:

1. Add tests to verify the NEXT_PHASE detection logic
2. Clarify relationship with packages/next-js/src/mod.ts - document why both places disable health checks or remove redundancy

Medium Priority:
3. Add JSDoc comment explaining this behavior for future maintainers
4. Consider whether config mutation is acceptable or if a copy should be made

Example JSDoc:

/**
 * Creates an instance of Client.
 * 
 * Note: Automatically disables health checks during Next.js build phase
 * to prevent connection errors when no server is running.
 */
public constructor(driver: ManagerDriver, config: ClientConfig) {

---

✅ Approval Status

This PR solves a real problem and is well-implemented. However, I recommend addressing the test coverage before merging to ensure the behavior is properly validated and won't regress.

Verdict: Approve with requested changes (add tests)

[claude]

PR Review: fix(next-js): disable health check during build phase

Overview

This PR adds automatic detection of Next.js build phase and disables health checks during that phase to prevent errors when the /metadata endpoint is unavailable.

Code Quality & Best Practices ✅

Strengths:

• Good use of existing utilities: Properly uses getEnvUniversal() for cross-runtime environment variable access
• Well-documented: Includes clear comment explaining why this is needed with a reference to Next.js source
• Follows project conventions: Log message is lowercase per CLAUDE.md guidelines
• Appropriate placement: The check is in the right location (constructor) before the driver is initialized

Suggestions:

1. Config mutation concern: The code directly mutates the config parameter:

   config.disableHealthCheck = true;

   This could have unexpected side effects if the caller reuses the config object. Consider a safer approach:

   if (getEnvUniversal("NEXT_PHASE") === "phase-production-build") {
       logger().info("detected next.js build phase, disabling health check");
       config = { ...config, disableHealthCheck: true };
   }

   However, this may be acceptable if the config object is not reused in practice. Worth considering the tradeoff.

2. Magic string: The value "phase-production-build" is a magic string. While it's documented with a GitHub link, consider:

   • Adding a constant at the top of the file or in a shared constants file
   • This would make it easier to maintain and test

   const NEXT_PHASE_PRODUCTION_BUILD = "phase-production-build";

Potential Bugs or Issues ⚠️

1. Limited scope: The check only covers "phase-production-build". Next.js has other build phases that might also lack the metadata endpoint:

   • phase-development-server
   • phase-production-server
   • phase-export

   Consider whether other phases should also skip health checks. Based on the existing Next.js adapter code (packages/next-js/src/mod.ts:16), it seems health checks are already disabled for the serverless runner, so this might only be an issue when creating a client directly during build.

2. Testing gap: There are no tests covering this new functionality. While it's a small change, consider adding a test that:

   • Sets NEXT_PHASE environment variable
   • Creates a client
   • Verifies disableHealthCheck is set to true

Performance Considerations ✅

• Minimal overhead: Single environment variable check is negligible
• No performance concerns: This change actually improves build performance by avoiding unnecessary network requests

Security Concerns ✅

• No security issues identified: Reading an environment variable is safe
• Defense in depth: Disabling health checks during build is appropriate as there's no server to connect to anyway

Test Coverage ⚠️

Missing test coverage:

• No unit tests for this Next.js-specific behavior
• Consider adding to packages/rivetkit/src/client/test.ts or creating a dedicated test file

Suggested test:

describe("Client constructor Next.js detection", () => {
  it("should disable health check during Next.js build phase", () => {
    const originalEnv = process.env.NEXT_PHASE;
    process.env.NEXT_PHASE = "phase-production-build";
    
    const config = {
      /* minimal config */
    };
    const client = new ClientRaw(mockDriver, config);
    
    expect(config.disableHealthCheck).toBe(true);
    
    process.env.NEXT_PHASE = originalEnv;
  });
});

Additional Notes

• This PR builds on fix(next-js): add webpackIgnore for optional dependencies #1408 which addresses webpack bundling issues - these are complementary fixes for Next.js compatibility
• The existing Next.js adapter (packages/next-js/src/mod.ts:16) already disables health checks, but this PR ensures it also works when users create clients directly in their Next.js apps during build

Recommendation

Approve with minor suggestions. The code solves a real problem and is generally well-implemented. The suggestions above are optional improvements that could enhance maintainability and test coverage, but the current implementation is functional and safe.

---

Review Confidence: High
Files Changed: 1
Lines Added: 9
Complexity: Low

[claude]

Code Review: Fix Next.js Build Phase Health Check

Thank you for this fix! I've reviewed the changes and have the following feedback:

✅ Positives

1. Root Cause Fix: Correctly identifies and solves the issue where Next.js build phase tries to perform health checks against a non-existent /metadata endpoint
2. Good Documentation: The comment references the exact Next.js source code that defines the NEXT_PHASE constant, making it easy to verify the approach
3. Consistent with Existing Patterns: Uses getEnvUniversal() which is the standard way to access environment variables across different runtimes (Deno/Node)
4. Appropriate Logging: Uses lowercase log message following the project's style guide
5. Follows Immutability Best Practice: Mutates runConfig directly (which is the input parameter) rather than creating a new object, consistent with how the config is processed

🔍 Code Quality Observations

Potential Issue - Direct Mutation:
The code mutates the runConfig parameter directly:

runConfig.disableHealthCheck = true;

This works because the parameter is passed by reference, but it's somewhat implicit. The mutation happens before the config is stored in this.#config. While this is functionally correct, consider:

1. Is this intentional behavior? Should the caller's config object be mutated, or should this be an internal decision?
2. Alternative approach: You could avoid mutation by setting the flag after assignment:

this.#config = runConfig;

if (getEnvUniversal("NEXT_PHASE") === "phase-production-build") {
    logger().info("detected next.js build phase, disabling health check");
    this.#config.disableHealthCheck = true;
}

However, the current approach is acceptable if the config is expected to be mutable at this stage.

💡 Suggestions for Improvement

1. Consider Browser Environment: The getEnvUniversal() function returns undefined in browser environments (packages/rivetkit/src/utils.ts:36). While Next.js build runs in Node.js, it's worth confirming this code path never executes in edge runtime or browser contexts where process.env might not exist.

2. Duplicate Logic: I noticed that packages/next-js/src/mod.ts:16 already sets inputConfig.disableHealthCheck = true for all Next.js usage. This new code adds a second location where health checks are disabled for Next.js. Consider:

   • Is this PR fixing a gap where the Next.js package config doesn't apply?
   • Should there be a comment explaining why both locations are needed?
   • Could this logic be centralized?

3. Test Coverage: There are no tests for RemoteManagerDriver constructor behavior. Consider adding a test that verifies:

   // Pseudo-code
   test("disables health check during Next.js build phase", () => {
       process.env.NEXT_PHASE = "phase-production-build";
       const driver = new RemoteManagerDriver(config);
       expect(config.disableHealthCheck).toBe(true);
   });

4. Type Safety: The string literal "phase-production-build" could be extracted to a constant to prevent typos and make it easier to maintain if Next.js changes this value.

🔐 Security

No security concerns identified. This is defensive code that prevents unnecessary network calls during build time.

⚡ Performance

Positive Impact: Skipping health checks during build phase will speed up Next.js builds by eliminating failed network requests.

📋 Summary

This is a well-targeted fix that solves a real problem in Next.js builds. The main question to clarify is whether this duplicates or complements the existing disableHealthCheck logic in the next-js package. Otherwise, the implementation is clean and follows project conventions.

Recommendation: ✅ Approve with minor suggestions

---

Review generated by Claude Code following RivetKit development guidelines

[graphite-app]

Merge activity

• Oct 19, 11:36 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Oct 19, 11:36 PM UTC: CI is running for this pull request on a draft pull request (#1410) due to your merge queue CI optimization settings.
• Oct 19, 11:37 PM UTC: Merged by the Graphite merge queue via draft PR: #1410.