[wip] feat: alternative clone implementation that is 50% faster - #264
[wip] feat: alternative clone implementation that is 50% faster#264ashwin153 wants to merge 2 commits into
Conversation
| "mcpServers": "./.mcp.json", | ||
| "name": "softlight", | ||
| "version": "5.103.0" | ||
| "version": "5.120.0" |
There was a problem hiding this comment.
Version inconsistency with sibling plugin files
.claude-plugin was bumped to 5.120.0 while .codex-plugin and .cursor-plugin were both bumped to 5.104.0. A 16-minor-version gap between files that should stay in lockstep looks like a copy-paste error — if the intent was to bump by one minor version (adding the new clone-app skill), all three should be 5.104.0.
| "version": "5.120.0" | |
| "version": "5.104.0" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/.claude-plugin/plugin.json
Line: 8
Comment:
**Version inconsistency with sibling plugin files**
`.claude-plugin` was bumped to `5.120.0` while `.codex-plugin` and `.cursor-plugin` were both bumped to `5.104.0`. A 16-minor-version gap between files that should stay in lockstep looks like a copy-paste error — if the intent was to bump by one minor version (adding the new `clone-app` skill), all three should be `5.104.0`.
```suggestion
"version": "5.104.0"
```
How can I resolve this? If you propose a fix, please make it concise.| ) | ||
| if imported_file_with_suffix.is_file(): | ||
| if imported_file_with_suffix not in source_files: | ||
| source_files.add(imported_file_with_suffix) |
There was a problem hiding this comment.
commonpath crashes on empty source directory
If source_dir is empty (or every file in it is skipped by the os.walk exclusion list), source_files will be an empty set and os.path.commonpath([]) raises ValueError: commonpath() arg is an empty sequence, crashing the script with no useful error message. A guard is needed before this line.
| source_files.add(imported_file_with_suffix) | |
| if not source_files: | |
| raise ValueError(f"No source files found in {source_dir}") | |
| root_dir = pathlib.Path(os.path.commonpath(source_files)) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/clone-app/create_clone.py
Line: 173
Comment:
**`commonpath` crashes on empty source directory**
If `source_dir` is empty (or every file in it is skipped by the `os.walk` exclusion list), `source_files` will be an empty set and `os.path.commonpath([])` raises `ValueError: commonpath() arg is an empty sequence`, crashing the script with no useful error message. A guard is needed before this line.
```suggestion
if not source_files:
raise ValueError(f"No source files found in {source_dir}")
root_dir = pathlib.Path(os.path.commonpath(source_files))
```
How can I resolve this? If you propose a fix, please make it concise.| "include": [ | ||
| "src", | ||
| ], |
There was a problem hiding this comment.
tsconfig include conflicts with index.html entry point
index.html references src="main.tsx" at the project root, but tsconfig.json only includes the "src" subdirectory. Any main.tsx placed at the project root (as SKILL.md step 3 instructs) will be bundled by Vite but silently skipped by tsc -b, so type errors in the entry file go undetected. Consider broadening the include to cover root-level .ts/.tsx files or moving the entry point to src/main.tsx.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/clone-app/create_clone.py
Line: 69-71
Comment:
**`tsconfig` `include` conflicts with `index.html` entry point**
`index.html` references `src="main.tsx"` at the project root, but `tsconfig.json` only includes the `"src"` subdirectory. Any `main.tsx` placed at the project root (as SKILL.md step 3 instructs) will be bundled by Vite but silently skipped by `tsc -b`, so type errors in the entry file go undetected. Consider broadening the include to cover root-level `.ts`/`.tsx` files or moving the entry point to `src/main.tsx`.
How can I resolve this? If you propose a fix, please make it concise.| source_files.add(imported_file_with_suffix) | ||
| queue.append(imported_file_with_suffix) | ||
|
|
||
| root_dir = pathlib.Path(os.path.commonpath(source_files)) | ||
|
|
||
| for source_file in source_files: |
There was a problem hiding this comment.
Copied source files can silently overwrite scaffold config
_copy_src is called after the scaffold files (package.json, tsconfig.json, vite.config.ts, index.html) are written. If the source directory contains any of these files at its root level, shutil.copy2 will overwrite the carefully constructed scaffold without warning. Consider skipping files whose target path collides with an existing scaffold file, or use a dedicated src/ subdirectory for copied source files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/clone-app/create_clone.py
Line: 173-178
Comment:
**Copied source files can silently overwrite scaffold config**
`_copy_src` is called after the scaffold files (`package.json`, `tsconfig.json`, `vite.config.ts`, `index.html`) are written. If the source directory contains any of these files at its root level, `shutil.copy2` will overwrite the carefully constructed scaffold without warning. Consider skipping files whose target path collides with an existing scaffold file, or use a dedicated `src/` subdirectory for copied source files.
How can I resolve this? If you propose a fix, please make it concise.
Greptile Summary
This PR introduces a new
clone-appskill for the softlight plugin — a Python script (create_clone.py) that scaffolds a Vite + React + TypeScript project from an existing source directory, resolves transitive relative imports, and installs dependencies via pnpm, along with aSKILL.mddescribing the 5-step workflow..claude-plugin/plugin.jsonwas bumped to5.120.0while.codex-pluginand.cursor-pluginwere both bumped to5.104.0; the 16-minor-version gap looks like a typo.os.path.commonpath(source_files)at line 173 raisesValueErrorwhensource_filesis empty (e.g., source dir only contained excluded directories).Confidence Score: 3/5
Not safe to merge as-is — one clear version-bump typo and one crash-inducing code path need to be resolved first.
Two P1 findings: the .claude-plugin version jumping to 5.120.0 (likely unintentional) and the unguarded commonpath([]) crash on empty source directories. P2 findings (tsconfig/entry mismatch and scaffold overwrite) are worth addressing before GA use but don't block merge on their own.
plugins/softlight/.claude-plugin/plugin.json (version typo) and plugins/softlight/skills/clone-app/create_clone.py (commonpath crash + tsconfig/entry mismatch)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["create_clone(source_dir)"] --> B["mkdtemp → target_dir"] B --> C["_create_package_json\n(merges source deps + vite/react devDeps)"] C --> D["_create_tsconfig_json\n(include: ['src'] only ⚠️)"] D --> E["_create_index_html\n(entry: main.tsx at root ⚠️)"] E --> F["_create_vite_config_ts"] F --> G["_copy_src(target_dir, source_dir)"] G --> G1["os.walk source_dir\nskipping node_modules/.git/dist/…"] G1 --> G2["BFS: follow relative imports\nin .ts/.tsx files"] G2 --> G3["commonpath(source_files) → root_dir\n⚠️ crashes if source_files empty"] G3 --> G4["copy each file to target_dir / file.relative_to(root_dir)\n⚠️ may overwrite scaffold files"] G4 --> H["_install_dependencies\npnpm install --prefer-offline"] H --> I["print(target_dir)"]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore: run pre-commit" | Re-trigger Greptile