Skip to content

Fix/loaded skills - #6

Closed
mostafamm44 wants to merge 4 commits into
yallma3:mainfrom
mostafamm44:fix/loaded-skills
Closed

mostafamm44 wants to merge 4 commits into
yallma3:mainfrom
mostafamm44:fix/loaded-skills

Conversation

@mostafamm44

@mostafamm44 mostafamm44 commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added an interactive init command to configure projects by selecting agents, modes, and workflows.
    • Added automatic downloading of selected files, configuration generation, execution guidance, and agent skill linking.
    • Added progress indicators, status messaging, overwrite prompts, and installation fallbacks.
    • Added support for manifest-based version tracking and updates.
  • Documentation

    • Expanded setup, requirements, development, and CLI usage guidance.
  • Breaking Changes

    • Removed the legacy Bash and PowerShell setup scripts.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR replaces the shell setup scripts with a TypeScript CLI. It adds interactive initialization, manifest processing, downloads, configuration generation, symlink handling, package tooling, documentation updates, and project ignore rules.

Changes

Interactive CLI migration

Layer / File(s) Summary
CLI entrypoint and interactive selection
package.json, tsconfig.json, src/index.ts, src/commands/init.ts, src/utils/ui.ts, src/utils/prompts.ts
Defines the CLI package, validates the target directory, presents interactive agent/mode/workflow selections, and orchestrates initialization.
Manifest interpretation and file acquisition
src/utils/manifest.ts, src/utils/download.ts, src/commands/init.ts
Classifies manifests, flattens selected files, compares versions, downloads remote content, and records local manifest data.
Configuration generation and skill linking
src/utils/generate-configs.ts, src/utils/symlinks.ts, src/commands/init.ts
Generates agent configuration and execution protocol files, then creates symlinks with elevated retry and copy fallback behavior.
Project setup and documentation updates
.gitignore, README.md, AGENTS.md, codewiser.sh, codewiser.ps1
Adds ignore rules, documents the CLI workflow, removes the prior execution protocol text, and deletes the legacy shell and PowerShell setup scripts.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • yallma3/codewiser#4: Also removes the Spec-Driven Execution Protocol from AGENTS.md.
  • yallma3/codewiser#5: Implements the same CLI rewrite and related module, documentation, and script changes.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and awkwardly phrased, and it does not clearly summarize the broad CLI and setup refactor in this PR. Rename it to a clear, specific summary such as "Refactor codewiser CLI setup and skill loading".
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Nitpick comments (1)
package.json (1)

11-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid "latest" for pinned dependency versions.

@types/bun at "latest" produces non-reproducible installs; CI/dev builds can silently pick up breaking type changes.

🤖 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 `@package.json` around lines 11 - 13, Replace the "latest" version constraint
for the `@types/bun` entry in devDependencies with a specific pinned version,
preserving reproducible installs and avoiding unintentional dependency updates.
🤖 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 @.gitignore:
- Around line 13-16: Update the log and report patterns in the ignore
configuration: replace the literal underscore log pattern with a wildcard
pattern that matches normal .log files, and replace the report pattern’s literal
underscores with wildcards so generated report JSON filenames are ignored.

In `@package.json`:
- Around line 1-22: Add a valid "version" field to package.json so meow can
resolve and display the CLI version for the --version flag. Keep the existing
package metadata and dependencies unchanged.

In `@README.md`:
- Line 124: Update the README description for the initialization flow to reflect
that setup reaches step 6, either by documenting all six steps or by removing
the fixed step count while preserving the existing description of
src/commands/init.ts.
- Line 64: Update the README initialization step describing symlink creation to
limit the claim to agents that actually receive symlinks, using wording such as
“Create supported symlinks” or explicitly naming Claude and Cursor; do not imply
OpenCode, Antigravity, or Kilo receive symlinks.
- Around line 94-97: Remove “or Node.js 18+” from the README Requirements entry
and update the corresponding architecture note to state Bun is the supported
runtime, since the documented download path relies on Bun.write and the run
script is Bun-based.
- Around line 101-105: Update the repository URL in the README development setup
clone command from yallma3/codewiser-cli to yallma3/codewiser, leaving the
surrounding installation instructions unchanged.
- Around line 48-52: Update the README run commands to invoke the published
package name codewiser-cli for both bunx and npx examples, while preserving the
existing codewiser binary usage and project-directory argument.

