fix: correct readFileSync calls in LinkInstaller to fix plugin installation

[JohnMcLear]

Summary

• pathToFileURL() was incorrectly wrapping file paths passed to readFileSync() in LinkInstaller.ts, causing ENOENT errors when reading plugin package.json files
• The errors were silently caught, making plugin installation appear to succeed while dependencies were never linked
• Replaced with plain file paths and 'utf-8' encoding, removed unnecessary as unknown as string casts

Test plan

• Type check passes
• Install a plugin via pnpm run plugins i ep_markdown and verify it appears in pnpm run plugins ls
• Verify no ENOENT errors in console output during install

Fixes #6811

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix readFileSync calls in LinkInstaller for plugin installation

🐞 Bug fix

Walkthroughs

Description

• Fixed incorrect pathToFileURL() wrapping in readFileSync() calls
• Replaced with plain file paths and 'utf-8' encoding parameter
• Removed unnecessary as unknown as string type casts
• Removed unused pathToFileURL import from node:url

Diagram

flowchart LR
  A["readFileSync with pathToFileURL"] -->|"Remove incorrect wrapping"| B["readFileSync with plain path"]
  A -->|"Remove unused import"| C["Remove pathToFileURL import"]
  A -->|"Remove type cast"| D["Remove as unknown as string"]
  B -->|"Add encoding parameter"| E["Add utf-8 encoding"]

Loading

File Changes

1. src/static/js/pluginfw/LinkInstaller.ts 🐞 Bug fix +4/-5

Fix readFileSync path handling and encoding

• Removed unused pathToFileURL import from node:url
• Fixed three readFileSync() calls by replacing pathToFileURL(path.join(...)) with plain
 path.join(...) and adding 'utf-8' encoding parameter
• Removed unnecessary as unknown as string type casts from all three readFileSync() calls
• Updated error logging to use plain file path instead of pathToFileURL().toString()

src/static/js/pluginfw/LinkInstaller.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Missing plugin install regression test 📘 Rule violation ⚙ Maintainability

Description

This PR changes plugin installation behavior in LinkInstaller to fix readFileSync() path
handling, but it does not add a regression test that would fail without the fix. Without coverage,
this bug can reappear unnoticed in future refactors.

Code

src/static/js/pluginfw/LinkInstaller.ts[R100-103]

  try {
    const json:IPluginInfo = JSON.parse(
-        readFileSync(pathToFileURL(path.join(pluginInstallPath, dependency, 'package.json'))) as unknown as string);
+        readFileSync(path.join(pluginInstallPath, dependency, 'package.json'), 'utf-8'));
    if(json.dependencies){

Evidence

PR Compliance ID 4 requires bug fixes to include a regression test. The diff shows the bug fix in
LinkInstaller.ts but no accompanying test addition in the changed code.

src/static/js/pluginfw/LinkInstaller.ts[100-103]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A bug fix was made to plugin installation file reading logic, but there is no regression test ensuring the installer reads `package.json` via a filesystem path string (not a file URL) and with an explicit encoding.
## Issue Context
This change fixes plugin installation failures that previously manifested as `ENOENT` errors and/or ineffective dependency linking. A test should fail if the old `pathToFileURL()` + `readFileSync()` usage is reintroduced.
## Fix Focus Areas
- src/static/js/pluginfw/LinkInstaller.ts[94-201]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Map updated after failure ☑ 🐞 Bug ≡ Correctness

Description

addSubDependency() updates dependenciesMap even when linking or package.json read/parse fails,
marking the dependency as tracked despite an incomplete/failed setup. This can block later cleanup
because removeSubDependency() returns early when dependenciesMap.has(dependency) is true.

Code

src/static/js/pluginfw/LinkInstaller.ts[R187-200]

      this.linkDependency(dependency)
      // Read sub dependencies
      const json:IPluginInfo = JSON.parse(
-          readFileSync(pathToFileURL(path.join(pluginInstallPath, dependency, 'package.json'))) as unknown as string);
+          readFileSync(path.join(pluginInstallPath, dependency, 'package.json'), 'utf-8'));
      if(json.dependencies){
        Object.keys(json.dependencies).forEach((subDependency: string) => {
          this.addSubDependency(dependency, subDependency)
        })
      }
    } catch (err) {
-        console.error(`Error reading package.json ${err} for ${pathToFileURL(path.join(pluginInstallPath, dependency, 'package.json')).toString()}`)
+        console.error(`Error reading package.json ${err} for ${path.join(pluginInstallPath, dependency, 'package.json')}`)
    }
    this.dependenciesMap.set(dependency, new Set([plugin]))
  }

