Repository navigation
Sync MCP configs without a shell wrapper - #237
Merged
wallstop merged 2 commits intoOct 14, 2026
Merged
Conversation
The script migration still leaves lifecycle MCP sync in Bash. The wrapper is executed, not sourced, so the earlier sourced-shell exception does not apply to it. Port the wrapper to typed, stdlib-only Python and call it through each lifecycle entry point's current interpreter. Keep the adjacent Node configurator and npm's direct Node entry unchanged. Preserve workspace selection, leading-dash flag forwarding, child cwd and environment, missing-node diagnostics, and child exit status. All five original sync integration cases and assertions are unchanged. Add wrapper checks for default roots from another cwd, relative paths with spaces, multiword flags, an empty first argument, exit propagation, missing Node, and a missing workspace. The sync suite no longer skips merely because Bash is absent. Update lifecycle stub expectations. Validation: all 38 self-test files pass; scoped fast check and all ten applicable gate steps pass. Strict pyright reports zero errors on the wrapper, three lifecycle files, and three affected test files. No old sync-mcp.sh references remain in tracked tooling. No Unity behavior or packaged asset changes; Unity suites and npm pack do not apply. Refs #171.
wallstop
force-pushed
the
port-mcp-sync-wrapper
branch
from
October 14, 2026 18:56
4ce2ff2 to
9129544
Compare
wallstop
changed the base branch from
main
to
docs-qualify-performance-baseline
October 14, 2026 18:56
wallstop
added this pull request to stack #238
October 14, 2026 18:56
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9129544. Configure here.
Agent trust and approval entries use the workspace path as a key. The Python wrapper resolved symlinks before exporting that path, unlike the old shell wrapper, so opening an alias could lose its trust identity. Use lexical absolute paths for both explicit workspaces and the default script-relative workspace. Extend the existing wrapper table to cover both alias paths on POSIX. Compare the exported path verbatim instead of resolving both sides, which hid this regression in the old assertion. Windows retains its existing path/argument coverage and explicitly skips the POSIX symlink fixture. Validation: the added alias case fails against the old wrapper and passes with this change. Strict pyright and check:fast pass. The combined stack gate passes all ten applicable steps, including the full 38-file suite. No Unity or packaged payload changes. Resolve the finding in PR #237 discussion_r4275057123. Refs #171.
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
1 participant
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.
DISCLOSURE: LLM-GENERATED TEXT
Why: Lifecycle MCP sync still launches a standalone Bash wrapper during script migration.
What:
Refs #171.
Note
Low Risk
Devcontainer hook wiring and a thin Python wrapper around unchanged
configure.mjs; behavior is contract-tested with no auth or data-path changes.Overview
Replaces the Bash MCP sync wrapper with a typed Python entrypoint (
sync_mcp.py) that still runs the existingconfigure.mjsNode configurator, keeping the same contract: optional workspace path, leading-dash args forwarded as flags,DATAVIZ_MCP_WORKSPACEset, and the child exit code propagated.Devcontainer lifecycle hooks (
on_create,post_start,update_content) now invokesync_mcp.pyviasys.executableinstead ofbash sync-mcp.sh;sync-mcp.shis removed and the MCP README documents the Python path for container hooks.Tests point at
sync_mcp.py, drive sync with Python only (no Bash/jq in the harness), and add wrapper-focused cases (flag/workspace forwarding, symlink workspace spelling, missing node, missing workspace) alongside the existing frontend sync contracts.Reviewed by Cursor Bugbot for commit 7d9eda0. Bugbot is set up for automated code reviews on this repo. Configure here.