• pnpm-lock.yaml: Language not supported

src/utils/is-wsl.ts (1)

4-18: LGTM! Well-implemented WSL detection.

The implementation correctly detects WSL using the standard approach: checking the platform, os.release(), and /proc/version for the "microsoft" identifier. The defensive error handling with try/catch is appropriate, and the synchronous file read is acceptable for this CLI initialization context.

💡 Optional: Consider memoizing for repeated calls

If this function is called frequently, you could memoize the result since WSL status won't change during runtime:

+let cachedResult: boolean | undefined;
+
 export const isWsl = () => {
+    if (cachedResult !== undefined) {
+        return cachedResult;
+    }
+
     if (process.platform !== 'linux') {
-        return false;
+        return (cachedResult = false);
     }
 
     if (os.release().toLowerCase().includes('microsoft')) {
-        return true;
+        return (cachedResult = true);
     }
 
     try {
-        return fs.readFileSync('/proc/version', 'utf8').toLowerCase().includes('microsoft');
+        return (cachedResult = fs.readFileSync('/proc/version', 'utf8').toLowerCase().includes('microsoft'));
     } catch {
-        return false;
+        return (cachedResult = false);
     }
 };

However, the current implementation is already very fast, so this is purely a micro-optimization.

src/utils/machine-id-utils.ts (2)

22-28: Consider modern alternatives to hasOwnProperty.

While the current implementation works, you could use the in operator or Object.hasOwn() (Node.js 16.9+) instead of calling hasOwnProperty directly on process.env.

🔎 Proposed refactor

 function isWindowsProcessMixedOrNativeArchitecture() {
-  // eslint-disable-next-line no-prototype-builtins
-  if (process.arch === 'ia32' && process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432')) {
+  if (process.arch === 'ia32' && 'PROCESSOR_ARCHITEW6432' in process.env) {
     return 'mixed'
   }
   return 'native'
 }

---

34-61: Minor inconsistency in error handling.

The expose() function throws an error for unsupported platforms (line 59), but getMachineId() already checks platform support at line 64 before calling expose(). This makes the throw statement unreachable in practice.

For consistency, you could either:

1. Remove the throw and return undefined for the default case, or
2. Remove the platform check in getMachineId() and rely on the exception

🔎 Option 1: Return undefined for unsupported platforms

     case 'freebsd':
       return result
         .toString()
         .replace(/\r+|\n+|\s+/gi, '')
         .toLowerCase()
     default:
-      throw new Error(`Unsupported platform: ${process.platform}`)
+      return undefined
   }
 }

.github/workflows/publish-release.yml (1)

58-72: Improve changelog generation robustness.

The changelog generation script may produce malformed output if commit messages contain special characters, backticks, or newlines that could break the heredoc delimiter or GitHub Actions output format.

🔎 Proposed improvement

       - name: Generate changelog
         id: changelog
         run: |
           PREVIOUS_TAG=$(git describe --tags --abbrev=0 2>/dev/null || echo "")
 
           if [ -z "$PREVIOUS_TAG" ]; then
-            CHANGELOG=$(git log --oneline --no-merges --format="* %s" HEAD)
+            CHANGELOG=$(git log --oneline --no-merges --format="* %s" HEAD | head -n 50)
           else
-            CHANGELOG=$(git log --oneline --no-merges --format="* %s" ${PREVIOUS_TAG}..HEAD)
+            CHANGELOG=$(git log --oneline --no-merges --format="* %s" ${PREVIOUS_TAG}..HEAD | head -n 50)
           fi
 
           if [ -z "$CHANGELOG" ]; then
             CHANGELOG="* Automated release"
           fi
 
-          echo "changelog<<EOF" >> $GITHUB_OUTPUT
-          echo "$CHANGELOG" >> $GITHUB_OUTPUT
-          echo "EOF" >> $GITHUB_OUTPUT
+          {
+            echo "changelog<<EOF"
+            echo "$CHANGELOG"
+            echo "EOF"
+          } >> $GITHUB_OUTPUT

Consider limiting the number of commits included and grouping the output commands to ensure atomicity.

package.json (1)

19-19: Fill in the author field.

The author field is currently empty. For proper package attribution and npm registry information, consider adding the author name or organization.

.github/workflows/claude-code-review.yml (1)

34-36: Consider pinning to a specific version or commit SHA.

The workflow uses @beta which is an unpinned, moving target. This could introduce unexpected breaking changes or bugs in the CI pipeline. For better stability and reproducibility, consider pinning to a specific version tag or commit SHA.

