fix(bindings): repair no longer clobbers wardline-owned launch flags in .mcp.json - #110
Conversation
…in .mcp.json The three-way binding repair rebuilt the wardline entry from scratch, keeping only the ceded --filigree-url. Every other wardline-owned launch flag (--trust-pack, --allow-custom-packs, --read-only, ...) was silently dropped, and the health check was exact args-array equality, so any hand-authored entry classified MissingOrStale forever and got rewritten on every install/doctor --fix — on elspeth this stripped the project's trust-grammar pack grant from a git-tracked .mcp.json and baked in the deterministic-port --loomweave-url. Now an existing sane entry (args invoke `mcp`) is carried forward verbatim — command included — with exactly one owned mutation: the --loomweave-url VALUE is refreshed when the flag is present with a stale value. An entry omitting the flag entirely is current (wardline's resolver falls through to the published .weft/loomweave/ephemeral.port rung). Fresh registrations still seed the full entry including the deterministic-port URL (ADR-044 unchanged). Closes clarion-c379a8c9ee. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts Loomweave’s Wardline .mcp.json “repair” behavior to avoid rewriting (and thereby clobbering) Wardline/operator-owned launch flags, while still keeping Loomweave’s owned --loomweave-url value up to date when present.
Changes:
- Updates the “is current” check for the Wardline MCP entry to treat missing
--loomweave-urlas acceptable and only validate Loomweave’s owned URL value when the flag exists. - Makes
install_wardline_mcppreserve existing saneargsverbatim and refresh only the--loomweave-urlvalue when stale (no whole-entry rewrite). - Adds regression tests to ensure repair preserves Wardline-owned flags and only refreshes the owned URL value.
Suppressed comments (1)
crates/loomweave-cli/src/integration_bindings.rs:273
- The surgical-repair path in
install_wardline_mcpconsiders an entry "sane" if it only has anargsarray starting with"mcp". That means partially malformed entries (e.g.,type/commandmissing) can fall into the early-return/no-op paths (especially when--loomweave-urlis omitted) and never get repaired to a runnable wardline registration. Tighten the "sane" predicate to require the expected structural fields before attempting an in-place edit; otherwise fall back to seeding a fresh entry.
if let Some(args) = servers
.get_mut("wardline")
.and_then(|entry| entry.get_mut("args"))
.and_then(Value::as_array_mut)
.filter(|args| args.first().and_then(Value::as_str) == Some("mcp"))
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Loomweave owns exactly one field of this entry: the --loomweave-url | ||
| // VALUE. Everything else — the command, --root, wardline-owned launch | ||
| // flags like --trust-pack/--allow-custom-packs, the ceded --filigree-url — | ||
| // is wardline's (or the operator's) and never judged here. An entry that | ||
| // omits --loomweave-url entirely is also current: wardline's resolver | ||
| // falls through to the published .weft/loomweave/ephemeral.port rung. | ||
| let Some(args) = entry.get("args").and_then(Value::as_array) else { | ||
| return Ok(false); | ||
| }; | ||
| if args.first().and_then(Value::as_str) != Some("mcp") { | ||
| return Ok(false); | ||
| } | ||
| Ok(loomweave_url_arg(args).is_none_or(|url| url == desired.loomweave_url)) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa53bcace2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let Some(args) = entry.get("args").and_then(Value::as_array) else { | ||
| return Ok(false); | ||
| }; | ||
| if args.first().and_then(Value::as_str) != Some("mcp") { | ||
| return Ok(false); | ||
| } | ||
| Ok(loomweave_url_arg(args).is_none_or(|url| url == desired.loomweave_url)) |
There was a problem hiding this comment.
With an existing .mcp.json where mcpServers.wardline.args begins with mcp but the entry lacks a string command (for example a partial/manual edit containing only {"args":["mcp"]}), this new predicate returns Present, and install_wardline_mcp uses the same loose shape to no-op instead of seeding the full entry. MCP clients cannot launch the stdio server without the command field, so doctor --fix can give a false all-clear while leaving an unusable binding; preserving custom commands still needs to distinguish a missing/non-string command as malformed or stale.
Useful? React with 👍 / 👎.
| if flag + 1 < args.len() { | ||
| args[flag + 1] = url; |
There was a problem hiding this comment.
When an existing entry has --loomweave-url without a value before another option, e.g. [..., "--loomweave-url", "--filigree-url", "http://..."], this branch overwrites --filigree-url with the Loomweave URL instead of inserting a missing value after the Loomweave flag. That silently drops Wardline's ceded emit flag and leaves its URL as a positional arg; the health check then accepts the corrupted argv, so one repair can break the wardline-owned flag preservation this change is meant to protect.
Useful? React with 👍 / 👎.
Summary
Owner-reported: "loomweave removes the trust pack when it repairs, it shouldn't do that." On elspeth, the three-way binding repair rewrote the tracked
.mcp.jsonwardline entry from["mcp","--root",".","--trust-pack","scripts.wardline_pack","--allow-custom-packs"]to["mcp","--root",".","--loomweave-url","http://127.0.0.1:10251"]— silently un-trusting the project's trust-grammar pack and baking the deterministic port into a git-tracked file.Root cause:
desired_wardline_argscomposed the entry from scratch (only--filigree-urlwas ceded/carried), andwardline_mcp_okwas exact args-array equality, so any wardline-owned flag made the entry permanently "stale".Fix
Same functionally-equivalent-is-healthy discipline as #109:
mcp) are carried forward verbatim — command,--root, trust flags, ceded--filigree-url. The single owned mutation: refresh the--loomweave-urlvalue when the flag is present and stale.--loomweave-urlis current — wardline's resolver (flag > env > published.weft/loomweave/ephemeral.port) discovers Loomweave at runtime — so no flag-injection churn on tracked files.--filigree-url.Closes clarion-c379a8c9ee.
Testing
repair_preserves_wardline_owned_flags_and_flagless_url_is_current(elspeth regression, byte-for-byte no-op incl. custom command),repair_refreshes_only_the_owned_loomweave_url_value(order + flags survive, only the value changes, check agrees with write).🤖 Generated with Claude Code