Fix npm install breaking on Node versions other than 22.0.0#207
Merged
Conversation
🤖 My Senior Dev — Analysis Complete👤 For @khaliqgant📁 Expert in View your contributor analytics → 📊 83 files reviewed • 47 need attention
🚀 Open Interactive Review →The full interface unlocks features not available in GitHub:
💬 Chat here: 📖 View all 12 personas & slash commandsYou can interact with me by mentioning In PR comments or on any line of code:
Slash commands:
AI Personas (mention to get their perspective):
For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews. |
khaliqgant
force-pushed
the
claude/fix-issue-204-uNZ6y
branch
from
43875b6 to
48758f1
Compare
Change .nvmrc from exact version v22.0.0 to major version 22, allowing any 22.x.x version to be used. This fixes compatibility issues for users running different minor/patch versions of Node 22. Fixes #204
- Remove node-pty from package.json dependencies - Update spawner.ts to require relay-pty binary (no fallback) - Simplify AgentWrapper type to only use RelayPtyOrchestrator This improves Node.js version compatibility by removing the native node-pty module that requires compilation for each Node version. The relay-pty Rust binary provides PTY functionality instead. Related to #204
BREAKING: Removes node-pty native module dependency and migrates all PTY functionality to use the relay-pty Rust binary. Changes: - Remove node-pty from package.json dependencies - Update spawner.ts to require relay-pty binary (no fallback) - Migrate cli-pty-runner.ts to use relay-pty via child_process.spawn - Migrate cli-auth.ts to use relay-pty for auth flows - Update onboarding.ts to remove IPty type dependency - Create wrapper-types.ts for shared event type definitions - Update imports in agent-manager.ts, persistence.ts to use wrapper-types.ts This improves Node.js version compatibility (including Node 25) by eliminating native module compilation requirements. The relay-pty Rust binary provides equivalent PTY functionality without version constraints. The relay-pty binary must be available for: - Agent spawning via Spawner - CLI authentication flows - CLI PTY runner operations Fixes #204
Add GitHub Actions workflow that tests npm install across multiple Node.js versions (18, 20, 22, 24, 25) using Docker containers. Tests include: - Full install and build in official Node images - npm pack and global installation (npm install -g) - ESM import verification - Minimal install in slim images (no build tools) This ensures the package installs correctly after removing the node-pty native module dependency. Related to #204
Replace references to node-pty with relay-pty in: - spawner.ts file header and spawn method doc - spawner.test.ts file header
The PtyWrapper class is no longer used after migrating to RelayPtyOrchestrator. Remove the file to fix TypeScript compilation errors from the missing node-pty dependency. Event types have been moved to wrapper-types.ts.
- Use local import (./dist/index.js) instead of global package import - Add npm test to verify functionality after build - Remove problematic global install test - Add fresh install test (deletes package-lock.json first) - Verify native modules compile correctly
Update test scripts in scripts/test-cli-auth/ to use relay-pty binary instead of node-pty: - ci-test-runner.ts: Migrated to child_process.spawn with relay-pty - test-oauth-flow.ts: Migrated to child_process.spawn with relay-pty - package.json: Removed node-pty dependency This completes the removal of node-pty from the entire codebase.
Remove scripts/test-pty-input.js and scripts/test-pty-input-auto.js which were manual debugging scripts that used node-pty. These are not part of the test suite and would fail without node-pty.
khaliqgant
force-pushed
the
claude/fix-issue-204-uNZ6y
branch
from
3baa1b0 to
093c42f
Compare
Add multi-stage Docker build that compiles relay-pty from source: - Stage 1: Build relay-pty using rust:1.75-slim - Stage 2: Copy binary to runtime image This fixes the CLI OAuth tests which now depend on relay-pty instead of node-pty for PTY emulation.
Add overrides for react and react-dom to resolve peer dependency conflict. drizzle-orm@0.45.1 has optional peer dependencies on React Native packages (expo-sqlite, @op-engineering/op-sqlite) that require React 19, but this web project uses React 18.3.1. The overrides tell npm to use React 18 regardless of what the optional React Native packages want. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change .nvmrc from exact version v22.0.0 to major version 22, allowing any 22.x.x version to be used. This fixes compatibility issues for users running different minor/patch versions of Node 22. Fixes #204
- Remove node-pty from package.json dependencies - Update spawner.ts to require relay-pty binary (no fallback) - Simplify AgentWrapper type to only use RelayPtyOrchestrator This improves Node.js version compatibility by removing the native node-pty module that requires compilation for each Node version. The relay-pty Rust binary provides PTY functionality instead. Related to #204
BREAKING: Removes node-pty native module dependency and migrates all PTY functionality to use the relay-pty Rust binary. Changes: - Remove node-pty from package.json dependencies - Update spawner.ts to require relay-pty binary (no fallback) - Migrate cli-pty-runner.ts to use relay-pty via child_process.spawn - Migrate cli-auth.ts to use relay-pty for auth flows - Update onboarding.ts to remove IPty type dependency - Create wrapper-types.ts for shared event type definitions - Update imports in agent-manager.ts, persistence.ts to use wrapper-types.ts This improves Node.js version compatibility (including Node 25) by eliminating native module compilation requirements. The relay-pty Rust binary provides equivalent PTY functionality without version constraints. The relay-pty binary must be available for: - Agent spawning via Spawner - CLI authentication flows - CLI PTY runner operations Fixes #204
Add GitHub Actions workflow that tests npm install across multiple Node.js versions (18, 20, 22, 24, 25) using Docker containers. Tests include: - Full install and build in official Node images - npm pack and global installation (npm install -g) - ESM import verification - Minimal install in slim images (no build tools) This ensures the package installs correctly after removing the node-pty native module dependency. Related to #204
Replace references to node-pty with relay-pty in: - spawner.ts file header and spawn method doc - spawner.test.ts file header
The PtyWrapper class is no longer used after migrating to RelayPtyOrchestrator. Remove the file to fix TypeScript compilation errors from the missing node-pty dependency. Event types have been moved to wrapper-types.ts.
- Use local import (./dist/index.js) instead of global package import - Add npm test to verify functionality after build - Remove problematic global install test - Add fresh install test (deletes package-lock.json first) - Verify native modules compile correctly
Update test scripts in scripts/test-cli-auth/ to use relay-pty binary instead of node-pty: - ci-test-runner.ts: Migrated to child_process.spawn with relay-pty - test-oauth-flow.ts: Migrated to child_process.spawn with relay-pty - package.json: Removed node-pty dependency This completes the removal of node-pty from the entire codebase.
Remove scripts/test-pty-input.js and scripts/test-pty-input-auto.js which were manual debugging scripts that used node-pty. These are not part of the test suite and would fail without node-pty.
Add multi-stage Docker build that compiles relay-pty from source: - Stage 1: Build relay-pty using rust:1.75-slim - Stage 2: Copy binary to runtime image This fixes the CLI OAuth tests which now depend on relay-pty instead of node-pty for PTY emulation.
Add overrides for react and react-dom to resolve peer dependency conflict. drizzle-orm@0.45.1 has optional peer dependencies on React Native packages (expo-sqlite, @op-engineering/op-sqlite) that require React 19, but this web project uses React 18.3.1. The overrides tell npm to use React 18 regardless of what the optional React Native packages want. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
khaliqgant
force-pushed
the
claude/fix-issue-204-uNZ6y
branch
from
89edb94 to
51ae0e4
Compare
4 tasks
The check for 'waitUntilCliReady' in pty is sufficient to determine if we can call the async method. The useRelayPty variable doesn't exist and was likely left over from incomplete refactoring. This resolves the TS2304 compilation error on line 642.
… claude/fix-issue-204-uNZ6y Resolve package-lock.json conflict by accepting remote and regenerating. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Regenerate package-lock.json with proper resolved/integrity fields for cross-platform optional dependencies (fixes rollup-linux-x64-gnu missing on CI) - Fix all req.params destructuring in Express route handlers to use explicit string casts, as Express 5 types params as `string | string[]` Affected files: - admin.ts, billing.ts, codex-auth-helper.ts, consensus.ts, coordinators.ts - daemons.ts, generic-webhooks.ts, github-app.ts, monitoring.ts, nango-auth.ts - onboarding.ts, policy.ts, providers.ts, repos.ts, teams.ts, test-helpers.ts - workspaces.ts, server.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add proper localStorage mock to Sidebar.archive.test.tsx to match the main Sidebar.test.tsx file. The test was failing because localStorage.clear() wasn't a function in the test environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Conflicts resolved: - src/cloud/api/providers.ts: Used main's variable rename (providerParam) - src/wrapper/pty-wrapper.ts: Kept deletion (file removed intentionally) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…rsion Fix: Update Docker Rust to 1.83 for Cargo.lock v4 support
Resolved conflict in src/cloud/server.ts GET /api/channels/:channel/members endpoint - kept the complete implementation from main that queries the database for channel members with fallback to dashboard proxy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Cursor agent support: CLI type detection, auth config, spawn modal, Docker install, cloud provider registry - Fix connection storm bug: UserBridge now exposes getRelayClient() to prevent duplicate relay clients for same username - Consolidate provider connection UI: Created shared ProviderConnectionList component used by both /app and /providers pages - Add relay-pty build stage to Dockerfile.local for local development - Remove remaining node-pty references (relay-pty is now the standard) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add --yolo flag to Gemini agents in spawner for auto-accept prompts - Add prompt handlers for Gemini API key confirmation and terms acceptance - Add Gemini to AI_PROVIDERS list in both onboarding pages - Pass workspaceId to mark-connected endpoint in TerminalProviderSetup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Gemini CLI shows an interactive menu where users can choose: - Login with Google (OAuth) - Enter Gemini API key Using terminal-based setup in both onboarding and workspace settings gives users the flexibility to choose their preferred auth method. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Conflict resolutions: - .trajectories/index.json: Kept all trajectories from both branches - src/cloud/server.ts: Used workspace.publicUrl routing for channel messages (per cloud-websocket-routing rule) Additional fix: - scripts/test-cli-auth/Dockerfile.real: Update Rust from 1.75 to 1.82 (Cargo.lock v4 requires Rust 1.78+) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Express query params can be string | string[] - use proper type narrowing instead of unsafe casts to fix TS2345 error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Without this mapping, when 'google' was passed as the CLI from TerminalProviderSetup, it wasn't being mapped to 'gemini', so the --yolo flag was never applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused imports (afterEach, vi, beforeEach, path, DeadLetter, PoolClient, ChannelJoinPayload, ChannelLeavePayload, RelayPtyOrchestratorConfig) - Prefix unused variables with underscore (displayName, channel, member, workspaceId, messages, errorMessage, channelId, upToMessageId, joinChannel, result, content, id, err) - Add eslint-disable comment for prefer-const on forward declaration - Change catch(err: any) to catch(_err: unknown) for type safety 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Gemini's interactive terminal allows users to choose between: - Login with Google (OAuth) - Use Gemini API Key Removing the separate API key input form simplifies the UX. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change .nvmrc from exact version v22.0.0 to major version 22, allowing
any 22.x.x version to be used. This fixes compatibility issues for users
running different minor/patch versions of Node 22.
Fixes #204