Example:

- name: Run Claude Code Review
  id: claude-review
  uses: anthropics/[email protected]  # or @<commit-sha>

scripts/post-build.ts (1)

10-16: Add error handling for file operations.

The file read/write operations lack explicit error handling. While the script will fail if files are missing, providing clearer error messages would improve debuggability during builds.

🔎 Proposed enhancement with error handling

  for (const file of filesToProcess) {
    console.log(`Processing ${file} for telemetry token...`)
    const filePath = path.join(__dirname, '..', file)
-   const content = fs.readFileSync(filePath, 'utf-8')
-   const updatedContent = content.replace('<TELEMETRY_TRACKING_TOKEN>', token)
-   fs.writeFileSync(filePath, updatedContent, 'utf-8')
+   try {
+     const content = fs.readFileSync(filePath, 'utf-8')
+     const updatedContent = content.replace('<TELEMETRY_TRACKING_TOKEN>', token)
+     fs.writeFileSync(filePath, updatedContent, 'utf-8')
+   } catch (error) {
+     throw new Error(`Failed to process ${file}: ${error instanceof Error ? error.message : String(error)}`)
+   }
  }

src/utils/is-docker.ts (1)

24-31: Consider updating the TODO comment or using ??=.

The TODO suggests waiting for Node.js 16, but the workflow and tsconfig indicate the project targets Node.js 20.x. The nullish coalescing assignment operator (??=) is available in Node.js 15+ and ES2021, so you can safely use it now.

🔎 Proposed modernization using ??=

 export default function isDocker() {
-    // TODO: Use `??=` when targeting Node.js 16.
-    if (isDockerCached === undefined) {
-        isDockerCached = hasDockerEnv() || hasDockerCGroup();
-    }
-
+    isDockerCached ??= hasDockerEnv() || hasDockerCGroup();
     return isDockerCached;
 }

src/utils/is-container.ts (1)

16-23: Consider updating the TODO comment or using ??=.

Similar to is-docker.ts, this TODO references Node.js 16, but the project targets Node.js 20.x. You can safely use the nullish coalescing assignment operator (??=) now.

🔎 Proposed modernization using ??=

 export function isInContainer() {
-    // TODO: Use `??=` when targeting Node.js 16.
-    if (cachedResult === undefined) {
-        cachedResult = hasContainerEnv() || isDocker();
-    }
-
+    cachedResult ??= hasContainerEnv() || isDocker();
     return cachedResult;
 }

src/index.ts (1)

57-82: Correct exit code handling for non-CommanderError cases.

Line 61 initializes exitCode to 1, but it's only updated to e.exitCode for CommanderError (line 69). For CliError and unhandled errors, the code will exit with 1, which is correct. However, the logic would be clearer if exit codes were explicitly set for each error type, and the process should exit immediately if telemetry is not tracking.

🔎 Proposed refactor for clarity

   let exitCode = 1
   try {
     await telemetry.trackCli(async () => {
       await program.parseAsync()
     })
+    exitCode = 0
   } catch (e: any) {
     if (e instanceof CommanderError) {
       // ignore
       exitCode = e.exitCode
     } else if (e instanceof CliError) {
       console.error(red(e.message))
+      exitCode = 1
     } else {
       console.error(red(`Unhandled error: ${e}`))
+      exitCode = 1
     }
-    if (telemetry.isTracking) {
-      // give telemetry a chance to send events before exit
-      setTimeout(() => {
-        process.exit(exitCode)
-      }, 200)
-    }
+  } finally {
+    if (exitCode !== 0 && telemetry.isTracking) {
+      // give telemetry a chance to send events before exit
+      setTimeout(() => {
+        process.exit(exitCode)
+      }, 200)
+    } else if (exitCode !== 0) {
+      process.exit(exitCode)
+    }
   }
 }

src/server.ts (2)

144-147: Reconsider manipulating module.paths.

Lines 144-146 directly manipulate module.paths to support pnpm monorepos. This approach is fragile and can lead to unexpected module resolution behavior, especially in complex dependency scenarios or different package managers.

Consider using require.resolve with explicit paths or documenting this as a known limitation for certain monorepo setups. Alternatively, provide a CLI option for users to specify the node_modules location.

---

196-199: Optimize adapter creation call.

