Skip to content

Conversation

akapug
Copy link
Member

@akapug akapug commented Sep 10, 2025

Ready for review Powered by Pull Request Badge

Summary

Fixes an issue where elide mcp (and other CLI entrypoints) could fail on WSL/Ubuntu with:

Error: Properties init: Could not determine current working directory.

Tracking: Fixes #1583

Root cause

In GraalVM native images, initialization of user.dir can throw if getcwd(2) fails in certain directories/configurations (seen on WSL). Our CLI startup referenced System.getProperty("user.dir") during early initialization and also relied on the process command path, which may be relative; together, this could surface the native image error on WSL when invoked from particular directories (e.g., ~/elide).

What changed

  • Add resolveBinaryPathForNative():
    • Prefer ProcessHandle.current().info().command() when absolute; resolve symlinks
    • If the command is relative or unavailable, fallback to /proc/self/exe (absolute, stable)
  • Add safeWorkingDirectory():
    • Try System.getProperty("user.dir")
    • On failure, fallback to /proc/self/cwd; then PWD; then HOME; finally .
  • Use the above in initializeEntry(...) when in native-image runtime code:
    • Compute binPath via resolveBinaryPathForNative()
    • Pass safeWorkingDirectory() to installStatics(...)

These fallbacks are only engaged when running as a native image on Linux/WSL; JVM behavior remains unchanged.

Why this works

  • /proc/self/exe gives a canonical absolute path to the running binary on Linux/WSL, independent of user.dir.
  • /proc/self/cwd provides an absolute working directory path even if user.dir initialization encountered issues inside SVM, avoiding the "Could not determine current working directory" failure mode.

Verification

  • Built and compiled :packages:cli successfully
  • Existing tests remain unaffected
  • Behavioral change is limited to native image path resolution and only when the standard properties are not safely available

Risk

Low. Changes are guarded for native image runtime and rely on standard Linux /proc fallbacks only when needed. No behavioral change for JVM runs.

— Augment Code

…/WSL\n\n- Resolve binary path via /proc/self/exe if ProcessHandle command is relative\n- Add safeWorkingDirectory fallback using /proc/self/cwd or PWD/HOME\n- Prevents SVM \"Could not determine current working directory\" error when running MCP in certain WSL dirs\n\nFixes #1583\n\nCo-authored-by: Augment Code <[email protected]>
@akapug akapug requested a review from sgammon as a code owner September 10, 2025 03:23
Copy link

codecov bot commented Sep 10, 2025

Codecov Report

❌ Patch coverage is 20.83333% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.98%. Comparing base (95ed0ff) to head (2c4414c).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...ackages/cli/src/main/kotlin/elide/tool/cli/main.kt 20.83% 17 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1633      +/-   ##
==========================================
+ Coverage   39.68%   39.98%   +0.30%     
==========================================
  Files         781      783       +2     
  Lines       37169    37450     +281     
  Branches     5253     5302      +49     
==========================================
+ Hits        14752    14976     +224     
- Misses      20648    20683      +35     
- Partials     1769     1791      +22     
Flag Coverage Δ
jvm 39.98% <20.83%> (+0.30%) ⬆️
lib 39.98% <20.83%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ackages/cli/src/main/kotlin/elide/tool/cli/main.kt 5.83% <20.83%> (+5.83%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95ed0ff...2c4414c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ir null/blank

- safeWorkingDirectory: handle null/blank user.dir; prefer /proc/self/cwd, then PWD/HOME, then "."
- Add WorkingDirectoryFallbackTest to assert non-empty result when user.dir is missing

Signed-off-by: David Anderson <[email protected]>
@akapug
Copy link
Member Author

akapug commented Sep 11, 2025

Validation update for MCP server CLI init on WSL

What changed in this PR (plus a small follow‑up I just pushed):

  • Hardened safeWorkingDirectory(): handle null/blank user.dir. Fallbacks are now: /proc/self/cwd → $PWD → $HOME → "."
  • Added a tiny JVM unit test (WorkingDirectoryFallbackTest) to assert a non‑blank result when user.dir is missing.
  • The change is restricted to CLI init and remains low‑risk; Linux/WSL gets robust /proc fallbacks, other OSes gracefully use env/".".

Why this matters

  • Addresses native‑image WSL error: "Could not determine current working directory" by removing reliance on a potentially failing getcwd(2) at early init.

Verification

  • Local: Ran :packages:cli:test for WorkingDirectoryFallbackTest with native tasks excluded → passed.
  • Prior CI (pre‑patch): Tests (All Suites) ✅, Gradle Wrapper ✅, Detekt ✅, Formatting ✅, API Check ❌ (advisory on PRs). DCO was missing.
  • Follow‑up commit includes DCO sign‑off. Awaiting updated CI on the branch.

Expected CI outcome

  • Gating checks: Tests + Gradle Wrapper should remain green.
  • Advisory: API Check may still show as failing (non‑blocking per repo rules).

Conclusion

  • High confidence the PR resolves the WSL native‑image init issue for elide mcp and similar commands.
  • With DCO in place and tests green, this should be ready to merge (API Check advisory acknowledged).

If anyone wants extra hardening, I can add a micro‑guard for blank $PWD/$HOME or wire a Linux‑only test that checks /proc path directly. Otherwise this is good to go.

- Add internal safeWorkingDirectoryForTest wrapper
- Update WorkingDirectoryFallbackTest to avoid private reflection

Signed-off-by: David Anderson <[email protected]>
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.

MCP server fails with "Could not determine current working directory" error on WSL/Ubuntu

1 participant