Skip to content

feat: add BC native MCP passthrough - #11

Merged
SShadowS merged 2 commits into
SShadowS:mainfrom
ianrayianray:feature/bc-native-mcp-passthrough
Jul 29, 2026
Merged

SShadowS merged 2 commits into
SShadowS:mainfrom
ianrayianray:feature/bc-native-mcp-passthrough

Conversation

@ianrayianray

@ianrayianray ianrayianray commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR ships roadmap item 8, BC native MCP passthrough, for the verified Business Central 28 cloud surface.

  • Adds bcdev_native_list to discover the dynamic native catalog for business actions, AL runtime, or an active paused debugger.
  • Adds bcdev_native_call to invoke an exact discovered tool while preserving the complete upstream CallToolResult, including content blocks, structured content, metadata, unknown fields, and isError.
  • Routes through the fixed Business Central cloud MCP endpoint using the existing launch configuration and Azure CLI identity. Endpoint, authorization, and routing headers are not caller-controlled.
  • Tracks confirmed NST session/host identity and paused/running debugger state so native troubleshooting is available only at a real manual-debugger break.
  • Shares the singleton test-run lock between native AL runtime calls, direct test runs, and debug-bound test runs.
  • Adds stable, redacted bridge errors, contextual next steps, and a dedicated embedded native-MCP skill.

Compatibility and safety

  • Existing first-class test, debugger, record-write, source, and snapshot-profile tool payloads remain compatible.
  • bcdev_native_call is conservatively marked destructive and non-idempotent because safety depends on the selected upstream tool.
  • Native passthrough is limited to cloud Sandbox and Production configurations. On-premises routing, arbitrary endpoints, arbitrary headers, prompts, and resources are excluded.
  • The public contexts are exactly business, runtime, and debugging. BC29/native profiling features are not exposed.
  • Authorization, initialization, and invocation share one bounded operation deadline; native-session termination and close are separately bounded and best effort.
  • Every list or call uses a fresh upstream MCP session. Native debugging snapshots the paused-session identity at call start, so agents are told not to resume the debugger concurrently.
  • Missing native server identity decoration returns server: null without discarding a successful list or call result.
  • Runtime locks release on every exit path. If an in-flight native runtime call times out, the structured error is non-retryable, reports upstreamRunCancelled: false, and warns that the server-side run may still be executing.
  • Debugger lifecycle transitions are callback-safe: a newer break/detach/fatal event cannot be overwritten by a pending continue or step invocation.
  • Authorization values, authenticated URLs, sensitive detail keys, and bare JWT-shaped values are redacted from bridge errors.

Validation

  • bun test: 622 passed, 0 failed
  • bun run typecheck: passed
  • bun run build: passed
  • Embedded-skill drift check: passed
  • Diff/whitespace check: passed

Deterministic coverage includes the installed Streamable HTTP SDK flow for both tools/list and tools/call, pagination, unknown-field preservation, upstream isError, phase-aware timeouts, runtime timeout recovery guidance, bare-JWT redaction, nullable server identity, DELETE cleanup, debugger callback ordering, paused-session identity gates, and native/direct/debug test-run contention.

SaaS Sandbox validation

Validated against a Business Central 28 SaaS Sandbox:

  • Business catalog discovery and a read-only bc_actions_search call
  • AL runtime catalog discovery and native run_tests against a disposable published fixture
  • Paused-debugger discovery and calls to get_stack_frames, get_variables, get_source_code, and add_breakpoint
  • Rejection before a debugger break and after continuing
  • Clean native-session, test-run-lock, and debugger cleanup
  • A correction rerun across all three contexts after the review fixes

No Production, on-premises, or native profiling call was made. Redacted evidence is recorded in:

  • scripts/e2e-native-mcp-passthrough-2026-07-27.md
  • scripts/e2e-native-mcp-passthrough-2026-07-28.md

@SShadowS SShadowS left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally on pr-11: bun run typecheck clean, bun test 619 pass / 0 fail across 43 files. Runtime deps unchanged (signalr, mcp sdk, zod), so the three dep rule holds.

Not verified: the SaaS Sandbox run. No SaaS credentials on this machine, so scripts/e2e-native-mcp-passthrough-2026-07-27.md is the only support for the new wire facts (Dev: ALRuntime, the mcp-troubleshooting-options shape, versionNumber: "1.0"). Those need a second pair of eyes from whoever has Sandbox access.

Strengths

The executionRevision work is the best part of this PR. Break arriving before sessionBound was a real ordering bug caught live, and rollbackResume correctly declines to restore state that a newer callback already owns. Both orderings have deterministic tests.

Other things done right:

  • nonblankHeader rejects CRLF on every header value at the core boundary, with zod regex on top at the tool boundary. Header injection closed twice.
  • requestHeaders re-checks debugIdentity even though targetFrom already guarantees it. Core does not trust its caller.
  • Loose zod schemas with the comment explaining that the SDK convenience helpers normalize unknown fields away. Correct call for a passthrough, and the reasoning is written down rather than assumed.
  • Endpoint, authorization, and routing headers are not caller controlled.
  • Conventions followed throughout: claim then finally lock matching test-tools.ts:111, // WIRE: provenance on every assumption, e2e checklist plus dated evidence, embedded skill with drift test, README table and count and roadmap all updated.

1. Runtime timeout releases the test-run lock without cancelling the upstream run

bcdev_native_call with context: "runtime" claims the singleton test-run lock, then releases it unconditionally in finally. On the whole-operation timeout (default 180s, hard max 300s) the local lock drops while Business Central's run_tests is still executing server side. A following bcdev_test_run is then free to start against a live run, which is the exact contention the lock exists to prevent. bcdev_test_run has no timeout of its own, it awaits hub completion, so this hole is new.

It gets worse from the error surface: TIMEOUT carries retryable: true and recoverySteps says "Retry the operation". An agent following that guidance starts a second test run on top of the first.

Suggested fix, keeping the release in finally and making the consequence explicit in the payload:

handler: async (params) => {
  const context = params["context"] as NativeMcpContext;
  const runLocked = context === "runtime";
  if (runLocked) claimTestRun(state);
  try {
    const target = targetFrom(state, deps, params);
    const toolName = params["toolName"] as string;
    const response = await deps.nativeMcpGateway.callTool(
      target,
      toolName,
      (params["arguments"] as Record<string, unknown> | undefined) ?? {},
    );
    return { context: target.context, toolName, ...response };
  } catch (error) {
    // A timeout stops the bridge waiting; it does not ask Business Central to stop
    // the native run. The test-run slot is released either way, so the caller has to
    // be told the server may still be executing before it retries or starts a run.
    if (runLocked && error instanceof BcDevError && error.code === "TIMEOUT") {
      throw new BcDevError(
        error.code,
        error.message,
        error.category,
        error.retryable,
        {
          ...error.details,
          upstreamRunCancelled: false,
          warning:
            "The native run was not cancelled and may still be executing on the server. "
            + "Confirm it finished before retrying or starting another test run.",
        },
        { cause: error },
      );
    }
    throw error;
  } finally {
    if (runLocked) state.testRunActive = false;
  }
},

details is typed Record<string, string | number | boolean | null>, so both fields fit without a type change. Worth a test asserting upstreamRunCancelled: false reaches the agent body, alongside the existing "releases the native runtime lock when setup or invocation fails" case.

Separately, consider whether 300s is the right ceiling for runtime at all. A real suite can exceed five minutes, so the cap turns normal runs into this failure mode rather than an edge case.

2. Bare JWT is outside the redactor (hardening, no demonstrated leak)

redactAuthorization matches URL userinfo, ?Authentication=, and a literal Authorization prefix. It does not match a bare token:

in:  Bearer eyJ0eXAi....SIGNATUREPART
out: Bearer eyJ0eXAi....SIGNATUREPART      (unchanged)