Line 197 calls createAdapter(zmodelConfig, zmodelSchemaDir) even when isClientEngine is false, only to assign null to the adapter. This unnecessarily executes the adapter creation logic and its potential side effects (logging, error throwing) when the result won't be used.

🔎 Proposed optimization

   const prisma = new PrismaClient({
-    adapter: isClientEngine ? createAdapter(zmodelConfig, zmodelSchemaDir) : null,
+    adapter: isClientEngine ? createAdapter(zmodelConfig, zmodelSchemaDir) : undefined,
     log: options.logLevel || [],
   })

Note: Changed null to undefined as that's typically the expected type for optional adapter parameter. Verify PrismaClient type definition.

src/telemetry.ts (1)

36-42: Consider handling additional DO_NOT_TRACK values.

Line 37 only checks if DO_NOT_TRACK equals the string '1'. The DO_NOT_TRACK convention also recognizes other truthy values like 'true', 'TRUE', or any non-empty string. Consider a more inclusive check to respect user privacy preferences.

🔎 Proposed enhancement

   constructor() {
-    if (process.env['DO_NOT_TRACK'] !== '1' && TELEMETRY_TRACKING_TOKEN) {
+    const doNotTrack = process.env['DO_NOT_TRACK']
+    const isOptedOut = doNotTrack && doNotTrack !== '0' && doNotTrack.toLowerCase() !== 'false'
+    if (!isOptedOut && TELEMETRY_TRACKING_TOKEN) {
       this.mixpanel = init(TELEMETRY_TRACKING_TOKEN, {
         geolocate: true,
       })
     }
   }

src/zmodel-parser.ts (2)

27-33: Comment removal regex may not handle all edge cases.

The regex patterns on lines 29 and 31 remove comments but don't account for comment-like patterns inside string literals. For example, a string containing "file://path" or "https://example.com" could be incorrectly modified.

While this is unlikely to cause issues in typical ZModel schemas, consider documenting this limitation or implementing a more robust parser that respects string boundaries if schema files are expected to contain such patterns.

---

177-177: Fix typo in comment.

Line 177 has a typo: "Exact engineType" should be "Extract engineType" to match the pattern of other comments in this function.

🔎 Proposed fix

-  // Exact engineType (optional)
+  // Extract engineType (optional)
   const engineTypeMatch = generatorBlock.match(/engineType\s*=\s*['"]([^'"]+)['"]/)

Configuration used: defaults

Review profile: CHILL

Plan: Pro

src/index.ts (1)

79-85: Telemetry may not flush on success path.

The setTimeout for telemetry flush only executes in the catch block. On successful CLI execution, the process may exit before Mixpanel's async track() calls complete, potentially losing telemetry data.

Consider adding a flush mechanism on successful completion as well, or ensure Mixpanel's callbacks complete before the process naturally exits.

src/zmodel-parser.ts (2)

5-5: Consider expanding DatabaseProvider type.

The type only includes sqlite, postgresql, and mysql, but Prisma supports additional providers like sqlserver, mongodb, and cockroachdb. If users specify these providers, the cast on line 113 will silently accept invalid values.

🔎 Proposed fix

-type DatabaseProvider = 'sqlite' | 'postgresql' | 'mysql'
+type DatabaseProvider = 'sqlite' | 'postgresql' | 'mysql' | 'sqlserver' | 'mongodb' | 'cockroachdb'

---

186-191: Regex assumes specific attribute ordering in plugin block.

The regex requires provider to appear before output in the plugin block. If users write output before provider, the match will fail.

🔎 Proposed fix to handle any ordering

 export function parsePrismaSchemaPath(content: string): string | undefined {
-  const match = content.match(
-    /plugin\s+\w+\s*\{[^}]*provider\s*=\s*['"]@core\/prisma['"][^}]*output\s*=\s*['"]([^'"]+)['"][^}]*\}/s
-  )
-  return match ? match[1] : undefined
+  // Match any plugin block
+  const pluginMatch = content.match(/plugin\s+\w+\s*\{([^}]+)\}/s)
+  if (!pluginMatch) return undefined
+  
+  const block = pluginMatch[1]
+  // Check if this is the @core/prisma provider
+  const providerMatch = block.match(/provider\s*=\s*['"]@core\/prisma['"]/)
+  if (!providerMatch) return undefined
+  
+  // Extract output
+  const outputMatch = block.match(/output\s*=\s*['"]([^'"]+)['"]/)
+  return outputMatch ? outputMatch[1] : undefined
 }

Configuration used: defaults

Review profile: CHILL

Plan: Pro