Fix #12: pass ./maho relative to cwd instead of the host absolute path - #13
Merged
Conversation
The LSP and MCP commands were built with path.join(workspaceFolder, 'maho'), an absolute host path. When maho.phpCommand runs PHP inside a container that path does not exist, so the server failed with "Could not open input file". Pass './maho' instead. cwd is already set to the workspace root for both the LanguageClient and the MCP server definition, so native setups are unchanged. The absolute path is still used for the fs.existsSync activation check. Docs: the Docker examples in README.md and package.json omitted -w, which fails with a relative script path. Document the container working directory requirement. AGENTS.md did not mention MCP registration at all. CI: add a workflow running typecheck, compile and vsce package on pushes to main and on PRs, and typecheck before the release build. esbuild does not typecheck, so a passing build proved nothing. None of this catches the bug above, which needs a test asserting the constructed argv.
|
For custom Docker configurations, the -i option must be added to the docker exec command. This keeps the process stdin open and prevents the LSP server from being closed immediately after startup. The documentation should be updated to highlight this requirement in the examples for maho.phpCommand and Docker-based configurations. (e.g. |
fballiano
added a commit
that referenced
this pull request
Aug 8, 2026
The LSP transport is stdio, so a container command without -i never attaches stdin: the server reads EOF and exits right after startup. Document -i alongside the existing -w requirement in README.md and the maho.phpCommand setting description, and warn against -t (allocating a TTY corrupts the protocol stream). Reported in #13.
Contributor
Author
|
@empiricompany documentation updated! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12.
The bug
activate()built the LSP and MCP commands withpath.join(workspaceFolder.uri.fsPath, 'maho'), an absolute host path. Whenmaho.phpCommandruns PHP inside a container, that path does not exist in the container's filesystem namespace, so the server failed withCould not open input file: /abs/host/path/maho.package.jsonand issue #1 both documented the setting as prepended to./maho dev:lsp:start, so the implementation had drifted from the documented contract.The fix
Pass
./mahoin the argv.cwdwas already set to the workspace root for both theLanguageClient(options.cwd) and the MCP server definition (definition.cwd), so native and custom-PHP-path setups are unchanged. The absolute path is still computed for thefs.existsSyncactivation check.maho.phpCommandphpphp /abs/path/maho …✓php ./maho …✓/usr/local/bin/php8.3docker exec -w /app phpdocker exec mycontainer phpDocs
The Docker examples in
README.mdandpackage.jsonuseddocker exec mycontainer php, with no-w. That case cannot work with a relative script path, and only worked before by accident when the host and container paths happened to match. Both now show-wand explain that./mahoresolves against the container's working directory.AGENTS.mddid not mention MCP registration at all, and now records the relative-path invariant so it does not regress.CI
New
.github/workflows/ci.ymlrunsnpm ci→typecheck→compile→vsce packageon pushes tomainand on all PRs. The packaging step matters because the release workflow only runs after the tag exists, and immutable releases mean a burned tag cannot be reused.release.ymlnow typechecks before compiling.To be clear about the limits: none of this CI would have caught this bug.
npm run compileis a bare esbuild bundle that strips types without checking them, and eventscsees two validstrings. Catching this needs a test asserting the constructed argv, which means introducing a test runner and avscodemodule mock. Not done here.Verification
npm run typecheckandnpm run compilepass; the bundle emitsvar MAHO_SCRIPT = "./maho". Not tested against a live Docker setup.Note
Developed with the help of AI.
As part of our commitment to GenAI transparency, we flag pull requests produced with AI assistance alongside human work. As with every change in Maho, a maintainer reviews and validates it before merge, we never merge purely AI-generated changes. See the GenAI transparency section for details.