That matters here because this is the first path that sends Authorization: Bearer <Entra JWT> to an external gateway and echoes that gateway's body back: SDK streamableHttp.js:364 builds Error POSTing to endpoint: ${text} from the raw response body, which reaches the agent via nativeErrorDetail into details.upstreamMessage.

To be clear about scope, I did not show that Business Central or the Entra front end ever echoes a token in a response body, and normally it will not. Our own outgoing headers are not in the message. So this is defense in depth on an unproven path, not a known exposure. One line in redactAuthorization closes it:

.replace(/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]*/g, "[REDACTED_JWT]")

3. serverIdentity() runs before the operation (low)

return await this.use(target, async (connection, options) => ({
  server: serverIdentity(connection),
  catalog: await connection.listTools(cursor, options),
}));

Property evaluation order means a server that omits serverInfo.name or version fails every list and call with PROTOCOL_ERROR, even though the request itself would have succeeded. serverInfo is spec required so this is unlikely, but the strictness buys nothing since identity is decoration. Degrading to null is strictly better than throwing.

4. Session per operation is undocumented (low)

Every bcdev_native_list and bcdev_native_call builds a fresh Client and transport: initialize, request, DELETE, three to four round trips, no reuse. Fine and deliberately stateless, but for the debugging context that is three-plus round trips per inspection against one hub invocation for bcdev_debug_variables. Worth a line in skills/bc-native-mcp/SKILL.md so agents do not reach for native calls where the first class tools are cheaper.

5. Debug identity is point in time (low)

nativeDebugIdentity is read once in targetFrom, then the call can run for minutes. A concurrent bcdev_debug_continue resumes the session mid flight and the native troubleshooting call proceeds against a resumed session. Needs a client that issues concurrent tool calls, which most do not do within a turn, so this is a documentation note rather than a code change.

6. Loose error message matching (cosmetic)

lower.includes("debug session") && lower.includes("identity") in fromKnownMessage catches any stray Error containing both words. Only reachable for non BcDevError throws and it matches existing house style. Leave it.

Verdict

Approve. Item 1 is the only one I would want resolved or consciously accepted before merge, since it converts a correctness guarantee into a silent one under a condition that real test suites will hit.

@gitguardian

gitguardian Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@ianrayianray

ianrayianray commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed all six review notes in 89c4c20.

  1. Runtime timeouts are now phase-aware. An operation-phase timeout is non-retryable, reaches the agent error body with upstreamRunCancelled: false, and warns that the server-side test run may still be executing. Authorization/connection timeouts remain retryable because no upstream tool call began.
  2. Bare JWT-shaped values are redacted from upstream error detail.
  3. Server identity is optional decoration: a successful operation now returns server: null when initialization identity is missing or invalid.
  4. The embedded skill and README document that each list/call creates and closes a fresh upstream MCP session and that first-class debugger tools are cheaper for repeated inspection.
  5. The skill and README document the point-in-time debug identity and require awaiting native debugging calls before continuing the debugger.
  6. The loose identity/paused-state message fallbacks were removed; current native debugger paths use typed errors.

I also reran the real handlers against the SaaS Sandbox after these corrections. Business search, native runtime test execution, and paused-debug get_stack_frames all returned non-error content; pre-break and post-continue state gates remained typed; locks released and detach was clean. Redacted evidence is in scripts/e2e-native-mcp-passthrough-2026-07-28.md.

Final local gates: 622 tests passed, typecheck clean, build clean, embedded-skill generation idempotent, and diff check clean. The synthetic JWT redaction vector is assembled at runtime to avoid a scanner false positive; both the repository verification job and GitGuardian now pass.

@ianrayianray
ianrayianray force-pushed the feature/bc-native-mcp-passthrough branch from 411d28d to 89c4c20 Compare July 28, 2026 22:47
@SShadowS
SShadowS merged commit 5b593ff into SShadowS:main Jul 29, 2026
2 checks passed
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.

2 participants