Evidence

The map write is unconditional after the try/catch in addSubDependency(), but cleanup logic in
removeSubDependency() short-circuits solely based on map presence, so a failed dependency can become
effectively undeletable by this logic.

src/static/js/pluginfw/LinkInstaller.ts[180-200]
src/static/js/pluginfw/LinkInstaller.ts[94-110]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`addSubDependency()` calls `this.dependenciesMap.set(dependency, ...)` even if the try block fails (linking or reading/parsing `package.json`). This can leave the dependency marked as “in use” and prevent removal later.
### Issue Context
`removeSubDependency()` returns early if `dependenciesMap.has(dependency)` is true, so an entry created after a failed add can block cleanup.
### Fix Focus Areas
- src/static/js/pluginfw/LinkInstaller.ts[180-200]
- src/static/js/pluginfw/LinkInstaller.ts[94-110]
### Suggested change
- Move `this.dependenciesMap.set(dependency, new Set([plugin]))` into the try block after successful link + successful `package.json` read/parse.
- If `package.json` read/parse fails, do **not** set the map entry; optionally rethrow so plugin install fails loudly rather than leaving partial state.
- (Optional hardening) After `linkDependency()`, verify the symlink/dir exists before marking success.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Swallowed package.json errors 🐞 Bug ☼ Reliability

Description

removeSubDependency() and unlinkSubDependency() silently ignore failures reading/parsing dependency
package.json, so transitive sub-dependencies might not be recursively removed/unlinked and the
underlying failure is hidden. This can leave orphaned dependency directories/symlinks behind with no
diagnostic output.

Code

src/static/js/pluginfw/LinkInstaller.ts[R99-105]

  // Read sub dependencies
  try {
    const json:IPluginInfo = JSON.parse(
-        readFileSync(pathToFileURL(path.join(pluginInstallPath, dependency, 'package.json'))) as unknown as string);
+        readFileSync(path.join(pluginInstallPath, dependency, 'package.json'), 'utf-8'));
    if(json.dependencies){
      for (let [subDependency] of Object.entries(json.dependencies)) {
        await this.removeSubDependency(dependency, subDependency)

Evidence

Both cleanup paths attempt to read and parse a dependency’s package.json for recursive traversal,
but errors are dropped via empty catch blocks, preventing recursion and suppressing the reason for
failure.

src/static/js/pluginfw/LinkInstaller.ts[94-110]
src/static/js/pluginfw/LinkInstaller.ts[148-170]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`removeSubDependency()` and `unlinkSubDependency()` swallow any exception while reading/parsing `package.json` for a dependency, which prevents recursive cleanup of transitive sub-dependencies and makes failures invisible.
### Issue Context
These functions are used during uninstall/unlink flows. If `package.json` is missing/malformed/unreadable, recursion is skipped with no log, potentially leaving orphaned installs/symlinks.
### Fix Focus Areas
- src/static/js/pluginfw/LinkInstaller.ts[99-110]
- src/static/js/pluginfw/LinkInstaller.ts[157-170]
### Suggested change
- Replace `catch (e) {}` with at least a `console.warn`/`console.error` that includes the dependency name and path.
- Consider whether cleanup should continue without recursion (log-and-continue) or fail fast (rethrow) depending on desired uninstall semantics.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-free-for-open-source-projects]

1. Missing plugin install regression test 📘 Rule violation ⚙ Maintainability

This PR changes plugin installation behavior in LinkInstaller to fix readFileSync() path
handling, but it does not add a regression test that would fail without the fix. Without coverage,
this bug can reappear unnoticed in future refactors.

Agent Prompt

## Issue description
A bug fix was made to plugin installation file reading logic, but there is no regression test ensuring the installer reads `package.json` via a filesystem path string (not a file URL) and with an explicit encoding.

## Issue Context
This change fixes plugin installation failures that previously manifested as `ENOENT` errors and/or ineffective dependency linking. A test should fail if the old `pathToFileURL()` + `readFileSync()` usage is reintroduced.

## Fix Focus Areas
- src/static/js/pluginfw/LinkInstaller.ts[94-201]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools