Skip to content

[Repo Assist] fix: replace deprecated async fs.exists with fs.existsSync in addRootPath#51

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-issue-36-executablepath-sync-796a5a385a069a2e
Draft

[Repo Assist] fix: replace deprecated async fs.exists with fs.existsSync in addRootPath#51
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-issue-36-executablepath-sync-796a5a385a069a2e

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Mar 9, 2026

🤖 This is an automated PR from Repo Assist, an AI assistant.

Summary

Replaces the deprecated async fs.exists() callback with the synchronous fs.existsSync() in addRootPath().

Root Cause

addRootPath() is called from loadSettings() to resolve relative executablePath values (e.g. ./vendor/bin/phpcbf) to absolute paths. The old code used fs.exists(path, callback) — a deprecated Node.js API that runs the existence check asynchronously:

// Before: async — the callback fires AFTER loadSettings() returns
fs.exists(tmpExecutablePath, exists => {
    if (exists) {
        this.executablePath = tmpExecutablePath;
    }
});

When loadSettings() returns, this.executablePath is still the original relative path. format() immediately calls getArgs() with that unresolved path, so cp.spawn() receives ./vendor/bin/phpcbf instead of the absolute path, causing a spawn failure.

This is the race condition behind issue #36: "after restart vscode it can't find the exePath anymore." The extension works after the second save (because the async callback has already fired by then), but fails on the first save after each restart.

Fix

// After: synchronous — executablePath is updated before loadSettings() returns
if (fs.existsSync(tmpExecutablePath)) {
    this.executablePath = tmpExecutablePath;
}

fs.existsSync() is synchronous, stable, and the idiomatic replacement for fs.exists() which has been deprecated since Node.js v4.

Files Changed

File Change
extension.js Replace fs.exists(path, callback) with if (fs.existsSync(path)) in addRootPath()

Test Status

This extension requires a live VS Code instance. No CI is configured for this repository.

Manual test steps:

  1. Set phpcbf.executablePath to a relative path (e.g. ./vendor/bin/phpcbf)
  2. Restart VS Code
  3. Open a PHP file and save immediately — phpcbf should resolve and run correctly on the first save (was: failed until second save)

Closes #36

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

…Path

The previous code used fs.exists() (deprecated async callback) to check
whether the resolved executable path exists before applying it. Because
the callback ran asynchronously, loadSettings() returned before the path
was updated. On the first save after a VS Code restart, getArgs() was
called with the still-relative executablePath, causing spawn to fail.

Fix: use fs.existsSync() so the path check and update are synchronous
and complete before loadSettings() returns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 9, 2026
Documents the bug fixes and improvements from the open Repo Assist PRs:
- Fix formatter hang on exit code 0 (PR #49, closes #39)
- Fix phpcbf.enable=false bypass (PR #56)
- Fix temp file leak on process error (PR #58)
- Fix onWillSaveTextDocument save timeout (PR #53, closes #35)
- Fix deprecated fs.exists (PR #51, closes #36)
- Fix undefined showErrorMessage on unknown exit code (PR #61)
- Add VS Code output channel for debug/error display (PR #50)
- Refactor getArgs standard variable (PR #60)
- Fix eslintrc and no-case-declarations lint issues

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Mar 10, 2026
- Replace deprecated async fs.exists() with synchronous fs.existsSync()
  in addRootPath(), eliminating the race condition where executablePath
  would be updated after cp.spawn() had already used the old value.
  Supersedes PR #51.

- Use path.join() for temp file construction instead of string concat
  with a hard-coded slash, fixing the double-slash on macOS when
  os.tmpdir() returns a path ending in '/'. Supersedes PR #49 (partial).

- Remove broken phpcbfError flag pattern. The flag was evaluated
  synchronously before any exit event fired, so the conditional stdout
  listener was never attached for exit code 3. Replace with an
  unconditional stdoutOutput accumulator used inside the exit handler.
  Supersedes PR #49 (partial).

- Reject the promise for exit code 0 (nothing to fix) so VS Code's
  formatter API is notified immediately instead of hanging until restart.
  Closes #39. Supersedes PR #48, PR #49.

- Show a meaningful error message for exit code 3 using buffered stdout
  (or a fallback string), then reject so the spinner clears. Previously
  the error message was silently swallowed.

- Clean up temp file in the error handler (exec.on('error')) to prevent
  orphaned files when the executable is not found. Supersedes PR #58.

- Add block scopes around case 1/2 and default to silence
  no-case-declarations lint errors without disabling the rule.

- Remove the unreachable 3: entry from the msgs lookup in the default
  case, and fall back gracefully for unknown exit codes instead of
  calling window.showErrorMessage(undefined). Supersedes PR #61.

- Use a local const for the resolved standard in getArgs() instead of
  writing back to this.standard, removing the accidental side effect
  that could clobber the instance state mid-format. Supersedes PR #60.

- Guard onDidChangeConfiguration reload with
  event.affectsConfiguration('phpcbf') so unrelated setting changes
  (e.g. font, theme) do not trigger unnecessary reloads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

executablePath broken

0 participants