In `@src/commands/init.ts`:
- Around line 198-206: Replace every hardcoded backslash path construction in
the init command, including the directory creation blocks and the locations
referenced around lines 211-213, 216-217, and 256-262, with the imported join()
helper using separate path segments. Cover agent directories, download
destinations, the local manifest path, and symlink relativeSrc/relativeDest
values while preserving the existing conditional behavior.

In `@src/index.ts`:
- Line 28: Update the targetDir resolution to join name directly with
process.cwd() without traversing to its parent, so init() receives the requested
project path under the current working directory.

In `@src/utils/download.ts`:
- Around line 4-17: Add a finite timeout to the fetch call in download in
src/utils/download.ts:4-17 using AbortSignal.timeout or an AbortController,
preserving the existing false return for timeout or other failures. Apply the
same timeout to the manifestUrl fetch inside the runSpinner callback in
src/commands/init.ts:100-106; no direct changes are needed elsewhere.

In `@src/utils/generate-configs.ts`:
- Around line 4-21: Update all four config generators, including
generateOpenCodeConfig and the generators for CLAUDE.md,
.antigravity/workflows.json, and .kilo/config.json, so existing destination
files are not silently overwritten. Before each writeFileSync, check
existsSync(dest) and use the established confirmPrompt/confirmOverwrite flow or
skip/back up according to the project’s existing overwrite behavior, while
preserving generation for missing files.

In `@src/utils/symlinks.ts`:
- Around line 35-56: Update the retry handling in the symlink creation catch
block: after detecting cancellation with isCancel(result), call cancel and
immediately return so no elevated command executes. Replace the interpolated
PowerShell command built from srcPath and destPath with a safely parameterized
or rigorously escaped invocation that treats both paths strictly as data,
preventing command injection. Preserve the existing post-retry symlink check and
fallback behavior.

In `@src/utils/ui.ts`:
- Around line 96-109: Remove the redundant s.stop(label) call from the catch
block in runSpinner, leaving the unconditional finally cleanup intact while
preserving error logging and the BACK return value.

In `@tsconfig.json`:
- Line 6: Update the TypeScript peer dependency range in package.json from ^5 to
^5.4, aligning the supported TypeScript version with the module setting in
tsconfig.json. Preserve all other dependency declarations unchanged.

---

Nitpick comments:
In `@package.json`:
- Around line 11-13: Replace the "latest" version constraint for the `@types/bun`
entry in devDependencies with a specific pinned version, preserving reproducible
installs and avoiding unintentional dependency updates.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 56ad4aeb-3253-4a32-93e9-20ee96a1aafd

📥 Commits

Reviewing files that changed from the base of the PR and between dffbb35 and 28477d5.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • .gitignore
  • AGENTS.md
  • README.md
  • codewiser.ps1
  • codewiser.sh
  • package.json
  • src/commands/init.ts
  • src/index.ts
  • src/utils/download.ts
  • src/utils/generate-configs.ts
  • src/utils/manifest.ts
  • src/utils/prompts.ts
  • src/utils/symlinks.ts
  • src/utils/ui.ts
  • tsconfig.json
💤 Files with no reviewable changes (3)
  • codewiser.ps1
  • codewiser.sh
  • AGENTS.md

Comment thread .gitignore
Comment on lines +13 to +16
# logs
logs
_.log
report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Fix the log and report ignore patterns.

_.log does not ignore normal log files, and the report pattern contains literal underscores instead of wildcards. This can leave generated logs or reports unignored and expose them to accidental commits.

Proposed fix
-_.log
-report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json
+*.log
+report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# logs
logs
_.log
report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json
# logs
logs
*.log
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
🤖 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 @.gitignore around lines 13 - 16, Update the log and report patterns in the
ignore configuration: replace the literal underscore log pattern with a wildcard
pattern that matches normal .log files, and replace the report pattern’s literal
underscores with wildcards so generated report JSON filenames are ignored.

Comment thread package.json
Comment on lines +1 to +22
{
"name": "codewiser-cli",
"bin": {
"codewiser": "src/index.ts"
},
"type": "module",
"private": true,
"scripts": {
"start": "bun src/index.ts"
},
"devDependencies": {
"@types/bun": "latest"
},
"peerDependencies": {
"typescript": "^5"
},
"dependencies": {
"@clack/prompts": "^1.7.0",
"meow": "^14.1.0",
"picocolors": "^1.1.1"
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Missing "version" field breaks meow's --version flag.

The CLI's help text advertises --version Show version, and meow (with importMeta: import.meta) resolves the version from this package.json. Without a "version" key, --version will report undefined/empty.

🔧 Proposed fix
 {
   "name": "codewiser-cli",
+  "version": "0.1.0",
   "bin": {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
"name": "codewiser-cli",
"bin": {
"codewiser": "src/index.ts"
},
"type": "module",
"private": true,
"scripts": {
"start": "bun src/index.ts"
},
"devDependencies": {
"@types/bun": "latest"
},
"peerDependencies": {
"typescript": "^5"
},
"dependencies": {
"@clack/prompts": "^1.7.0",
"meow": "^14.1.0",
"picocolors": "^1.1.1"
}
}
{
"name": "codewiser-cli",
"version": "0.1.0",
"bin": {
"codewiser": "src/index.ts"
},
"type": "module",
"private": true,
"scripts": {
"start": "bun src/index.ts"
},
"devDependencies": {
"`@types/bun`": "latest"
},
"peerDependencies": {
"typescript": "^5"
},
"dependencies": {
"`@clack/prompts`": "^1.7.0",
"meow": "^14.1.0",
"picocolors": "^1.1.1"
}
}
🤖 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 `@package.json` around lines 1 - 22, Add a valid "version" field to
package.json so meow can resolve and display the CLI version for the --version
flag. Keep the existing package metadata and dependencies unchanged.

Comment thread README.md
Comment on lines +48 to +52
# Interactively set up codewiser in a target project directory
bunx codewiser my-project

# 2. Select your AI agents and development mode (Prototype, Spec Driven, or Rigid)
# The script downloads the relevant skills, spec templates, and creates
# agent-specific configs with symlinks to the shared `.agents/skills/` directory.
# Or via npm (after publishing)
npx codewiser my-project

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '"name"|"bin"' package.json

Repository: yallma3/codewiser

Length of output: 197


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== package.json relevant fields =="
cat -n package.json | sed -n '1,80p'

echo
echo "== README setup commands around lines 48-52 =="
cat -n README.md | sed -n '40,60p'

echo
echo "== npm package availability/version/package names =="
npm view codewiser-cli name version bin --json 2>/tmp/npm_codewiser_err.log || { cat /tmp/npm_codewiser_err.log; echo; }
npm view codewiser name version bin --json 2>/tmp/npm_codewiser_cli_err.log || { cat /tmp/npm_codewiser_cli_err.log; echo; }

Repository: yallma3/codewiser

Length of output: 3797


Use codewiser-cli in the publishable run commands.

package.json declares the package as codewiser-cli with the codewiser binary, so bunx codewiser / npx codewiser resolve the wrong package. If the package stays named codewiser-cli, these commands should be updated too.

🤖 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 `@README.md` around lines 48 - 52, Update the README run commands to invoke the
published package name codewiser-cli for both bunx and npx examples, while
preserving the existing codewiser binary usage and project-directory argument.

Comment thread README.md
2. **Configure mode** — Pick a development mode (Prototype, Spec Driven, or Rigid) — or select workflows in legacy mode
3. **Download files** — Downloads shared skills, spec templates, and manifest from GitHub
4. **Generate configs** — Creates agent-specific configuration files
5. **Create symlinks** — Symlinks shared skills into each agent's private directory

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Limit the symlink claim to agents that actually receive symlinks.

The current initialization code creates symlinks for Claude and Cursor only; OpenCode, Antigravity, and Kilo reference shared files directly. Change this wording to “Create supported symlinks” or enumerate the applicable agents.

🤖 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 `@README.md` at line 64, Update the README initialization step describing
symlink creation to limit the claim to agents that actually receive symlinks,
using wording such as “Create supported symlinks” or explicitly naming Claude
and Cursor; do not imply OpenCode, Antigravity, or Kilo receive symlinks.

Comment thread README.md
Comment on lines +94 to +97
## Requirements

| Platform | Script | Source |
|---|---|---|
| Linux / macOS | `codewiser.sh` | Downloads `AGENTS.md`, skills, and specs from `https://github.com/yallma3/codewiser` |
| Windows | `codewiser.ps1` | Same logic via PowerShell with `Invoke-WebRequest` |
- **Bun** (recommended) or **Node.js 18+**
- Git

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'Bun\.write|writeFile|node:fs/promises' src package.json

Repository: yallma3/codewiser

Length of output: 832


🏁 Script executed:

#!/bin/bash
set -e
printf 'README relevant sections:\n'
sed -n '80,135p' README.md
printf '\npackage.json:\n'
cat package.json
printf '\nsrc/utils/download.ts:\n'
cat -n src/utils/download.ts
printf '\nAll Bun.write occurrences:\n'
rg -n 'Bun\.write' .

Repository: yallma3/codewiser

Length of output: 3995


Remove Node.js support until Bun.write() paths are Node-compatible.

README.md claims supported runtimes, but src/utils/download.ts uses Bun.write(dest, res) in the documented download path, and the only run script is bun-based. Use a Node-compatible file writer, or remove "or Node.js 18+" from Requirements and update the corresponding architecture note.

🤖 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 `@README.md` around lines 94 - 97, Remove “or Node.js 18+” from the README
Requirements entry and update the corresponding architecture note to state Bun
is the supported runtime, since the documented download path relies on Bun.write
and the run script is Bun-based.

Comment thread src/utils/download.ts
Comment on lines +4 to +17
export async function download(url: string, dest: string): Promise<boolean> {
const dir = dirname(dest);
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true });
}
try {
const res = await fetch(url);
if (!res.ok) return false;
await Bun.write(dest, res);
return true;
} catch {
return false;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

No timeout on any network fetch call. Both the shared file-download helper and the inline manifest fetch can hang indefinitely on a stalled/unresponsive network, with no user-facing recourse besides Ctrl+C.

  • src/utils/download.ts#L4-L17: wrap fetch(url) with AbortSignal.timeout(...) (or an AbortController) and treat timeout as a failed download (return false).
  • src/commands/init.ts#L100-L106: apply the same timeout to fetch(manifestUrl) inside the runSpinner callback so a slow manifest endpoint doesn't stall the whole CLI.
📍 Affects 2 files
  • src/utils/download.ts#L4-L17 (this comment)
  • src/commands/init.ts#L100-L106
🤖 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 `@src/utils/download.ts` around lines 4 - 17, Add a finite timeout to the fetch
call in download in src/utils/download.ts:4-17 using AbortSignal.timeout or an
AbortController, preserving the existing false return for timeout or other
failures. Apply the same timeout to the manifestUrl fetch inside the runSpinner
callback in src/commands/init.ts:100-106; no direct changes are needed
elsewhere.

Comment on lines +4 to +21
export function generateOpenCodeConfig(targetDir: string, skillDirs: string[], useOpencode: boolean): void {
if (!useOpencode) return;
const dest = join(targetDir, "opencode.json");

const config: Record<string, unknown> = {
$schema: "https://opencode.ai/config.json",
};

if (skillDirs.length > 0) {
config.skills = { paths: skillDirs.map((d) => `.agents/skills/${d}`) };
config.instructions = [...skillDirs.map((d) => `.agents/skills/${d}/**/SKILL.md`), "AGENTS.md"];
} else {
config.skills = { paths: [".agents/skills"] };
config.instructions = [".agents/skills/**/SKILL.md", "AGENTS.md"];
}

writeFileSync(dest, JSON.stringify(config, null, 2), "utf-8");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Config generators silently overwrite existing user files.

Unlike the skill-file download flow (which checks versions and prompts via confirmOverwrite), these four generators always writeFileSync regardless of existing content. Since detectInstalledAgents in src/commands/init.ts is explicitly designed to re-run init against an already-configured project, re-running the CLI will silently wipe any customizations in opencode.json, CLAUDE.md, .antigravity/workflows.json, and .kilo/config.json.

🔧 Suggested approach

Guard each write with an existsSync(dest) check and either skip, back up, or route through the existing confirmPrompt/confirmOverwrite helpers before overwriting, mirroring the SKILL.md download behavior.

Also applies to: 23-38, 40-59, 61-81

🤖 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 `@src/utils/generate-configs.ts` around lines 4 - 21, Update all four config
generators, including generateOpenCodeConfig and the generators for CLAUDE.md,
.antigravity/workflows.json, and .kilo/config.json, so existing destination
files are not silently overwritten. Before each writeFileSync, check
existsSync(dest) and use the established confirmPrompt/confirmOverwrite flow or
skip/back up according to the project’s existing overwrite behavior, while
preserving generation for missing files.

Comment thread src/utils/symlinks.ts
Comment on lines +35 to +56
try {
symlinkSync(destPath, srcPath, "junction");
} catch {
const result = await confirm({
message: `Symlink failed for ${cfg.label}. Retry as admin?`,
initialValue: false,
});
if (isCancel(result)) cancel("Cancelled");

if (result) {
const { spawnSync } = await import("child_process");
spawnSync("powershell.exe", [
"-NoProfile",
"-Command",
`Start-Process -Verb RunAs -Wait powershell.exe -ArgumentList '-NoProfile','-Command','& {New-Item -ItemType SymbolicLink -Path ''${srcPath}'' -Target ''${destPath}'' -Force}'`,
], { stdio: "inherit" });
}

if (!existsSync(srcPath) || !lstatSync(srcPath).isSymbolicLink()) {
copyDirContents(destPath, srcPath);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Elevated PowerShell command runs even after cancellation, and is vulnerable to injection via unescaped path interpolation.

Two issues here:

  1. cancel("Cancelled") only prints a message — it doesn't stop execution (the standard pattern is if (isCancel(result)) { cancel(...); return; } or process.exit(...)). Since there's no return after cancel(...), execution falls through to if (result), and the cancellation symbol is truthy, so the elevated PowerShell retry still runs even when the user cancels.
  2. srcPath/destPath are interpolated directly into a powershell.exe -Verb RunAs command string. Since ' and ; are legal in Windows path names, a target directory containing either can break out of the quoted -Path/-Target arguments and inject arbitrary commands that execute with admin privileges.
🔒 Proposed fix
     const result = await confirm({
       message: `Symlink failed for ${cfg.label}. Retry as admin?`,
       initialValue: false,
     });
-    if (isCancel(result)) cancel("Cancelled");
+    if (isCancel(result)) {
+      cancel("Cancelled");
+      return;
+    }

     if (result) {
       const { spawnSync } = await import("child_process");
+      const escapePs = (p: string) => p.replace(/'/g, "''");
       spawnSync("powershell.exe", [
         "-NoProfile",
         "-Command",
-        `Start-Process -Verb RunAs -Wait powershell.exe -ArgumentList '-NoProfile','-Command','& {New-Item -ItemType SymbolicLink -Path ''${srcPath}'' -Target ''${destPath}'' -Force}'`,
+        `Start-Process -Verb RunAs -Wait powershell.exe -ArgumentList '-NoProfile','-Command','& {New-Item -ItemType SymbolicLink -Path ''${escapePs(srcPath)}'' -Target ''${escapePs(destPath)}'' -Force}'`,
       ], { stdio: "inherit" });
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
symlinkSync(destPath, srcPath, "junction");
} catch {
const result = await confirm({
message: `Symlink failed for ${cfg.label}. Retry as admin?`,
initialValue: false,
});
if (isCancel(result)) cancel("Cancelled");
if (result) {
const { spawnSync } = await import("child_process");
spawnSync("powershell.exe", [
"-NoProfile",
"-Command",
`Start-Process -Verb RunAs -Wait powershell.exe -ArgumentList '-NoProfile','-Command','& {New-Item -ItemType SymbolicLink -Path ''${srcPath}'' -Target ''${destPath}'' -Force}'`,
], { stdio: "inherit" });
}
if (!existsSync(srcPath) || !lstatSync(srcPath).isSymbolicLink()) {
copyDirContents(destPath, srcPath);
}
}
try {
symlinkSync(destPath, srcPath, "junction");
} catch {
const result = await confirm({
message: `Symlink failed for ${cfg.label}. Retry as admin?`,
initialValue: false,
});
if (isCancel(result)) {
cancel("Cancelled");
return;
}
if (result) {
const { spawnSync } = await import("child_process");
const escapePs = (p: string) => p.replace(/'/g, "''");
spawnSync("powershell.exe", [
"-NoProfile",
"-Command",
`Start-Process -Verb RunAs -Wait powershell.exe -ArgumentList '-NoProfile','-Command','& {New-Item -ItemType SymbolicLink -Path ''${escapePs(srcPath)}'' -Target ''${escapePs(destPath)}'' -Force}'`,
], { stdio: "inherit" });
}
if (!existsSync(srcPath) || !lstatSync(srcPath).isSymbolicLink()) {
copyDirContents(destPath, srcPath);
}
}
🤖 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 `@src/utils/symlinks.ts` around lines 35 - 56, Update the retry handling in the
symlink creation catch block: after detecting cancellation with
isCancel(result), call cancel and immediately return so no elevated command
executes. Replace the interpolated PowerShell command built from srcPath and
destPath with a safely parameterized or rigorously escaped invocation that
treats both paths strictly as data, preventing command injection. Preserve the
existing post-retry symlink check and fallback behavior.

Comment thread src/utils/ui.ts
Comment on lines +96 to +109
export async function runSpinner<T>(label: string, fn: () => Promise<T>): Promise<T | typeof BACK> {
const s = spinner();
s.start(label);
try {
const result = await fn();
return result;
} catch (e) {
s.stop(label);
error(String(e));
return BACK;
} finally {
s.stop(label);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Redundant double spinner.stop() on error.

The catch block already calls s.stop(label) before returning BACK; the finally block then calls s.stop(label) again unconditionally. This runs on every network failure (manifest fetch, file downloads), risking a duplicate/odd status line.

🔧 Proposed fix
   try {
     const result = await fn();
     return result;
   } catch (e) {
-    s.stop(label);
     error(String(e));
     return BACK;
   } finally {
     s.stop(label);
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function runSpinner<T>(label: string, fn: () => Promise<T>): Promise<T | typeof BACK> {
const s = spinner();
s.start(label);
try {
const result = await fn();
return result;
} catch (e) {
s.stop(label);
error(String(e));
return BACK;
} finally {
s.stop(label);
}
}
export async function runSpinner<T>(label: string, fn: () => Promise<T>): Promise<T | typeof BACK> {
const s = spinner();
s.start(label);
try {
const result = await fn();
return result;
} catch (e) {
error(String(e));
return BACK;
} finally {
s.stop(label);
}
}
🤖 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 `@src/utils/ui.ts` around lines 96 - 109, Remove the redundant s.stop(label)
call from the catch block in runSpinner, leaving the unconditional finally
cleanup intact while preserving error logging and the BACK return value.

Comment thread tsconfig.json
// Environment setup & latest features
"lib": ["ESNext"],
"target": "ESNext",
"module": "Preserve",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## tsconfig.json"
cat -n tsconfig.json

echo
echo "## package.json peerDependencies/typescript"
python3 - <<'PY'
import json
with open("package.json") as f:
    data=json.load(f)
for key in ("dependencies","devDependencies","peerDependencies","optionalDependencies"):
    if key in data and "typescript" in data[key]:
        print(key, data[key]["typescript"])
PY

echo
echo "## package manager lock snippets if present"
for f in package-lock.json pnpm-lock.yaml yarn.lock bun.lockb; do
  [ -e "$f" ] && echo "--- $f ---" && (case "$f" in *.json) jq '.packages["node_modules/typescript"].version // .packages[""].dependencies.typescript // empty' "$f" 2>/dev/null || true ;; *) rg -n "(^typescript@|typescript:|^  /typescript@|^typescript=|typescript@)" "$f" | head -30 || true ;; esac)
done

echo
echo "## TypeScript release info from npm metadata (if network/npm is available)"
if command -v npm >/dev/null 2>&1; then
  npm view typescript version peerDependencies --json 2>/dev/null || true
  npm view typescript@5.4.0 version --json 2>/dev/null || true
  npm view typescript@5.3.0 version --json 2>/dev/null || true
fi

Repository: yallma3/codewiser

Length of output: 1899


🌐 Web query:

TypeScript module option preserve added 5.4 TypeScript 5.4 changelog

💡 Result:

The module option "preserve" was introduced in TypeScript 5.4 [1][2][3]. This setting is designed to better model the behavior of modern bundlers and runtimes (such as Bun) [1][3]. Under --module preserve: - ECMAScript import and export statements are preserved in the emitted output as-is [3]. - CommonJS-style statements, such as import x = require("...") and export =..., are emitted as CommonJS require calls and module.exports, respectively [3]. - This mode allows for the coexistence of both ECMAScript and CommonJS module syntaxes within the same file, as the format of each individual import or export statement is maintained rather than being coerced into a single format for the entire compilation [3]. Additionally, when you set --module to preserve, TypeScript implicitly configures the following compiler options [1][4]: - --moduleResolution: "bundler" - --esModuleInterop: true - --resolveJsonModule: true By providing this configuration, TypeScript ensures that its type checking and module resolution accurately reflect how your bundler or runtime will process your imports and exports [3].

Citations:


Align the TypeScript peer range with "module": "Preserve"

"module": "Preserve" requires TypeScript 5.4+, but package.json allows the satisfied range ^5 down to 5.0–5.3. Update the peer dependency to ^5.4 so installed TypeScript consumers won’t fail when type-checking with this config.

🤖 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 `@tsconfig.json` at line 6, Update the TypeScript peer dependency range in
package.json from ^5 to ^5.4, aligning the supported TypeScript version with the
module setting in tsconfig.json. Preserve all other dependency declarations
unchanged.

@mostafamm44 mostafamm44 closed this Aug 2, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 2, 2026
This was referenced Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant