chore(site): fix generating examples #3453

NathanFlurry · 2025-11-13T01:33:31Z

No description provided.

vercel · 2025-11-13T01:33:37Z

Deployment failed with the following error:

Too many requests - try again in 3 minutes (more than 120, code: "api-deployments-flood-pro").

Learn More: https://vercel.com/docs/limits#rate-limits

NathanFlurry · 2025-11-13T01:33:55Z

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

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

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 · 2025-11-13T01:39:29Z

PR Review: Fix generating examples

Summary

This PR refactors the example generation script to use local examples instead of cloning from a remote repository, and removes the unused README generation script. Overall this is a good improvement that simplifies the build process.

✅ Positive Changes

1. Simplified Architecture

Removing the git clone dependency makes the build process more reliable and faster
Using local examples from ../../examples is a cleaner approach for a monorepo
Good use of fileURLToPath and dirname for proper ESM path resolution

2. Improved Version Management

The new getLatestVersion() function with caching is excellent:

Fetches real npm versions instead of hardcoding
Caches versions to avoid repeated lookups
Has proper error handling with fallback

3. Better Dependency Handling

The refactored replaceWorkspaceDependencies() is more robust:

Processes both dependencies and devDependencies
Uses proper JSON parsing/stringification instead of regex
Maintains proper formatting with tabs

4. Documentation Cleanup

Removed unused generateReadme.mjs (169 lines)
Consolidated quickstart navigation in the docs

⚠️ Issues & Recommendations

1. Security: Command Injection Risk (High Priority)

Location: website/scripts/generateExamples.mjs:31

const result = execSync(`npm view ${packageName} version`, { encoding: 'utf-8' });

Issue: If a package.json contains a malicious package name with shell metacharacters, this could execute arbitrary commands.

Recommendation:

function getLatestVersion(packageName) {
  try {
    // Validate package name format
    if (!/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/.test(packageName)) {
      console.warn(`Warning: Invalid package name "${packageName}", using ^0.9.1`);
      return '0.9.1';
    }
    const result = execSync('npm view ' + JSON.stringify(packageName) + ' version', { 
      encoding: 'utf-8',
      shell: '/bin/sh'
    });
    return result.trim();
  } catch (error) {
    console.warn(`Warning: Could not fetch version for ${packageName}, using ^0.9.1`);
    return '0.9.1';
  }
}

2. Error Handling: Silent Failures

Location: website/scripts/generateExamples.mjs:163-166

The catch block silently skips files but throws an error that interrupts processing:

} catch (readError) {
  // Skip binary files or files that can't be read as text
  throw new Error(`Failed to read file ${relativePath}: ${readError.message}`);
}

Issue: The comment says "skip" but it actually throws. This could fail the entire build for a single unreadable file.

Recommendation:

} catch (readError) {
  // Skip binary files or files that can't be read as text
  console.warn(`Skipping file ${relativePath}: ${readError.message}`);
  continue;
}

3. Platform Compatibility Issue

Location: website/scripts/generateExamples.mjs:138

const allFiles = execSync('find . -type f -not -path "*/.git/*"', {

Issue: The find command is Unix-specific and will fail on Windows.

Recommendation: Use Node.js built-in functions for cross-platform compatibility:

import { readdirSync, statSync } from 'node:fs';

function getAllFilesRecursive(dir, baseDir = dir) {
  const files = [];
  for (const file of readdirSync(dir)) {
    const fullPath = join(dir, file);
    const stat = statSync(fullPath);
    if (stat.isDirectory() && file !== '.git') {
      files.push(...getAllFilesRecursive(fullPath, baseDir));
    } else if (stat.isFile()) {
      files.push(relative(baseDir, fullPath));
    }
  }
  return files;
}

4. Performance: Network Calls

Issue: The script makes an npm registry call for every unique package, which could be slow if there are many packages.

Recommendation: Consider batching or adding a timeout:

execSync('npm view ' + JSON.stringify(packageName) + ' version', { 
  encoding: 'utf-8',
  timeout: 5000  // 5 second timeout
});

5. Minor: Inconsistent Error Messages

Location: Multiple locations

Some errors use template literals, others use concatenation. Choose one style for consistency:

// Line 23
throw new Error(`Examples directory not found at ${EXAMPLES_DIR}`);
// Line 80
throw new Error(`Example defined in metadata but not found in repo: ${exampleName}`);

This is good and consistent.

6. Testing Gap

The PR doesn't include tests for the refactored script. Consider adding:

Unit tests for getLatestVersion()
Tests for replaceWorkspaceDependencies()
Integration test that runs the script on a test example

7. Documentation

The comment on line 1 of examplesData.mjs was updated but could be more descriptive:

// Shared example metadata used by generateExamples.mjs to build the examples gallery

🔍 Code Quality

Good Practices Observed:

✅ Proper cleanup with try/finally blocks
✅ Clear function names and separation of concerns
✅ Good error messages with context
✅ Consistent code style

Adherence to CLAUDE.md:

✅ Uses pnpm workspace correctly
✅ Follows conventional commit format: chore(site): fix generating examples
⚠️ The commit message could be more descriptive about what was actually fixed

📊 Impact Assessment

Risk Level: Low-Medium

The changes are isolated to build scripts
Removing remote dependency is a positive change
Security issue should be addressed but impact is limited to development

Breaking Changes: None

Output format remains the same
No API changes

Performance Impact: Positive

No more git cloning during builds
Faster local file access

🎯 Recommendations Priority

High Priority: Fix command injection vulnerability (Security)
Medium Priority: Fix platform compatibility (find command)
Medium Priority: Fix error handling to not throw on binary files
Low Priority: Add timeout to npm calls
Low Priority: Add tests for the refactored code

✅ Approval Status

Conditional Approve - The changes are good overall, but the security issue should be addressed before merging. The platform compatibility issue should also be fixed for Windows developers.

Great work simplifying the build process! 🎉

vercel · 2025-11-13T02:47:18Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-site	Error			Nov 13, 2025 9:10pm
rivetkit-serverless	Error			Nov 13, 2025 9:10pm

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 9:10pm
rivet-inspector	Ignored	Preview		Nov 13, 2025 9:10pm

claude · 2025-11-13T03:06:56Z

PR Review: chore(site): fix generating examples

Summary

This PR refactors the examples generation script to use local examples instead of cloning from a remote repository, removes the unused generateReadme.mjs script, and reorganizes the documentation structure by moving quickstart content to the main docs index.

Code Quality & Best Practices

Positive Changes:

Simplified Architecture: Removing the git clone approach in favor of local examples is cleaner and more maintainable
Better Dependency Version Resolution: The new getLatestVersion() function dynamically fetches package versions from npm
Improved Caching: Added versionCache to avoid redundant npm lookups (line 40)
Proper JSON Manipulation: Using JSON.parse() and JSON.stringify() instead of regex replacement is more robust
Good Error Handling: Added proper validation and error messages throughout
Cleanup Logic: Using try...finally blocks to ensure temp directories are cleaned up

Code Style:

Consistent with existing patterns
Good use of early returns and guard clauses
Clear function naming and separation of concerns

claude · 2025-11-13T03:07:07Z

Potential Issues & Concerns

1. Error Handling in getLatestVersion() (Line 29-36)

Issue: Silent fallback to hardcoded version 0.9.1 has a version string mismatch

The function returns '0.9.1' but the warning message and later usage (line 56) expect '^0.9.1'

Recommendation:

Fix: Change line 36 from return '0.9.1' to return '^0.9.1'
Consider logging the actual error for debugging
Consider making this a build failure if critical packages cannot be resolved

2. Potential Race Condition with Temp Directories

Issue: Using a shared TEMP_EXAMPLE_DIR could cause issues if multiple instances run concurrently

Recommendation: Use a unique temp directory per invocation (though this may be acceptable for a build script that typically runs sequentially)

3. Binary File Handling (Line 163-166)

Issue: The comment says Skip binary files but the code actually throws an error

Recommendation: Either skip binary files gracefully or update the comment to reflect the actual behavior

4. Documentation Navigation Change

The quickstart section was moved from /docs/quickstart to /docs/index.mdx. This is a breaking change for users with bookmarked links.

Recommendation: Consider adding a redirect from /docs/quickstart to /docs to avoid 404s

claude · 2025-11-13T03:07:17Z

Performance & Security

Performance - Positive:

Version Caching: The versionCache prevents redundant npm calls (good optimization!)
No Network Cloning: Using local examples is much faster than cloning

Performance - Potential Improvement:

Running npm install for each example could be slow. Consider parallel processing or caching node_modules between runs

Security Concerns:

Command Injection Risk (Line 138): Using find via shell execution could be vulnerable if directory paths contain special characters

Severity: Low (controlled environment)
Recommendation: Use Node.js native fs.readdirSync with recursive option instead of shell commands

Test Coverage

Missing: No tests were added or modified for this script

Recommendation: Consider adding:

Unit tests for replaceWorkspaceDependencies() function
Integration test that verifies examples can be generated successfully
Test that validates all examples in EXAMPLE_METADATA exist in the examples directory

claude · 2025-11-13T03:07:30Z

Summary & Recommendations

Overall Assessment: This is a good refactoring that simplifies the build process

Priority Fixes:

Fix the version string mismatch in line 36 of website/scripts/generateExamples.mjs (return '0.9.1' should be return '^0.9.1' to match the warning message and usage in line 56)
Decide whether binary files should throw or be skipped (line 163-166) - update either code or comment
Add a redirect for the removed /docs/quickstart route to avoid broken bookmarks

Nice to Have:

Use unique temp directories to avoid race conditions
Replace find command with Node.js native file operations
Add test coverage for the generation script
Consider parallel example processing for performance

Additional Observations:

generateReadme.mjs removal is justified
The quickstart page consolidation makes sense for better UX
Sitemap changes improve navigation by promoting quickstart to main docs level
Commented-out Use Cases section suggests ongoing restructuring - ensure this is intentional
Added @vbare/compiler and @biomejs/biome dependencies appear appropriate

Approval: Safe to merge after addressing the version string mismatch in priority fix #1

Review conducted following CLAUDE.md conventions

…pport it

…cket

- Added new Quickstart tab next to Overview and Integrations in docs navigation - Created new quickstart page at /docs/quickstart/ - Removed quickstart section from Overview tab sidebar - Updated all 'Get Started' buttons on home page to point to /docs/quickstart/ - Updated Local Development quickstart arrow to point to /docs/quickstart/ - Added quickstart to mobile navigation dropdown

claude · 2025-11-13T21:17:07Z

PR Review: Fix generating examples

Overview

This PR refactors the example generation system by removing the git cloning mechanism and instead using local examples directly from the monorepo. It also removes the generateReadme.mjs script and consolidates quickstart documentation.

✅ Positive Changes

Simplified Architecture: Removing the git clone mechanism in favor of local examples is a smart improvement that:
- Reduces external dependencies and potential network failures
- Speeds up the build process significantly
- Makes the system more reliable and easier to debug
- Ensures examples are always in sync with the current codebase
Better Dependency Management: The new getLatestVersion() function with caching is an improvement:
- Fetches actual published versions from npm instead of hardcoding
- Implements a version cache to avoid repeated lookups
- Provides fallback handling when npm queries fail
Cleaner Code Organization:
- Properly uses ES modules patterns (__dirname via fileURLToPath)
- Better separation of concerns with focused functions
- Improved error messages with more context
Documentation Consolidation: Moving quickstart content directly into /docs/index.mdx reduces navigation depth and improves UX

🐛 Potential Issues

Critical: Command Injection Risk

Location: generateExamples.mjs:138

const allFiles = execSync('find . -type f -not -path "*/.git/*"', { 
  cwd: tempExampleDir,
  encoding: 'utf-8'
}).trim().split('\n');

Issue: Using shell commands for file operations is unnecessary and could be a security risk if tempExampleDir is ever influenced by external input.

Recommendation: Replace with native Node.js file system operations:

import { readdirSync, statSync } from 'node:fs';

function getAllFilesRecursively(dir, fileList = []) {
  const files = readdirSync(dir);
  
  for (const file of files) {
    const filePath = join(dir, file);
    if (statSync(filePath).isDirectory()) {
      if (file !== '.git') {
        getAllFilesRecursively(filePath, fileList);
      }
    } else {
      fileList.push(filePath);
    }
  }
  
  return fileList;
}

Error Handling: Silent Failures

Location: generateExamples.mjs:29-36

function getLatestVersion(packageName) {
  try {
    const result = execSync(`npm view ${packageName} version`, { encoding: 'utf-8' });
    return result.trim();
  } catch (error) {
    console.warn(`Warning: Could not fetch version for ${packageName}, using ^0.9.1`);
    return '0.9.1';
  }
}

Issue: The fallback version 0.9.1 might be outdated and could cause compatibility issues. Users won't know about this silent fallback unless they read console output.

Recommendation:

Consider failing fast if npm queries fail, or
Make the fallback version configurable, or
At minimum, log the actual error for debugging purposes

Duplicate Directory Check

Location: generateExamples.mjs:66-69 and generateExamples.mjs:21-26

Both verifyExamplesDir() and getExamplesToProcess() check if EXAMPLES_DIR exists. This is redundant.

Recommendation: Remove the check from getExamplesToProcess() since verifyExamplesDir() is called first.

🔍 Code Quality Observations

1. Hard-coded Tab Indentation

Location: generateExamples.mjs:62

return JSON.stringify(packageJson, null, '\t');

While this follows the CLAUDE.md preference for hard tabs in Rust, it's applied to JavaScript JSON files here. Verify this is intentional for the generated package.json files. Most npm ecosystems use 2 spaces.

2. Error Message Improvement Needed

Location: generateExamples.mjs:165

The error message Failed to read file ${relativePath}: ${readError.message} is thrown but wraps legitimate binary file skips. Consider distinguishing between actual read errors and expected binary files.

3. Missing Cleanup on Early Exit

Location: generateExamples.mjs:202-205

if (exampleNames.length === 0) {
  console.error('No examples found in the repository');
  return;
}

If this early return happens, the TEMP_EXAMPLE_DIR created at line 195-197 won't be cleaned up (though the finally block at 254 would handle subsequent failures).

🎯 Performance Considerations

Good:

The version caching mechanism prevents redundant npm queries
Removing git operations significantly improves performance
Processing examples in sequence is appropriate given the npm install step

Consider:

For very large monorepos with many examples, processing could be parallelized (after npm install completes for each)
The npm install for each example could be slow; consider a progress indicator for better UX

🔒 Security Assessment

Low Risk Overall, but note:

Command injection risk with find command (mentioned above)
execSync with stdio: 'inherit' exposes output directly, which is fine for build scripts
No user input is processed, so risk is minimal
Temp directory usage (/tmp) is standard but could potentially have race conditions in multi-process scenarios

📋 Testing Recommendations

Verify the script works with all examples by running:
```
cd website && pnpm gen:examples
```
Test failure scenarios:
- What happens when an example has invalid package.json?
- What happens when npm is unavailable?
- What happens when an example defined in metadata doesn't exist?
Check generated output: Inspect website/src/data/examples/examples.ts to ensure:
- All examples are present
- Package.json files have correct versions
- No workspace references remain
Verify documentation changes by checking:
- Navigation still works correctly
- Quickstart links resolve properly
- No broken internal links

📊 Overall Assessment

Quality: Good ✅
Risk Level: Low-Medium ⚠️
Recommendation: Approve with minor changes

This is a solid refactoring that simplifies the architecture and removes external dependencies. The main concerns are:

The command injection risk with find (should be fixed)
Silent version fallback behavior (consider improving error visibility)

The documentation changes are sensible and improve UX. The removal of generateReadme.mjs is appropriate given the context.

🛠️ Suggested Action Items

Before Merge:

Replace find command with native Node.js file operations
Add better error logging for npm version lookup failures
Test the script with pnpm gen:examples to verify it works

Optional Improvements:

Remove duplicate directory existence checks
Add progress indicators for long-running npm installs
Consider adding a --dry-run flag for testing

Great work on this refactoring! 🚀

This was referenced Nov 13, 2025

feat(site): add typedoc docs #3446

Merged

chore: create branch for v3 website changes on v2 #3304

Open

chore(website): flatten actors docs structure #3280

Open

NathanFlurry force-pushed the 11-09-chore_site_fix_generating_examples branch from 0031d2e to 9ea29d6 Compare November 13, 2025 02:47

vercel bot had a problem deploying to Preview – rivet-site November 13, 2025 02:47 Failure

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 02:47 Failure

NathanFlurry force-pushed the 11-09-chore_site_fix_generating_examples branch from 9ea29d6 to ba3e30a Compare November 13, 2025 03:24

NathanFlurry force-pushed the 10-25-chore_website_changes_v3_on_v2 branch from 457d118 to 72676eb Compare November 13, 2025 03:24

vercel bot had a problem deploying to Preview – rivet-site November 13, 2025 03:24 Failure

NathanFlurry and others added 23 commits November 13, 2025 12:51

chore(rivetkit): add conn type

1f30194

chore(rivetkit): add WebSocketContext and RequestContext

57377f5

refactor(rivetkit): restructure connection lifecycle contexts

714660b

refactor(rivetkit): extract WebSocket protocol parsing to shared utility

e10206c

chore(rivetkit): add listing actors by only name to manager api

a8f3a9c

chore(rivetkit): add required zod validation for json encoding

1784d8f

feat(rivetkit): add json protocol support

412f83b

fix(rivetkit): remove incorrect getEndpoint call in metadata handler

0bc4b29

fix(rivetkit): standardize error metadata return type

68402d1

fix(rivetkit): fix race condition with websocket open events

afc9c8e

fix(rivetkit): skip sending RivetKit messages to conns that do not su…

51a32d6

…pport it

chore(rivetkit): add actor router to the openapi spec

898bc9a

feat(rivetkit): add asyncapi spec

9a434c7

chore(rivetkit): add support for conn params on onRequest and onWebSo…

7d2f44d

…cket

chore(rivetkit): remove deprecated packages

aaf96c9

feat(site): add typedoc docs

0e6327a

chore(rivetkit-typescript): remove dependency on node modules

7bda860

chore(rivetkit): switch dynamic node imports to use require

0ceae4a

chore(website): flatten actors docs structure

1a35970

chore: create branch for v3 website changes on v2

858d55d

chore(site): website changes v3

cad6172

chore(site): fix generating examples

0911e7e

NathanFlurry force-pushed the 10-25-chore_website_changes_v3_on_v2 branch from 72676eb to afd0d10 Compare November 13, 2025 21:09

NathanFlurry force-pushed the 11-09-chore_site_fix_generating_examples branch from ba3e30a to 0911e7e Compare November 13, 2025 21:09

vercel bot had a problem deploying to Preview – rivet-site November 13, 2025 21:10 Failure

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 21:10 Failure

NicholasKissel force-pushed the 10-25-chore_website_changes_v3_on_v2 branch from afd0d10 to 8010332 Compare November 15, 2025 02:22

NicholasKissel mentioned this pull request Nov 15, 2025

chre: v4 website changes #3472

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(site): fix generating examples #3453

chore(site): fix generating examples #3453

Uh oh!

NathanFlurry commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025

Uh oh!

NathanFlurry commented Nov 13, 2025 •

edited by NicholasKissel

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore(site): fix generating examples #3453

Are you sure you want to change the base?

chore(site): fix generating examples #3453

Uh oh!

Conversation

NathanFlurry commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025

Uh oh!

NathanFlurry commented Nov 13, 2025 • edited by NicholasKissel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Nov 13, 2025

PR Review: Fix generating examples

Summary

✅ Positive Changes

1. Simplified Architecture

2. Improved Version Management

3. Better Dependency Handling

4. Documentation Cleanup

⚠️ Issues & Recommendations

1. Security: Command Injection Risk (High Priority)

2. Error Handling: Silent Failures

3. Platform Compatibility Issue

4. Performance: Network Calls

5. Minor: Inconsistent Error Messages

6. Testing Gap

7. Documentation

🔍 Code Quality

Good Practices Observed:

Adherence to CLAUDE.md:

📊 Impact Assessment

🎯 Recommendations Priority

✅ Approval Status

Uh oh!

vercel bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 13, 2025

PR Review: chore(site): fix generating examples

Summary

Code Quality & Best Practices

Positive Changes:

Code Style:

Uh oh!

claude bot commented Nov 13, 2025

Potential Issues & Concerns

1. Error Handling in getLatestVersion() (Line 29-36)

2. Potential Race Condition with Temp Directories

3. Binary File Handling (Line 163-166)

4. Documentation Navigation Change

Uh oh!

claude bot commented Nov 13, 2025

Performance & Security

Performance - Positive:

Performance - Potential Improvement:

Security Concerns:

Test Coverage

Uh oh!

claude bot commented Nov 13, 2025

Summary & Recommendations

Priority Fixes:

Nice to Have:

Additional Observations:

Uh oh!

claude bot commented Nov 13, 2025

PR Review: Fix generating examples

Overview

✅ Positive Changes

🐛 Potential Issues

Critical: Command Injection Risk

Error Handling: Silent Failures

Duplicate Directory Check

🔍 Code Quality Observations

1. Hard-coded Tab Indentation

2. Error Message Improvement Needed

3. Missing Cleanup on Early Exit

🎯 Performance Considerations

🔒 Security Assessment

NathanFlurry commented Nov 13, 2025 •

edited by NicholasKissel

Loading

vercel bot commented Nov 13, 2025 •

edited

Loading