feat(mcp): rewrite bcli_mcp to consume bcli describe (AIP Phase 5)

docs/mcp-server.md

@@ -7,9 +7,17 @@ server that lets MCP-aware clients drive bcli. The intended caller is Claude
 Desktop, but it also works with the official [MCP Inspector](https://github.com/modelcontextprotocol/inspector)
 and any other client that speaks the spec.
 
-The server is deliberately small (4 read-only tools) and delegates every call
-to the bcli CLI as a subprocess. Profile resolution, auth, retry, telemetry,
-and the read-only `disable_writes` gate are inherited from the CLI for free.
+The server generates its tool list **dynamically** by subprocessing
+`bcli describe --format json` once on startup, and delegates every call to the
+bcli CLI as a subprocess. Profile resolution, auth, retry, telemetry, and the
+read-only `disable_writes` gate are inherited from the CLI for free. New CLI
+commands light up automatically as MCP tools; deprecated ones disappear.
+
+Read commands return parsed stdout JSON. Mutating commands (`bcli_post`,
+`bcli_patch`, `bcli_delete`, `bcli_attach_upload`, `bcli_batch_run`) pass
+`--result-out <tmp>` and return the AIP §Phase 2 result envelope content as
+the tool result. A `status="failed"` envelope surfaces as an MCP `ToolError`
+with the BC correlation id quoted so the agent can cite it.
 
 ## Install
 
@@ -47,8 +55,15 @@ Add a `mcpServers` entry to `~/Library/Application Support/Claude/claude_desktop
 }
 ```
 
-Restart Claude Desktop. You should see four tools register: `query`,
-`list_endpoints`, `describe_endpoint`, `list_companies`.
+Restart Claude Desktop. The server registers one tool per command in
+`bcli describe`'s output — typically ~20 tools, one per read/mutating verb in
+the CLI. Use the MCP Inspector to enumerate them, or run
+`bcli describe --format json` directly to preview what'll appear.
+
+Renames from the pre-Phase-5 surface: the old hand-written `query`,
+`list_endpoints`, `describe_endpoint`, and `list_companies` are now
+`bcli_get`, `bcli_endpoint_list`, `bcli_endpoint_info`, and `bcli_company_list`
+respectively (matching the CLI subcommand paths).
 
 If `bcli-mcp` isn't on Claude Desktop's PATH (uv tool install paths can be
 tricky), use the full path:
@@ -66,17 +81,25 @@ tricky), use the full path:
 
 ## Tool surface
 
+The tool list is generated from `bcli describe --format json` on startup, so
+the canonical reference is the describe output for your install. The most
+useful subset:
+
 | Tool | What it does | Notes |
 |------|--------------|-------|
-| `query` | Run an OData query against an entity. | `top` defaults to 50, capped at 1000. Use `select` to keep payloads small. |
-| `list_endpoints` | List entities the active profile can reach. | Honours `disable_standard_api`, `allowed_categories`, `allowed_endpoints`. |
-| `describe_endpoint` | Show fields, key, supported ops, and route for one entity. | `fields` is populated only after `bcli endpoint fields <name>` has been run. |
-| `list_companies` | Companies on the active environment. | Returns `[{id, name, alias, is_default}]`. |
-
-Mutating commands (`post` / `patch` / `delete`), file uploads, batch runs, and
-admin/setup flows are deliberately not exposed. Claude can always fall back to
-`Bash` + `bcli` directly when those are needed — and that path trips the
-existing `disable_writes` confirmation prompt.
+| `bcli_get` | Run an OData query against an entity. | `top` defaults to 50, capped at 1000 (carried from describe's `limits`). Use `select` to keep payloads small. |
+| `bcli_endpoint_list` | List entities the active profile can reach. | Honours `disable_standard_api`, `allowed_categories`, `allowed_endpoints`. |
+| `bcli_endpoint_info` | Show fields, key, supported ops, and route for one entity. | `fields` is populated only after `bcli endpoint fields <name>` has been run. |
+| `bcli_endpoint_fields` | Discover the fields for one entity and persist them to the local registry. | One BC API call; populates the cache for every future call. |
+| `bcli_company_list` | Companies on the active environment. | Returns `[{id, name, alias, is_default}]`. |
+| `bcli_q` | Run a saved query by name with `${{ params.X }}` substitution. | The "daily questions" surface — hides OData syntax. |
+| `bcli_post` / `bcli_patch` / `bcli_delete` | Mutating verbs. | Server passes `--result-out <tmp>` and returns the AIP §Phase 2 result envelope as the tool result. `status="failed"` raises `ToolError` with the BC correlation id. |
+| `bcli_attach_upload` | Two-phase document attachment upload. | Same envelope contract as the other mutating verbs. |
+| `bcli_batch_run` | Execute a YAML batch file. | Returns an envelope whose `record_id` is the batch ledger run id; pivot to `bcli_describe`'s batch state subcommand for per-step detail. |
+| `bcli_describe` | Re-emit the full describe payload. | The MCP server itself uses this on startup; tools can call it to discover what else is exposed. |
+
+`auth login`, `config init`, and other interactive commands (`effects:
+["other"]` in describe) are filtered out — those are command-line-only.
 
 ## Trust model — why the server resets cwd
 
@@ -95,20 +118,20 @@ sources are then exactly:
 Per-tool calls do not honour a per-request `cwd` argument. The server runs
 with a single fixed working directory for its lifetime.
 
-## BC query objects vs entity pages — what to expect from `query()`
+## BC query objects vs entity pages — what to expect from `bcli_get`
 
 Not every endpoint in BC's OData surface is a fully-featured entity. Some
 are "query objects" — read-only summary pages exposed via OData (e.g.
 `customerSales`, `vendorPurchases`). They behave like entities for `GET`
 but Microsoft's runtime drops `$orderby` and `$filter` support on most of
-them. A `query()` call against one of these with `orderby=` or `filter=`
+them. A `bcli_get` call against one of these with `orderby=` or `filter=`
 will 400 from BC.
 
 How to recognise one: there's no flag in the registry today (it'd require
 a hint per endpoint), so the practical signal is the 400 itself. The
 recovery pattern that works:
 
-1. `query(entity="customerSales", top=1000)` — pull a bounded page.
+1. `bcli_get(endpoint="customerSales", top=1000)` — pull a bounded page.
 2. Sort the result client-side in your reasoning step.
 3. Take the top N.
 
@@ -119,24 +142,22 @@ you may miss the actual top customer. For BC tenants with very large
 summary pages, fall back to `bcli get …` via Bash with `--all` (which
 follows pagination).
 
-Entity pages (most of `bcli endpoint list`) support full OData. Use
-`describe_endpoint(name)` to see whether `fields_discovered` is `true`
+Entity pages (most of `bcli_endpoint_list`) support full OData. Use
+`bcli_endpoint_info(name)` to see whether `fields_discovered` is `true`
 and what `fields` look like; the registry doesn't currently track which
 endpoints are query objects vs entities, so you'll learn this empirically.
 
-## Discovering field names — `discover_fields`
+## Discovering field names
 
-`describe_endpoint(name)` returns `fields: []` and `fields_discovered:
+`bcli_endpoint_info(name)` returns `fields: []` and `fields_discovered:
 false` if the local registry hasn't probed BC for that entity yet. Two
 ways to recover, in order of cost:
 
-1. Cheapest: call `query(entity=name, top=1)` once and read the keys off
-   the returned record. No registry mutation, zero cache pollution.
-2. One-time: pass `discover_fields=true` to `describe_endpoint`. The
-   tool runs `bcli endpoint fields <name>` first, which fetches one
-   record, persists the field names to the local registry, and then
-   returns the populated metadata. Every subsequent call (any user, any
-   session) gets `fields_discovered: true` for free.
+1. Cheapest: call `bcli_get(endpoint=name, top=1)` once and read the keys
+   off the returned record. No registry mutation, zero cache pollution.
+2. One-time: call `bcli_endpoint_fields(name)`. That fetches one record,
+   persists the field names to the local registry, and every subsequent
+   call (any user, any session) gets `fields_discovered: true` for free.
 
 Pick (1) for one-shot analysis. Pick (2) when the entity is one you'll
 revisit a lot — the registry-cached field list also feeds bcli's
@@ -148,9 +169,11 @@ Pairing an MCP server with a CLI tool is empirically token-favorable for
 **bounded, schema-stable** responses. The OSS server ships with two
 guard-rails baked in:
 
-* `query.top` defaults to 50 (max 1000) — an unbounded request can't pull a
-  whole table into context.
-* Tool docstrings are short. The schema-payload Claude sees is small.
+* `bcli_get.top` defaults to 50 (max 1000) — the safety bound is carried from
+  `bcli describe`'s `limits` field, so an unbounded request can't pull a whole
+  table into context.
+* Tool descriptions come straight from the CLI's docstrings. They're short by
+  construction — the schema-payload Claude sees is small.
 
 It is **not** universally a token win. For browse-style "show me everything"
 workflows, falling back to `bcli get <entity> --format markdown` via Bash is

src/bcli/result_envelope.py

@@ -107,4 +107,34 @@ def write_envelope(
         os.close(fd)
 
 
-__all__ = ["ENVELOPE_VERSION", "ResultEnvelope", "write_envelope"]
+def read_envelope(path: Path) -> ResultEnvelope:
+    """Inverse of :func:`write_envelope` — load a JSON envelope file.
+
+    Used by ``bcli_mcp`` to pick up the result of a mutating CLI
+    invocation. Tolerates missing optional fields so an older envelope
+    written by a previous bcli version still loads (forward-compat).
+    """
+    raw = json.loads(Path(path).read_text(encoding="utf-8"))
+    return ResultEnvelope(
+        version=raw.get("version", ENVELOPE_VERSION),
+        invocation_id=raw["invocation_id"],
+        tool_version=raw.get("tool_version", ""),
+        profile=raw.get("profile"),
+        environment=raw.get("environment"),
+        company=raw.get("company"),
+        method=raw["method"],
+        endpoint=raw["endpoint"],
+        resolved_url=raw.get("resolved_url"),
+        record_id=raw.get("record_id"),
+        dry_run=bool(raw.get("dry_run", False)),
+        status=raw["status"],
+        exit_code=int(raw["exit_code"]),
+        bc_correlation_id=raw.get("bc_correlation_id"),
+        telemetry_event_id=raw.get("telemetry_event_id"),
+        audit_log_offset=raw.get("audit_log_offset"),
+        started_at=raw.get("started_at", ""),
+        duration_ms=int(raw.get("duration_ms", 0)),
+    )
+
+
+__all__ = ["ENVELOPE_VERSION", "ResultEnvelope", "read_envelope", "write_envelope"]

src/bcli_cli/commands/describe_cmd.py

@@ -160,30 +160,86 @@ def _supported_formats_from_signature(sig: inspect.Signature) -> list[str]:
     return ["json"]
 
 
-def _options_from_signature(sig: inspect.Signature) -> list[dict[str, Any]]:
+# Safety bounds the MCP server's auto-generated tools must honour. Keys
+# are the (command-path-tuple, long-flag-name); the value rides as the
+# option's ``limits`` sub-object in describe's JSON output. This is the
+# *one* place that says "an agent must clamp this before invoking" —
+# Phase 5's tool generator (``bcli_mcp._tool_generator``) reads it
+# straight off describe.
+_OPTION_LIMITS: dict[tuple[tuple[str, ...], str], dict[str, Any]] = {
+    (("get",), "--top"): {"default": 50, "minimum": 1, "maximum": 1000},
+}
+
+
+def _options_from_signature(
+    sig: inspect.Signature, *, path: tuple[str, ...] = (),
+) -> list[dict[str, Any]]:
     options: list[dict[str, Any]] = []
     for name, param in sig.parameters.items():
         info = param.default
         param_decls = getattr(info, "param_decls", None)
         if not param_decls:
-            # Positional argument — describe lists options only.
+            # Positional argument — handled by ``_positionals_from_signature``.
             continue
         # Pick the longest decl (typically the ``--foo`` form).
         long_name = sorted(param_decls, key=lambda d: (-len(d), d))[0]
         type_name = _annotation_to_name(param.annotation)
         opt: dict[str, Any] = {"name": long_name, "type": type_name}
+        # Required options expose ``typer.Option(..., ...)`` — the
+        # default is the literal ellipsis. Phase 5's tool generator
+        # marks these as required in the JSON Schema so an agent
+        # doesn't construct a missing-argument call.
+        option_default = getattr(info, "default", None)
+        if option_default is Ellipsis:
+            opt["required"] = True
         # Crude validator hint — the spec example shows
         # ``validates: "odata-filter"`` for ``--filter``.
         if long_name == "--filter":
             opt["validates"] = "odata-filter"
+        # Optional safety-bounds for clamp-on-call (Phase 5 MCP wiring).
+        limits = _OPTION_LIMITS.get((path, long_name))
+        if limits is not None:
+            opt["limits"] = dict(limits)
         options.append(opt)
     return options
 
 
+def _positionals_from_signature(sig: inspect.Signature) -> list[dict[str, Any]]:
+    """List of positional arguments (Typer ``Argument``) per command.
+
+    Each entry has ``{name, type, required}``. ``required=True`` when
+    the parameter has no default; ``False`` when it does (Typer renders
+    optional positionals via ``typer.Argument(None, ...)``).
+    """
+    positionals: list[dict[str, Any]] = []
+    for name, param in sig.parameters.items():
+        info = param.default
+        param_decls = getattr(info, "param_decls", None)
+        if param_decls:
+            # Option / flag — skip.
+            continue
+        # ``typer.Argument(...)`` instances expose ``default`` on the info
+        # object; a missing default (``...``) means required.
+        argument_default = getattr(info, "default", None)
+        required = argument_default is Ellipsis or argument_default is None
+        # ``typer.Argument(None, ...)`` is the idiomatic optional form
+        # — default is ``None`` and the function accepts it. Treat as
+        # not-required to match the CLI's behaviour.
+        if argument_default is None:
+            required = False
+        positionals.append({
+            "name": name,
+            "type": _annotation_to_name(param.annotation),
+            "required": bool(required),
+        })
+    return positionals
+
+
 def _annotation_to_name(ann: Any) -> str:
     """Render a type annotation as a JSON-friendly string.
 
-    ``Optional[int]`` → ``"int"``, ``bool`` → ``"bool"``, etc.
+    ``Optional[int]`` → ``"int"``, ``bool`` → ``"bool"``,
+    ``list[str]`` → ``"list[str]"`` (preserved so MCP can branch on it).
     """
     if ann is inspect.Parameter.empty:
         return "string"
@@ -192,7 +248,15 @@ def _annotation_to_name(ann: Any) -> str:
         s = ann
     else:
         s = getattr(ann, "__name__", None) or str(ann)
-    s = s.replace("Optional[", "").rstrip("]")
+    s = s.replace("Optional[", "")
+    # Strip one trailing ``]`` that the Optional unwrap leaves behind.
+    if s.endswith("]") and s.count("[") < s.count("]"):
+        s = s[:-1]
+    # Take the last dotted component but preserve the bracket payload.
+    if "[" in s:
+        head, rest = s.split("[", 1)
+        head = head.split(".")[-1].lower()
+        return f"{head}[{rest}".rstrip()
     s = s.split(".")[-1]
     return s.lower() or "string"
 
@@ -223,7 +287,8 @@ def _walk_typer(typer_app, parent_path: tuple[str, ...] = ()) -> list[dict[str,
         entry: dict[str, Any] = {
             "path": list(path),
             "summary": _summary_from_callback(callback),
-            "options": _options_from_signature(sig),
+            "options": _options_from_signature(sig, path=path),
+            "positionals": _positionals_from_signature(sig),
             "effects": _classify_effects(path),
             "supported_formats": _supported_formats_from_signature(sig),
         }

src/bcli_mcp/__main__.py

@@ -22,15 +22,15 @@ def main() -> None:
     os.chdir(os.path.expanduser("~"))
 
     try:
-        from bcli_mcp._server import mcp
+        from bcli_mcp._server import get_server
     except ImportError as exc:
         sys.stderr.write(
             f"bcli-mcp: failed to import server: {exc}\n"
             "Hint: install the MCP extra: pip install 'bc-cli[mcp]'\n"
         )
         raise SystemExit(1) from exc
 
-    mcp.run()
+    get_server().run()
 
 
 if __name__ == "__main__":