fix(install): exclude __pycache__ and Python bytecode from install plans - #2728
Conversation
listFilesRecursive skipped node_modules and .git but descended into __pycache__, so any bytecode left in the repo was planned as an installable source file and copied into the install root (e.g. ~/.claude/scripts/lib/__pycache__/*.pyc). Running the bundled Tkinter dashboard imports scripts/lib/ecc_dashboard_runtime.py and creates that directory, so a normal `npm run dashboard` followed by an install shipped stale bytecode -- possibly built for a different Python version than the one in use. package.json `files` already excludes **/__pycache__/**, *.pyc, *.pyo and *.pyd, so npm pack dropped these while the installer picked them up. Align the planner with that intent. Closes affaan-m#2727 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe install planner now excludes ChangesInstall planner filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/lib/install-executor.test.js`:
- Around line 57-58: Update the install-executor test fixtures in both relevant
fixture sections to include excluded .pyo and .pyd files alongside the existing
.pyc files, including the manifest fixture. Extend each planned-operations
assertion to verify that none of .pyc, .pyo, or .pyd is included.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8e7b119-9741-4ceb-99b7-c6e32b9ea5cc
📒 Files selected for processing (2)
scripts/lib/install-executor.jstests/lib/install-executor.test.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
{package.json,*.config.js,scripts/**/*.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Package manager detection should support npm, pnpm, yarn, and bun, with configuration via CLAUDE_PACKAGE_MANAGER environment variable or project config.
Files:
scripts/lib/install-executor.js
scripts/**/*.js
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure cross-platform support for Windows, macOS, and Linux via Node.js scripts in the scripts/ directory.
Files:
scripts/lib/install-executor.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Delegate complex features to a planner agent, architectural decisions to an architect agent, modified code to a code-reviewer agent, and security-sensitive work to a security-reviewer agent; use parallel agents for independent operations.
Never compromise security; validate all inputs and prevent hardcoded secrets, injection, XSS, CSRF, authentication or authorization failures, sensitive error leakage, and missing rate limits.
Never hardcode secrets; use environment variables or a secret manager, validate required secrets at startup, and rotate exposed secrets immediately.
Always create new objects and never mutate existing ones.
Plan complex features before implementation, identifying dependencies, risks, and phases.
Prefer many small, focused files; keep functions under 50 lines, files under 800 lines where practical, avoid nesting deeper than four levels, and use readable, well-named identifiers.
Handle errors at every level, provide user-friendly UI messages, log detailed server-side context, and never silently swallow errors.
Validate all user input at system boundaries using schema-based validation; fail fast with clear messages and never trust external data.
Required tests include unit tests, integration tests for APIs and database operations, and end-to-end tests for critical user flows.
Follow the mandatory TDD cycle: write a failing test, implement the minimum passing solution, then refactor and verify coverage.
Use a consistent API response envelope containing a success indicator, data payload, error message, and pagination metadata.
Encapsulate data access behind a repository interface with operations such asfindAll,findById,create,update, anddelete; business logic must depend on the abstraction rather than storage details.
Files:
scripts/lib/install-executor.jstests/lib/install-executor.test.js
{scripts,bin}/**
⚙️ CodeRabbit configuration file
{scripts,bin}/**: Focus on command injection, unsafe subprocess usage, path traversal, SSRF, secret exposure, and missing tests for new CLI behavior.
Files:
scripts/lib/install-executor.js
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write tests before implementation (test-driven development); target 80%+ coverage
Achieve minimum 80% test coverage across all three layers: Unit, Integration, and E2E
Use AAA structure (Arrange / Act / Assert) in tests with descriptive test names that explain behavior under test
Files:
tests/lib/install-executor.test.js
🧠 Learnings (1)
📚 Learning: 2026-07-16T15:23:29.177Z
Learnt from: nankingjing
Repo: affaan-m/ECC PR: 2495
File: tests/lib/shell-substitution.test.js:12-24
Timestamp: 2026-07-16T15:23:29.177Z
Learning: In this repository, standalone JavaScript test suites under tests/lib/ follow a local runner convention: they use mutable `passed`/`failed` counters and print per-test console output. During code reviews, treat this as the expected harness style and generally avoid recommending one-off refactors to immutable counters for new/modified suites. Only request such counter refactors if the repository-wide test harness/convention is being changed.
Applied to files:
tests/lib/install-executor.test.js
🔇 Additional comments (1)
scripts/lib/install-executor.js (1)
83-84: LGTM!Also applies to: 105-107
| writeFile(root, path.join('rules', 'common', '__pycache__', 'ignored.cpython-314.pyc'), 'ignored\n'); | ||
| writeFile(root, path.join('rules', 'common', 'stray.pyc'), 'ignored\n'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover all excluded Python extensions.
The new filter excludes .pyc, .pyo, and .pyd, but these fixtures and assertions exercise only .pyc. Add .pyo and .pyd files to both fixtures. Assert that none of the three extensions appears in planned operations.
Suggested test update
writeFile(root, path.join('rules', 'common', 'stray.pyc'), 'ignored\n');
+ writeFile(root, path.join('rules', 'common', 'stray.pyo'), 'ignored\n');
+ writeFile(root, path.join('rules', 'common', 'stray.pyd'), 'ignored\n');
- assert.ok(!plan.operations.some(operation => operation.sourceRelativePath.endsWith('.pyc')));
+ assert.ok(!plan.operations.some(operation => (
+ ['.pyc', '.pyo', '.pyd'].some(extension => (
+ operation.sourceRelativePath.endsWith(extension)
+ ))
+ )));Apply the same changes to the manifest fixture and assertion.
Also applies to: 116-117, 199-200, 363-364
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lib/install-executor.test.js` around lines 57 - 58, Update the
install-executor test fixtures in both relevant fixture sections to include
excluded .pyo and .pyd files alongside the existing .pyc files, including the
manifest fixture. Extend each planned-operations assertion to verify that none
of .pyc, .pyo, or .pyd is included.
|
haelyra
left a comment
haelyra
left a comment
There was a problem hiding this comment.
Thank you for fixing this installer leak. Excluding Python cache directories and bytecode keeps install plans aligned with the package publish surface and prevents stale interpreter artifacts from being copied into user installations. All 16 focused installer tests pass, ESLint is clean on both changed files, and the diff is clean. Approved for the serial main-branch merge gate.
|
Superseded by merged installer hotfix #2784. Your Python bytecode exclusion was integrated, tested in the full cross-platform matrix, and preserved with co-author credit. Thank you. |
2 participants
Fixes #2727.
Problem
listFilesRecursiveinscripts/lib/install-executor.jsskippednode_modulesand.git, but descended into__pycache__. Any Python bytecode left in the repo was planned as an installable source file and copied into the install root:This is reachable from normal use:
npm run dashboardrunsecc_dashboard.py, which importsscripts/lib/ecc_dashboard_runtime.pyand createsscripts/lib/__pycache__/. Every install after that ships the bytecode — potentially compiled for a different Python version than the user runs.package.jsonfilesalready carries"!**/__pycache__/**","!**/*.pyc","!**/*.pyo"and"!**/*.pyd", sonpm packdropped these while the installer picked them up. This change aligns the planner with that existing intent.Change
IGNORED_DIRECTORY_NAMESgains__pycache__.IGNORED_FILE_EXTENSIONS(.pyc,.pyo,.pyd) covers bytecode that lands outside a__pycache__directory.Tests
tests/lib/install-executor.test.jsalready asserted thatnode_modulesand.gitare excluded, using fixtures that write a decoy file into each. This follows the same shape: both fixtures now write__pycache__/ignored.cpython-314.pycand a loosestray.pyc, with matching negative assertions in the legacy-compat and manifest plan tests.Verified the tests fail without the source change:
Full suite:
yarn test→ 3713 passed, 0 failed.eslintclean on both touched files.🤖 Generated with Claude Code