mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-06 07:06:54 +01:00
Merge pull request #24 from Curabis/fix/bc-mcp-config-template-and-error-surfacing
Ret bc-mcp: config-skabelon matcher broen + fejl svælges ikke længere
This commit is contained in:
commit
97f3e60ce1
5 changed files with 136 additions and 4 deletions
|
|
@ -0,0 +1,61 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: mcp
|
||||||
|
keywords: [businesscentral, mcp, bc-mcp-bridge, error-handling, sse, timeout, troubleshooting]
|
||||||
|
technologies: [al, mcp]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# bc-mcp-bridge.js Must Check `!r.ok` Before Any SSE Parsing, Unconditionally
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`bc-mcp-bridge.js`'s `forward()` function decides how to parse the BC MCP
|
||||||
|
endpoint's response body based on its `content-type` header. Before
|
||||||
|
2026-07-31 it only treated a response as an error when `!r.ok && !text` —
|
||||||
|
i.e. only when there was no body at all. A non-2xx response WITH a body
|
||||||
|
(the common case — BC returns structured JSON error objects) fell through
|
||||||
|
to the content-type branch instead.
|
||||||
|
|
||||||
|
## Incident (2026-07-31)
|
||||||
|
|
||||||
|
BC returned a 400 with a plain JSON error body (`{"Error": {"Message":
|
||||||
|
"..."}}`) while still labelling the response `content-type:
|
||||||
|
text/event-stream`. `parseSSE()` only extracts lines starting with `data:` —
|
||||||
|
a plain JSON body has none, so it returned `[]` silently. The stdin loop's
|
||||||
|
`for (const out of responses) process.stdout.write(...)` then had nothing to
|
||||||
|
iterate, so the bridge produced **zero output** — not even to stderr — for a
|
||||||
|
request that BC had already answered in under 200ms. Claude Code had no
|
||||||
|
signal to work with and waited out its own 30-second client-side timeout,
|
||||||
|
which was the only thing the developer actually saw.
|
||||||
|
|
||||||
|
## Rule
|
||||||
|
|
||||||
|
Check `!r.ok` before considering content-type at all, and throw
|
||||||
|
unconditionally (with the body text included) when it's true. Never let a
|
||||||
|
non-2xx response reach `parseSSE` — a server is free to mislabel an error
|
||||||
|
body's content-type, and the client must not depend on that label being
|
||||||
|
honest.
|
||||||
|
|
||||||
|
## Anti-Pattern
|
||||||
|
|
||||||
|
const ct = r.headers.get("content-type") || "";
|
||||||
|
const text = await r.text();
|
||||||
|
if (!r.ok && !text) throw new Error(`HTTP ${r.status}`);
|
||||||
|
return ct.includes("text/event-stream") ? parseSSE(text) : [text.trim()];
|
||||||
|
// A 400 WITH a body silently falls through to parseSSE and returns [].
|
||||||
|
|
||||||
|
## Compliant
|
||||||
|
|
||||||
|
const ct = r.headers.get("content-type") || "";
|
||||||
|
const text = await r.text();
|
||||||
|
if (!r.ok) throw new Error(`HTTP ${r.status}: ${text || "(empty body)"}`);
|
||||||
|
return ct.includes("text/event-stream") ? parseSSE(text) : [text.trim()];
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
`bc-mcp-bridge.js` specifically, but the underlying principle generalizes to
|
||||||
|
any stdio MCP bridge that branches parsing logic on a server-supplied
|
||||||
|
content-type header: validate the HTTP status first, independent of what
|
||||||
|
the header claims the body's shape is.
|
||||||
|
|
@ -0,0 +1,57 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: mcp
|
||||||
|
keywords: [businesscentral, mcp, bc-mcp-bridge, company, header, display-name, config, troubleshooting]
|
||||||
|
technologies: [al, mcp]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# BC MCP `Company` Header Must Be the Exact `Navn` Field, Not `Vist navn`
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`~/.bc-mcp.config.json`'s `company` value is sent as the literal `Company`
|
||||||
|
HTTP header to the BC MCP endpoint. It must exactly match the company's
|
||||||
|
**`Navn`** field in Business Central's company list — not the **`Vist navn`**
|
||||||
|
(display name) field. The two are often different strings for the same
|
||||||
|
company, and BC's own company picker UI shows the display name more
|
||||||
|
prominently, making it the natural (wrong) one to copy.
|
||||||
|
|
||||||
|
## Incident (2026-07-31)
|
||||||
|
|
||||||
|
CURABIS's own `businesscentral` MCP server failed after "working all
|
||||||
|
evening" with a generic client-side "connection timed out after 30000ms" —
|
||||||
|
no useful error surfaced to the developer. Root cause: `company` was set to
|
||||||
|
`"CURABIS ApS"` (the `Vist navn`), while BC's actual `Navn` field is
|
||||||
|
`"Curabis ApS"`. BC's own API rejected the mismatched header with a fast,
|
||||||
|
clear 400 error — but `bc-mcp-bridge.js` had a separate bug
|
||||||
|
(`bc-mcp-bridge-must-surface-non-2xx-responses-before-sse-parsing`, same
|
||||||
|
incident) that swallowed the error body, turning a sub-200ms server error
|
||||||
|
into a 30-second client-side hang with no diagnostic.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
If `businesscentral` MCP fails, check BC's company list page (Virksomheder /
|
||||||
|
Companies) and compare the `Navn` column — not `Vist navn` — against
|
||||||
|
`~/.bc-mcp.config.json`'s `company` value, character for character. Do not
|
||||||
|
assume the value that "looks right" from the picker UI is the one the API
|
||||||
|
needs.
|
||||||
|
|
||||||
|
## Anti-Pattern
|
||||||
|
|
||||||
|
// WRONG: copied from BC's company switcher, which shows Vist navn
|
||||||
|
{ "company": "CURABIS ApS" }
|
||||||
|
|
||||||
|
## Compliant
|
||||||
|
|
||||||
|
// CORRECT: copied from the Navn column on the company list page
|
||||||
|
{ "company": "Curabis ApS" }
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
Every machine with `~/.bc-mcp.config.json` configured — this is a
|
||||||
|
machine-local file, not something Mode B can fix centrally. The template
|
||||||
|
(`bc-mcp.config.template.json`) carries an explicit warning about this
|
||||||
|
distinction as of 2026-07-31, but a machine already onboarded before that
|
||||||
|
date needs its existing file checked manually.
|
||||||
|
|
@ -92,7 +92,14 @@ async function forward(msg) {
|
||||||
const sid = r.headers.get("mcp-session-id"); if (sid) sessionId = sid;
|
const sid = r.headers.get("mcp-session-id"); if (sid) sessionId = sid;
|
||||||
const ct = r.headers.get("content-type") || "";
|
const ct = r.headers.get("content-type") || "";
|
||||||
const text = await r.text();
|
const text = await r.text();
|
||||||
if (!r.ok && !text) throw new Error(`HTTP ${r.status}`);
|
// 2026-07-31: BC has returned error bodies as plain JSON while still labelling
|
||||||
|
// content-type text/event-stream (e.g. "company not found"). parseSSE only
|
||||||
|
// extracts lines starting with "data:" - a plain JSON error body has none, so
|
||||||
|
// it silently returned []. The stdin loop then wrote nothing at all, and the
|
||||||
|
// client (Claude Code) waited out its own 30s timeout instead of seeing the
|
||||||
|
// real error immediately. Check !r.ok BEFORE any SSE parsing, unconditionally -
|
||||||
|
// never let a non-2xx response fall through to parseSSE.
|
||||||
|
if (!r.ok) throw new Error(`HTTP ${r.status}: ${text || "(empty body)"}`);
|
||||||
return ct.includes("text/event-stream") ? parseSSE(text) : (text.trim() ? [text.trim()] : []);
|
return ct.includes("text/event-stream") ? parseSSE(text) : (text.trim() ? [text.trim()] : []);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -205,7 +205,12 @@ If it does NOT exist:
|
||||||
2. Write it to `~/.bc-mcp.config.json` as-is
|
2. Write it to `~/.bc-mcp.config.json` as-is
|
||||||
3. Tell the developer:
|
3. Tell the developer:
|
||||||
> "⚠️ `~/.bc-mcp.config.json` er oprettet fra CURABIS-template.
|
> "⚠️ `~/.bc-mcp.config.json` er oprettet fra CURABIS-template.
|
||||||
> Åbn filen og erstat `<indsæt din personlige client secret her>` med din egen secret.
|
> Udfyld ALLE placeholder-felter (tenant, clientId, client secret, company)
|
||||||
|
> — ikke kun secret'en. For `company`: brug PRÆCIS firmanavnet fra BC's
|
||||||
|
> 'Navn'-kolonne på virksomhedslisten, IKKE 'Vist navn' — de to kan være
|
||||||
|
> forskellige strenge for samme firma (2026-07-31: 'CURABIS ApS' vs.
|
||||||
|
> 'Curabis ApS' forårsagede et 30-sekunders timeout uden brugbar fejl —
|
||||||
|
> se `bc-mcp-company-header-must-match-exact-company-name`).
|
||||||
> Gem filen — BC MCP er klar når du genstarter Claude Code."
|
> Gem filen — BC MCP er klar når du genstarter Claude Code."
|
||||||
|
|
||||||
#### 3c. bcquality-knowledge, roster agents, find-altool.ps1, MCP registration (v24)
|
#### 3c. bcquality-knowledge, roster agents, find-altool.ps1, MCP registration (v24)
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,8 @@
|
||||||
{
|
{
|
||||||
"tenantId": "CURABIS-TENANT-ID",
|
"tenant": "CURABIS-TENANT-ID",
|
||||||
"clientId": "CURABIS-CLIENT-ID",
|
"clientId": "CURABIS-CLIENT-ID",
|
||||||
"clientSecret": "<indsæt din personlige client secret her>",
|
"clientSecret": "<indsæt din personlige client secret her>",
|
||||||
"baseUrl": "https://api.businesscentral.dynamics.com"
|
"environment": "Production",
|
||||||
|
"company": "<PRÆCIS firmanavnet fra BC's 'Navn'-felt - IKKE 'Vist navn'. 2026-07-31: 'CURABIS ApS' (Vist navn) fejlede med 'company not found' hos BC MCP; det korrekte var 'Curabis ApS' (Navn). Tjek Virksomheder-siden i BC, kolonnen 'Navn', hvis usikker.>",
|
||||||
|
"configurationName": "CURABIS_DEV"
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue